* [PATCH v3 0/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. @ 2022-12-21 13:12 Stephan Rohr 2022-12-21 13:12 ` [PATCH v3 1/1] " Stephan Rohr 0 siblings, 1 reply; 9+ messages in thread From: Stephan Rohr @ 2022-12-21 13:12 UTC (permalink / raw) To: gdb-patches; +Cc: Rohr, Stephan From: "Rohr, Stephan" <stephan.rohr@intel.com> Hi all, this is version 3 of my patch and implements the following changes on top of version 2: * Corrected indentation for git commit message. * Extended testcase to also test if warning is emitted. I appreciate your feedback. Thanks Stephan Rohr, Stephan (1): gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. gdb/dwarf2/expr.c | 3 -- gdb/testsuite/gdb.dwarf2/shortpiece.exp | 53 ++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 4 deletions(-) -- 2.25.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] 9+ messages in thread
* [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. 2022-12-21 13:12 [PATCH v3 0/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type Stephan Rohr @ 2022-12-21 13:12 ` Stephan Rohr 2023-01-05 19:52 ` Tom Tromey 0 siblings, 1 reply; 9+ messages in thread From: Stephan Rohr @ 2022-12-21 13:12 UTC (permalink / raw) To: gdb-patches; +Cc: Rohr, Stephan From: "Rohr, Stephan" <stephan.rohr@intel.com> The 'rw_pieced_value' function is executed when fetching a (lazy) variable described by 'DW_OP_piece' or 'DW_OP_bit_piece'. The function checks the 'type' and 'enclosing_type' fields of the value for identity. * The 'type' field describes the type of a value. * In most cases, the 'enclosing_type' field is identical to the 'type' field. * Scenarios where the 'type' and 'enclosing_type' of an object differ are described in 'gdb/value.c'. Possible cases are: * If a value represents a C++ object, then the 'type' field gives the object's compile-time type. If the object actually belongs to some class derived from `type', perhaps with other base classes and additional members, then `type' is just a subobject of the real thing, and the full object is probably larger than `type' would suggest. * If 'type' is a dynamic class (i.e. one with a vtable), then GDB can actually determine the object's run-time type by looking at the run-time type information in the vtable. GDB may then elect to read the entire object. * If the user casts a variable to a different type (e.g. 'print (<type> []) <variable>'), the value's type is updated before reading the value. If a lazy value is fetched, GDB allocates space based on the enclosing type's length and typically reads the 'full' object. This is not implemented for pieced values and causes an internal error if 'type' and 'enclosing_type' of a value are not identical. However, GDB can read the value based on its type. Thus, this patch fixes the previously mentioned cases by removing the check for identity. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28605 gdb/ChangeLog: 2022-04-13 Stephan Rohr <stephan.rohr@intel.com> * dwarf2/loc.c (rw_pieced_value): Fix check on 'type' and 'enlcosing_type' when reading pieced value 'v'. gdb/testsuite/ChangeLog: 2022-04-13 Stephan Rohr <stephan.rohr@intel.com> * gdb.dwarf2/shortpiece.exp: Added test cases. --- gdb/dwarf2/expr.c | 3 -- gdb/testsuite/gdb.dwarf2/shortpiece.exp | 53 ++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c index 73dfd4b4ffb..fa08ad89024 100644 --- a/gdb/dwarf2/expr.c +++ b/gdb/dwarf2/expr.c @@ -161,9 +161,6 @@ rw_pieced_value (value *v, value *from, bool check_optimized) } else { - if (value_type (v) != value_enclosing_type (v)) - internal_error (_("Should not be able to create a lazy value with " - "an enclosing type")); if (check_optimized) v_contents = nullptr; else diff --git a/gdb/testsuite/gdb.dwarf2/shortpiece.exp b/gdb/testsuite/gdb.dwarf2/shortpiece.exp index f5a933e521b..6f45887a881 100644 --- a/gdb/testsuite/gdb.dwarf2/shortpiece.exp +++ b/gdb/testsuite/gdb.dwarf2/shortpiece.exp @@ -29,7 +29,7 @@ Dwarf::assemble $asm_file { cu { addr_size 4 } { compile_unit {} { - declare_labels int_label ushort_label struct_label + declare_labels int_label ushort_label struct_label struct_label_2 int_label: DW_TAG_base_type { {DW_AT_byte_size 4 DW_FORM_udata} @@ -59,6 +59,23 @@ Dwarf::assemble $asm_file { } } + struct_label_2: DW_TAG_structure_type { + {DW_AT_name "S_2"} + {DW_AT_byte_size 6 DW_FORM_udata} + } { + DW_TAG_member { + {DW_AT_name "a"} + {DW_AT_type :${int_label}} + {DW_AT_data_member_location 0 DW_FORM_udata} + } + + DW_TAG_member { + {DW_AT_name "b"} + {DW_AT_type :${ushort_label}} + {DW_AT_data_member_location 4 DW_FORM_udata} + } + } + DW_TAG_variable { {DW_AT_name "s1"} {DW_AT_type :${struct_label}} @@ -86,6 +103,20 @@ Dwarf::assemble $asm_file { DW_OP_piece 8 } SPECIAL_expr} } + + DW_TAG_variable { + {DW_AT_name "s3"} + {DW_AT_type :${struct_label_2}} + {DW_AT_external 1 DW_FORM_flag} + {DW_AT_location { + DW_OP_constu 0 + DW_OP_stack_value + DW_OP_piece 4 + DW_OP_constu 1 + DW_OP_stack_value + DW_OP_piece 2 + } SPECIAL_expr} + } } } } @@ -98,3 +129,23 @@ if { [prepare_for_testing "failed to prepare" ${testfile} \ gdb_test "p s1" " = {a = 1, b = 0}" gdb_test "p s2" \ "access outside bounds of object referenced via synthetic pointer" + +# When fetching a lazy value, GDB typically tries to fetch the 'full' +# object based on the enclosing type. GDB does not support the reading +# of a pieced value with a (possibly larger) enclosing type. However, +# the user may want to print a value casted to a different type, +# e.g. print (<type> []) <variable>. This cast causes an update of the +# value's type. In case of a pieced value, GDB failed to fetch the +# value's content. +# This test verifies that GDB can print a pieced value casted to a +# different type. +gdb_test "p (int \[\]) s1" " = \\{1\\, 0\\}" +gdb_test "p (short \[\]) s1" " = \\{1\\, 0\\, 0\\, <synthetic pointer>\\}" + +# Test for correct output if the size of the original object is not a +# multiple of the array's element size. +gdb_test "p s3" " = {a = 0, b = 1}" +gdb_test "p (int \[\]) s3" [multi_line \ + "warning: array element type size does not divide object size in cast" \ + "\\$\\d = \\{0\\}"] + -- 2.25.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] 9+ messages in thread
* Re: [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. 2022-12-21 13:12 ` [PATCH v3 1/1] " Stephan Rohr @ 2023-01-05 19:52 ` Tom Tromey 2023-01-06 19:22 ` Simon Marchi 2023-01-09 8:12 ` Rohr, Stephan 0 siblings, 2 replies; 9+ messages in thread From: Tom Tromey @ 2023-01-05 19:52 UTC (permalink / raw) To: Stephan Rohr via Gdb-patches; +Cc: Stephan Rohr >>>>> Stephan Rohr via Gdb-patches <gdb-patches@sourceware.org> writes: > From: "Rohr, Stephan" <stephan.rohr@intel.com> > The 'rw_pieced_value' function is executed when fetching a (lazy) > variable described by 'DW_OP_piece' or 'DW_OP_bit_piece'. The > function checks the 'type' and 'enclosing_type' fields of the value > for identity. Thanks for the patch. > If a lazy value is fetched, GDB allocates space based on the enclosing > type's length and typically reads the 'full' object. This is not > implemented for pieced values and causes an internal error if 'type' > and 'enclosing_type' of a value are not identical. > However, GDB can read the value based on its type. Thus, this patch > fixes the previously mentioned cases by removing the check for identity. I thought there was some other discussion & idea about the cause of this patch... something like, setting a value's type should reset the enclosing type? Or vice versa? I am wondering if you tried this approach instead. Tom ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. 2023-01-05 19:52 ` Tom Tromey @ 2023-01-06 19:22 ` Simon Marchi 2023-01-06 20:36 ` Tom Tromey 2023-01-09 8:12 ` Rohr, Stephan 1 sibling, 1 reply; 9+ messages in thread From: Simon Marchi @ 2023-01-06 19:22 UTC (permalink / raw) To: Tom Tromey, Stephan Rohr via Gdb-patches; +Cc: Stephan Rohr On 1/5/23 14:52, Tom Tromey wrote: >>>>>> Stephan Rohr via Gdb-patches <gdb-patches@sourceware.org> writes: > >> From: "Rohr, Stephan" <stephan.rohr@intel.com> >> The 'rw_pieced_value' function is executed when fetching a (lazy) >> variable described by 'DW_OP_piece' or 'DW_OP_bit_piece'. The >> function checks the 'type' and 'enclosing_type' fields of the value >> for identity. > > Thanks for the patch. > >> If a lazy value is fetched, GDB allocates space based on the enclosing >> type's length and typically reads the 'full' object. This is not >> implemented for pieced values and causes an internal error if 'type' >> and 'enclosing_type' of a value are not identical. > >> However, GDB can read the value based on its type. Thus, this patch >> fixes the previously mentioned cases by removing the check for identity. > > I thought there was some other discussion & idea about the cause of this > patch... something like, setting a value's type should reset the > enclosing type? Or vice versa? I am wondering if you tried this > approach instead. For reference, here was my last attempt at understanding what happens: https://inbox.sourceware.org/gdb-patches/dd04b3b4-ea6a-07ba-d734-0e542ca136b3@simark.ca/ I don't have a strong opinion on the matter, I don't know if one solution is more corrent than the other. The thing is, I can't explain the logic behind the existing assert. It's missing a comment explaining why it would be a bad thing to "create a lazy value with an enclosing type". If we can't add such a justification, I think it's ok to remove it... Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. 2023-01-06 19:22 ` Simon Marchi @ 2023-01-06 20:36 ` Tom Tromey 0 siblings, 0 replies; 9+ messages in thread From: Tom Tromey @ 2023-01-06 20:36 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, Stephan Rohr via Gdb-patches, Stephan Rohr >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: Simon> I don't have a strong opinion on the matter, I don't know if one Simon> solution is more corrent than the other. The thing is, I can't explain Simon> the logic behind the existing assert. It's missing a comment explaining Simon> why it would be a bad thing to "create a lazy value with an enclosing Simon> type". If we can't add such a justification, I think it's ok to remove Simon> it... I think the idea is that the enclosing type stuff is all there for "set print object on" -- and so for a value to have an enclosing type, it must already have contents, because the enclosing type is the runtime type, which can only be determined by reading memory. Tom ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. 2023-01-05 19:52 ` Tom Tromey 2023-01-06 19:22 ` Simon Marchi @ 2023-01-09 8:12 ` Rohr, Stephan 2023-01-09 19:28 ` Tom Tromey 1 sibling, 1 reply; 9+ messages in thread From: Rohr, Stephan @ 2023-01-09 8:12 UTC (permalink / raw) To: Tom Tromey, Stephan Rohr via Gdb-patches; +Cc: Simon Marchi Hi Tom. > -----Original Message----- > From: Tom Tromey <tom@tromey.com> > Sent: Thursday, January 5, 2023 8:53 PM > To: Stephan Rohr via Gdb-patches <gdb-patches@sourceware.org> > Cc: Rohr, Stephan <stephan.rohr@intel.com> > Subject: Re: [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values > casted to different type. > > >>>>> Stephan Rohr via Gdb-patches <gdb-patches@sourceware.org> > writes: > > > From: "Rohr, Stephan" <stephan.rohr@intel.com> The 'rw_pieced_value' > > function is executed when fetching a (lazy) variable described by > > 'DW_OP_piece' or 'DW_OP_bit_piece'. The function checks the 'type' > > and 'enclosing_type' fields of the value for identity. > > Thanks for the patch. > > > If a lazy value is fetched, GDB allocates space based on the enclosing > > type's length and typically reads the 'full' object. This is not > > implemented for pieced values and causes an internal error if 'type' > > and 'enclosing_type' of a value are not identical. > > > However, GDB can read the value based on its type. Thus, this patch > > fixes the previously mentioned cases by removing the check for identity. > > I thought there was some other discussion & idea about the cause of this > patch... something like, setting a value's type should reset the enclosing > type? Or vice versa? I am wondering if you tried this approach instead. > > Tom There was a discussion with Simon whether the value cast should update the enclosing type. Simon tried it out but as I understood he's also not clear on what the right answer answers are. From my point of view, this would also be a separate patch. Thanks Stephan 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] 9+ messages in thread
* Re: [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. 2023-01-09 8:12 ` Rohr, Stephan @ 2023-01-09 19:28 ` Tom Tromey 2023-01-10 9:29 ` Rohr, Stephan 0 siblings, 1 reply; 9+ messages in thread From: Tom Tromey @ 2023-01-09 19:28 UTC (permalink / raw) To: Rohr, Stephan via Gdb-patches; +Cc: Tom Tromey, Rohr, Stephan, Simon Marchi > There was a discussion with Simon whether the value cast should update the > enclosing type. Simon tried it out but as I understood he's also not clear on > what the right answer answers are. From my point of view, this would also > be a separate patch. Ok, that's fair. I think the patch is ok. Tom ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. 2023-01-09 19:28 ` Tom Tromey @ 2023-01-10 9:29 ` Rohr, Stephan 2023-01-10 21:31 ` Tom Tromey 0 siblings, 1 reply; 9+ messages in thread From: Rohr, Stephan @ 2023-01-10 9:29 UTC (permalink / raw) To: Tom Tromey, Rohr, Stephan via Gdb-patches; +Cc: Simon Marchi Hi Tom. > -----Original Message----- > From: Tom Tromey <tom@tromey.com> > Sent: Monday, January 9, 2023 8:29 PM > To: Rohr, Stephan via Gdb-patches <gdb-patches@sourceware.org> > Cc: Tom Tromey <tom@tromey.com>; Rohr, Stephan > <stephan.rohr@intel.com>; Simon Marchi <simark@simark.ca> > Subject: Re: [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values > casted to different type. > > > There was a discussion with Simon whether the value cast should update > > the enclosing type. Simon tried it out but as I understood he's also > > not clear on what the right answer answers are. From my point of > > view, this would also be a separate patch. > > Ok, that's fair. > I think the patch is ok. > > Tom Thank you for your review. This is my first patch. Can you push it for me? I don't have any write access. I plan to upstream more patches in the future and would like to have "write after approval" access. Could I put your email address in the field which says "email address of person who approved request" in the application form at https://sourceware.org/cgi-bin/pdw/ps_form.cgi ? Thanks Stephan 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] 9+ messages in thread
* Re: [PATCH v3 1/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type. 2023-01-10 9:29 ` Rohr, Stephan @ 2023-01-10 21:31 ` Tom Tromey 0 siblings, 0 replies; 9+ messages in thread From: Tom Tromey @ 2023-01-10 21:31 UTC (permalink / raw) To: Rohr, Stephan via Gdb-patches; +Cc: Tom Tromey, Rohr, Stephan, Simon Marchi >> Thank you for your review. This is my first patch. Can you push it for me? >> I don't have any write access. Yep. I fixed a whitespace error (an extra blank line at the end of the test file) that git pointed out. >> I plan to upstream more patches in the future and would like to have >> "write after approval" access. Could I put your email address in the field >> which says "email address of person who approved request" in the application >> form at https://sourceware.org/cgi-bin/pdw/ps_form.cgi ? Yes. Tom ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-01-10 21:31 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-21 13:12 [PATCH v3 0/1] gdb/dwarf2: Fix 'rw_pieced_value' for values casted to different type Stephan Rohr 2022-12-21 13:12 ` [PATCH v3 1/1] " Stephan Rohr 2023-01-05 19:52 ` Tom Tromey 2023-01-06 19:22 ` Simon Marchi 2023-01-06 20:36 ` Tom Tromey 2023-01-09 8:12 ` Rohr, Stephan 2023-01-09 19:28 ` Tom Tromey 2023-01-10 9:29 ` Rohr, Stephan 2023-01-10 21:31 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).