This patch teaches llvm-dsymutil to follow references to clang modules and recursively clone the debug info therein.
This does not yet resolve external type references.
Details
- Reviewers
friss
Diff Detail
- Repository
- rL LLVM
Event Timeline
test/tools/dsymutil/X86/modules.map | ||
---|---|---|
2–9 | The name of that file is a bit confusing because of the context it addresses. I'd like to see the actual module map file committed, and if you could rework that test to look like the other newer ones (and thus get rid of this map in favor of dummy-debug-map.map), this would be great. | |
test/tools/dsymutil/X86/modules.test | ||
4 | Is that copying really necessary? | |
5–6 | I usually ditch the tmp file in favor of -o - | llvm-dwarfdump | |
tools/dsymutil/DwarfLinker.cpp | ||
395 | I think that for your work, this line would be sufficient in this loop. The rest will be needed for --update support, but I need to first figure out accelerator tables. | |
1145–1146 | Can you do that refactoring in a preparatory NFC commit? | |
1479 | StringSet? | |
1499–1501 | Making that a static helper could be a separate NFC commit too, right? | |
2653–2657 | Took me some time to remember why we need that (or rather why we didn't need it before). Usually, the last DIE is always a NULL. It can happen that the input file is just a single compile_unit DIE though and that would crash here because we don't have a terminating NULL. A comment explaining that we should do that optimization would be nice. | |
3119–3121 | It might help the reader if there was a comment describing the layout of debug info containing a module reference so that it's trivial to understand how this function works. | |
3174 | this should be sys::TimeValue::PosixZeroTime(). | |
3188 | That condition seems unlikely to trigger. | |
tools/dsymutil/dsymutil.h | ||
31 | Separate NFC patch? |
LGTM once you address one nit:
tools/dsymutil/DwarfLinker.cpp | ||
---|---|---|
2653–2656 | The next DIE isn't usually NULL, but it is usually valid because there's nearly always at least a NULL after it. |
Is that copying really necessary?