From 705402787eb8751f8e75ca2373c5ff9c6847ef40 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 17 Jul 2023 12:08:14 -0700 Subject: [PATCH 1/9] In the Canonical ABI, disallow empty types. In C++, all types must have a non-zero size, so empty structs have size 1. In C, structs with no members are non-standard, though widely supported, with size 0, making them ABI-incompatible with C++. In Go, the spec says "Two distinct zero-size variables may have the same address in memory", and as a user, my understanding is the fact that it says "may" means one isn't guaranteed it's always one way or the other. To avoid these complexities, for now, disallow empty record and flags types. --- design/mvp/CanonicalABI.md | 6 ++++-- design/mvp/canonical-abi/definitions.py | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/design/mvp/CanonicalABI.md b/design/mvp/CanonicalABI.md index c5d8e0c5..643bdd92 100644 --- a/design/mvp/CanonicalABI.md +++ b/design/mvp/CanonicalABI.md @@ -161,7 +161,8 @@ below. ### Size Each value type is also assigned a `size`, measured in bytes, which corresponds -the `sizeof` operator in C: +the `sizeof` operator in C. Empty types, such as records with no fields, are +not permitted, to avoid complications in source languages. ```python def size(t): match despecialize(t): @@ -184,6 +185,7 @@ def size_record(fields): for f in fields: s = align_to(s, alignment(f.t)) s += size(f.t) + trap_if(s == 0) return align_to(s, alignment_record(fields)) def align_to(ptr, alignment): @@ -201,7 +203,7 @@ def size_variant(cases): def size_flags(labels): n = len(labels) - if n == 0: return 0 + trap_if(n == 0) if n <= 8: return 1 if n <= 16: return 2 return 4 * num_i32_flags(labels) diff --git a/design/mvp/canonical-abi/definitions.py b/design/mvp/canonical-abi/definitions.py index dcdd37c9..758bea24 100644 --- a/design/mvp/canonical-abi/definitions.py +++ b/design/mvp/canonical-abi/definitions.py @@ -249,6 +249,7 @@ def size_record(fields): for f in fields: s = align_to(s, alignment(f.t)) s += size(f.t) + trap_if(s == 0) return align_to(s, alignment_record(fields)) def align_to(ptr, alignment): @@ -266,7 +267,7 @@ def size_variant(cases): def size_flags(labels): n = len(labels) - if n == 0: return 0 + trap_if(n == 0) if n <= 8: return 1 if n <= 16: return 2 return 4 * num_i32_flags(labels) From 1f6c4b20f0a951925ef37eb5dbfff1882ba16e44 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 19 Jul 2023 08:38:18 -0700 Subject: [PATCH 2/9] Commented out empty record and flags tests. --- design/mvp/canonical-abi/run_tests.py | 29 ++++++++++++++++----------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/design/mvp/canonical-abi/run_tests.py b/design/mvp/canonical-abi/run_tests.py index 4539a2e5..5cd7485a 100644 --- a/design/mvp/canonical-abi/run_tests.py +++ b/design/mvp/canonical-abi/run_tests.py @@ -96,11 +96,13 @@ def test_name(): if not equal_modulo_string_encoding(got, lower_v): fail("{} re-lift expected {} but got {}".format(test_name(), lower_v, got)) -test(Record([]), [], {}) +# Empty record types are not permitted yet. +#test(Record([]), [], {}) test(Record([Field('x',U8()), Field('y',U16()), Field('z',U32())]), [1,2,3], {'x':1,'y':2,'z':3}) test(Tuple([Tuple([U8(),U8()]),U8()]), [1,2,3], {'0':{'0':1,'1':2},'1':3}) -t = Flags([]) -test(t, [], {}) +# Empty flags types are not permitted yet. +#t = Flags([]) +#test(t, [], {}) t = Flags(['a','b']) test(t, [0], {'a':False,'b':False}) test(t, [2], {'a':False,'b':True}) @@ -242,7 +244,8 @@ def test_heap(t, expect, args, byte_array): cx = mk_cx(heap.memory) test(t, args, expect, cx) -test_heap(List(Record([])), [{},{},{}], [0,3], []) +# Empty record types are not permitted yet. +#test_heap(List(Record([])), [{},{},{}], [0,3], []) test_heap(List(Bool()), [True,False,True], [0,3], [1,0,1]) test_heap(List(Bool()), [True,False,True], [0,3], [1,0,2]) test_heap(List(Bool()), [True,False,True], [3,3], [0xff,0xff,0xff, 1,0,1]) @@ -276,8 +279,9 @@ def test_heap(t, expect, args, byte_array): [6,0, 7, 0x0ff, 8,0, 9, 0xff]) test_heap(List(Tuple([Tuple([U16(),U8()]),U8()])), [mk_tup([4,5],6),mk_tup([7,8],9)], [0,2], [4,0, 5,0xff, 6,0xff, 7,0, 8,0xff, 9,0xff]) -test_heap(List(Union([Record([]),U8(),Tuple([U8(),U16()])])), [{'0':{}}, {'1':42}, {'2':mk_tup(6,7)}], [0,3], - [0,0xff,0xff,0xff,0xff,0xff, 1,0xff,42,0xff,0xff,0xff, 2,0xff,6,0xff,7,0]) +# Empty record types are not permitted yet. +#test_heap(List(Union([Record([]),U8(),Tuple([U8(),U16()])])), [{'0':{}}, {'1':42}, {'2':mk_tup(6,7)}], [0,3], +# [0,0xff,0xff,0xff,0xff,0xff, 1,0xff,42,0xff,0xff,0xff, 2,0xff,6,0xff,7,0]) test_heap(List(Union([U32(),U8()])), [{'0':256}, {'1':42}], [0,2], [0,0xff,0xff,0xff,0,1,0,0, 1,0xff,0xff,0xff,42,0xff,0xff,0xff]) test_heap(List(Tuple([Union([U8(),Tuple([U16(),U8()])]),U8()])), @@ -285,12 +289,13 @@ def test_heap(t, expect, args, byte_array): [1,0xff,5,0,6,0xff,7,0xff, 0,0xff,8,0xff,0xff,0xff,9,0xff]) test_heap(List(Union([U8()])), [{'0':6},{'0':7},{'0':8}], [0,3], [0,6, 0,7, 0,8]) -t = List(Flags([])) -test_heap(t, [{},{},{}], [0,3], - []) -t = List(Tuple([Flags([]), U8()])) -test_heap(t, [mk_tup({}, 42), mk_tup({}, 43), mk_tup({}, 44)], [0,3], - [42,43,44]) +# Empty flags types are not permitted yet. +#t = List(Flags([])) +#test_heap(t, [{},{},{}], [0,3], +# []) +#t = List(Tuple([Flags([]), U8()])) +#test_heap(t, [mk_tup({}, 42), mk_tup({}, 43), mk_tup({}, 44)], [0,3], +# [42,43,44]) t = List(Flags(['a','b'])) test_heap(t, [{'a':False,'b':False},{'a':False,'b':True},{'a':True,'b':True}], [0,3], [0,2,3]) From 7b8a515df1a2809fd9c9b45672d10d8dbd10e06e Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 19 Jul 2023 10:19:30 -0700 Subject: [PATCH 3/9] Require `flags`, `record`, and `tuple` to be non-empty. --- design/mvp/Binary.md | 6 +++--- design/mvp/Explainer.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/design/mvp/Binary.md b/design/mvp/Binary.md index fe838ea4..878f7c8d 100644 --- a/design/mvp/Binary.md +++ b/design/mvp/Binary.md @@ -196,11 +196,11 @@ primvaltype ::= 0x7f => bool | 0x74 => char | 0x73 => string defvaltype ::= pvt: => pvt - | 0x72 lt*:vec() => (record (field lt)*) + | 0x72 lt*:vec() => (record (field lt)+) | 0x71 case*:vec() => (variant case*) | 0x70 t: => (list t) - | 0x6f t*:vec() => (tuple t*) - | 0x6e l*:vec(