public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Zoran Zaric <Zoran.Zaric@amd.com>
To: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: Re: [PATCH 10/13] gdb/testsuite: add .debug_loclists tests
Date: Fri, 29 Jan 2021 16:58:10 +0000	[thread overview]
Message-ID: <d2ecbaa6-555a-8a9b-902b-ae3e912acf8c@amd.com> (raw)
In-Reply-To: <beaa7ac3-e33e-2033-b807-2a7f900f4c19@polymtl.ca>


>>>>>
>>>>> Add tests for the various issues fixed in the previous patches.
>>>>>
>>>>> Add a new "loclists" procedure to the DWARF assembler, to allow
>>>>> generating .debug_loclists sections.
>>>>>
>>>>
>>>> Thank you for this contribution.
>>>>
>>>> Having a loclists support in DWARF assembler gives us so considerable testing flexibility and decouples the gdb testing even more from what compiler is expected to generate.
>>>>
>>>> The only thing missing now (at least in my mind) is the CFI support but that is a big project for the future.
>>>
>>> Given that:
>>>
>>>    - the actual assembler (GNU as or other) already has support for
>>>      specifying and generating CFI, and
>>>    - a test case that wants to use specific CFI would contain some
>>>      assembly code already, to control exactly which instructions are
>>>      generated
>>
>> This is not exactly true, you can always define a CFI that doesn't need any assembly code considering that register values can be set from the the test itself or if only CFI register rules that target a memory locations are used.
> 
> Isn't a CFI table essentially a big mapping that says:
> 
>    - At address X + 0, here's how you'll find the saved registers
>    - At address X + 2, here's how you'll find the saved registers
>    - ... and so on
> 
> So ultimately you want in your test case to know exactly which
> instructions are generated, and their addresses, to generate the CFI
> table, don't you?  And for that, won't you write assembly?

You can still write a CFI information that covers the whole function 
without going into details on a per instruction basis. In the same way 
how you can write a single location description for a variable right now 
without thinking if that variable exist in the actual code or is it 
mapped to only one location.

If someone wants to do it per instruction basis later, more power to them.

It is true that one really needs to know what they are doing but that is 
no different to the current symbolic information support.

> 
> I'm not sure I know what "CFI register rules that target a memory
> location" means.  A register from a previous frame that is currently
> saved in memory?  I don't really understand what that changes.

My point was that in that case you can still write a fairly complex CFI 
information that never touches ABI sensitive resources like a specific 
register.

This will get even more useful with future addition of address spaces, 
where pieces of a register might be spilled to different kinds of memory 
like AMD has in the case of their large vector registers.

> 
>> With the new extensions that I've contributed (and are currently under the review) the register rules mechanism now supports any location description to be part of the DWARF expression. With this extension, you can imagine that a very complex DWARF expression that doesn't use any potentially ABI sensitive resource, can still be written and tested in the same way as any variable location.
> 
> Hmm but in the end it's still just a sequence of opcodes, isn't it?

Same goes for variable location descriptions right? And we still support 
those in the DWARF assembler.

I have written full debug information in assembly in the past, and apart 
from encoding the DIE relationship by hand, I didn't really see that 
much difference between encoding variable location to a CFI entries.

Both equally unpleasant.

>> Another option is to only hand write a CFI table for the top level function (frame 0) and design a way to merge the original CFI generated by the compiler for other functions with the hand written one.
> 
> I don't remember how exactly things are merged by the linker, but I
> suppose that would work.  But again, you'd probably want to write that
> top-level frame 0 function in assembly - I think.  Or if don't need to
> write different rules for different instructions in the function, I
> guess that function could be written in C, and you make CFI rules for
> the whole function's range.  But you'd need a way to tell the compiler
> to not generate CFI for that particular function.

This function could be an empty function with a single asm statement 
that could be compiled separately (with the instructions to the compiler 
to not generate debug info for it).

We don't do that right now but it doesn't sound like a deal breaker.

> 
>>
>> Also, with a potential new operation DW_OP_LLVM_call_frame_entry_reg described at length here:
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fllvm.org%2Fdocs%2FAMDGPUDwarfExtensionsForHeterogeneousDebugging.html&amp;data=04%7C01%7CZoran.Zaric%40amd.com%7C20c7017e3b7f47e5f39808d8c46e9356%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637475326499683800%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Ww3TjCpYOjj7FtUuTSr93VqHEErO1LoLJFRJGK3hhFg%3D&amp;reserved=0
>>
>> This would allow us to test any CFI expression of the frame we are currently stopped in.
>>
>>>
>>> I don't think our assembler needs to know how to generate CFI, you'd
>>> just write it in platform-specific assembly.
>>>
>>
>> But, isn't this the case for any part of the DWARF assembler functionality?
>>
>> Writing any complex DWARF expression fast in any assembly is hard and time consuming.
> 
> No, you can't express symbolically any DWARF expression at the assembly
> level.  For example, you can't describe the tree of DWARF DIEs using
> directives in assembly.  All you can do is output the raw bytes using
> the .byte and friends directives, but that wouldn't be humanly feasible.
> So instead we have this higher level language (our DWARF assembler)
> where we describe the tree of DIEs and have it output the .byte
> directives for us.

