Should a DW_OP_implicit_value be taken from the left end?

Should a DW_OP_implicit_value be taken from the left end?

[Andreas Arnez]

The following patch changed how sub-values of a DW_OP_implicit_value are
extracted on big-endian platforms:

  https://sourceware.org/ml/gdb-patches/2011-08/msg00123.html

The patch seems to assume that the placement rule is similar as for
DW_OP_stack_value.  I think that's wrong; DW_OP_implicit_value should
always be taken from the left end.  (See rationale below for an
explanation.)  And I'd like to know whether anyone objects.

I'm in the process of preparing a patch series for fixing various
problems with DWARF piece handling, and the series currently contains a
patch for reverting the above patch's effect on DW_OP_implicit_value.

Rationale:

Since DW_OP_implicit_value can express arbitrary amounts of structured
or unstructured data, the notion of a "least significant byte" is
meaningless.  The patch above makes it impossible for a
DW_OP_implicit_pointer operation to refer to any sub-value within a
DW_OP_implicit_value on big-endian targets.

E.g., consider this code snippet:

  const char foo[] = "Hello, world!";
  char *a = &foo[0];
  char *b = &foo[7];

IMHO the compiler should be able to describe `foo' with a single
DW_OP_implicit_value operation and `a' and `b' as DW_OP_implicit_pointer
operations pointing into that value.

Any objections?

--
Andreas

Re: Should a DW_OP_implicit_value be taken from the left end?

[Jan Kratochvil]

On Wed, 21 Dec 2016 20:42:25 +0100, Andreas Arnez wrote:
> Since DW_OP_implicit_value can express arbitrary amounts of structured
> or unstructured data, the notion of a "least significant byte" is
> meaningless.

That depends on what the compiler-debugger negotiate together, as specified by
the DWARF standard which is ambiguous in this case IIUC.


> The patch above makes it impossible for a
> DW_OP_implicit_pointer operation to refer to any sub-value within a
> DW_OP_implicit_value on big-endian targets.

When looking at the patch I think the patch may be wrong, I think the
endianity should affect at least also the lines:

        case DWARF_VALUE_LITERAL:
...
            ldata = ctx.data + byte_offset;
            n -= byte_offset;


> E.g., consider this code snippet:
> 
>   const char foo[] = "Hello, world!";
>   char *a = &foo[0];
>   char *b = &foo[7];
> 
> IMHO the compiler should be able to describe `foo' with a single
> DW_OP_implicit_value operation and `a' and `b' as DW_OP_implicit_pointer
> operations pointing into that value.

I would need a compilable + compiled code + DWARF output + GDB output to say
more.


> Any objections?

I really do not mind reverting the patch if you think so, that is up to the
maintainers.

But when you ask me I miss here stating what the current GCC version does
produce.  Is GDB behavior fixed with current GCC by your proposed patch
revert?  If it breaks do you plan to change/fix also GCC?  Does the GDB
testsuite (particulerly the entryval testcases) have no regressions on s390*?

From my mail you reference I understand it as that my patch did fix some
entryval testcases with GCC that time.  Unfortunately the entryval testcases
are provided as .S files prebuilt by GCC that time and they are difficult to
reproduce with newer GCC as -O2 -g code changes too much with different GCC
version, breaking various compiled code assumptions of the .exp file.

My feeling is that my patch did fix some entryval testcases on bigendian but
I also guess my patch was wrong - it only accidentally fixed them, the right
fix for big endian should look differently.


Jan

Re: Should a DW_OP_implicit_value be taken from the left end?

[Andreas Arnez]

