Before this patch, S_[L|G][THREAD32|DATA32] records were emitted with a simple name, not the fully qualified name (namespace + class scope). I've simply reused what @akhuang did for S_CONSTANT
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp | ||
---|---|---|
3134–3135 | Yep- thanks for catching that! |
Thanks!
I have a backlog of issues I'm noting along the way, I will only fix them if they have an impact in regards to debugging in Visual Studio.
- The list of namespaces, ie.S_NAMESPACE isn't emitted in the first module.
- extern int A; doesn't emit anything in the debug stream (of a .OBJ). MSVC emits S_GDATA32, but maybe this doesn't matter as it will be emitted probably by the TU that defines it.
- S_OBJNAME is not emitted (see D43002, I will get back to this soon).
- LF_BUILDINFO does not contain the full repro for rebuilding the .OBJ. I will fix this soon as well, this is blocking us.
- LF_NESTTYPE emits empty names for inner types (at least not a unnamed enum).
- Negative enumerations are emitted as U64, not signed. I need to check if this has an impact in the VS debugger.
- S_UDT isn't emitted for a global enum.
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp | ||
---|---|---|
3134–3135 | Can I leave the comment for 'enum'? There are still a few things missing in the debug stream, in regards to enums (see other comment). |
lgtm, modulo ifdef adjustment
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp | ||
---|---|---|
2989 | This probably needs to be #ifndef NDEBUG, or a build with asserts but without ABI breaking checks (blech) will break. Besides, it's not an ABI breaking check. | |
3134–3135 | I'd leave the FIXME out, as you have it now. I don't think we're getting value from it here. At this point, it's in the frontend's hands to decide if the enum needs emitting, so the code fix would be far from this comment. |
- Added assert in emitDebugInfoForUDTs to ensure no lowering can occur during UDT emission, as suggested by @rnk
It seems we hit the assert in Chromium. Here's a reproducer: https://bugs.chromium.org/p/chromium/issues/detail?id=1083877#c3 (I'll try to create a shorter one.)
I've reverted in 525a591f0f48b9d54018bf5245f2abee09c9c1c8 in the meantime.
Reduced repro:
$ cat /tmp/x.ii typedef int a; struct b; class c { c(); a b::*d; }; c::c() = default; $ clang -cc1 -triple x86_64-pc-windows-msvc19.16.0 -emit-obj -gcodeview -debug-info-kind=limited /tmp/x.ii clang: /work/llvm.monorepo/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:2997: void llvm::CodeViewDebug::emitDebugInfoForUDTs(const std::vector<std::pair<std::string, const DIType *>> &): Assertion `OriginalSize == UDTs.size()' failed.
This probably needs to be #ifndef NDEBUG, or a build with asserts but without ABI breaking checks (blech) will break. Besides, it's not an ABI breaking check.