RE: [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions

["Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>]

Kindly pinging.

Thanks
-Baris


On Monday, February 15, 2021 4:52 PM, Aktemur, Tankut Baris wrote:
> GDB reports duplicate local vars with "<optimized out>" values for
> inlined functions that are compiled with Clang.
> 
> Suppose we have
> 
>   __attribute__((always_inline))
>   static void aFunction() {
>     int a = 42;
>     if(a > 2) {
>       int value = a;
>       value += 10; /* break here */
>     }
>   }
> 
> The "info locals" command at the "break here" line gives the following
> output:
> 
>   ...
>   Breakpoint 1, aFunction () at test.c:6
>   6           value += 10; /* break here */
>   (gdb) info locals
>   value = 42
>   a = 42
>   value = <optimized out>
>   (gdb)
> 
> The reason is, inlined functions that are compiled by Clang do not
> contain DW_AT_abstract_origin attributes in the DW_TAG_lexical_block
> entries.  E.g. the DIE of the inlined function above is
> 
> 0x00000087:     DW_TAG_inlined_subroutine
>                   DW_AT_abstract_origin (0x0000002a "aFunction")
>                   DW_AT_low_pc  (0x00000000004004b2)
>                   DW_AT_high_pc (0x00000000004004d2)
>                   DW_AT_call_file       ("/tmp/test.c")
>                   DW_AT_call_line       (11)
>                   DW_AT_call_column     (0x03)
> 
> 0x0000009b:       DW_TAG_variable
>                     DW_AT_location      (DW_OP_fbreg -4)
>                     DW_AT_abstract_origin       (0x00000032 "a")
> 
> 0x000000a3:       DW_TAG_lexical_block
>                     DW_AT_low_pc        (0x00000000004004c3)
>                     DW_AT_high_pc       (0x00000000004004d2)
> 
> 0x000000b0:         DW_TAG_variable
>                       DW_AT_location    (DW_OP_fbreg -8)
>                       DW_AT_abstract_origin     (0x0000003e "value")
> 
> This causes GDB to fail matching the concrete lexical scope with the
> corresponding abstract entry.  Hence, the locals vars of the abstract
> function that are contained in the lexical scope are read separately
> (and thus, in addition to) the locals vars of the concrete scope.
> Because the abstract definition of the vars do not contain location
> information, we see the extra 'value = <optimized out>' above.
> 
> This bug is highly related to PR gdb/25695, but the root cause is not
> exactly the same.  In PR gdb/25695, GCC emits an extra
> DW_TAG_lexical_block without an DW_AT_abstract_origin that wraps the
> body of the inlined function.  That is, the trees of the abstract DIE
> for the function and its concrete instance are structurally not the
> same.  In the case of using Clang, the trees have the same structure.
> 
> To tackle the Clang case, when traversing the children of the concrete
> instance root, keep a reference to the child of the abstract DIE that
> corresponds to the concrete child, so that we can match the two DIEs
> heuristically in case of missing DW_AT_abstract_origin attributes.
> 
> The updated gdb.opt/inline-locals.exp test has been checked with GCC
> 5-10 and Clang 5-11.
> 
> gdb/ChangeLog:
> 2021-02-15  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> 
> 	* dwarf2/read.c (inherit_abstract_dies): Keep a reference to the
> 	corresponding child of the abstract DIE when iterating the
> 	children of the concrete DIE.
> 
> gdb/testsuite/ChangeLog:
> 2021-02-15  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> 
> 	* gdb.opt/inline-locals.c (scoped): New function.
> 	(main): Call 'scoped'.
> 	* gdb.opt/inline-locals.exp: Update with "info locals" tests
> 	for scoped variables.
> ---
>  gdb/dwarf2/read.c                       | 48 ++++++++++++++++++++++++-
>  gdb/testsuite/gdb.opt/inline-locals.c   | 20 +++++++++++
>  gdb/testsuite/gdb.opt/inline-locals.exp | 30 ++++++++++++++++
>  3 files changed, 97 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 51bf0fbeea5..11696cebd96 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -13536,6 +13536,36 @@ inherit_abstract_dies (struct die_info *die, struct dwarf2_cu *cu)
>  	       sect_offset_str (die->sect_off),
>  	       sect_offset_str (origin_die->sect_off));
> 
> +  /* Find if the concrete and abstract trees are structurally the
> +     same.  This is a shallow traversal and it is not bullet-proof;
> +     the compiler can trick the debugger into believing that the trees
> +     are isomorphic, whereas they actually are not.  However, the
> +     likelyhood of this happening is pretty low, I think, and a
> +     full-fledged check would be an overkill.  */
> +  bool are_isomorphic = true;
> +  die_info *concrete_child = die->child;
> +  die_info *abstract_child = origin_die->child;
> +  while (concrete_child != nullptr || abstract_child != nullptr)
> +    {
> +      if (concrete_child == nullptr
> +	  || abstract_child == nullptr
> +	  || concrete_child->tag != abstract_child->tag)
> +	{
> +	  are_isomorphic = false;
> +	  break;
> +	}
> +
> +      concrete_child = concrete_child->sibling;
> +      abstract_child = abstract_child->sibling;
> +    }
> +
> +  /* Walk the origin's children in parallel to the concrete children.
> +     This helps match an origin child in case the debug info misses
> +     DW_AT_abstract_origin attributes.  Keep in mind that the abstract
> +     origin tree may not have the same tree structure as the concrete
> +     DIE, though.  */
> +  die_info *corresponding_abstract_child = origin_die->child;
> +
>    std::vector<sect_offset> offsets;
> 
>    for (child_die = die->child;
> @@ -13552,7 +13582,12 @@ inherit_abstract_dies (struct die_info *die, struct dwarf2_cu *cu)
>  	 one.  */
>        if (child_die->tag == DW_TAG_call_site
>  	  || child_die->tag == DW_TAG_GNU_call_site)
> -	continue;
> +	{
> +	  if (are_isomorphic)
> +	    corresponding_abstract_child
> +	      = corresponding_abstract_child->sibling;
> +	  continue;
> +	}
> 
>        /* For each CHILD_DIE, find the corresponding child of
>  	 ORIGIN_DIE.  If there is more than one layer of
> @@ -13571,6 +13606,14 @@ inherit_abstract_dies (struct die_info *die, struct dwarf2_cu *cu)
>  					     &child_origin_cu);
>  	}
> 
> +      /* If missing DW_AT_abstract_origin, try the corresponding child
> +	 of the origin.  Clang emits such lexical scopes.  */
> +      if (child_origin_die == child_die
> +	  && dwarf2_attr (child_die, DW_AT_abstract_origin, cu) == nullptr
> +	  && are_isomorphic
> +	  && child_die->tag == DW_TAG_lexical_block)
> +	child_origin_die = corresponding_abstract_child;
> +
>        /* According to DWARF3 3.3.8.2 #3 new entries without their abstract
>  	 counterpart may exist.  */
>        if (child_origin_die != child_die)
> @@ -13590,6 +13633,9 @@ inherit_abstract_dies (struct die_info *die, struct dwarf2_cu *cu)
>  	  else
>  	    offsets.push_back (child_origin_die->sect_off);
>  	}
> +
> +      if (are_isomorphic)
> +	corresponding_abstract_child = corresponding_abstract_child->sibling;
>      }
>    std::sort (offsets.begin (), offsets.end ());
>    sect_offset *offsets_end = offsets.data () + offsets.size ();
> diff --git a/gdb/testsuite/gdb.opt/inline-locals.c b/gdb/testsuite/gdb.opt/inline-locals.c
> index b949152a3c0..f8910dadca3 100644
> --- a/gdb/testsuite/gdb.opt/inline-locals.c
> +++ b/gdb/testsuite/gdb.opt/inline-locals.c
> @@ -53,6 +53,24 @@ inline ATTR int func2(int arg2)
>    return x * func1 (arg2);
>  }
> 
> +inline ATTR
> +void
> +scoped (int s)
> +{
> +  int loc1 = 10;
> +  if (s > 0)
> +    {
> +      int loc2 = 20;
> +      s++; /* bp for locals 1 */
> +      if (s > 1)
> +	{
> +	  int loc3 = 30;
> +	  s++; /* bp for locals 2 */
> +	}
> +    }
> +  s++; /* bp for locals 3 */
> +}
> +
>  int main (void)
>  {
>    int val;
> @@ -67,5 +85,7 @@ int main (void)
>    val = func2 (result);
>    result = val;
> 
> +  scoped (40);
> +
>    return 0;
>  }
> diff --git a/gdb/testsuite/gdb.opt/inline-locals.exp b/gdb/testsuite/gdb.opt/inline-
> locals.exp
> index 2d8af285a88..d0acb4ae8b5 100644
> --- a/gdb/testsuite/gdb.opt/inline-locals.exp
> +++ b/gdb/testsuite/gdb.opt/inline-locals.exp
> @@ -124,3 +124,33 @@ if { ! $no_frames } {
>  }
> 
>  gdb_test "print array\[0\]" "\\\$$decimal = 184" "print local 3"
> +
> +# Test printing scoped local variables.
> +
> +proc check_scoped_locals {bp_label pass_re} {
> +    global srcfile
> +
> +    set locals_bp [gdb_get_line_number $bp_label ${srcfile}]
> +    gdb_breakpoint $srcfile:$locals_bp
> +
> +    gdb_continue_to_breakpoint "$bp_label" ".*$srcfile:$locals_bp.*"
> +    set kfail_re [multi_line $pass_re ".*<optimized out>"]
> +    gdb_test_multiple "info locals" "scoped info locals at $bp_label" {
> +	-re -wrap $pass_re {
> +	    pass $gdb_test_name
> +	}
> +	-re -wrap $kfail_re {
> +	    if {[test_compiler_info {gcc-[0-8]-*-*}]} {
> +		kfail gdb/25695 $gdb_test_name
> +	    } else {
> +		fail $gdb_test_name
> +	    }
> +	}
> +    }
> +}
> +
> +if {! $no_frames } {
> +    check_scoped_locals "bp for locals 1" "loc2 = 20\r\nloc1 = 10"
> +    check_scoped_locals "bp for locals 2" "loc3 = 30\r\nloc2 = 20\r\nloc1 = 10"
> +    check_scoped_locals "bp for locals 3" "loc1 = 10"
> +}
> --
> 2.17.1


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928