dwarf-reader: Bug 29811 - Support updating of variable type
authorDodji Seketeli <dodji@redhat.com>
Fri, 30 Dec 2022 21:15:46 +0000 (22:15 +0100)
committerDodji Seketeli <dodji@redhat.com>
Sun, 1 Jan 2023 17:21:56 +0000 (18:21 +0100)
Let's look at the source code reported at
https://sourceware.org/bugzilla/show_bug.cgi?id=29811:

    extern unsigned int is_basic_table[];

    unsigned int is_basic_table[] = {0};

Let's look at the DWARF output from GCC.  The variable is_basic_table
is described by the DIE at offset 0x51:

 [    51]    variable             abbrev: 7
             specification        (ref4) [    2f]
             decl_line            (data1) 3
             decl_column          (data1) 14
             type                 (ref4) [    3b]
             location             (exprloc)
              [ 0] addr .bss+0 <is_basic_table>

The type of the variable is defined at the offset 0x3b:

 [    3b]    array_type           abbrev: 1
             type                 (ref4) [    29]
             sibling              (ref4) [    4b]
 [    44]      subrange_type        abbrev: 6
               type                 (ref4) [    4b]
               upper_bound          (data1) 0

But then, we see that the DIE at 0x51 has a DW_AT_specification
attribute that refers to the DIE at offset 0x2f:

 [    2f]    variable             abbrev: 5
             name                 (strp) "is_basic_table"
             decl_file            (data1) test-v2.c (1)
             decl_line            (data1) 1
             decl_column          (data1) 21
             type                 (ref4) [    1e]
             external             (flag_present) yes
             declaration          (flag_present) yes

That DIE at offset 0x2f represents the first external variable
declared in the source code.  It's type is an array defined at offset
0x1e:

 [    1e]    array_type           abbrev: 1
             type                 (ref4) [    29]
             sibling              (ref4) [    29]
 [    27]      subrange_type        abbrev: 4

This array has one dimension of 'unknown' size; this is because the
dimension is described by the DIE at offset 0x27 of kind
DW_TAG_subrange_type and has no DW_AT_upper_bound DIE.

But then, I said earlier, the real type of the is_basic_table variable
is the DIE at offset 0x3b, which is an array which single dimension
described by the DIE at offset 0x44 of kind DW_TAG_subrange_type with
a DW_AT_upper_bound attribute of value 0.

Let's see the output of abidw on this program, from the DWARF debug info:

     1 <abi-corpus version='2.1' path='test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o' architecture='elf-amd-x86_64'>
     2   <elf-variable-symbols>
     3     <elf-symbol name='is_basic_table' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     4   </elf-variable-symbols>
     5   <abi-instr address-size='64' path='test-PR29811-unknown-size-array-dwarf-ctf.c' comp-dir-path='/home/dodji/git/libabigail/PR29811/prtests' language='LANG_C11'>
     6     <type-decl name='unsigned int' size-in-bits='32' id='type-id-1'/>
     7     <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='32' id='type-id-2'>
     8       <subrange length='1' type-id='type-id-3' id='type-id-4'/>
     9     </array-type-def>
    10     <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='infinite' id='type-id-5'>
    11       <subrange length='infinite' id='type-id-6'/>
    12     </array-type-def>
    13     <type-decl name='unsigned long int' size-in-bits='64' id='type-id-3'/>
    14     <var-decl name='is_basic_table' type-id='type-id-5' mangled-name='is_basic_table' visibility='default' filepath='/home/dodji/git/libabigail/PR29811/prtests/test-PR29811-unknown-size-array-dwarf-ctf.c' line='10' column='1' elf-symbol-id='is_basic_table'/>
    15   </abi-instr>
    16 </abi-corpus>

The variable is_basic_table is described by the element at line 14:

    14     <var-decl name='is_basic_table' type-id='type-id-5' mangled-name='is_basic_table' visibility='default' filepath='/home/dodji/git/libabigail/PR29811/prtests/test-PR29811-unknown-size-array-dwarf-ctf.c' line='10' column='1' elf-symbol-id='is_basic_table'/>

Its type has the ID 'type-id-5' which is defined at line 10:

    10     <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='infinite' id='type-id-5'>
    11       <subrange length='infinite' id='type-id-6'/>
    12     </array-type-def>

