[PATCH] Add a dwarf unit type to represent 24 bit values.

[PATCH] Add a dwarf unit type to represent 24 bit values.

[John Darrington]

	* include/dwarf2.h (enum dwarf_unit_type) [DE_EH_PE_udata3]: New member.
---
 include/dwarf2.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dwarf2.h b/include/dwarf2.h
index cf0039a92a..0fe88a3a7e 100644
--- a/include/dwarf2.h
+++ b/include/dwarf2.h
@@ -474,6 +474,7 @@ enum dwarf_unit_type
 #define DW_EH_PE_udata2		0x02
 #define DW_EH_PE_udata4		0x03
 #define DW_EH_PE_udata8		0x04
+#define DW_EH_PE_udata3		0x05
 #define DW_EH_PE_sleb128	0x09
 #define DW_EH_PE_sdata2		0x0A
 #define DW_EH_PE_sdata4		0x0B
-- 
2.11.0

Re: [PATCH] Add a dwarf unit type to represent 24 bit values.

[John Darrington]

Ping!

Does anyone have any objections or comments?

J'

Re: [PATCH] Add a dwarf unit type to represent 24 bit values.

[Jason Merrill]

On Mon, Aug 27, 2018 at 8:20 PM, John Darrington
<john@darrington.wattle.id.au> wrote:
>         * include/dwarf2.h (enum dwarf_unit_type) [DE_EH_PE_udata3]: New member.

This is a new macro, not a member of dwarf_unit_type.

What's the rationale?  Do you have a separate patch that uses this new macro?

Jason

Re: [PATCH] Add a dwarf unit type to represent 24 bit values.

[John Darrington]

On Mon, Sep 10, 2018 at 03:36:26PM +0100, Jason Merrill wrote:
     On Mon, Aug 27, 2018 at 8:20 PM, John Darrington
     <john@darrington.wattle.id.au> wrote:
     >         * include/dwarf2.h (enum dwarf_unit_type) [DE_EH_PE_udata3]: New member.
     
     
     What's the rationale?  Do you have a separate patch that uses this new macro?
     
Yes.   I there is an upcoming patch for GDB.  See 
https://sourceware.org/ml/gdb-patches/2018-08/msg00731.html

J'

Re: [PATCH] Add a dwarf unit type to represent 24 bit values.

[Jason Merrill]

On Mon, Sep 10, 2018 at 3:42 PM, John Darrington
<john@darrington.wattle.id.au> wrote:
> On Mon, Sep 10, 2018 at 03:36:26PM +0100, Jason Merrill wrote:
>      On Mon, Aug 27, 2018 at 8:20 PM, John Darrington
>      <john@darrington.wattle.id.au> wrote:
>      >         * include/dwarf2.h (enum dwarf_unit_type) [DE_EH_PE_udata3]: New member.
>
>
>      What's the rationale?  Do you have a separate patch that uses this new macro?
>
> Yes.   I there is an upcoming patch for GDB.  See
> https://sourceware.org/ml/gdb-patches/2018-08/msg00731.html

This looks like support for reading fixed 3-byte values from the
exception handling unwind information.  Do you expect this information
to ever need to store 3-byte values?  The offsets in the unwind info
don't need to correspond exactly to target word sizes, and if you use
an assembler that supports it (such as GNU as), the table will use
variable-length encoding anyway.

Jason

Re: [PATCH] Add a dwarf unit type to represent 24 bit values.

[John Darrington]

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

On Mon, Sep 10, 2018 at 04:40:58PM +0100, Jason Merrill wrote:
     On Mon, Sep 10, 2018 at 3:42 PM, John Darrington
     <john@darrington.wattle.id.au> wrote:
     > On Mon, Sep 10, 2018 at 03:36:26PM +0100, Jason Merrill wrote:
     >      On Mon, Aug 27, 2018 at 8:20 PM, John Darrington
     >      <john@darrington.wattle.id.au> wrote:
     >      >         * include/dwarf2.h (enum dwarf_unit_type) [DE_EH_PE_udata3]: New member.
     >
     >
     >      What's the rationale?  Do you have a separate patch that uses this new macro?
     >
     > Yes.   I there is an upcoming patch for GDB.  See
     > https://sourceware.org/ml/gdb-patches/2018-08/msg00731.html
     
     This looks like support for reading fixed 3-byte values from the
     exception handling unwind information.  Do you expect this information
     to ever need to store 3-byte values? 

Yes,  I've already come across that (which is why I created the patch).
Without this patch, GDB will barf.  See the attached backtrace.

     The offsets in the unwind info
     don't need to correspond exactly to target word sizes,

