Re: [PATCH 5/5] ctf-front-end: Fix array size representation

[Dodji Seketeli <dodji@seketeli.org>]

Hello Guillermo,

"Guillermo E. Martinez via Libabigail" <libabigail@sourceware.org> a
écrit:

> Array with a single element is represented as infinite array.
>
> <array-type-def dimensions='1' ... size-in-bits='infinite' ...>
>     <subrange length='infinite' type-id='type-id-3' .../>
> </array-type-def>

Hmmh, actually I think it's a little bit more involved than that.

This case is for "variable length arrays", for instance, defined as:

int a[];

There is also an extension known as "zero-length arrays", supported by
GCC and various other compilers since forever:
https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html.  It's an extension
because strictly speaking an array can't have a zero size in C.

I see the "zero-length array" as another way to specify a variable
length array in the context of the "flexible array member" idiom:
https://en.wikipedia.org/wiki/Flexible_array_member.  Those were
standardized in C99, by the way.

So, it's those types of array that are represented in the libabigail IR
as being of "non known size".  The IR uses the "infinite" keyword,
similarly to what DWARF does.

I have thus updated the introductory paragraphs of the commit log accordingly.
>
> 	* src/abg-ctf-reader.cc (build_array_ctf_range): Use
> 	* tests/data/Makefile.am: Add new test.
> 	`upper_bound' and number of elements to indicate infinite
> 	array size.
> 	* tests/data/test-read-ctf/test-array-size.abi: New test.
> 	* tests/data/test-read-ctf/test-array-size.c: Likewise.
> 	* tests/data/test-read-ctf/test-array-size.o: Likewise.
> 	* tests/test-read-ctf.cc: Update testsuite.
>
> Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>

The patch otherwise looks good to me.  Please find below what was
actually applied to the master repository.

[...]

Cheers,

From 77ae31417697dde8d3119e042dafd7dddb9538e1 Mon Sep 17 00:00:00 2001
From: "Guillermo E. Martinez via Libabigail" <libabigail@sourceware.org>
Date: Wed, 16 Nov 2022 21:43:05 -0600
Subject: [PATCH] ctf-reader: Fix array size representation

A variable length array (VLA), or a flexible array member (with its
size set to zero to recognize such cases) is represented in the
libabigail IR as an array with "non-finite" length.  This is a way to
say that the length of the array is not statically known.

The ABIXML array-type-def element looks like:

<array-type-def dimensions='1' ... size-in-bits='infinite' ...>
    <subrange length='infinite' type-id='type-id-3' .../>
</array-type-def>

The patch teaches the ctf-reader to correctly set the size of the
array for VLAs.

	* src/abg-ctf-reader.cc (build_array_ctf_range): Use
	* tests/data/Makefile.am: Add new test.
	`upper_bound' and number of elements to indicate infinite
	array size.
	* tests/data/test-read-ctf/test-array-size.abi: New test.
	* tests/data/test-read-ctf/test-array-size.c: Likewise.
	* tests/data/test-read-ctf/test-array-size.o: Likewise.
	* tests/test-read-ctf.cc: Update testsuite.

Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 src/abg-ctf-reader.cc                        |   2 +-
 tests/data/Makefile.am                       |   3 +++
 tests/data/test-read-ctf/test-array-size.abi |  23 +++++++++++++++++++
 tests/data/test-read-ctf/test-array-size.c   |   4 ++++
 tests/data/test-read-ctf/test-array-size.o   | Bin 0 -> 1432 bytes
 tests/data/test-read-ctf/test9.o.abi         |   4 ++--
 tests/test-read-ctf.cc                       |   9 ++++++++
 7 files changed, 42 insertions(+), 3 deletions(-)
 create mode 100644 tests/data/test-read-ctf/test-array-size.abi
 create mode 100644 tests/data/test-read-ctf/test-array-size.c
 create mode 100644 tests/data/test-read-ctf/test-array-size.o

diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc
index b4b957d2..e0b8bfb1 100644
--- a/src/abg-ctf-reader.cc
+++ b/src/abg-ctf-reader.cc
@@ -1194,7 +1194,7 @@ build_array_ctf_range(reader *rdr, ctf_dict_t *dic,
   upper_bound.set_unsigned(nelems > 0 ? nelems - 1 : 0U);
 
   /* for VLAs number of array elements is 0 */
-  if (upper_bound.get_unsigned_value() == 0)
+  if (upper_bound.get_unsigned_value() == 0 && nelems == 0)
     is_infinite = true;
 
   subrange.reset(new array_type_def::subrange_type(rdr->env(),
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index d04ecf3f..e994aff3 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -716,6 +716,9 @@ test-read-ctf/test-const-array.o	\
 test-read-ctf/test-array-mdimension.abi	\
 test-read-ctf/test-array-mdimension.c	\
 test-read-ctf/test-array-mdimension.o	\
+test-read-ctf/test-array-size.abi	\
+test-read-ctf/test-array-size.c		\
+test-read-ctf/test-array-size.o		\
 \
 test-annotate/test0.abi			\
 test-annotate/test1.abi			\
diff --git a/tests/data/test-read-ctf/test-array-size.abi b/tests/data/test-read-ctf/test-array-size.abi
new file mode 100644
index 00000000..3fbc65b6
--- /dev/null
+++ b/tests/data/test-read-ctf/test-array-size.abi
@@ -0,0 +1,23 @@
+<abi-corpus version='2.1' path='data/test-read-ctf/test-array-size.o'>
+  <elf-variable-symbols>
+    <elf-symbol name='bar' size='1' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='baz' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='foo' size='2' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+  </elf-variable-symbols>
+  <abi-instr address-size='64' language='LANG_C'>
+    <type-decl name='char' size-in-bits='8' id='type-id-1'/>
+    <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='8' id='type-id-2'>
+      <subrange length='1' type-id='type-id-3' id='type-id-4'/>
+    </array-type-def>
+    <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='16' id='type-id-5'>
+      <subrange length='2' type-id='type-id-3' id='type-id-6'/>
+    </array-type-def>
+    <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='infinite' id='type-id-7'>
+      <subrange length='infinite' type-id='type-id-3' id='type-id-8'/>
+    </array-type-def>
+    <type-decl name='unsigned long int' size-in-bits='64' id='type-id-3'/>
+    <var-decl name='bar' type-id='type-id-2' mangled-name='bar' visibility='default' elf-symbol-id='bar'/>
+    <var-decl name='baz' type-id='type-id-7' mangled-name='baz' visibility='default' elf-symbol-id='baz'/>
+    <var-decl name='foo' type-id='type-id-5' mangled-name='foo' visibility='default' elf-symbol-id='foo'/>
+  </abi-instr>
+</abi-corpus>
diff --git a/tests/data/test-read-ctf/test-array-size.c b/tests/data/test-read-ctf/test-array-size.c
new file mode 100644
index 00000000..fe2f3fef
--- /dev/null
+++ b/tests/data/test-read-ctf/test-array-size.c
@@ -0,0 +1,4 @@
+/* gcc -gctf -c test-array-size.c -o test-array-size.o */
+char foo[2];
+char bar[1];
+char baz[0];
diff --git a/tests/data/test-read-ctf/test-array-size.o b/tests/data/test-read-ctf/test-array-size.o
new file mode 100644
index 0000000000000000000000000000000000000000..8e4d8b147bcf9ae2fcfe3d6f2caca05868f6b341
GIT binary patch
literal 1432
zcmbVM&2G~`5T29}elMj5kU+vnNVEd4op7QDWT}Em2ysK=!eQ-f6AQ<#?5!I20*}Ev
zaN!Yn3~ulW@B%QOvzx3};lfDc`R1E%Ry$)SA78wFxic7$wE;b+O=el5?>ifubTFYC
zbe(?x7_KW1HxjzZ7=Fii%<(h3W*@R&V!zCOj~$x8VAOz70}q|K&j|4<QzMRSBgT{f
zt?U*8G`P_EQbax8oCRr*2SM3f)>KsGEI2Ehd{(Myke8ZHs)|ysr1FBId9_ec+NNz(
z)iNto)YMs2<f%;anaqo@ROgXajc%f;)UtzNt>iS!^vNodbuHVl$uE@1h>nkr4uk#U
zH*XJu@lz3tIGDtf$#@(;3-;CWRMq)Hm0A`D-3`0l1q`ttZsIuy->@@l1Cu**+EOo~
zKBQlNdL$=%?7Q98R~5U*+lIHfJ^!xZ@t%5y9fZA6-8GVE+J%-WM_sSt+#;=(ngq5-
zB5fKHyg7bU3oL>}S!pGB^svz~`(W}}c_vQlir1yv-ZlO=!CaWpd~54tk~QiByAR7g
zU;xhFOJW6H^!p!i9pCrA`rUVqqx}9STR7+vJlGPi!Aoh(8gTgCy7tKkAG;)D+i&Bc
z!@1HgR^XCbw6B=E!fwAeEB|Wx{d-|ou0iJaqW;tLFOB$`(b<0d=fJ@Gy9fWy9C{y#
UWBypXT-(-vZu;Lly?}oIUzApI#Q*>R

literal 0
HcmV?d00001

diff --git a/tests/data/test-read-ctf/test9.o.abi b/tests/data/test-read-ctf/test9.o.abi
index 9f5a7466..5c89f821 100644
--- a/tests/data/test-read-ctf/test9.o.abi
+++ b/tests/data/test-read-ctf/test9.o.abi
@@ -7,8 +7,8 @@
     <array-type-def dimensions='1' type-id='type-id-2' size-in-bits='448' alignment-in-bits='64' id='type-id-3'>
       <subrange length='7' type-id='type-id-4' id='type-id-5'/>
     </array-type-def>
-    <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='infinite' id='type-id-6'>
-      <subrange length='infinite' type-id='type-id-4' id='type-id-7'/>
+    <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='8' id='type-id-6'>
+      <subrange length='1' type-id='type-id-4' id='type-id-7'/>
     </array-type-def>
     <type-decl name='double' size-in-bits='64' id='type-id-8'/>
     <array-type-def dimensions='2' type-id='type-id-8' size-in-bits='960' id='type-id-9'>
diff --git a/tests/test-read-ctf.cc b/tests/test-read-ctf.cc
index f3d46118..236ccbf2 100644
--- a/tests/test-read-ctf.cc
+++ b/tests/test-read-ctf.cc
@@ -381,6 +381,15 @@ static InOutSpec in_out_specs[] =
     "output/test-read-ctf/test-array-mdimension.abi",
     "--ctf",
   },
+  {
+    "data/test-read-ctf/test-array-size.o",
+    "",
+    "",
+    SEQUENCE_TYPE_ID_STYLE,
+    "data/test-read-ctf/test-array-size.abi",
+    "output/test-read-ctf/test-array-size.abi",
+    "--ctf",
+  },
   // This should be the last entry.
   {NULL, NULL, NULL, SEQUENCE_TYPE_ID_STYLE, NULL, NULL, NULL}
 };
-- 
2.38.1


-- 
		Dodji