This all depends of ones view of what is convenient and what is not.

 From this discussion it sounds like the DIE relationship was the main 
motivation for developing the DWARF assembler. I always though that it 
was for general convenience of writing debug information in one test.

While I agree that writing DIE relationships in a raw byte format is 
daunting, so is writing a really complex DWARF expression. This will get 
even more daunting when lane masking, address spaces and large vector 
registers come into play.

I have seen DWARF expression with over 15 operations and those are not 
fun to write in a byte format.

And why not give the ability to mix and match symbolic variable 
information over a frame change with hand written CFI rules in one test?

> 
>>
>> I agree that adding a CFI support requires more thought and effort and better understanding about how the CFI works and what is safe to use from a test writer perspective, but I can definitely see the benefit of adding that support in the future.
> 
> In the event we want to test a CFI construct that isn't yet supported by
> the assembler, then it could make sense to write own assembler that
> emits the right bytes directly.

I agree that what we have is good enough right now, but I wouldn't want 
us to dismiss the idea completely.

I might be tempted to implement it in the future :)

Zoran

  reply	other threads:[~2021-01-29 16:58 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-20  5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
2021-01-20  5:39 ` [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors Simon Marchi
2021-01-28 15:17   ` Zoran Zaric
2021-01-28 15:42     ` Simon Marchi
2021-02-25 19:20       ` Tom Tromey
2021-01-20  5:39 ` [PATCH 02/13] gdb/dwarf: fix bound check in read_rnglist_index Simon Marchi
2021-01-28 15:22   ` Zoran Zaric
2021-01-20  5:39 ` [PATCH 03/13] gdb/dwarf: add missing bound check to read_loclist_index Simon Marchi
2021-01-20  5:39 ` [PATCH 04/13] gdb/dwarf: remove unnecessary check in read_{rng, loc}list_index Simon Marchi
2021-01-20  5:39 ` [PATCH 05/13] gdb/dwarf: few fixes for handling DW_FORM_{rng, loc}listx Simon Marchi
2021-01-28 15:30   ` [PATCH 05/13] gdb/dwarf: few fixes for handling DW_FORM_{rng,loc}listx Zoran Zaric
2021-01-20  5:39 ` [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng, loc}list_index Simon Marchi
2021-01-28 15:39   ` [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index Zoran Zaric
2021-01-28 15:49     ` Simon Marchi
2021-01-28 15:54       ` Zoran Zaric
2021-01-20  5:39 ` [PATCH 07/13] gdb/dwarf: read DW_AT_ranges value as unsigned in partial_die_info::read Simon Marchi
2021-01-28 15:41   ` Zoran Zaric
2021-01-28 15:51     ` Simon Marchi
2021-01-20  5:39 ` [PATCH 08/13] gdb/testsuite: add .debug_rnglists tests Simon Marchi
2021-01-28 16:24   ` Zoran Zaric
2021-01-20  5:39 ` [PATCH 09/13] gdb/testsuite: DWARF assembler: add context parameters to _location Simon Marchi
2021-01-28 16:30   ` Zoran Zaric
2021-01-20  5:39 ` [PATCH 10/13] gdb/testsuite: add .debug_loclists tests Simon Marchi
2021-01-28 16:52   ` Zoran Zaric
2021-01-28 17:47     ` Simon Marchi
2021-01-29 10:13       ` Zoran Zaric
2021-01-29 15:57         ` Simon Marchi
2021-01-29 16:58           ` Zoran Zaric [this message]
2021-01-29 17:37             ` Simon Marchi
2021-01-20  5:39 ` [PATCH 11/13] gdb/dwarf: split dwarf2_cu::ranges_base in two Simon Marchi
2021-01-20  5:39 ` [PATCH 12/13] gdb/dwarf: make read_{loc, rng}list_index return sect_offset Simon Marchi
2021-02-25 19:26   ` Tom Tromey
2021-01-20  5:39 ` [PATCH 13/13] gdb/testsuite: add test for .debug_{rng, loc}lists section without offset array Simon Marchi
2021-02-02 15:43 ` [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d2ecbaa6-555a-8a9b-902b-ae3e912acf8c@amd.com \
    --to=zoran.zaric@amd.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@efficios.com \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).