DW_AT_export_symbols is an attribute introduced in DWARF-5, which should not be generated under previous DWARF versions. And this unexpected behavior caused the DBX to fail.
@aprantl @probinson @jhenderson - any of you folks want this in older versions? I think this is for C++ inline namespaces - so if you used a modern C++ standard library (that uses inline namespaces like "std::__1::basic_string", etc) users would have to use these extra qualifiers when naming types, functions, etc. Not unworkable, but a bit inconvenient.
That's right. LLDB is using this attribute even when parsing DWARF v4 to model inline namespaces (such as libc++'s __1 namespace). If we don't have the attribute then we end up with a bunch of __1 in the type names we show to the user (+ some other minor changes in behaviour). It shouldn't break any LLDB scripts though as the type names used in scripts always specify inline namespaces.
If we decide to remove this for all consumers we can just special-case the namespace parsing in LLDB to infer that std::__1 is an inline namespace. It's the only namespace that is important for users (and the test suite).
Generally we figure emitting new tags/attributes is OK, because consumers are meant to ignore them if they don't recognize them - and some consumers can recognize them as an extension. (such as lldb)
This conversation's gotten fragmented over a bunch of reviews, unfortunately - but it sounds like the direction it's headed is that maybe -gstrict-dwarf will be wired up to do all this pedantry/not emit things from future standards, and DBX tuning will opt in to -gstrict-dwarf mode.
Thanks for the context. (from other threads, sounds like we might be moving more towards DBX folks wiring up -gstrict-dwarf to implement all this stuff and enabling -gstrict-dwarf when targeting DBX by default - leaving all these future-spec-feature-in-prior-spec-mode on by default for everyone else)
There may be an opportunity to do this more robustly, instead of scattering the checks all over the place. Note that the Dwarf.def file already knows the version for each tag and attribute; DwarfDebug already knows the version we're emitting; so if DwarfDebug also had the strict-dwarf flag, it would be easy to add a helper predicate or two that would do the checking for any tag or attribute.
Then the methods that add attributes could generally call the predicate themselves, instead of having higher-level code do it.
For tags you probably do want the higher-level code to call the predicate, but it seems like it would be a lot simpler to do something like if (canEmitTag(dwarf::DW_TAG_rvalue_reference)) than the kind of thing we're seeing in these patches.
Does that seem reasonable?
Agreed - brought up something similar here: https://reviews.llvm.org/D100826#2702135
(For the tags, yeah, I'm not sure - wonder if the code for emitting the DIEs could return null/empty DIE/something to indicate that the DIE wasn't created - though that might be hard to retrofit if it means revisiting every place that constructs a DIE to ensure it can handle a "nothing" return)