- User Since
- May 9 2013, 11:10 AM (236 w, 2 d)
The FORM that's appropriate for DW_AT_location to use as a reference varies with the DWARF version. In v2 and v3, it's FORM_data4 or FORM_data8, in v4 it's FORM_sec_offset. I haven't looked at the verifier much but it seems like using the correct FORM is something that ought to be verifiable.
Thu, Nov 16
Thanks for accepting, Jonas! FWIW accepting by itself does not cause Phab to send email, there still needs to be a comment.
And committing this has to wait on D39854 which still needs review.
Wed, Nov 15
@rafael this looks pretty benign to me, but MC is not my area. What do you think?
Tue, Nov 14
Use address-size zero for the DWARFDataExtractor, in the .dwo section.
This required relaxing an assertion in the line-table parser. If there's no line-number program, the address size isn't needed, and if there is one, attempting to retrieve an address will assert farther down.
Mon, Nov 13
The results suggest a different problem. Emitting the aranges section should happen after the "kittens" and "rainbows" symbols are emitted. If the symbols were emitted, they should have nonzero and unique order. I don't see how shuffling the list can result in a different result for the aranges section.
Although MCStreamer::AssignFragment looks like it might be a victim of unspecified order-of-evaluation, still I would think the assigned orders remain nonzero and unique per symbol, and that's all that matters here.
So there can be multiple symbols with the same offset, in this list? This change implies that the insertion order in the list is important and worth preserving. Why do we know that's true?
Fri, Nov 10
Oh, head-smack! You don't need a unit to know the address size; the line table has always had that info, although indirectly.
DW_LNE_set_address (the only opcode that specifies an actual address) is an extended opcode, which has a ULEB telling you how long the operands are. Which is, one byte for the opcode, and.... the size of the address. Doh!
Drive by comment:
Thu, Nov 9
(Moving the why-do-we-need-the-unit discussion out of the inline comments.)
Address @dblaikie comments.
Tue, Nov 7
I left the existing test (dwarfdump-header.s) as a checked-in binary, because it tests both normal and split headers, and I don't know what MachO section names to use for the .dwo sections. My intuition is that MachO will never bother supporting split DWARF so maybe it's fine to leave it as is. If somebody does want me to convert the test, I'll need a list of section names to use, and I would do it as a follow-up. Just thought I'd put that out there.
Mon, Nov 6
It does seem like this could be very useful in a sequence of RUN commands, where previously we'd have to add a bunch of run-specific checks.
Fri, Nov 3
At Sony we build our own unified tree by pasting together several subtrees, including debuginfo-tests, and moving one will cause a bit of disruption. I remember the idea of moving debuginfo-tests has come up previously, and in principle I have no problem with it, but a more explicit announcement on llvm-dev/cfe-dev would be appropriate.
Wed, Nov 1
Have you tried the GDB suite yet? If it has no problems, and we can make LLDB happy, I'm okay with it.
Tue, Oct 31
Make the new test work for both ELF and Mach-O.
Simplify LLD change to never pass a Unit, as there's no test for it yet.
Mon, Oct 30
Fri, Oct 27
By passing a DWARFUnit into the line-table parser (if we have one), we allow it to handle indirect string forms, which require finding other sections. Previously the line-table parser couldn't do that (and didn't need to, prior to v5).
By passing DWARFFormParams to DWARFFormValue::extractValue() separately from the DWARFUnit, we allow interpreting forms according to the line-table's version/format, which could be different from the Unit's version/format.
Thu, Oct 26
Has this gone in? I'm wondering because I starting playing with the monorepo, ran cmake with -DLLDB_TEST_COMPILER=$PWD/bin/clang, and today's test failure seems to be trying to build the test program with the system compiler (gcc) rather than my copy of clang. But it looks like you're deprecating LLDB_TEST_COMPILER?
Tue, Oct 24
Have you tried this change against the GDB and LLDB test suites? If they have failures then we should think about whether those tests are over-specific or whether we should put this under a tuning option.
Mon, Oct 23
Anytime the code between saveIP() and restoreIP() could set the current debug location, it needs to be saved/restored along with the insertion point. I have to say the problem is not obvious to me here, so maybe saveIP/restoreIP should be changed (or eliminated in favor of always using InsertPointGuard). I'm not seeing a lot of places where saveIP/restoreIP are used.
Oct 12 2017
Oct 6 2017
Sep 29 2017
Abandoning. This change is irrelevant to the SCE debugger, and while I believe it could be made more complete and better reflect the original source (which is what the DWARF spec says names should be), I do not have time to pursue it.
Sep 28 2017
I think I will go ahead and commit this; it doesn't change the status quo for CodeView and if something is warranted we can do that in a follow-up.
Sep 27 2017
+rnk for the CodeView question.
command-line option is cc1 not driver
internal flag moved from LangOpts to CodeGenOpts and renamed
Sep 26 2017
Add a command-line flag to control emitting the template parameter children. Default to on for SCE debugger tuning.
I am not super excited about my choice of option name or the help text; alternate suggestions welcome.
Sep 21 2017
Well then! LGTM.
This came out of our sample-PGO work, wasn't sure it would be kosher for me to approve a patch from the guy in the next cube.
It's completely in line with the other similar patches that Andrea and Dehao have been doing. So if Craig is happy, I'm happy.
Sep 20 2017
Sep 15 2017
A few minor commentary points; nothing huge. I've been looking at this mainly with the thought of having to do something similar for DWARF 5 eventually, but it seems likely there won't be a lot to leverage as CodeView and DWARF appear to associate the hashes with files pretty differently. Oh well.
Sep 14 2017
My impression is that .eh_frame and .debug_frame are interchangeable and in fact you get only one or the other in any given compilation. So, if you ask for .debug_frame and the object has .eh_frame, I think we should still dump it; and vice versa.
Sep 13 2017
Sep 12 2017
UI is tricky. I think I lean in the direction that if the section doesn't exist, don't print anything, even the "contents:" line.
This is certainly how I want --all to behave, and obviously it's easiest and self-consistent if that's how the tool always behaves. So for example --debug-types when there is no .debug_types section should print nothing.
I could see an argument that if I explicitly asked for a particular section and it doesn't exist, I should get the "contents:" line with nothing after it. But that's really not how I want --all to behave.
If you use cmake to generate Visual Studio projects on Windows, do they use lit to run the tests? If so, do they run with this patch correctly? I have no opinion one way or the other but it's an environment nobody seems to have thought of before (including me).
Sep 11 2017
I would kind of like one option to dump whichever of .debug_info or .debug_info.dwo (etc) there is, so I don't have to know up front which section to be asking for. Would also be more user-friendly to people who kind of know DWARF but not at that level of detail.
I see that findPrologueEndLoc() is also checking the FrameSetup flag, so at least this makes the handling consistent, and relying on FrameSetup actually seems cleaner than relying on source locations. If you have time to investigate why the source locations seem wrong, that would be great.
LGTM after you fix the comment Adrian pointed out.
Sep 8 2017
I think the argument (and the test) would be stronger if it was based on the instruction sequence rather than offsets. For example if you generate asm and then show prologue_end is immediately before the call to get_arg.
Also we generally like to trim the IR source by removing unnecessary attribute sets.
r312803. Forgot to put the tag in the commit message....
Sep 7 2017
The new file goes in the debuginfo-tests directory. It's a separate project so that's probably not obvious from the diff.
Seriously simplified test (derived from DebugInfo/X86/discriminator2.ll, as suggested by dblaikie).
Sep 5 2017
The patch seems trivial enough but I do not have a good idea of the subtle differences in the various environments.
I have subscribed a couple of people who probably can tell whether this change makes sense.
An MIR test that set up the situation more directly might be better, although I am not super confident in my ability to hand-reduce an MIR input.
Aug 31 2017
I now think the tactic to pursue is making sure the containing DICompositeType(s) for the method have complete descriptions, and RAUW the temporary nodes, prior to calling CloneFunction. Actually wading around in the MDNodes, Types, and Decls to accomplish that result promises to be "interesting" and "educational" (advice/suggestions welcome).
Aug 29 2017
Aug 28 2017
Dumping the whole module, the temporary DICompositeType node for CBdVfsImpl is used both as the scope node for the DISubprogram, and also as the baseType of a DIDerivedType which is a pointer type in the type list for the subprogram.
Trying to understand the broader context here, I looked back through the list of revisions mentioned in PR33930 to see if that helped.
Aug 25 2017
Sorry it took so long to wrap my head around this, but it has been about 20 years since I've had to accommodate an intentional ABI breakage, and that's actually what you want to do. Remembering that case, I have a model for this one, and I understand what you are doing now.
I'm happy, but Adrian should have a chance to comment too.