[PATCH] D118754: [DebugInfo] Always emit `.debug_names` with dwarf 5 for Apple platforms

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D118754#3315821 <https://reviews.llvm.org/D118754#3315821>, @JDevlieghere wrote:

> In D118754#3315791 <https://reviews.llvm.org/D118754#3315791>, @dblaikie wrote:
>
>> In D118754#3315739 <https://reviews.llvm.org/D118754#3315739>, @JDevlieghere wrote:
>>
>>> Both of you seem to prefer not to add a new `DebugNameTableKind` which makes me curious as to why. I'm totally fine with the suggested alternatives (I considered them myself) but having this encoded in the `NameTableKind` means we have more information in both the front and the backend so I'm interested in understanding why we would pick something that doesn't do that.
>>
>> Mostly for me I don't object to changing it - but that it seems like a separate thing than the DWARFv5 debug_names issue. Since it enables a bunch of functionality we didn't have before - whereas just fixing the debug_names issue in a way that's consistent with the existing MachO hardcoded apple names functionality seems like the narrower/status quo fix.
>
> What additional functionality are you thinking of?

The ability to enable/disable accelerators on MachO (not currently possible - since it's hardcoded in the backend) - eg: someone adding `-gno-pubnames` to reduce debug info size, but in the process breaking lldb's behavior due to missing accelerator tables. (or now being able to request non-accelerator tables pubnames (debug_{gnu_,}pub{names,types}) which would be similarly broken.

I guess even if it's represented in the IR level, it isn't necessarily/doesn't have to be exposed on the command line - though, would still mean some more possibility of failure if the IR happened to contain the wrong pubnames kind for the platform, etc.

>> If there's some desire to be able to configure debug names/accelerators on MachO, sure, happy to discuss how that might look - and yeah, having a name table kind seems plausible. (so default == debug_pubnames/pubtypes in v4, debug_names in v5, gnu == debug_gnu_pubnames/pubtypes, apple == apple_names, none == none) then changing the frontend to default to Apple on apple platforms in v4, and default on apple platforms in v5, sure.
>>
>> But that's a lot of changes if the only issue is missing debug_names in v5 - unless the extra flexibility is desirable, or if the existing hardcoded apple names is considered hacky/technical debt, etc.
>
> I personally consider it to be that yes. The fact that this is broken is the result of implicit assumptions that held before we had debug_names support and then nobody noticed the discrepancy between the front and the backend. I guess subconsciously I'm worried about this breaking again which is why I'm leaning towards being more explicit.

Fair enough. I'm OK with it if that's the goal.

Though I don't know that `nameTableKind = Apple` should mean `debug_names` in DWARFv5 - I'm not sure why it's even partly implemented that way, presumably apple names is compatible with DWARfv5 (I don't see any reason it wouldn't be) - though also unnecessary given how close apple names is to debug_names? I guess... (sorry, I'm rambling, in short, it seems like `nameTableKind = Apple` should mean `apple_names` in any version and if you want DWARFv5 to use debug_names on Apple platforms, that should be requested with `nameTableKind = Default`?)

But then again, things do get complicated when the DWARF version is a module flag but the name table kind is per-CU? If they do happen to be inconsistent not sure what the outcome is - whether we build separate name tables covering just the CUs that asked for that kind of name table... I think /maybe/ that works.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118754/new/

https://reviews.llvm.org/D118754