[-- Attachment #1: Type: text/plain, Size: 2213 bytes --]

Thanks for looking into this.

On Wed, Dec 21 2016, Jan Kratochvil wrote:

> When looking at the patch I think the patch may be wrong, I think the
> endianity should affect at least also the lines:
>
>         case DWARF_VALUE_LITERAL:
> ...
>             ldata = ctx.data + byte_offset;
>             n -= byte_offset;

Right, that's another issue.  If we revert the change for
DW_OP_implicit_value, it doesn't matter anymore, but the same bug occurs
with DW_OP_stack_value as well.  I'll handle that separately.

> I really do not mind reverting the patch if you think so, that is up to the
> maintainers.

Note that I don't intend to revert the whole patch, only the part
affecting DW_OP_implicit_value (DWARF_VALUE_LITERAL).

> But when you ask me I miss here stating what the current GCC version does
> produce.  Is GDB behavior fixed with current GCC by your proposed patch
> revert?  If it breaks do you plan to change/fix also GCC?  Does the GDB
> testsuite (particulerly the entryval testcases) have no regressions on s390*?

Right, the test suite has no regressions on s390*.  And GCC can indeed
emit DWARF code that breaks with current GDB on s390x; and the proposed
patch would fix that.  See attachment for an example.

> From my mail you reference I understand it as that my patch did fix some
> entryval testcases with GCC that time.  Unfortunately the entryval testcases
> are provided as .S files prebuilt by GCC that time and they are difficult to
> reproduce with newer GCC as -O2 -g code changes too much with different GCC
> version, breaking various compiled code assumptions of the .exp file.

Your patch certainly fixed a real problem with DW_OP_stack_value, but I
doubt that the changes to the handling of DW_OP_implicit_value did.  In
the patch description you stated:

  "I am not completely sure with the DWARF standard understanding for
 DW_OP_implicit_value (DWARF_VALUE_LITERAL) but I think it also should
 be fixed (if such DWARF is valid at all)."

And Tom Tromey answered:

  "I think I asked about this when implementing this DWARF addition in
  gdb, but I can't find the messages right now."

Is there any any other information about the rationale of that change?

--
Andreas


[-- Attachment #2: implvalue.c --]
[-- Type: text/plain, Size: 603 bytes --]

/* DWARF test case that gets the compiler to emit DW_OP_implicit_value as
   well as multiple instances of DW_OP_implicit_pointer, each pointing
   into the implicit value with varying offsets.  */

volatile int v;

static void
bar (char *a, char *b, char *c, char *d)
{
  v = *a + *b + *c + *d;
}

int
main ()
{
  __int128 u = (((__int128) 0x11223344556677 << 64) | 0x8899aabbccddeeff);
  char *a = (char *) &u;

  bar (a, a + 4, a + 8, a + 12);

  /* Ensure that a location list is needed for u, so DW_AT_const_value can
     not be used.  */
  u = ~u;
  bar (a, a + 4, a + 8, a + 12);

  return 0;
}

[-- Attachment #3: readelf -wio output --]
[-- Type: text/plain, Size: 6366 bytes --]

Contents of the .debug_info section:

  Compilation Unit @ offset 0x0:
   Length:        0x159 (32-bit)
   Version:       4
   Abbrev Offset: 0x0
   Pointer Size:  8
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_producer    : (indirect string, offset: 0x21): GNU C11 7.0.0 20161222 (experimental) -march=zEC12 -mno-vx -mzarch -m64 -g -O3
    <10>   DW_AT_language    : 12	(ANSI C99)
    <11>   DW_AT_name        : (indirect string, offset: 0x10): implvalue.c
    <15>   DW_AT_comp_dir    : (indirect string, offset: 0x0): /home/arnez/tmp
    <19>   DW_AT_ranges      : 0x0
    <1d>   DW_AT_low_pc      : 0x0
    <25>   DW_AT_stmt_list   : 0x0
 <1><29>: Abbrev Number: 2 (DW_TAG_variable)
    <2a>   DW_AT_name        : v
    <2c>   DW_AT_decl_file   : 1
    <2d>   DW_AT_decl_line   : 5
    <2e>   DW_AT_type        : <0x43>
    <32>   DW_AT_external    : 1
    <32>   DW_AT_location    : 9 byte block: 3 0 0 0 0 10 0 20 3c 	(DW_OP_addr: 1000203c)
 <1><3c>: Abbrev Number: 3 (DW_TAG_base_type)
    <3d>   DW_AT_byte_size   : 4
    <3e>   DW_AT_encoding    : 5	(signed)
    <3f>   DW_AT_name        : int
 <1><43>: Abbrev Number: 4 (DW_TAG_volatile_type)
    <44>   DW_AT_type        : <0x3c>
 <1><48>: Abbrev Number: 5 (DW_TAG_subprogram)
    <49>   DW_AT_external    : 1
    <49>   DW_AT_name        : (indirect string, offset: 0x1c): main
    <4d>   DW_AT_decl_file   : 1
    <4e>   DW_AT_decl_line   : 14
    <4f>   DW_AT_type        : <0x3c>
    <53>   DW_AT_low_pc      : 0x10000418
    <5b>   DW_AT_high_pc     : 0x18
    <63>   DW_AT_frame_base  : 1 byte block: 9c 	(DW_OP_call_frame_cfa)
    <65>   DW_AT_GNU_all_call_sites: 1
    <65>   DW_AT_sibling     : <0x11b>
 <2><69>: Abbrev Number: 6 (DW_TAG_variable)
    <6a>   DW_AT_name        : u
    <6c>   DW_AT_decl_file   : 1
    <6d>   DW_AT_decl_line   : 16
    <6e>   DW_AT_type        : <0x11b>
    <72>   DW_AT_location    : 0x0 (location list)
 <2><76>: Abbrev Number: 7 (DW_TAG_variable)
    <77>   DW_AT_name        : a
    <79>   DW_AT_decl_file   : 1
    <7a>   DW_AT_decl_line   : 17
    <7b>   DW_AT_type        : <0x122>
    <7f>   DW_AT_location    : 6 byte block: f2 0 0 0 69 0 	(DW_OP_GNU_implicit_pointer: <0x69> 0)
 <2><86>: Abbrev Number: 8 (DW_TAG_inlined_subroutine)
    <87>   DW_AT_abstract_origin: <0x12f>
    <8b>   DW_AT_low_pc      : 0x10000418
    <93>   DW_AT_high_pc     : 0xc
    <9b>   DW_AT_call_file   : 1
    <9c>   DW_AT_call_line   : 19
    <9d>   DW_AT_sibling     : <0xd2>
 <3><a1>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <a2>   DW_AT_abstract_origin: <0x137>
    <a6>   DW_AT_location    : 6 byte block: f2 0 0 0 69 0 	(DW_OP_GNU_implicit_pointer: <0x69> 0)
 <3><ad>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <ae>   DW_AT_abstract_origin: <0x140>
    <b2>   DW_AT_location    : 6 byte block: f2 0 0 0 69 4 	(DW_OP_GNU_implicit_pointer: <0x69> 4)
 <3><b9>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <ba>   DW_AT_abstract_origin: <0x149>
    <be>   DW_AT_location    : 6 byte block: f2 0 0 0 69 8 	(DW_OP_GNU_implicit_pointer: <0x69> 8)
 <3><c5>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <c6>   DW_AT_abstract_origin: <0x152>
    <ca>   DW_AT_location    : 6 byte block: f2 0 0 0 69 c 	(DW_OP_GNU_implicit_pointer: <0x69> 12)
 <3><d1>: Abbrev Number: 0
 <2><d2>: Abbrev Number: 10 (DW_TAG_inlined_subroutine)
    <d3>   DW_AT_abstract_origin: <0x12f>
    <d7>   DW_AT_low_pc      : 0x10000424
    <df>   DW_AT_high_pc     : 0x6
    <e7>   DW_AT_call_file   : 1
    <e8>   DW_AT_call_line   : 24
 <3><e9>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <ea>   DW_AT_abstract_origin: <0x137>
    <ee>   DW_AT_location    : 6 byte block: f2 0 0 0 69 0 	(DW_OP_GNU_implicit_pointer: <0x69> 0)
 <3><f5>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <f6>   DW_AT_abstract_origin: <0x140>
    <fa>   DW_AT_location    : 6 byte block: f2 0 0 0 69 4 	(DW_OP_GNU_implicit_pointer: <0x69> 4)
 <3><101>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <102>   DW_AT_abstract_origin: <0x149>
    <106>   DW_AT_location    : 6 byte block: f2 0 0 0 69 8 	(DW_OP_GNU_implicit_pointer: <0x69> 8)
 <3><10d>: Abbrev Number: 9 (DW_TAG_formal_parameter)
    <10e>   DW_AT_abstract_origin: <0x152>
    <112>   DW_AT_location    : 6 byte block: f2 0 0 0 69 c 	(DW_OP_GNU_implicit_pointer: <0x69> 12)
 <3><119>: Abbrev Number: 0
 <2><11a>: Abbrev Number: 0
 <1><11b>: Abbrev Number: 11 (DW_TAG_base_type)
    <11c>   DW_AT_byte_size   : 16
    <11d>   DW_AT_encoding    : 5	(signed)
    <11e>   DW_AT_name        : (indirect string, offset: 0x70): __int128
 <1><122>: Abbrev Number: 12 (DW_TAG_pointer_type)
    <123>   DW_AT_byte_size   : 8
    <124>   DW_AT_type        : <0x128>
 <1><128>: Abbrev Number: 11 (DW_TAG_base_type)
    <129>   DW_AT_byte_size   : 1
    <12a>   DW_AT_encoding    : 8	(unsigned char)
    <12b>   DW_AT_name        : (indirect string, offset: 0x79): char
 <1><12f>: Abbrev Number: 13 (DW_TAG_subprogram)
    <130>   DW_AT_name        : bar
    <134>   DW_AT_decl_file   : 1
    <135>   DW_AT_decl_line   : 8
    <136>   DW_AT_prototyped  : 1
    <136>   DW_AT_inline      : 1	(inlined)
 <2><137>: Abbrev Number: 14 (DW_TAG_formal_parameter)
    <138>   DW_AT_name        : a
    <13a>   DW_AT_decl_file   : 1
    <13b>   DW_AT_decl_line   : 8
    <13c>   DW_AT_type        : <0x122>
 <2><140>: Abbrev Number: 14 (DW_TAG_formal_parameter)
    <141>   DW_AT_name        : b
    <143>   DW_AT_decl_file   : 1
    <144>   DW_AT_decl_line   : 8
    <145>   DW_AT_type        : <0x122>
 <2><149>: Abbrev Number: 14 (DW_TAG_formal_parameter)
    <14a>   DW_AT_name        : c
    <14c>   DW_AT_decl_file   : 1
    <14d>   DW_AT_decl_line   : 8
    <14e>   DW_AT_type        : <0x122>
 <2><152>: Abbrev Number: 14 (DW_TAG_formal_parameter)
    <153>   DW_AT_name        : d
    <155>   DW_AT_decl_file   : 1
    <156>   DW_AT_decl_line   : 8
    <157>   DW_AT_type        : <0x122>
 <2><15b>: Abbrev Number: 0
 <1><15c>: Abbrev Number: 0

Contents of the .debug_loc section:

    Offset   Begin            End              Expression
    00000000 0000000010000418 0000000010000424 (DW_OP_implicit_value 16 byte block: 0 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff )
    00000024 0000000010000424 000000001000042a (DW_OP_implicit_value 16 byte block: ff ee dd cc bb aa 99 88 77 66 55 44 33 22 11 0 )
    00000048 <End of list>


[-- Attachment #4: GDB session --]
[-- Type: text/plain, Size: 676 bytes --]

Reading symbols from implvalue...done.
(gdb) b main
Breakpoint 1 at 0x10000418: file implvalue.c, line 15.
(gdb) r
Starting program: /home/arnez/tmp/implvalue 

Breakpoint 1, main () at implvalue.c:19
(gdb) si
0x000000001000041e in bar (a=<synthetic pointer>, b=<synthetic pointer>, 
    c=<synthetic pointer>, d=<synthetic pointer>) at implvalue.c:15
(gdb) p/x (char [4]) {*a, *b, *c, *d}
$1 = {0xff, 0xff, 0xff, 0xff}
(gdb) p/x *(char (*)[16]) a
$2 = {0x0, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 
  0xee, 0xff}
(gdb) p/x (char [8]) {a[0], a[1], a[2], a[3], a[4], a[5], a[6], a[7]}
$3 = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}

Re: Should a DW_OP_implicit_value be taken from the left end?

[Jan Kratochvil]

On Fri, 23 Dec 2016 19:18:22 +0100, Andreas Arnez wrote:
> Right, that's another issue.  If we revert the change for
> DW_OP_implicit_value, it doesn't matter anymore,

After checking your provided example I agree my patch was wrong.

I agree that DW_OP_implicit_value should be correct after reverting my patch.


> but the same bug occurs
> with DW_OP_stack_value as well.  I'll handle that separately.

Yes, DW_OP_stack_value I probably fixed for my specific testcase accidentally.
It needs some different fix than the fix by me.  I am fine/prefer to even
revert that part assuming you apply some correct/new fix afterwards instead.


> Note that I don't intend to revert the whole patch, only the part
> affecting DW_OP_implicit_value (DWARF_VALUE_LITERAL).

I am fine with either way, reverting just DWARF_VALUE_LITERAL or both.


> Is there any any other information about the rationale of that change?

I do everything based on real execution results and my fix (accidentally)
fixed the entry-values testcase for me on big-endian.


Thanks,
Jan