Which has an unknown size.

But the, at line 7, there is another array type defined with a size of
32 bits:

     7     <array-type-def dimensions='1' type-id='type-id-1' size-in-bits='32' id='type-id-2'>
     8       <subrange length='1' type-id='type-id-3' id='type-id-4'/>
     9     </array-type-def>

So, libabigail links the is_basic_table variable to the wrong array
type.

This is because when the DWARF reader builds the internal
representation for the DW_TAG_variable DIE at offset 0x51, it first
builds it with the type (and the other properties such as the name for
instance) of the "declaration" DIE specified by the
DW_AT_specification attribute.  But then, this DW_TAG_variable DIE has
its own type at offset 0x3b ; libabigail should update the internal
representation it just built to set the type to the one referred to at
offset 0x3b.  It's that updating that is not being done.  So the
variable wrongly points to the type of the "declaration" DIE at offset
0x2f.

This patch fixes build_var_decl to make it update the type of the
variable when necessary.

* include/abg-ir.h (var_decl::set_type): Declare new member
function.
* src/abg-ir.cc (var_decl::priv::set_type): Define new member
function.
(var_decl::set_type): Likewise.
* src/abg-dwarf-reader.cc (build_var_decl): In "updating mode",
update the type of the variable as well.
* tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-CTF.o:
Add new test binary input.
* tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o:
Likewise.
* tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-report.txt:
Add test reference output.
* tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf.c:
Add source code of the new test binary input.
* tests/data/Makefile.am: Add the new files above to source
distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the input binaries
to the test harness.
* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Adjust.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
include/abg-ir.h
src/abg-dwarf-reader.cc
src/abg-ir.cc
tests/data/Makefile.am
tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi
tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-CTF.o [new file with mode: 0644]
tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o [new file with mode: 0644]
tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-report.txt [new file with mode: 0644]
tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf.c [new file with mode: 0644]
tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi
tests/test-diff-filter.cc

index ada61e9feaa08b645a08bbec50b2446694d9594e..93959e92feec5f9143a7497fa1a450150bedbbf6 100644 (file)
@@ -2910,6 +2910,9 @@ public:
   const type_base_sptr
   get_type() const;
 
+  void
+  set_type(type_base_sptr&);
+
   const type_base*
   get_naked_type() const;
 
index 4d581ee510dc7768d5a55577223176b96973f2c8..d2848a30862ad83347e08c000cb88c68ae3bd99e 100644 (file)
@@ -14447,7 +14447,7 @@ build_var_decl(reader&  rdr,
       ABG_ASSERT(type);
     }
 
-  if (!type)
+  if (!type && !result)
     return result;
 
   string name, linkage_name;
@@ -14460,9 +14460,12 @@ build_var_decl(reader& rdr,
     {
       // We were called to append properties that might have been
       // missing from the first version of the variable.  And usually
-      // that missing property is the mangled name.
+      // that missing property is the mangled name or the type.
       if (!linkage_name.empty())
        result->set_linkage_name(linkage_name);
+
+      if (type)
+       result->set_type(type);
     }
 
   // Check if a variable symbol with this name is exported by the elf
index 6e41d8497da7097abb44859d96f5d034040c482a..33bd3a0e5466e8e9204bebfeb1bc57c664df6e4e 100644 (file)
@@ -18900,6 +18900,16 @@ struct var_decl::priv
       naked_type_(t.get()),
       binding_(b)
   {}
+
+  /// Setter of the type of the variable.
+  ///
+  /// @param t the new variable type.
+  void
+  set_type(type_base_sptr t)
+  {
+    type_ = t;
+    naked_type_ = t.get();
+  }
 }; // end struct var_decl::priv
 
 /// Constructor of the @ref var_decl type.
@@ -18936,6 +18946,13 @@ const type_base_sptr
 var_decl::get_type() const
 {return priv_->type_.lock();}
 
+/// Setter of the type of the variable.
+///
+/// @param the new type of the variable.
+void
+var_decl::set_type(type_base_sptr& t)
+{priv_->set_type(t);}
+
 /// Getter of the type of the variable.
 ///
 /// This getter returns a bare pointer, as opposed to a smart pointer.
