* [PATCH][gdb/symtab] Fix element type modification in read_array_type @ 2021-02-05 12:28 Tom de Vries 2021-02-05 16:30 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Tom de Vries @ 2021-02-05 12:28 UTC (permalink / raw) To: gdb-patches Hi, When running test-case gdb.fortran/function-calls.exp with target board unix/gdb:debug_flags=-gdwarf-5, I run into: ... (gdb) PASS: gdb.fortran/function-calls.exp: \ p derived_types_and_module_calls::pass_cart(c) p derived_types_and_module_calls::pass_cart_nd(c_nd)^M ^M Program received signal SIGSEGV, Segmentation fault.^M 0x0000000000400f73 in derived_types_and_module_calls::pass_cart_nd \ (c=<error reading variable: Cannot access memory at address 0xc>) at \ function-calls.f90:130^M 130 pass_cart_nd = ubound(c%d,1,4)^M The program being debugged was signaled while in a function called from GDB.^M GDB has restored the context to what it was before the call.^M To change this behavior use "set unwindonsignal off".^M Evaluation of the expression containing the function^M (derived_types_and_module_calls::pass_cart_nd) will be abandoned.^M (gdb) FAIL: gdb.fortran/function-calls.exp: p ... The problem originates in read_array_type, when reading a DW_TAG_array_type with a dwarf-5 DW_TAG_generic_subrange child. This is not supported, and the fallout of this is that rather than constructing a new array type, the code proceeds to modify the element type. Fix this conservatively by bailing out in read_array_type when not being able to construct an array type. Tested on x86_64-linux. Any comments? Thanks, - Tom [gdb/symtab] Fix element type modification in read_array_type gdb/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> PR symtab/27341 * dwarf2/read.c (read_array_type): Return NULL when not being able to construct an array type. --- gdb/dwarf2/read.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index f60e418172c..98189f7b566 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -17243,6 +17243,9 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu) } } + if (type == element_type) + return NULL; + /* Understand Dwarf2 support for vector types (like they occur on the PowerPC w/ AltiVec). Gcc just adds another attribute to the array type. This is not part of the Dwarf2/3 standard yet, but a ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][gdb/symtab] Fix element type modification in read_array_type 2021-02-05 12:28 [PATCH][gdb/symtab] Fix element type modification in read_array_type Tom de Vries @ 2021-02-05 16:30 ` Tom Tromey 2021-02-07 8:38 ` Tom de Vries 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2021-02-05 16:30 UTC (permalink / raw) To: Tom de Vries; +Cc: gdb-patches >>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: Tom> Fix this conservatively by bailing out in read_array_type when not being able Tom> to construct an array type. Perhaps it should issue a complaint? It seems to me though that if gcc emits DW_TAG_generic_subrange, then gdb ought to handle it. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][gdb/symtab] Fix element type modification in read_array_type 2021-02-05 16:30 ` Tom Tromey @ 2021-02-07 8:38 ` Tom de Vries 2021-02-09 20:24 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Tom de Vries @ 2021-02-07 8:38 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 750 bytes --] On 2/5/21 5:30 PM, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: > > Tom> Fix this conservatively by bailing out in read_array_type when not being able > Tom> to construct an array type. > > Perhaps it should issue a complaint? > Indeed. I've added one. > It seems to me though that if gcc emits DW_TAG_generic_subrange, then > gdb ought to handle it. True, but that's not the issue I'm trying to handle here. I'm trying to fix the bug that the element type is modified, which could also happen f.i. when an array type without any subrange children is processed. I want to file an enhancement PR for supporting DW_TAG_generic_subrange, which I then can mention in the KFAIL I've also added. WDYT? Thanks, - Tom [-- Attachment #2: 0006-gdb-symtab-Fix-element-type-modification-in-read_array_type.patch --] [-- Type: text/x-patch, Size: 6759 bytes --] [gdb/symtab] Fix element type modification in read_array_type When running test-case gdb.fortran/function-calls.exp with target board unix/gdb:debug_flags=-gdwarf-5, I run into: ... (gdb) PASS: gdb.fortran/function-calls.exp: \ p derived_types_and_module_calls::pass_cart(c) p derived_types_and_module_calls::pass_cart_nd(c_nd)^M ^M Program received signal SIGSEGV, Segmentation fault.^M 0x0000000000400f73 in derived_types_and_module_calls::pass_cart_nd \ (c=<error reading variable: Cannot access memory at address 0xc>) at \ function-calls.f90:130^M 130 pass_cart_nd = ubound(c%d,1,4)^M The program being debugged was signaled while in a function called from GDB.^M GDB has restored the context to what it was before the call.^M To change this behavior use "set unwindonsignal off".^M Evaluation of the expression containing the function^M (derived_types_and_module_calls::pass_cart_nd) will be abandoned.^M (gdb) FAIL: gdb.fortran/function-calls.exp: p ... The problem originates in read_array_type, when reading a DW_TAG_array_type with a dwarf-5 DW_TAG_generic_subrange child. This is not supported, and the fallout of this is that rather than constructing a new array type, the code proceeds to modify the element type. Fix this conservatively by issuing a complaint and bailing out in read_array_type when not being able to construct an array type, such that we have: ... (gdb) maint expand-symtabs function-calls.f90^M During symbol reading: unable to handle array child DIE \ DW_TAG_generic_subrange - DIE at 0xe1e [in module function-calls]^M During symbol reading: unable to handle array child DIE \ DW_TAG_generic_subrange - DIE at 0xe1e [in module function-calls]^M (gdb) KFAIL: gdb.fortran/function-calls.exp: no complaints in srcfile \ (PRMS: symtab/nnnnn) ... Tested on x86_64-linux. gdb/ChangeLog: 2021-02-06 Tom de Vries <tdevries@suse.de> PR symtab/27341 * dwarf2/read.c (read_array_type): Return NULL when not being able to construct an array type. Add assert to ensure that element_type is not being modified. 2021-02-06 Tom de Vries <tdevries@suse.de> PR symtab/27341 * lib/gdb.exp (with_complaints): New proc, factored out of ... (gdb_load_no_complaints): ... here. * gdb.fortran/function-calls.exp: Add test-case. --- gdb/dwarf2/read.c | 11 +++++++ gdb/testsuite/gdb.fortran/function-calls.exp | 16 ++++++++++ gdb/testsuite/lib/gdb.exp | 46 ++++++++++++++++++++-------- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index b0b37358866..2f8869bd11c 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -17246,6 +17246,15 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu) range_types.push_back (child_type); } } + else + { + complaint (_("unable to handle array child DIE %s" + " - DIE at %s [in module %s]"), + dwarf_tag_name (child_die->tag), + sect_offset_str (die->sect_off), + objfile_name (cu->per_objfile->objfile)); + return NULL; + } child_die = child_die->sibling; } @@ -17278,6 +17287,8 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu) } } + gdb_assert (type != element_type); + /* Understand Dwarf2 support for vector types (like they occur on the PowerPC w/ AltiVec). Gcc just adds another attribute to the array type. This is not part of the Dwarf2/3 standard yet, but a diff --git a/gdb/testsuite/gdb.fortran/function-calls.exp b/gdb/testsuite/gdb.fortran/function-calls.exp index a14cfaeb08a..d1d781ff59f 100644 --- a/gdb/testsuite/gdb.fortran/function-calls.exp +++ b/gdb/testsuite/gdb.fortran/function-calls.exp @@ -24,6 +24,22 @@ if {[prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug f90}]} { return -1 } +with_complaints 5 { + set cmd "maint expand-symtabs $srcfile" + set cmd_regexp [string_to_regexp $cmd] + set re_kfail [concat "During symbol reading:" \ + " unable to handle array child" \ + " DIE DW_TAG_generic_subrange"] + gdb_test_multiple $cmd "no complaints in srcfile" { + -re -wrap "$re_kfail.*" { + kfail symtab/nnnnn $gdb_test_name + } + -re "^$cmd_regexp\r\n$gdb_prompt $" { + pass $gdb_test_name + } + } +} + if {![runto [gdb_get_line_number "post_init"]]} then { perror "couldn't run to breakpoint post_init" continue diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index dbbf239dab9..69a81353b05 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -5096,11 +5096,11 @@ proc gdb_load { arg } { } # -# gdb_load_no_complaints -- As gdb_load, but in addition verifies that -# loading caused no symbol reading complaints. +# with_complaints -- Execute BODY and set complaints temporary to N for the +# duration. # -proc gdb_load_no_complaints { arg } { - global gdb_prompt gdb_file_cmd_msg decimal +proc with_complaints { n body } { + global decimal # Save current setting of complaints. set save "" @@ -5112,23 +5112,43 @@ proc gdb_load_no_complaints { arg } { } } - # Fall back to regular gdb_load if we couldn't get the current setting - # of complaints. if { $save == "" } { - return gdb_load $arg + perror "Did not manage to set complaints" + } else { + # Set complaints. + gdb_test_no_output "set complaints $n" "" } - # Temporarily set complaint to a small non-zero number. - gdb_test_no_output "set complaints 5" "" + set code [catch {uplevel 1 $body} result] + + # Restore saved setting of complaints. + if { $save != "" } { + gdb_test_no_output "set complaints $save" "" + } + + if {$code == 1} { + global errorInfo errorCode + return -code $code -errorinfo $errorInfo -errorcode $errorCode $result + } else { + return -code $code $result + } +} - gdb_load $arg +# +# gdb_load_no_complaints -- As gdb_load, but in addition verifies that +# loading caused no symbol reading complaints. +# +proc gdb_load_no_complaints { arg } { + global gdb_prompt gdb_file_cmd_msg decimal + + # Temporarily set complaint to a small non-zero number. + with_complaints 5 { + gdb_load $arg + } # Verify that there were no complaints. set re "\r\nDuring symbol reading: " gdb_assert {![regexp $re $gdb_file_cmd_msg]} "No complaints" - - # Restore saved setting of complaints. - gdb_test_no_output "set complaints $save" "" } # gdb_reload -- load a file into the target. Called before "running", ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][gdb/symtab] Fix element type modification in read_array_type 2021-02-07 8:38 ` Tom de Vries @ 2021-02-09 20:24 ` Tom Tromey 2021-02-09 22:29 ` Tom de Vries 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2021-02-09 20:24 UTC (permalink / raw) To: Tom de Vries; +Cc: Tom Tromey, gdb-patches >>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: Tom> + { Tom> + complaint (_("unable to handle array child DIE %s" Tom> + " - DIE at %s [in module %s]"), Tom> + dwarf_tag_name (child_die->tag), Tom> + sect_offset_str (die->sect_off), Tom> + objfile_name (cu->per_objfile->objfile)); Tom> + return NULL; Tom> + } This seems a bit too strong to me. A normal DWARF extension mechanism is to add new "unexpected" children to a DIE. In many (maybe most) cases, it's fine to just ignore these. Maybe another approach would be to bail out if range_types.empty(), issuing a complaint then. This wouldn't mention DW_TAG_generic_subrange, but that seems ok to me. Users don't really read complaints. Maybe even gdb developers don't ;) Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][gdb/symtab] Fix element type modification in read_array_type 2021-02-09 20:24 ` Tom Tromey @ 2021-02-09 22:29 ` Tom de Vries 2021-02-10 12:23 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Tom de Vries @ 2021-02-09 22:29 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 904 bytes --] On 2/9/21 9:24 PM, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: > > Tom> + { > Tom> + complaint (_("unable to handle array child DIE %s" > Tom> + " - DIE at %s [in module %s]"), > Tom> + dwarf_tag_name (child_die->tag), > Tom> + sect_offset_str (die->sect_off), > Tom> + objfile_name (cu->per_objfile->objfile)); > Tom> + return NULL; > Tom> + } > > This seems a bit too strong to me. A normal DWARF extension mechanism > is to add new "unexpected" children to a DIE. In many (maybe most) > cases, it's fine to just ignore these. > > Maybe another approach would be to bail out if range_types.empty(), > issuing a complaint then. This wouldn't mention > DW_TAG_generic_subrange, but that seems ok to me. Ack, done. > Users don't really read complaints. Maybe even gdb developers don't ;) Heh :) Committed as attached. Thanks, - Tom [-- Attachment #2: 0001-gdb-symtab-Fix-element-type-modification-in-read_array_type.patch --] [-- Type: text/x-patch, Size: 6269 bytes --] [gdb/symtab] Fix element type modification in read_array_type When running test-case gdb.fortran/function-calls.exp with target board unix/gdb:debug_flags=-gdwarf-5, I run into: ... (gdb) PASS: gdb.fortran/function-calls.exp: \ p derived_types_and_module_calls::pass_cart(c) p derived_types_and_module_calls::pass_cart_nd(c_nd)^M ^M Program received signal SIGSEGV, Segmentation fault.^M 0x0000000000400f73 in derived_types_and_module_calls::pass_cart_nd \ (c=<error reading variable: Cannot access memory at address 0xc>) at \ function-calls.f90:130^M 130 pass_cart_nd = ubound(c%d,1,4)^M The program being debugged was signaled while in a function called from GDB.^M GDB has restored the context to what it was before the call.^M To change this behavior use "set unwindonsignal off".^M Evaluation of the expression containing the function^M (derived_types_and_module_calls::pass_cart_nd) will be abandoned.^M (gdb) FAIL: gdb.fortran/function-calls.exp: p ... The problem originates in read_array_type, when reading a DW_TAG_array_type with a dwarf-5 DW_TAG_generic_subrange child. This is not supported, and the fallout of this is that rather than constructing a new array type, the code proceeds to modify the element type. Fix this conservatively by issuing a complaint and bailing out in read_array_type when not being able to construct an array type, such that we have: ... (gdb) maint expand-symtabs function-calls.f90^M During symbol reading: unable to find array range \ - DIE at 0xe1e [in module function-calls]^M During symbol reading: unable to find array range \ - DIE at 0xe1e [in module function-calls]^M (gdb) KFAIL: gdb.fortran/function-calls.exp: no complaints in srcfile \ (PRMS: symtab/27388) ... Tested on x86_64-linux. gdb/ChangeLog: 2021-02-06 Tom de Vries <tdevries@suse.de> PR symtab/27341 * dwarf2/read.c (read_array_type): Return NULL when not being able to construct an array type. Add assert to ensure that element_type is not being modified. 2021-02-06 Tom de Vries <tdevries@suse.de> PR symtab/27341 * lib/gdb.exp (with_complaints): New proc, factored out of ... (gdb_load_no_complaints): ... here. * gdb.fortran/function-calls.exp: Add test-case. --- gdb/dwarf2/read.c | 10 +++++++ gdb/testsuite/gdb.fortran/function-calls.exp | 15 ++++++++++ gdb/testsuite/lib/gdb.exp | 43 +++++++++++++++++++++------- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 3f60ce6a4f1..4b901c565f8 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -17217,6 +17217,14 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu) child_die = child_die->sibling; } + if (range_types.empty ()) + { + complaint (_("unable to find array range - DIE at %s [in module %s]"), + sect_offset_str (die->sect_off), + objfile_name (cu->per_objfile->objfile)); + return NULL; + } + /* Dwarf2 dimensions are output from left to right, create the necessary array types in backwards order. */ @@ -17246,6 +17254,8 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu) } } + gdb_assert (type != element_type); + /* Understand Dwarf2 support for vector types (like they occur on the PowerPC w/ AltiVec). Gcc just adds another attribute to the array type. This is not part of the Dwarf2/3 standard yet, but a diff --git a/gdb/testsuite/gdb.fortran/function-calls.exp b/gdb/testsuite/gdb.fortran/function-calls.exp index a14cfaeb08a..00036d96c7c 100644 --- a/gdb/testsuite/gdb.fortran/function-calls.exp +++ b/gdb/testsuite/gdb.fortran/function-calls.exp @@ -24,6 +24,21 @@ if {[prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug f90}]} { return -1 } +with_complaints 5 { + set cmd "maint expand-symtabs $srcfile" + set cmd_regexp [string_to_regexp $cmd] + set re_kfail [concat "During symbol reading:" \ + " unable to find array range"] + gdb_test_multiple $cmd "no complaints in srcfile" { + -re -wrap "$re_kfail.*" { + kfail symtab/27388 $gdb_test_name + } + -re "^$cmd_regexp\r\n$gdb_prompt $" { + pass $gdb_test_name + } + } +} + if {![runto [gdb_get_line_number "post_init"]]} then { perror "couldn't run to breakpoint post_init" continue diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 53ac9f1408c..1406b917151 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -5096,11 +5096,11 @@ proc gdb_load { arg } { } # -# gdb_load_no_complaints -- As gdb_load, but in addition verifies that -# loading caused no symbol reading complaints. +# with_complaints -- Execute BODY and set complaints temporary to N for the +# duration. # -proc gdb_load_no_complaints { arg } { - global gdb_prompt gdb_file_cmd_msg decimal +proc with_complaints { n body } { + global decimal # Save current setting of complaints. set save "" @@ -5112,16 +5112,39 @@ proc gdb_load_no_complaints { arg } { } } - # Fall back to regular gdb_load if we couldn't get the current setting - # of complaints. if { $save == "" } { - return gdb_load $arg + perror "Did not manage to set complaints" + } else { + # Set complaints. + gdb_test_no_output "set complaints $n" "" } - # Temporarily set complaint to a small non-zero number. - gdb_test_no_output "set complaints 5" "" + set code [catch {uplevel 1 $body} result] + + # Restore saved setting of complaints. + if { $save != "" } { + gdb_test_no_output "set complaints $save" "" + } + + if {$code == 1} { + global errorInfo errorCode + return -code $code -errorinfo $errorInfo -errorcode $errorCode $result + } else { + return -code $code $result + } +} + +# +# gdb_load_no_complaints -- As gdb_load, but in addition verifies that +# loading caused no symbol reading complaints. +# +proc gdb_load_no_complaints { arg } { + global gdb_prompt gdb_file_cmd_msg decimal - gdb_load $arg + # Temporarily set complaint to a small non-zero number. + with_complaints 5 { + gdb_load $arg + } # Verify that there were no complaints. set re "^Reading symbols from \[^\r\n\]*\r\n$gdb_prompt $" ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][gdb/symtab] Fix element type modification in read_array_type 2021-02-09 22:29 ` Tom de Vries @ 2021-02-10 12:23 ` Tom Tromey 0 siblings, 0 replies; 6+ messages in thread From: Tom Tromey @ 2021-02-10 12:23 UTC (permalink / raw) To: Tom de Vries; +Cc: Tom Tromey, gdb-patches >>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: Tom> 2021-02-06 Tom de Vries <tdevries@suse.de> Tom> PR symtab/27341 Tom> * dwarf2/read.c (read_array_type): Return NULL when not being able to Tom> construct an array type. Add assert to ensure that element_type is Tom> not being modified. Looks good. Thank you. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-10 12:23 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-05 12:28 [PATCH][gdb/symtab] Fix element type modification in read_array_type Tom de Vries 2021-02-05 16:30 ` Tom Tromey 2021-02-07 8:38 ` Tom de Vries 2021-02-09 20:24 ` Tom Tromey 2021-02-09 22:29 ` Tom de Vries 2021-02-10 12:23 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox