[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D138618#4080706 <https://reviews.llvm.org/D138618#4080706>, @labath wrote:

> In D138618#4077851 <https://reviews.llvm.org/D138618#4077851>, @ayermolo wrote:
>
>> In D138618#4073329 <https://reviews.llvm.org/D138618#4073329>, @labath wrote:
>>
>>> I think that the 1-based index thingy helps a lot here, but I haven't seen anything (in your reponse, or in the new patch) that would address my concernt DIERef<->user_id conversion ambiguity. I.e. how is one supposed to know what is the "right" way to convert a DIERef to a user_id:
>>>
>>> - `die_ref.get_id()`
>>> - or `symbol_file.GetUID(die_ref)` (which, funnily enough, will construct another DIERef, and *then* call get_id? (`return DIERef(GetID(), ref.section(), ref.die_offset()).get_id();`)
>>>
>>> What's your position on that? That we should live with the ambiguity?
>>
>> Searching for GetUID doesn't look like it's used all that often, maybe follow up patch is just to get rid of it, and replace with DIERef?
>
> If you could make that work, that would be awesome, but I think that's going to be fairly hard. It's true that there aren't that many call sites of this functions, but the ones that are there are very crucial. The user_id_t type represents a symbol-file-neutral identifier (cookie, if you will) that different symbol file implementations use to identify parsed objects (types, mostly). SymbolFileDWARF uses it (via DIERef et al.) to identify the DIE belonging to that type. PDB symbol files use it differently, but the idea is the same. If we wanted to remove that, we'd have to come up with a whole new way to parse/link types -- and one that would work for non-dwarf symbol files as well.

SymbolFileDWARF is the only SymbolFile that inherits from UserID. So this is a DWARF internal thing only where symbol files have user_id_t values. So as long as we make this work for all things DWARF we are good to go.

> (also, this would not really address my concern, because the question would then become "which DIERef is safe to be used as a Type cookie" (answer: only the one which has the OSO field set), but the reduction in complexity resulting from removing one step from the conversion process (DWARFDIE->DIERef->user_id) might be well worth it.)

DIERef should be used anywhere a DIE needs a user_id_t. If DIERef now uses the file index (OSO index, or DWO index) in the same way where we always ask the SymbolFileDWARFXXX::GetID() as the file index, then things will be the same between the two (DWO or OSO).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618