index 0f1f4e2696b6bf57c4ef84bf7771a71cbe9c7fb3..c995930f337ec981a2db2bb022a61afa4e6727da 100644 (file)
@@ -1131,6 +1131,10 @@ test-diff-filter/test-PR29387-v0.c        \
 test-diff-filter/test-PR29387-v1.o      \
 test-diff-filter/test-PR29387-v0.o      \
 test-diff-filter/test-PR29387-report.txt \
+test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-CTF.o \
+test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o \
+test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-report.txt \
+test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf.c \
 \
 test-diff-suppr/test0-type-suppr-v0.cc \
 test-diff-suppr/test0-type-suppr-v1.cc \
index cdecfae73e28280dd22dcbb6e76aad3e6324fed9..2b19b1dedd2af348c229df532160222b66f2c4e2 100644 (file)
         </data-member>
         <data-member access='private' static='yes'>
           <!-- static tcmalloc::PageHeapAllocator<tcmalloc::Span> tcmalloc::Static::span_allocator_ -->
-          <var-decl name='span_allocator_' type-id='type-id-1507' mangled-name='_ZN8tcmalloc6Static15span_allocator_E' visibility='default' filepath='src/static_vars.h' line='99' column='1' elf-symbol-id='_ZN8tcmalloc6Static15span_allocator_E'/>
+          <var-decl name='span_allocator_' type-id='type-id-1281' mangled-name='_ZN8tcmalloc6Static15span_allocator_E' visibility='default' filepath='src/static_vars.h' line='99' column='1' elf-symbol-id='_ZN8tcmalloc6Static15span_allocator_E'/>
         </data-member>
         <data-member access='private' static='yes'>
           <!-- static tcmalloc::PageHeapAllocator<tcmalloc::StackTrace> tcmalloc::Static::stacktrace_allocator_ -->
-          <var-decl name='stacktrace_allocator_' type-id='type-id-1510' mangled-name='_ZN8tcmalloc6Static21stacktrace_allocator_E' visibility='default' filepath='src/static_vars.h' line='100' column='1' elf-symbol-id='_ZN8tcmalloc6Static21stacktrace_allocator_E'/>
+          <var-decl name='stacktrace_allocator_' type-id='type-id-1147' mangled-name='_ZN8tcmalloc6Static21stacktrace_allocator_E' visibility='default' filepath='src/static_vars.h' line='100' column='1' elf-symbol-id='_ZN8tcmalloc6Static21stacktrace_allocator_E'/>
         </data-member>
         <data-member access='private' static='yes'>
           <!-- static tcmalloc::Span tcmalloc::Static::sampled_objects_ -->
         </data-member>
         <data-member access='private' static='yes'>
           <!-- static tcmalloc::PageHeapAllocator<tcmalloc::StackTraceTable::Bucket> tcmalloc::Static::bucket_allocator_ -->
-          <var-decl name='bucket_allocator_' type-id='type-id-1554' mangled-name='_ZN8tcmalloc6Static17bucket_allocator_E' visibility='default' filepath='src/static_vars.h' line='102' column='1' elf-symbol-id='_ZN8tcmalloc6Static17bucket_allocator_E'/>
+          <var-decl name='bucket_allocator_' type-id='type-id-1305' mangled-name='_ZN8tcmalloc6Static17bucket_allocator_E' visibility='default' filepath='src/static_vars.h' line='102' column='1' elf-symbol-id='_ZN8tcmalloc6Static17bucket_allocator_E'/>
         </data-member>
         <data-member access='private' static='yes'>
           <!-- static tcmalloc::StackTrace* tcmalloc::Static::growth_stacks_ -->
