* RE: [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions
2021-02-15 15:51 [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions Tankut Baris Aktemur
@ 2021-03-16 18:18 ` Aktemur, Tankut Baris
2021-03-29 14:16 ` Aktemur, Tankut Baris
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Aktemur, Tankut Baris @ 2021-03-16 18:18 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions
2021-02-15 15:51 [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions Tankut Baris Aktemur
2021-03-16 18:18 ` Aktemur, Tankut Baris
@ 2021-03-29 14:16 ` Aktemur, Tankut Baris
2021-04-12 12:47 ` Aktemur, Tankut Baris
2021-04-12 14:50 ` Simon Marchi
3 siblings, 0 replies; 6+ messages in thread
From: Aktemur, Tankut Baris @ 2021-03-29 14:16 UTC (permalink / raw)
To: gdb-patches
Kindly pinging.
Regards
-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
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions
2021-02-15 15:51 [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions Tankut Baris Aktemur
2021-03-16 18:18 ` Aktemur, Tankut Baris
2021-03-29 14:16 ` Aktemur, Tankut Baris
@ 2021-04-12 12:47 ` Aktemur, Tankut Baris
2021-04-12 14:50 ` Simon Marchi
3 siblings, 0 replies; 6+ messages in thread
From: Aktemur, Tankut Baris @ 2021-04-12 12:47 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions
2021-02-15 15:51 [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions Tankut Baris Aktemur
` (2 preceding siblings ...)
2021-04-12 12:47 ` Aktemur, Tankut Baris
@ 2021-04-12 14:50 ` Simon Marchi
2021-04-13 14:44 ` Aktemur, Tankut Baris
3 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2021-04-12 14:50 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches
On 2021-02-15 10:51 a.m., Tankut Baris Aktemur via Gdb-patches 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.
Hi Baris,
In addition to updating this test, could you try to make a test using
the DWARF assembler in gdb.dwarf2/ ? The next clang version might not
generate this "bug" anymore, but we still want to make sure this
behavior keeps working. Also, it would make sure the behavior is tested
even when running the testsuite with gcc (which most people do, because
it's the default).
Speaking of "bug", do you think it's something that should be reported
to clang so they can fix their output? If there's already a clang bug
open for this, then can you add it to the commit message?
Otherwise, the patch looks good to me, see minor comments below.
>
> 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. */
Just a nit: remove the "I think". I think [:)] that first-person in
comments sounds weird, because once this is merged, this is no longer
you speaking, it's the code.
But I agree with you, I think it's is an acceptable assumption.
Especially since you restrict the fix to DW_TAG_lexical_block DIEs.
> + 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;
I'd suggest leaving corresponding_abstract_child to nullptr if
are_isomorphic is false. This will ensure we won't use the value by
mistake (since it's not meaningful in that case).
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] gdb/dwarf2: fix "info locals" for clang-compiled inlined functions
2021-04-12 14:50 ` Simon Marchi
@ 2021-04-13 14:44 ` Aktemur, Tankut Baris
0 siblings, 0 replies; 6+ messages in thread
From: Aktemur, Tankut Baris @ 2021-04-13 14:44 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On Monday, April 12, 2021 4:51 PM, Simon Marchi wrote:
> On 2021-02-15 10:51 a.m., Tankut Baris Aktemur via Gdb-patches 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.
>
> Hi Baris,
>
> In addition to updating this test, could you try to make a test using
> the DWARF assembler in gdb.dwarf2/ ? The next clang version might not
> generate this "bug" anymore, but we still want to make sure this
> behavior keeps working. Also, it would make sure the behavior is tested
> even when running the testsuite with gcc (which most people do, because
> it's the default).
Hi Simon,
Sure, I'll try it. Thanks for this suggestion.
> Speaking of "bug", do you think it's something that should be reported
> to clang so they can fix their output? If there's already a clang bug
> open for this, then can you add it to the commit message?
I haven't seen this reported in the Clang bug database. I hesitated to
open a ticket, because LLDB shows the local variables properly when using
the same program. Nevertheless, it makes sense to report it. I'll do it.
> Otherwise, the patch looks good to me, see minor comments below.
>
> >
> > 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. */
>
> Just a nit: remove the "I think". I think [:)] that first-person in
> comments sounds weird, because once this is merged, this is no longer
> you speaking, it's the code.
Fixed.
> But I agree with you, I think it's is an acceptable assumption.
> Especially since you restrict the fix to DW_TAG_lexical_block DIEs.
>
> > + 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;
>
> I'd suggest leaving corresponding_abstract_child to nullptr if
> are_isomorphic is false. This will ensure we won't use the value by
> mistake (since it's not meaningful in that case).
Fixed.
I'll send the v2 after I write the gdb.dwarf2 test and submit the Clang report.
Thanks
-Baris
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
^ permalink raw reply [flat|nested] 6+ messages in thread