So far as I can tell, the dwarf-unwinder assumes that they do.  Chaning
it would require a rewrite.  I doubt the GDB team would be happy with
that.

     and if you use
     an assembler that supports it (such as GNU as), the table will use
     variable-length encoding anyway.

I must admit I don't understand what you are saying here.  I was using
GNU as.  To what table are you refering?
     

J'

[-- Attachment #2: bt --]
[-- Type: text/plain, Size: 6297 bytes --]

(top-gdb) bt
#0  internal_error (file=0x100afeac0 <_ZN3gdbL8in_placeE+86> "/home/john/binutils-gdb/gdb/dwarf2-frame.c", line=1535,
    fmt=0x100aff05b <dwarf2_frame_base+219> "Unsupported address size %d") at /home/john/binutils-gdb/gdb/common/errors.c:54
#1  0x000000010055bce1 in encoding_for_size (size=3) at /home/john/binutils-gdb/gdb/dwarf2-frame.c:1535
#2  0x000000010055be64 in read_encoded_value (unit=0x60086bf80, encoding=0 '\000', ptr_len=3, buf=0x60087c300 "",
    bytes_read_ptr=0xffffb82c, func_base=0) at /home/john/binutils-gdb/gdb/dwarf2-frame.c:1590
#3  0x000000010055cd63 in decode_frame_entry_1 (unit=0x60086bf80, start=0x60087c2f8 "", eh_frame_p=0, cie_table=0xffffb970,
    fde_table=0xffffb960, entry_type=EH_CIE_OR_FDE_TYPE_ID) at /home/john/binutils-gdb/gdb/dwarf2-frame.c:2046
#4  0x000000010055cefd in decode_frame_entry (unit=0x60086bf80, start=0x60087c2f8 "", eh_frame_p=0, cie_table=0xffffb970,
    fde_table=0xffffb960, entry_type=EH_CIE_OR_FDE_TYPE_ID) at /home/john/binutils-gdb/gdb/dwarf2-frame.c:2101
#5  0x000000010055d3fc in dwarf2_build_frame_info (objfile=0x60068a300) at /home/john/binutils-gdb/gdb/dwarf2-frame.c:2297
#6  0x000000010055c355 in dwarf2_frame_find_fde (pc=0xffffbac0, out_offset=0x0) at /home/john/binutils-gdb/gdb/dwarf2-frame.c:1719
#7  0x000000010055b97c in dwarf2_frame_sniffer (self=0x100afef00 <dwarf2_frame_unwind>, this_frame=0x600810530, this_cache=0x600810548)
    at /home/john/binutils-gdb/gdb/dwarf2-frame.c:1349
#8  0x00000001005cbd78 in frame_unwind_try_unwinder (this_frame=0x600810530, this_cache=0x600810548,
    unwinder=0x100afef00 <dwarf2_frame_unwind>) at /home/john/binutils-gdb/gdb/frame-unwind.c:106
#9  0x00000001005cbf35 in frame_unwind_find_by_frame (this_frame=0x600810530, this_cache=0x600810548)
    at /home/john/binutils-gdb/gdb/frame-unwind.c:165
#10 0x00000001005ca77c in get_frame_type (frame=0x600810530) at /home/john/binutils-gdb/gdb/frame.c:2623
#11 0x00000001006cb9c7 in print_frame_info (frame=0x600810530, print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1)
    at /home/john/binutils-gdb/gdb/stack.c:795
#12 0x00000001006ca373 in print_stack_frame (frame=0x600810530, print_level=0, print_what=SRC_AND_LOC, set_current_sal=1)
    at /home/john/binutils-gdb/gdb/stack.c:176
#13 0x0000000100611c8f in print_stop_location (ws=0xffffbec0) at /home/john/binutils-gdb/gdb/infrun.c:8042
#14 0x0000000100611ceb in print_stop_event (uiout=0x6006784d0) at /home/john/binutils-gdb/gdb/infrun.c:8059
#15 0x000000010041b911 in tui_on_normal_stop (bs=0x0, print_frame=1) at /home/john/binutils-gdb/gdb/tui/tui-interp.c:98
#16 0x00000001009a8986 in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&,                                  int&&) (__functor=..., __args#0=<unknown type in /home/john/bb/gdb/gdb, CU 0x546d40, DIE 0x65b331>,
    __args#1=<unknown type in /home/john/bb/gdb/gdb, CU 0x546d40, DIE 0x65b340>)
    at /usr/lib/gcc/x86_64-linux-gnu/7.3.0/include/c++/bits/std_function.h:316
#17 0x00000001009273ed in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const (this=0x600155a30, __args#0=0x0, __args#1=1)
    at /usr/lib/gcc/x86_64-linux-gnu/7.3.0/include/c++/bits/std_function.h:706