diff --git a/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-CTF.o b/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-CTF.o
new file mode 100644 (file)
index 0000000..da169cb
Binary files /dev/null and b/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-CTF.o differ
diff --git a/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o b/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o
new file mode 100644 (file)
index 0000000..94f5ca4
Binary files /dev/null and b/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o differ
diff --git a/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-report.txt b/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-report.txt
new file mode 100644 (file)
index 0000000..e69de29
diff --git a/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf.c b/tests/data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf.c
new file mode 100644 (file)
index 0000000..3e86a5d
--- /dev/null
@@ -0,0 +1,12 @@
+/*
+   Compile this file twice.  Once with DWARF debug info and once with
+   CTF debug info:
+
+   gcc -g -c test-PR29811-unknown-size-array-dwarf-ctf.c -o test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o
+
+  gcc -gctf -c test-PR29811-unknown-size-array-dwarf-ctf.c -o test-PR29811-unknown-size-array-dwarf-ctf-CTF.o
+
+*/
+extern unsigned int is_basic_table[];
+
+unsigned int is_basic_table[] = {0};
index 92a0b7757d611a77cc534b01a51b467779197a0e..8b468a5b11ae4868401f9f39aa1d8d873869995a 100644 (file)
           <var-decl name='central_cache_' type-id='type-id-1431' mangled-name='_ZN8tcmalloc6Static14central_cache_E' visibility='default' filepath='src/static_vars.h' line='98' column='1' elf-symbol-id='_ZN8tcmalloc6Static14central_cache_E'/>
         </data-member>
         <data-member access='private' static='yes'>
-          <var-decl name='span_allocator_' type-id='type-id-1507' mangled-name='_ZN8tcmalloc6Static15span_allocator_E' visibility='default' filepath='src/static_vars.h' line='99' column='1' elf-symbol-id='_ZN8tcmalloc6Static15span_allocator_E'/>
+          <var-decl name='span_allocator_' type-id='type-id-1281' mangled-name='_ZN8tcmalloc6Static15span_allocator_E' visibility='default' filepath='src/static_vars.h' line='99' column='1' elf-symbol-id='_ZN8tcmalloc6Static15span_allocator_E'/>
         </data-member>
         <data-member access='private' static='yes'>
-          <var-decl name='stacktrace_allocator_' type-id='type-id-1510' mangled-name='_ZN8tcmalloc6Static21stacktrace_allocator_E' visibility='default' filepath='src/static_vars.h' line='100' column='1' elf-symbol-id='_ZN8tcmalloc6Static21stacktrace_allocator_E'/>
+          <var-decl name='stacktrace_allocator_' type-id='type-id-1147' mangled-name='_ZN8tcmalloc6Static21stacktrace_allocator_E' visibility='default' filepath='src/static_vars.h' line='100' column='1' elf-symbol-id='_ZN8tcmalloc6Static21stacktrace_allocator_E'/>
         </data-member>
         <data-member access='private' static='yes'>
           <var-decl name='sampled_objects_' type-id='type-id-144' mangled-name='_ZN8tcmalloc6Static16sampled_objects_E' visibility='default' filepath='src/static_vars.h' line='101' column='1' elf-symbol-id='_ZN8tcmalloc6Static16sampled_objects_E'/>
         </data-member>
         <data-member access='private' static='yes'>
-          <var-decl name='bucket_allocator_' type-id='type-id-1554' mangled-name='_ZN8tcmalloc6Static17bucket_allocator_E' visibility='default' filepath='src/static_vars.h' line='102' column='1' elf-symbol-id='_ZN8tcmalloc6Static17bucket_allocator_E'/>
+          <var-decl name='bucket_allocator_' type-id='type-id-1305' mangled-name='_ZN8tcmalloc6Static17bucket_allocator_E' visibility='default' filepath='src/static_vars.h' line='102' column='1' elf-symbol-id='_ZN8tcmalloc6Static17bucket_allocator_E'/>
         </data-member>
         <data-member access='private' static='yes'>
           <var-decl name='growth_stacks_' type-id='type-id-1560' mangled-name='_ZN8tcmalloc6Static14growth_stacks_E' visibility='default' filepath='src/static_vars.h' line='108' column='1' elf-symbol-id='_ZN8tcmalloc6Static14growth_stacks_E'/>
index 37966d5aa6a0ac8af4b0dbcde095da07f437fba9..ac7855da23561a88be8ccc5fcdad767a0511f2e7 100644 (file)
@@ -822,6 +822,15 @@ InOutSpec in_out_specs[] =
    "data/test-diff-filter/test-PR29387-report.txt",
    "output/test-diff-filter/test-PR29387-report.txt",
   },
+#ifdef WITH_CTF
+  {
+   "data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o",
+   "data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-CTF.o",
+   "--ctf --no-default-suppression",
+   "data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-report.txt",
+   "output/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-report.txt",
+  },
+#endif
   // This should be the last entry
   {NULL, NULL, NULL, NULL, NULL}
 };