[llvm-dev] [RFC] Generate Debug Information for Labels in Function
Hsiangkai Wang via llvm-dev
I would like to enhance LLVM debug info that supports setting
breakpoint on labels in function.
Generally, if users use GDB as their debugger, they could set
breakpoints on labels in function. Following is an example.
// C program
static int
myfunction (int arg)
{
int i, j, r;
j = 0; /* myfunction location */
r = arg;
top:
++j; /* top location */
if (j == 10)
goto done;
for (i = 0; i < 10; ++i)
{
r += i;
if (j % 2)
goto top;
}
done:
return r;
}
int
main (void)
{
int i, j;
for (i = 0, j = 0; i < 1000; ++i)
j += myfunction (0);
return 0;
}
Following is the GDB commands to illustrate how to set breakpoints on labels.
(gdb) b main
Breakpoint 1 at 0x10298: file explicit.c, line 50.
(gdb) r
Starting program: /home/users/kai/sandbox/gdbtest/explicit-gcc
Breakpoint 1, main () at explicit.c:50
50 for (i = 0, j = 0; i < 1000; ++i)
(gdb) b myfunction:top
Breakpoint 2 at 0x10214: file explicit.c, line 26.
(gdb) c
Continuing.
Breakpoint 2, myfunction (arg=0) at explicit.c:27
27 ++j; /\* top location */
(gdb)
However, LLVM does not generate debug information for labels. So, the
feature could not work for binaries generated by clang. I also found
that the problem is reported in PR35526 and PR36420. I propose an
implementation plan to support debug information for labels.
Following are the steps I propose to implement the feature.
1. Define debug metadata and intrinsic functions for labels.
First of all, we need to record debug information in LLVM IR. In LLVM
IR, LLVM uses metadata and intrinsic function to keep debug
information. So, I need to define new kind of metadata, DILabel, and
new intrinsic function, llvm.dbg.label, to associate DILabel with
label statement.
DILabel will contain name of the label, file metadata, line number,
and scope metadata.
Intrinsic function llvm.dbg.label uses DILabel metadata as its parameter.
2. Create MI instruction DBG_LABEL.
I create new MI instruction DBG_LABEL to keep debug information after
LLVM IR converted to MI.
DBG_LABEL uses DILabel metadata as its parameter.
3. Create data structure, SDDbgLabel, to store debug information of
labels in SelectionDAG.
In SelectionDAG, we need a data structure to keep debug information of
label. It will keep DILabel metadata.
4. Convert SDDbgLabel to DBG_LABEL in SelectionDAG.
After EmitSchedule(), SelectionDAG will be converted to a list of MI
instructions. In the function, we will generate DBG_LABEL MachineInstr
from SDDbgLabel.
For FastISel and GlobalISel, we could convert llvm.dbg.label to
DBG_LABEL directly.
5. Collect debug information of labels from MI listing to DebugHandlerBase.
Before generating actual debug information in assembly format or
object format, we need to keep debug format-independent data in
DebugHandlerBase. Afterwards, we could convert these data to CodeView
format or DWARF format.
6. Create DWARF DIE specific data structure in DwarfDebug.
In class DwarfDebug, we keep DWARF specific data structure for DILabel.
7. Generate DW_TAG_label and fill details of DW_TAG_label.
Finally, generating DW_TAG_label DIE and its attributes into DIE structure.
I am looking forward to any thoughts & feedback!
_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reid Kleckner via llvm-dev
陳韋任 via llvm-dev
like to review it. ;)
2018-03-28 10:41 GMT+08:00 Hsiangkai Wang via llvm-dev
<llvm...@lists.llvm.org>:
--
Wei-Ren Chen (陳韋任)
Homepage: https://people.cs.nctu.edu.tw/~chenwj
Hsiangkai Wang via llvm-dev
Thanks for your suggestions. I dug in the implementation of
llvm.codeview.annotation. It seems to create a temp symbol as handling
Intrinsic::codeview_annotation and keep the mapping of this temp
symbol to some metadata. The mapping will be kept in MachineFunction.
Although it converts this intrinsic function to ANNOTATION_LABEL MI,
it seems no use of ANNOTATION_LABEL MI. All the debug information
generated from CodeViewDebug depends on mappings kept in
MachineFunction.
It is very different with the scenario to enable debugging with label.
Debugging with label needs to know file, line, and address of labels
in functions. It needs to keep these information from LLVM IR down to
MI and generate assembly through AsmPrinter finally. I will send
patches after I finish some clean up of these patches. Thanks a lot
anyway.
Hsiangkai Wang via llvm-dev
I will send my patches after I polish them in more detail. I am
looking forward to your review. Thanks a lot.
Adrian Prantl via llvm-dev
Thank you for working on this! I think it would be good to support labels better. IIRC it currently only generates them from assembler sources in place of a DW_TAG_subprogram.
> Following are the steps I propose to implement the feature.
>
> 1. Define debug metadata and intrinsic functions for labels.
>
> First of all, we need to record debug information in LLVM IR. In LLVM
> IR, LLVM uses metadata and intrinsic function to keep debug
> information. So, I need to define new kind of metadata, DILabel, and
> new intrinsic function, llvm.dbg.label, to associate DILabel with
> label statement.
>
> DILabel will contain name of the label, file metadata, line number,
> and scope metadata.
>
> Intrinsic function llvm.dbg.label uses DILabel metadata as its parameter.
Looking at your testcase in https://reviews.llvm.org/D45043
br label %top
top:
call void @llvm.dbg.label(metadata !10), !dbg !11
%0 = load i32, i32* %a.addr, align 4
Modelling the IR this way is problematic. In a llvm.dbg.value intrinsic we tie the SSA value the intrinsic describes to the intrinsic by making it an explicit argument of the intrinsic. In the example above, this is not the case, and optimizations will likely move the label and the intrinsic further apart, or even duplicate the intrinsic during loop unrolling. If you want to have additional metadata for a label, I think it would be better to allow a BasicBlock to carry a !dbg attachment. In IR assembler this could look like this:
top, !label !10, !dbg !11:
That said, perhaps this isn't even necessary. The only information that is stored in DILabel is the name of the label (which is redundant with the actual name of the label) and its source location, which is also stored in the DILocation (!11). I'm wondering if the DILocation of a label is even useful. When a debugger user sets a breakpoint of a label, we might as well use the location of the first instruction in the basic block described by the label, since that is where execution will continue.
Based on that I think it might be sufficient to have a flag on an IR label that marks a user-originated label and triggers the backend to create a DW_TAG_label for it. If we do need source location information for the DW_TAG_label, we could grab it from the first instruction.
Let me know what you think!
-- adrian
via llvm-dev
> that marks a user-originated label and triggers the backend to create a
> DW_TAG_label for it. If we do need source location information for the
> DW_TAG_label, we could grab it from the first instruction.
Are there languages where labels are scoped? If so we'd need explicit
metadata to identify the parent scope. In C/C++ the only scope is the
containing function, which I'm guessing we wouldn't need to specify
(except for inlined functions? that might need some care to get right).
I feel compelled to point out that the top instructions in a block do not
necessarily have a valid source location so we might want to carry that
explicitly just for sanity's sake.
--paulr
via llvm-dev
> -----Original Message-----
> From: llvm-dev [mailto:llvm-dev...@lists.llvm.org] On Behalf Of via
> llvm-dev
> Sent: Thursday, March 29, 2018 1:54 PM
> To: apr...@apple.com; hsia...@gmail.com
> Cc: llvm...@lists.llvm.org
> Subject: Re: [llvm-dev] [RFC] Generate Debug Information for Labels in
> Function
>
> > Based on that I think it might be sufficient to have a flag on an IR
> label
> > that marks a user-originated label and triggers the backend to create a
> > DW_TAG_label for it. If we do need source location information for the
> > DW_TAG_label, we could grab it from the first instruction.
>
> Are there languages where labels are scoped? If so we'd need explicit
> metadata to identify the parent scope. In C/C++ the only scope is the
> containing function, which I'm guessing we wouldn't need to specify
> (except for inlined functions? that might need some care to get right).
>
> I feel compelled to point out that the top instructions in a block do not
> necessarily have a valid source location so we might want to carry that
> explicitly just for sanity's sake.
Adrian Prantl via llvm-dev
> On Mar 29, 2018, at 10:55 AM, paul.r...@sony.com wrote:
>
>
>
>> -----Original Message-----
>> From: llvm-dev [mailto:llvm-dev...@lists.llvm.org] On Behalf Of via
>> llvm-dev
>> Sent: Thursday, March 29, 2018 1:54 PM
>> To: apr...@apple.com; hsia...@gmail.com
>> Cc: llvm...@lists.llvm.org
>> Subject: Re: [llvm-dev] [RFC] Generate Debug Information for Labels in
>> Function
>>
>>> Based on that I think it might be sufficient to have a flag on an IR
>> label
>>> that marks a user-originated label and triggers the backend to create a
>>> DW_TAG_label for it. If we do need source location information for the
>>> DW_TAG_label, we could grab it from the first instruction.
>>
>> Are there languages where labels are scoped? If so we'd need explicit
>> metadata to identify the parent scope. In C/C++ the only scope is the
>> containing function, which I'm guessing we wouldn't need to specify
>> (except for inlined functions? that might need some care to get right).
>>
>> I feel compelled to point out that the top instructions in a block do not
>> necessarily have a valid source location so we might want to carry that
>> explicitly just for sanity's sake.
>
> That is, carry the source location explicitly on the label.
Thanks that's a good point. I still wonder what a debugger would do with the source location of a label though? For setting breakpoints it seems redundant with the location of the next instruction. Does GCC emit source location for a label, or just its name? If yes, does GDB do anything with that information?
-- adrian
via llvm-dev
A GUI debugger might show the source text near the label definition.
Suppose the first code after the label is an inlined function call.
Using the instruction's location would take you to the source for the
inlined function, not to the label and the source for the call.
--paulr
Adrian Prantl via llvm-dev
Yes, but LLDB at least only looks at .debug_line to determine the source to display for a given PC. This is quite annoying at -O0 with always_inline constructors in libcxx.
A label immediately followed by an inlined instruction will be invisible to the .debug_line table. It would also be ambiguous whether the label is part of a (lexical, inlined) scope in the .debug_info section if it appears at the beginning that scope.
-- adrian
via llvm-dev
> with
> >> the source location of a label though? For setting breakpoints it seems
> >> redundant with the location of the next instruction. Does GCC emit
> source
> >> location for a label, or just its name? If yes, does GDB do anything
> with
> >> that information?
> >>
> >> -- adrian
> >
> > A GUI debugger might show the source text near the label definition.
> > Suppose the first code after the label is an inlined function call.
> > Using the instruction's location would take you to the source for the
> > inlined function, not to the label and the source for the call.
>
> Yes, but LLDB at least only looks at .debug_line to determine the source
> to display for a given PC. This is quite annoying at -O0 with
> always_inline constructors in libcxx.
If you're stopped at the goto, you could do a "show me the definition"
of the label, and you'd want the source location of the label.
>
> A label immediately followed by an inlined instruction will be invisible
> to the .debug_line table. It would also be ambiguous whether the label is
> part of a (lexical, inlined) scope in the .debug_info section if it
> appears at the beginning that scope.
>
> -- adrian
>
The label DIE does need to be in the correct scope. Hmm it would need
to be a child of the inlined subprogram DIE because its code address
would be different for each inlined instance.
--paulr
Hsiangkai Wang via llvm-dev
better solution. I will change my design to convey debug metadata
through basic block instead of intrinsic.
https://reviews.llvm.org/D45078
> That said, perhaps this isn't even necessary. The only information that is stored in DILabel is the name of the label (which is redundant with the actual name of the label) and its source location, which is also stored in the DILocation (!11). I'm wondering if the DILocation of a label is even useful. When a debugger user sets a breakpoint of a label, we might as well use the location of the first instruction in the basic block described by the label, since that is where execution will continue.
>
> Based on that I think it might be sufficient to have a flag on an IR label that marks a user-originated label and triggers the backend to create a DW_TAG_label for it. If we do need source location information for the DW_TAG_label, we could grab it from the first instruction.
I still think that we should collect debug information from source
code level instead of infer from instructions in the basic block. As
Paul said, "the top instructions in a block do not necessarily have a
valid source location." So, I will keep DILabel metadata and remove
llvm.dbg.label intrinsic.
Adrian Prantl via llvm-dev
> On Mar 29, 2018, at 11:29 PM, Hsiangkai Wang <hsia...@gmail.com> wrote:
>
> I agree with you. Attach debug metadata to basic block will be a
> better solution. I will change my design to convey debug metadata
> through basic block instead of intrinsic.
>
> https://reviews.llvm.org/D45078
In this revised design it is now possible to attach a DILabel to a BasicBlock. When the basic block is inlined it will be ambiguous to which function the DILabel belongs. For instructions, we store the inline information in the inlinedAt: field of its DILocation. In order to handle inlining for DILabels we have two options:
1. Also attach a DILocation to be associated with the label to carry the inline information, and teach the inliner to correctly update the DILocation on basic blocks during inlining. This would also solve the issue of hypothetical scoped labels that Paul brought up. We'll also need to figure out what to do when two labels are being merged by a transformation.
2. Teach the inliner to drop all metadata attachments on basic blocks.
Option (2) is obviously going to be easier to implement and might be a good as a first step.
>
>> That said, perhaps this isn't even necessary. The only information that is stored in DILabel is the name of the label (which is redundant with the actual name of the label) and its source location, which is also stored in the DILocation (!11). I'm wondering if the DILocation of a label is even useful. When a debugger user sets a breakpoint of a label, we might as well use the location of the first instruction in the basic block described by the label, since that is where execution will continue.
>>
>> Based on that I think it might be sufficient to have a flag on an IR label that marks a user-originated label and triggers the backend to create a DW_TAG_label for it. If we do need source location information for the DW_TAG_label, we could grab it from the first instruction.
> I still think that we should collect debug information from source
> code level instead of infer from instructions in the basic block. As
> Paul said, "the top instructions in a block do not necessarily have a
> valid source location." So, I will keep DILabel metadata and remove
> llvm.dbg.label intrinsic.
I'm still not convinced that this information will be useful to a debugger, but if you have a compelling use-case please let me know.
-- adrian
Adrian Prantl via llvm-dev
> On Mar 30, 2018, at 9:25 AM, Adrian Prantl via llvm-dev <llvm...@lists.llvm.org> wrote:
>
>
>
>> On Mar 29, 2018, at 11:29 PM, Hsiangkai Wang <hsia...@gmail.com> wrote:
>>
>> I agree with you. Attach debug metadata to basic block will be a
>> better solution. I will change my design to convey debug metadata
>> through basic block instead of intrinsic.
>>
>> https://reviews.llvm.org/D45078
>
> In this revised design it is now possible to attach a DILabel to a BasicBlock. When the basic block is inlined it will be ambiguous to which function the DILabel belongs. For instructions, we store the inline information in the inlinedAt: field of its DILocation. In order to handle inlining for DILabels we have two options:
>
> 1. Also attach a DILocation to be associated with the label to carry the inline information, and teach the inliner to correctly update the DILocation on basic blocks during inlining. This would also solve the issue of hypothetical scoped labels that Paul brought up. We'll also need to figure out what to do when two labels are being merged by a transformation.
>
> 2. Teach the inliner to drop all metadata attachments on basic blocks.
>
> Option (2) is obviously going to be easier to implement and might be a good as a first step.
I'm really sorry for not realizing this yesterday, but the problems pertaining to inlining made me realize that your original design with the dbg.label intrinsic might actually be a better approach especially when considering optimized code. We will get inlining support for free because it is just another instruction and it can deal with more than one label at the same address. It looks a bit more complicated in unoptimized code, but that seems like a small price to pay.
We just need to make sure that the backend doesn't get confused when loop unrolling duplicates a dbg.label but that should be doable.
-- adrian
Reid Kleckner via llvm-dev
I'm really sorry for not realizing this yesterday, but the problems pertaining to inlining made me realize that your original design with the dbg.label intrinsic might actually be a better approach especially when considering optimized code. We will get inlining support for free because it is just another instruction and it can deal with more than one label at the same address. It looks a bit more complicated in unoptimized code, but that seems like a small price to pay.
We just need to make sure that the backend doesn't get confused when loop unrolling duplicates a dbg.label but that should be doable.
Hsiangkai Wang via llvm-dev
I am sorry that I didn’t carefully think about how to handle labels in
inlined function.
Today, I did some simple experiments and found that some basic block
may be removed in CFGSimplifyPass and the attached metadata will be
eliminated. So, if the optimization is turned on, label metadata will
disappear. However, intrinsic will keep existing in the function if we
handle it correctly. So, if we take care about optimization and
inlining, to keep label metadata through intrinsic could be right for
free.
I learned a lot from discussions. Thanks for your comments and
suggestions. If there is no other concern, I will keep my original
implementation and send patches to Phabricator again. Sorry have
bothered you.
Any comments?
via llvm-dev
> -----Original Message-----
> From: Hsiangkai Wang [mailto:hsia...@gmail.com]
> Sent: Sunday, April 01, 2018 12:12 PM
> To: Reid Kleckner
> Cc: Adrian Prantl; llvm-dev; Robinson, Paul; David Blaikie; Eric
> Christopher
> Subject: Re: [llvm-dev] [RFC] Generate Debug Information for Labels in
> Function
>
>
> I am sorry that I didn’t carefully think about how to handle labels in
> inlined function.
>
> Today, I did some simple experiments and found that some basic block
> may be removed in CFGSimplifyPass and the attached metadata will be
> eliminated. So, if the optimization is turned on, label metadata will
> disappear. However, intrinsic will keep existing in the function if we
> handle it correctly. So, if we take care about optimization and
> inlining, to keep label metadata through intrinsic could be right for
> free.
in ways such that it would require a lot of attention to preserve metadata
correctly. An intrinsic is harder to lose. :-) I suppose if the entire
basic block is optimized away, the label is really dead, and in that case
it is okay to remove the intrinsic.
>
> I learned a lot from discussions. Thanks for your comments and
> suggestions. If there is no other concern, I will keep my original
> implementation and send patches to Phabricator again. Sorry have
> bothered you.
the conclusion is "it was right the first time."
>
> Any comments?
The previous patch set looked reasonable, however please post patches
in testable units, with their tests in the same review. This helps
reviewers to understand what you are doing, and makes it easier to see
whether there are missing pieces.
Thanks!
--paulr
Adrian Prantl via llvm-dev
> On Apr 1, 2018, at 9:12 AM, Hsiangkai Wang <hsia...@gmail.com> wrote:
>
> Hi all,
>
> I am sorry that I didn’t carefully think about how to handle labels in
> inlined function.
There is no need to apologize! Thank you very much for engaging in the discussion and for contributing you patches.
>
> Today, I did some simple experiments and found that some basic block
> may be removed in CFGSimplifyPass and the attached metadata will be
> eliminated. So, if the optimization is turned on, label metadata will
> disappear. However, intrinsic will keep existing in the function if we
> handle it correctly. So, if we take care about optimization and
> inlining, to keep label metadata through intrinsic could be right for
> free.
>
> I learned a lot from discussions. Thanks for your comments and
> suggestions. If there is no other concern, I will keep my original
> implementation and send patches to Phabricator again.
That sounds like a good plan.
-- adrian
Son Tuan VU via llvm-dev
Hsiangkai Wang via llvm-dev
About Clang part, I have submitted in https://reviews.llvm.org/D45045.
However, it has been reverted due to broken Chromium build. I have
submitted a patch to fix the problem in
https://reviews.llvm.org/D46738.
About DWARF debug emission, I have submitted in
https://reviews.llvm.org/D45556 and the patch is under reviewing.
Welcome to give suggestions in these patches. Thanks.
Kai
Björn Pettersson A via llvm-dev
I also see that there has been some talk about loop unrolling etc. when
discussing this RFC. Was there a consensus on what should happen if we
end up with multiple dbg.label intrinsics for the same label due to
code duplication?
Here is an example where we get two "#DEBUG_LABEL: main:foo" but only one
DW_TAG_label for "foo": https://godbolt.org/z/oTpHQp
Maybe that is as expected, or is it allowed to have multiple DW_TAG_label
for the same label name?
/Björn
> -----Original Message-----
> From: llvm-dev <llvm-dev...@lists.llvm.org> On Behalf Of Adrian
> Prantl via llvm-dev
> Sent: den 30 mars 2018 18:40
> To: Adrian Prantl <apr...@apple.com>
> Cc: llvm-dev <llvm...@lists.llvm.org>
> Subject: Re: [llvm-dev] [RFC] Generate Debug Information for Labels in
> Function
>
>
>
David Blaikie via llvm-dev
I suspect the rightiest thing to do is probably multiple DW_TAG_labels with the same name, if that's what's happened.
I guess another question/possibility - what happens if some inline asm references the label? I guess it probably constrains the optimizer not to unroll? Maybe? If the label can be duplicated and inline asm can still correctly reference the label, maybe that's the answer - describe the label the same way inline asm "sees" it.
Björn Pettersson A via llvm-dev
Well, I haven’t seen that GCC is emitting more than one DW_TAG_label per label either. So maybe clang is on the same “level” now (or I haven’t triggered that kind of optimization where you get several DW_TAG_label when using GCC).
Maybe setting breakpoints on labels aren’t supposed to be that safe when using -O1 and higher (you aren’t guaranteed that you get a break every time execution passes the label).
Or we should try to emit multiple DW_TAG_labels with the same name to see what happens.
This is not that important to me, just something I discovered when trying out the new feature. I thought that perhaps this was well known and in the category “not done yet / work still ongoing”.
/Björn
via llvm-dev
I think it's more like "nobody brought it up before" rather than intentional.
Emitting multiple copies of the DW_TAG_label… might confuse debuggers, especially as they'll all have the same name. But we don't want to muck with the name to discriminate the instances. I don't have an answer here, unless we want words in the DWARF spec that say "you might see multiple labels with the same name in the same scope when…"
The situation feels vaguely like when a function is inlined multiple places and might also have an out-of-line instance. "Break on foo" might only catch the out-of-line instance, which if it's never called, is not a great debugging experience either.