#18 0x00000001008c891f in gdb::observers::observable<bpstats*, int>::notify (this=0x100cf2aa0 <gdb::observers::normal_stop>, args#0=0x0,
    args#1=1) at /home/john/binutils-gdb/gdb/common/observable.h:106
#19 0x00000001006123f9 in normal_stop () at /home/john/binutils-gdb/gdb/infrun.c:8339
#20 0x0000000100607bf1 in start_remote (from_tty=1) at /home/john/binutils-gdb/gdb/infrun.c:3236
#21 0x000000010068fdcf in remote_target::start_remote (this=0x600850d00, from_tty=1, extended_p=0)
    at /home/john/binutils-gdb/gdb/remote.c:4767
#22 0x0000000100691278 in remote_target::open_1 (name=0x600051cde ":2159", from_tty=1, extended_p=0)
    at /home/john/binutils-gdb/gdb/remote.c:5638
#23 0x0000000100690005 in remote_target::open (name=0x600051cde ":2159", from_tty=1) at /home/john/binutils-gdb/gdb/remote.c:4862
#24 0x00000001006ed810 in open_target (args=0x600051cde ":2159", from_tty=1, command=0x60063e800)
    at /home/john/binutils-gdb/gdb/target.c:338
#25 0x00000001007583ff in do_sfunc (c=0x60063e800, args=0x600051cde ":2159", from_tty=1)
    at /home/john/binutils-gdb/gdb/cli/cli-decode.c:122
#26 0x000000010075b2a9 in cmd_func (cmd=0x60063e800, args=0x600051cde ":2159", from_tty=1)
    at /home/john/binutils-gdb/gdb/cli/cli-decode.c:1856
#27 0x000000010070e654 in execute_command (p=0x600051ce2 "9", from_tty=1) at /home/john/binutils-gdb/gdb/top.c:630
#28 0x00000001005b6ebb in command_handler (command=0x600051cd0 "") at /home/john/binutils-gdb/gdb/event-top.c:583
#29 0x00000001005b72a5 in command_line_handler (rl=0x600765110 "") at /home/john/binutils-gdb/gdb/event-top.c:774
#30 0x00000001005b6674 in gdb_rl_callback_handler (rl=0x600765110 "") at /home/john/binutils-gdb/gdb/event-top.c:213
#31 0x000000010079450b in rl_callback_read_char () at /home/john/binutils-gdb/readline/callback.c:220
#32 0x00000001005b6599 in gdb_rl_callback_read_char_wrapper_noexcept () at /home/john/binutils-gdb/gdb/event-top.c:175
#33 0x00000001005b6601 in gdb_rl_callback_read_char_wrapper (client_data=0x600051cf0) at /home/john/binutils-gdb/gdb/event-top.c:192
#34 0x00000001005b6d4a in stdin_event_handler (error=0, client_data=0x600051cf0) at /home/john/binutils-gdb/gdb/event-top.c:511
#35 0x00000001005b55c2 in handle_file_event (file_ptr=0x600689070, ready_mask=1) at /home/john/binutils-gdb/gdb/event-loop.c:733
#36 0x00000001005b5950 in gdb_wait_for_event (block=1) at /home/john/binutils-gdb/gdb/event-loop.c:859
#37 0x00000001005b4a57 in gdb_do_one_event () at /home/john/binutils-gdb/gdb/event-loop.c:347
#38 0x00000001005b4a92 in start_event_loop () at /home/john/binutils-gdb/gdb/event-loop.c:371
#39 0x00000001006338cf in captured_command_loop () at /home/john/binutils-gdb/gdb/main.c:330
#40 0x0000000100634e93 in captured_main (data=0xffffcb50) at /home/john/binutils-gdb/gdb/main.c:1173
#41 0x0000000100634f52 in gdb_main (args=0xffffcb50) at /home/john/binutils-gdb/gdb/main.c:1189
#42 0x00000001004010d0 in main (argc=2, argv=0xffffcbc0) at /home/john/binutils-gdb/gdb/gdb.c:32
(top-gdb)

Re: [PATCH] Add a dwarf unit type to represent 24 bit values.

[Jason Merrill]

On Thu, Sep 13, 2018 at 1:35 AM, John Darrington
<john@darrington.wattle.id.au> wrote:
> On Mon, Sep 10, 2018 at 04:40:58PM +0100, Jason Merrill wrote:
>      On Mon, Sep 10, 2018 at 3:42 PM, John Darrington
>      <john@darrington.wattle.id.au> wrote:
>      > On Mon, Sep 10, 2018 at 03:36:26PM +0100, Jason Merrill wrote:
>      >      On Mon, Aug 27, 2018 at 8:20 PM, John Darrington
>      >      <john@darrington.wattle.id.au> wrote:
>      >      >         * include/dwarf2.h (enum dwarf_unit_type) [DE_EH_PE_udata3]: New member.
>      >
>      >
>      >      What's the rationale?  Do you have a separate patch that uses this new macro?
>      >
>      > Yes.   I there is an upcoming patch for GDB.  See
>      > https://sourceware.org/ml/gdb-patches/2018-08/msg00731.html
>
>      This looks like support for reading fixed 3-byte values from the
>      exception handling unwind information.  Do you expect this information
>      to ever need to store 3-byte values?
>
> Yes,  I've already come across that (which is why I created the patch).
> Without this patch, GDB will barf.  See the attached backtrace.

Well, that's curious, given that GCC doesn't have that address type either.

Ah, looking closer, I see that we aren't dealing with the EH unwind
information, but rather the normal DWARF unwind information, which
uses

      /* The encoding for FDE's in a normal .debug_frame section
         depends on the target address size.  */
      cie->encoding = DW_EH_PE_absptr;

it seems strange that GDB then wants to use one of the other codes as
a proxy for loading a particular number of bytes.

I also notice that the default definition of DWARF2_ADDR_SIZE in GCC
has a comment,

/* The size of addresses as they appear in the Dwarf 2 data.
   Some architectures use word addresses to refer to code locations,
   but Dwarf 2 info always uses byte addresses.  On such machines,
   Dwarf 2 addresses need to be larger than the architecture's
   pointers.  */
#define DWARF2_ADDR_SIZE ((POINTER_SIZE + BITS_PER_UNIT - 1) / BITS_PER_UNIT)

And many targets with smaller pointers override this in GCC and GAS, e.g.

./gas/config/tc-msp430.h:#define DWARF2_ADDR_SIZE(bfd) 4
./gas/config/tc-s12z.h:#define DWARF2_ADDR_SIZE(bfd) 4
./gas/config/tc-wasm32.h:#define DWARF2_ADDR_SIZE(bfd)           4
./gas/config/tc-xgate.h:#define DWARF2_ADDR_SIZE(bfd) 4
./gas/config/tc-aarch64.c:/* Implement DWARF2_ADDR_SIZE.  */
./gas/config/tc-csky.h:#define DWARF2_ADDR_SIZE(bfd)   4
./gas/config/tc-avr.h:#define DWARF2_ADDR_SIZE(bfd) 4

Is this appropriate for your target, as well?

Perhaps GCC should double check that DWARF2_ADDR_SIZE is 2, 4, or 8.

Jason

Re: [PATCH] Add a dwarf unit type to represent 24 bit values.

[John Darrington]

Thank you for your insight.  I will investigate further and see if I can
find out what is going on.

J'

On Thu, Sep 13, 2018 at 01:33:34PM -0400, Jason Merrill wrote:

     Well, that's curious, given that GCC doesn't have that address type either.
     
     Ah, looking closer, I see that we aren't dealing with the EH unwind
     information, but rather the normal DWARF unwind information, which
     uses
     
           /* The encoding for FDE's in a normal .debug_frame section
              depends on the target address size.  */
           cie->encoding = DW_EH_PE_absptr;
     
     it seems strange that GDB then wants to use one of the other codes as
     a proxy for loading a particular number of bytes.
     
     I also notice that the default definition of DWARF2_ADDR_SIZE in GCC
     has a comment,
     
     /* The size of addresses as they appear in the Dwarf 2 data.
        Some architectures use word addresses to refer to code locations,
        but Dwarf 2 info always uses byte addresses.  On such machines,
        Dwarf 2 addresses need to be larger than the architecture's
        pointers.  */
     #define DWARF2_ADDR_SIZE ((POINTER_SIZE + BITS_PER_UNIT - 1) / BITS_PER_UNIT)
     
     And many targets with smaller pointers override this in GCC and GAS, e.g.
     
     ./gas/config/tc-msp430.h:#define DWARF2_ADDR_SIZE(bfd) 4
     ./gas/config/tc-s12z.h:#define DWARF2_ADDR_SIZE(bfd) 4
     ./gas/config/tc-wasm32.h:#define DWARF2_ADDR_SIZE(bfd)           4
     ./gas/config/tc-xgate.h:#define DWARF2_ADDR_SIZE(bfd) 4
     ./gas/config/tc-aarch64.c:/* Implement DWARF2_ADDR_SIZE.  */
     ./gas/config/tc-csky.h:#define DWARF2_ADDR_SIZE(bfd)   4
     ./gas/config/tc-avr.h:#define DWARF2_ADDR_SIZE(bfd) 4
     
     Is this appropriate for your target, as well?
     
     Perhaps GCC should double check that DWARF2_ADDR_SIZE is 2, 4, or 8.
     
     Jason

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.