[LLDB] Enable 64 bit debug/type offset

[ayermolo]

Summary

This came out of from https://discourse.llvm.org/t/dwarf-dwp-4gb-limit/63902
With big binaries we can have .dwp files where .debug_info.dwo section can grow
beyond 4GB. We would like to support this in LLVM and in LLDB.

The plan is to enable manual parsing of cu/tu index in DWARF library
(https://reviews.llvm.org/D137882), and then
switch internal index data structure to 64 bit.
For the second part is to enable 64bit offset support in LLDB with
this patch.

[labath]

I am puzzled by the OSO changes in the DIERef class. How do they tie in with the increase in the offset size? It seems like it should at best be a separate patch...

[ayermolo]

In D138618#3948707, @labath wrote:

I am puzzled by the OSO changes in the DIERef class. How do they tie in with the increase in the offset size? It seems like it should at best be a separate patch...

That part was more about having a consistent interface for setting and getting lldb::user_id_t to avoid bugs, and having implicit assumptions what bit layout looks like.

For example here:
user_id_t SymbolFileDWARF::GetUID(DIERef ref) {

if (GetDebugMapSymfile())
  return GetID() | ref.die_offset();

[ayermolo]

ping

[clayborg]

In D138618#3948707, @labath wrote:

I am puzzled by the OSO changes in the DIERef class. How do they tie in with the increase in the offset size? It seems like it should at best be a separate patch...

I have been helping Alexander get this patch ready for open source. We needed to do these changes or this patch doesn't work and would break mac debugging. The reason is some people were manually creating lldb::user_id_t IDs and then manually decoding them. If we change the DIE offset size, then the people that were manually creating user_id_t would now be encoding bits into the wrong bits if they were every put into a DIERef we would extract the wrong information.

Part of what this patch is doing is allowing a DIERef to get a user_id_t from the object and also create itself from a user_id_t. This allows a single consistent interface. No one should be manually encoding user_id_t values now, and it should always be done through DIERef.

[labath]

The explanation makes sense, and I *think* the patch is ok, but it's hard to review it with all the noise. I still believe the DIERef change would be better off as a separate patch, so that the change is not obscured by the (hopefully mechanical) aspects of increasing the size of the offset field.

[ayermolo]

In D138618#3963767, @labath wrote:

The explanation makes sense, and I *think* the patch is ok, but it's hard to review it with all the noise. I still believe the DIERef change would be better off as a separate patch, so that the change is not obscured by the (hopefully mechanical) aspects of increasing the size of the offset field.

I don't think that would be mechanical, because of implicit and explicit assumptions of bit layout, and trying to keep the size of DIERef from doubling.

[clayborg]

In D138618#3963767, @labath wrote:

The explanation makes sense, and I *think* the patch is ok, but it's hard to review it with all the noise. I still believe the DIERef change would be better off as a separate patch, so that the change is not obscured by the (hopefully mechanical) aspects of increasing the size of the offset field.

I am saying we can't make the DIERef change separately because it breaks the buildbots. Without modifying the OSO stuff, we can't make this patch work. If we need to change the max DIE offsets size, we need all these fixes.

[clayborg]

We have modified the DIERef class over the years and made it work for both DWARF in .o files (OSO) for mac and then for fission (.dwo). The two made different changes that never affected either one but with these changes they need to coexist and actually work with DIERef in the same way with no assumptions on the DIE offset being in the lower 32 bits of a user_id_t.

[labath]

I don't believe there's no way to split this patch up. I mean, just half of it is dedicated to changing PRIx32 into PRIx64. Surely that can be a patch of it's own, even if it meant that DWARFDIE::GetOffset temporarily returns something other than dw_offset_t. And if we first changed that to use the llvm::formatv function (which does not require width specifiers), then changing the size of dw_offset_t would be a no-op there. And the fact that the patch description can be summed up into one sentence (enable 64-bit offsets) means that I as a reviewer have to reverse-engineer from scratch what all of these (very subtle) changes do and how they interact.

For example, I only now realized that this patch makes the DIERef class essentially interchangable with the user_id_t type (in has an (implicit!) constructor and a get_id() function). That's not necessarily bad, if every DIERef can really be converted to a user_id_t. However, if that's the case, then why does SymbolFileDWARF::GetUID still exist?

Previously the DIERef class did not encode information about which "OSO" DWARF file in the debug map it is referring to, and they way that was enforced was by having all conversions go through that function (which was the only way to convert from one to the other). If every DIERef really does carry the OSO information then this function (and it's counterpart, DecodeUID) should not exist. If it doesn't carry that information, then we're losing some type safety because we now have two ways to do the conversion, and one of them is (sometimes?) incorrect. Maybe there's no way to avoid that, it's definitely worth discussing, and it that would be a lot easier without the other changes in the way.

As for the discussion, I am still undecided about whether encoding the OSO information into the DIERef is a good thing. In some ways, it is very similar to dwo files (whose information is encoded there), but OTOH, they are also very different. An OSO is essentially a completely self-contained dwarf file, and we represent it in lldb as such (with its own Module, SymbolFile objects, etc.). A DWO file is only syntactically independent (e.g. its DIE tree can be parsed independently), but there's no way to interpret the information inside it without accessing the parent object file (as that contains all the address information). This is also reflected in how they're represented in LLDB. The don't have their own Module objects, and the SymbolFileDWARFDwo class is just a very thin wrapper that forwards everything to the real symbol file. Therefore, it does not seem *un*reasonable to have one way/object to reference a DIE inside a single SymbolFileDWARF (and all the DWO files it references), and another to reference *any* DIE in the set of all SymbolFileDWARFs (either a single object, or multiple objects managed by a SymbolFileDWARFDebugMap) which provide the information for this module.

lldb/source/Plugins/SymbolFile/DWARF/DIERef.h

51

At least make this explicit so it can't be constructed from any random integer. I'd consider even making this a named (static) function (e.g. DIERef fromUID(user_id_t)), as one should be extra careful around these conversions.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

15

This would look much better in the block on line 60, next to the other includes from this directory.
Or, even better, if you just delete all the empty lines between the includes, then clang-format will automatically sort the whole thing.

[ayermolo]

Addressed inlined comments.

[ayermolo]

In D138618#3966539, @labath wrote:

I don't believe there's no way to split this patch up. I mean, just half of it is dedicated to changing PRIx32 into PRIx64. Surely that can be a patch of it's own, even if it meant that DWARFDIE::GetOffset temporarily returns something other than dw_offset_t. And if we first changed that to use the llvm::formatv function (which does not require width specifiers), then changing the size of dw_offset_t would be a no-op there. And the fact that the patch description can be summed up into one sentence (enable 64-bit offsets) means that I as a reviewer have to reverse-engineer from scratch what all of these (very subtle) changes do and how they interact.

For example, I only now realized that this patch makes the DIERef class essentially interchangable with the user_id_t type (in has an (implicit!) constructor and a get_id() function). That's not necessarily bad, if every DIERef can really be converted to a user_id_t. However, if that's the case, then why does SymbolFileDWARF::GetUID still exist?

Previously the DIERef class did not encode information about which "OSO" DWARF file in the debug map it is referring to, and they way that was enforced was by having all conversions go through that function (which was the only way to convert from one to the other). If every DIERef really does carry the OSO information then this function (and it's counterpart, DecodeUID) should not exist. If it doesn't carry that information, then we're losing some type safety because we now have two ways to do the conversion, and one of them is (sometimes?) incorrect. Maybe there's no way to avoid that, it's definitely worth discussing, and it that would be a lot easier without the other changes in the way.

As for the discussion, I am still undecided about whether encoding the OSO information into the DIERef is a good thing. In some ways, it is very similar to dwo files (whose information is encoded there), but OTOH, they are also very different. An OSO is essentially a completely self-contained dwarf file, and we represent it in lldb as such (with its own Module, SymbolFile objects, etc.). A DWO file is only syntactically independent (e.g. its DIE tree can be parsed independently), but there's no way to interpret the information inside it without accessing the parent object file (as that contains all the address information). This is also reflected in how they're represented in LLDB. The don't have their own Module objects, and the SymbolFileDWARFDwo class is just a very thin wrapper that forwards everything to the real symbol file. Therefore, it does not seem *un*reasonable to have one way/object to reference a DIE inside a single SymbolFileDWARF (and all the DWO files it references), and another to reference *any* DIE in the set of all SymbolFileDWARFs (either a single object, or multiple objects managed by a SymbolFileDWARFDebugMap) which provide the information for this module.

How would you like me to break up this diff? Is factoring OSO into another diff enough, or do you want more granular one?

[labath]

In D138618#3967933, @ayermolo wrote:

How would you like me to break up this diff? Is factoring OSO into another diff enough, or do you want more granular one?

Hard to say without seeing what the patches would look like, but yes, in general, I'd say that the OSO/DIERef integration is the most fundamental part of this patch, and I'd optimize things such that this part stands out as much as possible. If you can do that, then maybe everything else can be in the second patch (sequenced either before or after it). Another obvious patch could be to create formatv-based version of the Module::ReportWarning function, and all of the PRIx64 parts of this patch to call that instead. That will reduce the blast radius of the subsequent dw_offset_t size change.

[ayermolo]

Separated format patch, and oso patch. Although without OSO changes a lot of tests fail on mac.

[labath]

Thanks for splitting this up. We still need to figure out what to do with the first patch, but these two are looking very good now.

At least, in the sense that one can clearly see what is happening -- I'd still like to have to discussion about the DIERef->user_id conversion. Essentially, I'd like to see if we can preserve the property that the conversion always produces a valid user_id. With this patch that is not true anymore, because the OSO DIERefs need to be passed through the SymbolFileDWARF::GetUID function, even though it is _very_ tempting to just call get_id() on them. Previously, there was no get_id function, so one had no choice but to call GetUID. If we can make sure that the OSO symbol files always construct DIERefs with the oso field populated correctly, then I think this approach would be fine (and we should be able to delete the GetUID function). Otherwise, I'd like to explore some options of keeping the DIERef->user_id conversion tightly controlled. Personally, I am still not convinced that doing the conversion in the GetUID function is a bad thing. IIUC, the main problem is the part where we do a bitwise or of the DIERef (the die offset part) and the OSO ID (GetID() | ref.die_offset()), but I don't see why we couldn't do something about that. Basically we could just create an inverse function of GetOSOIndexFromUserID (which extracts the OSO symbol file from the user id) -- and make sure the functions are close to each other and their implementation matches.

[ayermolo]

In D138618#3999154, @labath wrote:

Thanks for splitting this up. We still need to figure out what to do with the first patch, but these two are looking very good now.

At least, in the sense that one can clearly see what is happening -- I'd still like to have to discussion about the DIERef->user_id conversion. Essentially, I'd like to see if we can preserve the property that the conversion always produces a valid user_id. With this patch that is not true anymore, because the OSO DIERefs need to be passed through the SymbolFileDWARF::GetUID function, even though it is _very_ tempting to just call get_id() on them. Previously, there was no get_id function, so one had no choice but to call GetUID. If we can make sure that the OSO symbol files always construct DIERefs with the oso field populated correctly, then I think this approach would be fine (and we should be able to delete the GetUID function). Otherwise, I'd like to explore some options of keeping the DIERef->user_id conversion tightly controlled. Personally, I am still not convinced that doing the conversion in the GetUID function is a bad thing. IIUC, the main problem is the part where we do a bitwise or of the DIERef (the die offset part) and the OSO ID (GetID() | ref.die_offset()), but I don't see why we couldn't do something about that. Basically we could just create an inverse function of GetOSOIndexFromUserID (which extracts the OSO symbol file from the user id) -- and make sure the functions are close to each other and their implementation matches.

I don't think creation of ID was that tightly controlled.
For example
oso_symfile->SetID(((uint64_t)m_cu_idx + 1ull) << 32ull);
in SymbolFileDWARFDebugMap.cpp
But on more general note all the assumptions about bit layout scattered through few places:
GetOSOIndexFromUserID
GetUID
SymbolFileDWARFDwo constructor
GetDwoNum
Encode/Decode UID
and even something like
const dw_offset_t function_die_offset = func.GetID();
return DecodedUID{

*dwarf, {std::nullopt, DIERef::Section::DebugInfo, dw_offset_t(uid)}};

So probably moving to something uniform to construct uid for dwo and oso cases, and extract relevant fields is a step in a right direction.

I do see your point that it's currently not quite there.

This

if (GetDebugMapSymfile()) {
    DIERef die_ref(GetID());
    die_ref.set_die_offset(ref.die_offset());
    return die_ref.get_id();
  }

Is kind of not ideal.
In other places it does work.

static uint32_t GetOSOIndexFromUserID(lldb::user_id_t uid) {
    llvm::Optional<uint32_t> OsoNum = DIERef(uid).oso_num();
    lldbassert(OsoNum && "Invalid OSO Index");
    return *OsoNum;
  }

oso_symfile->SetID(DIERef(DIERef::IndexType::OSONum, m_cu_idx,
                                      DIERef::Section(0), 0)
                                   .get_id());

llvm::Optional<uint32_t> GetDwoNum() override {
    return DIERef(GetID()).dwo_num();
  }

I guess main issue with GetUID is that we rely on an internal state of SymbolFileDWARF to

1. figure out if we are dealing with dwo or oso with check for GetDebugMapSymfile
2. get extra data GetDwoNum(), and GetID()

We can either push that logic on the caller side of things (not I deal I would think) and remove GetUID, or extend the constructor to be a bit more explicit. This way all the bit settings are still consolidated, but the logic of when to create what is still hidden in GetDebugMapSymfile.

What do you think?

[labath]

I think that the main reason we've arrived at such different conclusions is that I treat the "user IDs of DIEs" and and "user IDs of symbol files" as essentially two different things (namespaces if you will). Obviously, one needs the symbol file ID in order to compute the DIERef ID, but theoretically those two can use completely different encodings. With this take on things, I stand by my assertion that DIERef->user_id conversions are tightly controlled. The symbol file ID computations are a mess.

You, if I understand correctly, see the ID of a symbol file as a special case of an all-encompassing user id -- essentially a user_id (or a DIERef) pointing to the first byte of the symbol file. with this world view, the entirety of user ID computation is a mess. :)
I can definitely see the appeal of viewing the world that way. It's nice and uniform and unambiguous (since you can't have a DIE at offset zero) -- it's just not the view I had when I was writing this code a couple of years ago. :) And it has the disadvantage of obscuring the DIERef->user_id transition (for DIEs at least), and that's what I'm weight against the other advantages of that approach.

In D138618#4002747, @ayermolo wrote:

I guess main issue with GetUID is that we rely on an internal state of SymbolFileDWARF to

1. figure out if we are dealing with dwo or oso with check for GetDebugMapSymfile
2. get extra data GetDwoNum(), and GetID()

We can either push that logic on the caller side of things (not I deal I would think) and remove GetUID, or extend the constructor to be a bit more explicit. This way all the bit settings are still consolidated, but the logic of when to create what is still hidden in GetDebugMapSymfile.

What do you think?

I'm not entirely sure what you mean by that, but I think either of those could be fine. Essentally, what I'm trying to achieve is to make sure is that if the DIERef<->user_id conversion is trivial, then it is always valid to perform it (i.e. there are no partially constructed DIERefs). Ideally, there wouldn't be partially constructed DIERefs in any case, but that is not as important if one is forced to provide that additional information in order to do the conversion.

However, I also want to throw out this alternative. This one goes in the completely opposite direction. Instead of centralizing the conversions, it federates it (which is I think is roughly what I had in mind when I worked on this last time). There is no single place which controls the conversion, but there are multiple disjoint places which do that:

• one for the OSO case. This includes the following problematic lines you've listed:

GetOSOIndexFromUserID
GetUID (1/2)
Encode/Decode UID (1/2)
return DecodedUID{

*dwarf, {std::nullopt, DIERef::Section::DebugInfo, dw_offset_t(uid)}};

• one for the DWO case:

GetUID (1/2)
Encode/Decode UID (1/2)

• one for Symbol File IDs (which is does a +1 on the internal index -- bacause the main object file has ID 0)

oso_symfile->SetID(((uint64_t)m_cu_idx + 1ull) << 32ull);
SymbolFileDWARFDwo constructor
GetDwoNum (cancels out the previous one)

And I don't think it's an obstacle for making the die offsets larger -- I've included comments on how I think that could happen.

It doesn't handle this one, which seems just wrong, and should be made to use GetUID/DecodeUID

const dw_offset_t function_die_offset = func.GetID();

[ayermolo]

In D138618#4004866, @labath wrote:

You, if I understand correctly, see the ID of a symbol file as a special case of an all-encompassing user id -- essentially a user_id (or a DIERef) pointing to the first byte of the symbol file. with this world view, the entirety of user ID computation is a mess. :)

Ah I see. Thanks for providing context. Yeah that is kind of the way I see it after talking with Greg. Looking over your proposal it makes sense with the way you described how you see this. You and @clayborg have a historical context for this code. He is on PTO right now, lets see what he thinks when he is back in a couple weeks? :)

In meantime what do you think about closing on D139955, so it can land? I think it is fully independent of design decision in this diff, and can land separately. Would be great to juggle one less diff on a stack.

[clayborg]

In D138618#4004866, @labath wrote:

I think that the main reason we've arrived at such different conclusions is that I treat the "user IDs of DIEs" and and "user IDs of symbol files" as essentially two different things (namespaces if you will). Obviously, one needs the symbol file ID in order to compute the DIERef ID, but theoretically those two can use completely different encodings. With this take on things, I stand by my assertion that DIERef->user_id conversions are tightly controlled. The symbol file ID computations are a mess.

You, if I understand correctly, see the ID of a symbol file as a special case of an all-encompassing user id -- essentially a user_id (or a DIERef) pointing to the first byte of the symbol file. with this world view, the entirety of user ID computation is a mess. :)
I can definitely see the appeal of viewing the world that way. It's nice and uniform and unambiguous (since you can't have a DIE at offset zero) -- it's just not the view I had when I was writing this code a couple of years ago. :) And it has the disadvantage of obscuring the DIERef->user_id transition (for DIEs at least), and that's what I'm weight against the other advantages of that approach.

FWIW: User IDs of symbol files is not part of any API. It was added to SymbolFileDWARF to allow us to identify .o files for mac without dSYM and was used by the fission code that Pavel wrote as well. Since this is internal, it doesn't matter at all how we make or use the IDs. No public interface will ever expect a SymboFile to have a lldb::user_id_t. Therefore it is just down to how to use these IDs within the DWARF symbol plug-ins for both mac with .o files and for fission with .dwo files.

Things that are created by the DWARF, like lldb_private::CompileUnit, lldb_private::Function, lldb_private::Block, and lldb_private::Type do have lldb::user_id_t and they are expected to make IDs that make sense for the individual SymbolFile plug-in to be able to easily match up with something in the DWARF. All of these objects have DIEs in the DWARF, so we must be able to make a lldb::user_id_t that allows us to easily answer more questions about something at a later date in the DWARF. Like we can make a lldb_private::CompileUnit without making any functions, blocks or types. If we are later asked to find all functions for a compile unit, we should be able to take the lldb::user_id_t of the compile unit and easily do this.

So how DIERef is used is solely up to the DWARF symbol file plug-in.

So we could just assign the user IDs of each SymbolFileDWARF to be the index of the .o file for mac, or the index of the .dwo file for fission. It doesn't really matter. As long as we can easily take a user_id_t from a virtual interface and track it back to the DIE we care about.

In D138618#4002747, @ayermolo wrote:

I guess main issue with GetUID is that we rely on an internal state of SymbolFileDWARF to

1. figure out if we are dealing with dwo or oso with check for GetDebugMapSymfile
2. get extra data GetDwoNum(), and GetID()

We can either push that logic on the caller side of things (not I deal I would think) and remove GetUID, or extend the constructor to be a bit more explicit. This way all the bit settings are still consolidated, but the logic of when to create what is still hidden in GetDebugMapSymfile.

What do you think?

I'm not entirely sure what you mean by that, but I think either of those could be fine. Essentally, what I'm trying to achieve is to make sure is that if the DIERef<->user_id conversion is trivial, then it is always valid to perform it (i.e. there are no partially constructed DIERefs). Ideally, there wouldn't be partially constructed DIERefs in any case, but that is not as important if one is forced to provide that additional information in order to do the conversion.

However, I also want to throw out this alternative. This one goes in the completely opposite direction. Instead of centralizing the conversions, it federates it (which is I think is roughly what I had in mind when I worked on this last time). There is no single place which controls the conversion, but there are multiple disjoint places which do that:

• one for the OSO case. This includes the following problematic lines you've listed:

GetOSOIndexFromUserID
GetUID (1/2)
Encode/Decode UID (1/2)
return DecodedUID{

*dwarf, {std::nullopt, DIERef::Section::DebugInfo, dw_offset_t(uid)}};

• one for the DWO case:

GetUID (1/2)
Encode/Decode UID (1/2)

• one for Symbol File IDs (which is does a +1 on the internal index -- bacause the main object file has ID 0)

oso_symfile->SetID(((uint64_t)m_cu_idx + 1ull) << 32ull);
SymbolFileDWARFDwo constructor
GetDwoNum (cancels out the previous one)

And I don't think it's an obstacle for making the die offsets larger -- I've included comments on how I think that could happen.

It doesn't handle this one, which seems just wrong, and should be made to use GetUID/DecodeUID

const dw_offset_t function_die_offset = func.GetID();

yes this is wrong and should be changed. It only used to work because we knew that the bottom 32 bits of a a 64 bit user_id_t was the DIE offset, which isn't true anymore.

Since the user IDs of SymbolFileDWARF plug-ins mean nothing to anyone else, we can make them what we need them to be so they work for us. I would suggest to remove the use of DIERef from calculating the IDs of symbol files and have .o files for mac and .dwo files for fission use a 1 based index as their ID to make it easy to encode into a DIERef when needed for lldb::user_id_t values that _are_ included in objects that we hand out. Is there anything else that would need to be done to keep everyone happy here?

[ayermolo]

updated based on Gregs suggestion

[labath]

In D138618#4060565, @clayborg wrote:

...
Since the user IDs of SymbolFileDWARF plug-ins mean nothing to anyone else, we can make them what we need them to be so they work for us. I would suggest to remove the use of DIERef from calculating the IDs of symbol files and have .o files for mac and .dwo files for fission use a 1 based index as their ID to make it easy to encode into a DIERef when needed for lldb::user_id_t values that _are_ included in objects that we hand out. Is there anything else that would need to be done to keep everyone happy here?

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?

lldb/source/Plugins/SymbolFile/DWARF/DIERef.h

72

Can we remove this function now?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

11

nor this

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

12

I guess this isn't necessary anymore.

[ayermolo]

In D138618#4073329, @labath wrote:

In D138618#4060565, @clayborg wrote:

...
Since the user IDs of SymbolFileDWARF plug-ins mean nothing to anyone else, we can make them what we need them to be so they work for us. I would suggest to remove the use of DIERef from calculating the IDs of symbol files and have .o files for mac and .dwo files for fission use a 1 based index as their ID to make it easy to encode into a DIERef when needed for lldb::user_id_t values that _are_ included in objects that we hand out. Is there anything else that would need to be done to keep everyone happy here?

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?

[labath]

In D138618#4077851, @ayermolo wrote:

In 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.

(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.)

[clayborg]

In D138618#4080706, @labath wrote:

In D138618#4077851, @ayermolo wrote:

In 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).

[clayborg]

We just need to create all DIERef objects using the GetID() from the symbol file as the file index, and we should be able to remove the SymbolFile::GetUID() function now. As long as file index zero is reserved for "vanilla DWARF that doesn't use DWO or OSO we will know the difference. We might want to not have SymbolFileDWARF inherit from UserID at all, and switch over to have SymbolFileDWARF add a virtual function:

uint32_t m_file_index = 0; // Zero means main DWARF file, 1...N identifies the Nth DWO file or OSO file
virtual uint32_t GetFileIndex() { return m_file_index; }

Then anyone can set the file index correctly for DWO or OSO files. And we avoid using user_id_t values for the symbol files since they aren't needed.

lldb/source/Plugins/SymbolFile/DWARF/DIERef.h

72

We should be able to, and we should move this lldbassert to the constructor to ensure that die_offset is not too large.

[labath]

In D138618#4083481, @clayborg wrote:

We just need to create all DIERef objects using the GetID() from the symbol file as the file index, and we should be able to remove the SymbolFile::GetUID() function now. As long as file index zero is reserved for "vanilla DWARF that doesn't use DWO or OSO we will know the difference. We might want to not have SymbolFileDWARF inherit from UserID at all, and switch over to have SymbolFileDWARF add a virtual function:

uint32_t m_file_index = 0; // Zero means main DWARF file, 1...N identifies the Nth DWO file or OSO file
virtual uint32_t GetFileIndex() { return m_file_index; }

Then anyone can set the file index correctly for DWO or OSO files. And we avoid using user_id_t values for the symbol files since they aren't needed.

This isn't about the "user id" of a symbol file. I'm totally happy with the changes there -- though I also wouldn't be opposed to changing the "user id" field to something more explicit (like the file index).

My problem is with the "user id"s of individual DIEs. Currently, if I have a DWARFDIE, the only way to get its user id is to do something like die.GetDWARF()->GetUID(die). With this patch, there are two ways:

1. the same as before
2. die.GetDIERef()->get_id()

The problem is that the second way is not going to be correct for OSO files because that path will not set the "oso" component of the DIERef. The worst part is that the second method is much shorter than the first one, so I think it will be very tempting to use it -- and it will actually be right most of the time, until that code is used in an OSO context.

[ayermolo]

addressed comments

[ayermolo]

Created commit that removes getUID(...) https://reviews.llvm.org/D142775
Seems like it's isolated to SymbolFile and DWARF code.
So now userid goes through DIERef.

[labath]

I'm sorry, but that patch does not fix the problem I am trying to point out. In fact, I think it makes things a lot worse.

We clearly have some kind of a communication problem, but I am running out of ideas of what can I do about it. Let me try rephrasing it one more time:

• this patch creates two path for converting a DIERef to a user_id_t -- a) ref.get_id(); and b) dwarf.GetUID(ref)
• of those two ways, one is clearly more intuitive
• of those two ways, one is always correct
• those two ways aren't the same -- (a) is simpler; (b) is correct
• you can't fix that by simply taking (b) away. All that does is make the API misuse even more likely. That patch essentially just deletes GetUID, and inlines it into all its callers.

Forget about the what the code does for a moment, and tell me which of these snippets looks better:
i)

if (IsValid())
  return GetDWARF()->GetUID(*this);

ii)

const std::optional<DIERef> &ref = this->GetDIERef();
if (ref)
  return DIERef(GetID(), ref->section(), ref->die_offset()).get_id();

iii)

if (IsValid())
  return GetDIERef()->get_id();

Now look up the implementation and tell me which one is correct.

[ayermolo]

In D138618#4086789, @labath wrote:

I'm sorry, but that patch does not fix the problem I am trying to point out. In fact, I think it makes things a lot worse.

We clearly have some kind of a communication problem, but I am running out of ideas of what can I do about it. Let me try rephrasing it one more time:

• this patch creates two path for converting a DIERef to a user_id_t -- a) ref.get_id(); and b) dwarf.GetUID(ref)
• of those two ways, one is clearly more intuitive
• of those two ways, one is always correct
• those two ways aren't the same -- (a) is simpler; (b) is correct
• you can't fix that by simply taking (b) away. All that does is make the API misuse even more likely. That patch essentially just deletes GetUID, and inlines it into all its callers.

Forget about the what the code does for a moment, and tell me which of these snippets looks better:
i)

if (IsValid())
  return GetDWARF()->GetUID(*this);

ii)

const std::optional<DIERef> &ref = this->GetDIERef();
if (ref)
  return DIERef(GetID(), ref->section(), ref->die_offset()).get_id();

iii)

if (IsValid())
  return GetDIERef()->get_id();

Now look up the implementation and tell me which one is correct.

Thank you for providing an example. Yes sometimes it's hard to communicate over comments.
In this context yes first one is better.
Question is what should it look "under the hood".
For example:
DIERef::Decode
SymbolFileDWARF::GetUID
SymbolFileDWARF::DecodeUID
There are all these bit shifts scattered around.

If this is such a blocker I did expand on your diff, and added 64 bit support (it still has to be cleaned up a bit like various static constexpr probably moved out of DIERef to #define in dwarfh, but for illustrative purposes) https://reviews.llvm.org/D142779

[clayborg]

Looks good to me. Pavel?

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

67

Why is this needed? No casting should be needed for using the llvm formatting stuff?

198

Needed? Same as above

[clayborg]

In D138618#4086789, @labath wrote:

I'm sorry, but that patch does not fix the problem I am trying to point out. In fact, I think it makes things a lot worse.

We clearly have some kind of a communication problem, but I am running out of ideas of what can I do about it. Let me try rephrasing it one more time:

• this patch creates two path for converting a DIERef to a user_id_t -- a) ref.get_id(); and b) dwarf.GetUID(ref)
• of those two ways, one is clearly more intuitive
• of those two ways, one is always correct
• those two ways aren't the same -- (a) is simpler; (b) is correct
• you can't fix that by simply taking (b) away. All that does is make the API misuse even more likely. That patch essentially just deletes GetUID, and inlines it into all its callers.

Forget about the what the code does for a moment, and tell me which of these snippets looks better:
i)

if (IsValid())
  return GetDWARF()->GetUID(*this);

ii)

const std::optional<DIERef> &ref = this->GetDIERef();
if (ref)
  return DIERef(GetID(), ref->section(), ref->die_offset()).get_id();

iii)

if (IsValid())
  return GetDIERef()->get_id();

Now look up the implementation and tell me which one is correct.

Sorry a lot of noise on all of these things. Forget me last comment, I hit submit too quickly.

How about we make DIERef constructor always take all the info that is needed to construct the objects correctly:

DIERef(DWARFDie die);
DIERef(SymbolFileDWARF *dwarf, dw_offset_t die_offset); // might not need this one?
DIERef(user_id_t uid);

We might not need all of these. But in this case, we can't incorrectly use the APIs since all of the objects that are needed to fill it in are in the constructor args. We take away the ability to manually fill in the DWO num and other fields. Would that fix the issues you have with this patch Pavel?

[clayborg]

So looks like this function needs to be fixed:

llvm::Optional<DIERef> DWARFBaseDIE::GetDIERef() const {
  if (!IsValid())
    return llvm::None;

  return DIERef(m_cu->GetSymbolFileDWARF().GetDwoNum(), m_cu->GetDebugSection(),
                m_die->GetOffset());
}

This currently only works for DWO files, but not for OSO files. If we make the DIERef constructor that this function uses "protected", then only this function can use it. And it should be able to do things correctly.

If we switched the DIERef constructor to take a SymbolFileDWARF as a mandatory first arg, then we can manually supply the section + offset, then DWARFBaseDie can always do this correctly for both DWO files and OSO files.

We might need to add an accessor to SymbolFileDWARF like:

virtual uint32_t SymbolFileDWARF::GetFileIndex();

And there can be a setter for this as well. Then the OSO stuff would set the file index to a 1 based index, and the DWO files would also set this as the file index in the SymoblFileDWARF base class and then this all works.

So my suggestion would be:

• make only one DIERef constructor: "DIERef(SymbolFileDWARF *dwarf, Section section, dw_offset_t die_offset)"
• add SymbolFileDWARF::GetFileIndex() and SymbolFileDWARF::SetFileIndex(...) and a backing ivar
• OSO and DWO files will set the file index manually early on so it is always available and can always create valid DIERef objects in DWARFBaseDIE::GetDIERef
• Change DWARFBaseDIE::GetDIERef to use the GetFileIndex() where it expects zero for a non DWO or OSO file and a 1 based index for DWO or OSO stuff
• Don't allow anyone else to create manually DIERef objects unless you ask for it from a DWARFBaseDie except for the encode/decode functions for saving to/from cache

[labath]

In D138618#4092040, @clayborg wrote:

How about we make DIERef constructor always take all the info that is needed to construct the objects correctly:

DIERef(DWARFDie die);
DIERef(SymbolFileDWARF *dwarf, dw_offset_t die_offset); // might not need this one?
DIERef(user_id_t uid);

We might not need all of these. But in this case, we can't incorrectly use the APIs since all of the objects that are needed to fill it in are in the constructor args. We take away the ability to manually fill in the DWO num and other fields. Would that fix the issues you have with this patch Pavel?

Yes, I believe it would, but I do want to add two things:

• I don't consider it important whether most of the construction work happens inside the DIERef constructor, or outside of it. So, I would consider these two implementations equally fine

DIERef(DWARFDie die); // compute this somehow
DWARFDie::GetDIERef() { return DIERef(*this); }

vs.

DIERef(dwo_id, type_unit_flag, die_offset, ...); // a dumb constructor
DWARFDie::GetDIERef() { return DIERef(...); } // computation happens here

The first one is what you described -- the second one is how it roughly how it works right now.

• My main source of frustration was that my concern is getting overlooked/ignored (not necessarily your fault -- I've been told I am not always sufficiently clear). I think that is something we could live with, if we thing the other cleanups in this patch are worth it (which could very well be the case) -- however, I would want us to be clear that's what we're doing.

[clayborg]

• My main source of frustration was that my concern is getting overlooked/ignored (not necessarily your fault -- I've been told I am not always sufficiently clear). I think that is something we could live with, if we thing the other cleanups in this patch are worth it (which could very well be the case) -- however, I would want us to be clear that's what we're doing.

I do want to state that if we fix things the way I describe it will work seamlessly with OSO or DWO files. The current state of things is the DWO stuff only uses the fancy DIERef constructor and fills in the dwo number correctly only to have it overwritten in SymbolFile::GetUID(...). The SymbolFile::GetUID(...) is needed for OSOs currently because the DIERef that SymbolFileDWARF (which is used for OSO) doesn't correctly create DIERef objects since they always return llvm::None for SymbolFileDWARF::GetDwoNum(). But the new API will have SymbolFileDWARF::GetFileIndex() to be used instead of SymbolFileDWARF::GetDwoNum(), and the file index will be set correctly for both DWO and OSO files. We can then change DIERef away from DWO specific naming, and have DIERef have a "m_file_index" and "m_file_index_valid" instead of the dwo specific members. As long as both OSO and DWO files can be found from the user_id_t API calls we are all good. Not sure if this addresses all of your issues or not.

If all of your concerns are not clarified above, can you clarify what is still being overlooked? Both Alexander and I are usually thinking we are addressing everything you want, but we obviously still aren't, so restating your remaining concerns would help us get this patch moving.

[labath]

In D138618#4094933, @clayborg wrote:

• My main source of frustration was that my concern is getting overlooked/ignored (not necessarily your fault -- I've been told I am not always sufficiently clear). I think that is something we could live with, if we thing the other cleanups in this patch are worth it (which could very well be the case) -- however, I would want us to be clear that's what we're doing.

I do want to state that if we fix things the way I describe it will work seamlessly with OSO or DWO files. The current state of things is the DWO stuff only uses the fancy DIERef constructor and fills in the dwo number correctly only to have it overwritten in SymbolFile::GetUID(...). The SymbolFile::GetUID(...) is needed for OSOs currently because the DIERef that SymbolFileDWARF (which is used for OSO) doesn't correctly create DIERef objects since they always return llvm::None for SymbolFileDWARF::GetDwoNum(). But the new API will have SymbolFileDWARF::GetFileIndex() to be used instead of SymbolFileDWARF::GetDwoNum(), and the file index will be set correctly for both DWO and OSO files. We can then change DIERef away from DWO specific naming, and have DIERef have a "m_file_index" and "m_file_index_valid" instead of the dwo specific members. As long as both OSO and DWO files can be found from the user_id_t API calls we are all good. Not sure if this addresses all of your issues or not.

If all of your concerns are not clarified above, can you clarify what is still being overlooked? Both Alexander and I are usually thinking we are addressing everything you want, but we obviously still aren't, so restating your remaining concerns would help us get this patch moving.

Everything is clarified is and fine. I agree with your plan Thanks. I was trying to return the favor and clarify my own (potentially rude) responses.

[ayermolo]

In D138618#4102275, @labath wrote:

In D138618#4094933, @clayborg wrote:

• My main source of frustration was that my concern is getting overlooked/ignored (not necessarily your fault -- I've been told I am not always sufficiently clear). I think that is something we could live with, if we thing the other cleanups in this patch are worth it (which could very well be the case) -- however, I would want us to be clear that's what we're doing.

I do want to state that if we fix things the way I describe it will work seamlessly with OSO or DWO files. The current state of things is the DWO stuff only uses the fancy DIERef constructor and fills in the dwo number correctly only to have it overwritten in SymbolFile::GetUID(...). The SymbolFile::GetUID(...) is needed for OSOs currently because the DIERef that SymbolFileDWARF (which is used for OSO) doesn't correctly create DIERef objects since they always return llvm::None for SymbolFileDWARF::GetDwoNum(). But the new API will have SymbolFileDWARF::GetFileIndex() to be used instead of SymbolFileDWARF::GetDwoNum(), and the file index will be set correctly for both DWO and OSO files. We can then change DIERef away from DWO specific naming, and have DIERef have a "m_file_index" and "m_file_index_valid" instead of the dwo specific members. As long as both OSO and DWO files can be found from the user_id_t API calls we are all good. Not sure if this addresses all of your issues or not.

If all of your concerns are not clarified above, can you clarify what is still being overlooked? Both Alexander and I are usually thinking we are addressing everything you want, but we obviously still aren't, so restating your remaining concerns would help us get this patch moving.

Everything is clarified is and fine. I agree with your plan Thanks. I was trying to return the favor and clarify my own (potentially rude) responses.

Great, thank you for your and Greg feedback. Let me modify my changes according to Gregs suggestion.

[ayermolo]

Implemented Gregs suggestion. Also consolidated three diffs back into one.
To make it clear the scope of changes, and for ease of testing.

[clayborg]

Very clean patch now, just a few nits about asserts!

lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

129

Does this assert really need to exist? Why would we not require a .dwo file (old code) be able to index? Can we remove this assert? It seems wrong?

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

403–404

Does this assert really need to exist? Why would we not require a .dwo file (old code) be able to index? Can we remove this assert? It seems wrong?

lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp

52–53

Does this assert really need to exist? Why would we not require a .dwo file (old code) be able to index? Can we remove this assert? It seems wrong?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

1404

Pavel: note we now really on "SymbolFileDWARF::GetDie(user_id_t)" to be the one source of truth when finding a DIE. We could make "SymbolFileDWARF:GetDie(DIERef ref)" be the one source of truth and then have "SymbolFileDWARF::GetDie(user_id_t)" just create a local DIERef and then call "SymbolFileDWARF:GetDie(DIERef ref)" if that would be cleaner.

1682

Pavel: note we now really on "SymbolFileDWARF::GetDie(user_id_t)" to be the one source of truth when finding a DIE. We could make "SymbolFileDWARF:GetDie(DIERef ref)" be the one source of truth and then have "SymbolFileDWARF::GetDie(user_id_t)" just create a local DIERef and then call "SymbolFileDWARF:GetDie(DIERef ref)" if that would be cleaner.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

570–572

[labath]

lldb/source/Plugins/SymbolFile/DWARF/DIERef.h

21

I don't understand this sentence.

31

they're not actually protected

lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp

75

Is this the only call site of the DIERef(SymbolFileDWARF&) constructor?

If so, and if we make it such that DWARFBaseDIE::GetDIERef returns the fully filled in DIERef, then this function can just call get_id() on the result, and we can delete that constructor.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

1682

+1

[clayborg]

lldb/source/Plugins/SymbolFile/DWARF/DIERef.h

19–24

31

There were for a bit to control access to this, but in reality we ended up friending too many classes and then ran into issues with the DIERefTest stuff, so we decided to make them public again. We can remove this comment.

lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp

75

This line doesn't make sense. If we got a valid DIERef back from GetDIERef(), then we just return that as it would have used the SymbolFileDWARF to fill everything in already. So we might not need that extra constructor if this is the only place as Pavel suggested.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

1682

Ok. So lets do this - change "DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t uid)" to just be:

DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t uid) {
  return GetDIE(DIERef(uid));
}

And then change the current "DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t uid)" to be the one that does all of the work:

DWARFDIE SymbolFileDWARF::GetDIE(DIERef die_ref) {
  std::optional<uint32_t> file_index = die_ref.file_index();
  if (file_index) {
    if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile())
      symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO case
    else if (*file_index == DIERef::k_file_index_mask)
      symbol_file = m_dwp_symfile.get(); // DWP case
    else
      symbol_file = this->DebugInfo()
                        .GetUnitAtIndex(*die_ref.file_index())
                        ->GetDwoSymbolFile(); // DWO case
  } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
    symbol_file = nullptr;
  } else {
    symbol_file = this;
  }

  if (symbol_file)
    return symbol_file->GetDIE(die_ref);

  return DWARFDIE();
}

[ayermolo]

lldb/source/Plugins/SymbolFile/DWARF/DIERef.h

31

Oops, forgot to remove. For one of the internal revisions I experimented with making them protected.

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

198

m_offset is is bit field now, so without it clang produces error.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

1682

ah, yes, great suggestion.

[ayermolo]

addressed comments

[clayborg]

Looks good to me. Pavel?

[labath]

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

403–404

That was because it a split dwarf setup, there are two compile units, two symbol files and two CU DIEs. In a DIERef, the unit offset refers to the offset of the main unit within the main symbol file (because that's globally unique), but the die offset refers to the offset in the separate file (because that's where the dies are). The indexing process needs to start with the main unit (not the one from the split file) in order for the DIERefs to come out right, and these assertions were enforcing that. Therefore, I think we should put all of these back in.

Or at least, that was the case at some point in the past... I don't know whether this has changed since then, but I wouldn't expect it to.

[labath]

Thank you for your patience. I'm really happy with the overall result here.

[ayermolo]

Added asserts back in

[ayermolo]

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

403–404

Thank you for elaborating, Will put them back in.

[ayermolo]

Had to revert, broke some build bots. :(

[ayermolo]

fixed cross-project tests, and also normal test that I somehow missed.
Need to get access to windows machine to figure out why that fails.

[ayermolo]

small clenaup

[ayermolo]

OK, managed to setup windows VM, and couldn't repro.
After reverting my fix for cross-projects tests locally, I saw test failures.
Will try to push again and see if it triggers bot build failures.

[mib]

Hi @ayermolo!

This patch is causing some failure on the macOS lldb bot: https://green.lab.llvm.org/green/job/lldb-cmake/51257/

Could you take a look ? If you don't have the time, we can revert your patch until you manage to reproduce these failures.

Let me know if you need help with that :)

[ayermolo]

In D138618#4133717, @mib wrote:

Hi @ayermolo!

This patch is causing some failure on the macOS lldb bot: https://green.lab.llvm.org/green/job/lldb-cmake/51257/

Could you take a look ? If you don't have the time, we can revert your patch until you manage to reproduce these failures.

Let me know if you need help with that :)

Sorry about that. I ran on mac and tests passed. Let me see if I can repro tomorrow.
In meantime reverted.

[ayermolo]

This comment was removed by ayermolo.

[ayermolo]

Fixed logic for DwarfMap, also removed an assert in AppleDWARFIndex::GetGlobalVariables.
Before when it would invoke GetDwoNum it will go to a virtual API that alays returned nullopt.
The ID was handled through GetOSOIndexFromUserID. Now that it's all consolidated under
GetFileIndex it's no longer an apporpriate assert I think.

[ayermolo]

removed two more asserts. It made one of the tests fail as "Unresolved".

[ayermolo]

https://green.lab.llvm.org/green/job/lldb-cmake/51484/ passed.
There were other built bots failures, but looks like it was built failure related to other diff that was part of testing. Since I only changed things on mac side, and previously other built bots passed, fingers crossed this is it. :)

[mib]

In D138618#4145644, @ayermolo wrote:

https://green.lab.llvm.org/green/job/lldb-cmake/51484/ passed.
There were other built bots failures, but looks like it was built failure related to other diff that was part of testing. Since I only changed things on mac side, and previously other built bots passed, fingers crossed this is it. :)

Thank you @ayermolo ! I really appreciate that you took the time to fix this :)

[ayermolo]

In D138618#4145804, @mib wrote:

In D138618#4145644, @ayermolo wrote:

https://green.lab.llvm.org/green/job/lldb-cmake/51484/ passed.
There were other built bots failures, but looks like it was built failure related to other diff that was part of testing. Since I only changed things on mac side, and previously other built bots passed, fingers crossed this is it. :)

Thank you @ayermolo ! I really appreciate that you took the time to fix this :)

No, problem. Thank you for bringing it to my attention. Hopefully no more reverts will be necessary. :)

lldb/include/lldb/Core/dwarf.h

 typedef int32_t dw_sleb128_t;
 typedef uint16_t dw_attr_t;
 typedef uint16_t dw_form_t;
 typedef llvm::dwarf::Tag dw_tag_t;
 typedef uint64_t dw_addr_t; // Dwarf address define that must be big enough for
                             // any addresses in the compile units that get
                             // parsed
 
-typedef uint32_t dw_offset_t; // Dwarf Debug Information Entry offset for any
+typedef uint64_t dw_offset_t; // Dwarf Debug Information Entry offset for any
                               // offset into the file
 
 /* Constants */
-#define DW_INVALID_OFFSET (~(dw_offset_t)0)
+#define DW_DIE_OFFSET_MAX_BITSIZE 40
+#define DW_INVALID_OFFSET (((uint64_t)1u << DW_DIE_OFFSET_MAX_BITSIZE) - 1)
 #define DW_INVALID_INDEX 0xFFFFFFFFul
 
 // #define DW_ADDR_none 0x0
 
 #define DW_EH_PE_MASK_ENCODING 0x0F
 
 typedef lldb_private::RangeVector<dw_addr_t, dw_addr_t, 2> DWARFRangeList;
 
 #endif // LLDB_CORE_DWARF_H

lldb/include/lldb/Symbol/DWARFCallFrameInfo.h

private:
 
   bool IsEHFrame() const;
 
   std::optional<FDEEntryMap::Entry>
   GetFirstFDEEntryInRange(const AddressRange &range);
 
   void GetFDEIndex();
 
-  bool FDEToUnwindPlan(uint32_t offset, Address startaddr,
+  bool FDEToUnwindPlan(dw_offset_t offset, Address startaddr,
                        UnwindPlan &unwind_plan);
 
   const CIE *GetCIE(dw_offset_t cie_offset);
 
   void GetCFIData();
 
   // Applies the specified DWARF opcode to the given row. This function handle
   // the commands operates only on a single row (these are the ones what can
private:
 
   FDEEntryMap m_fde_index;
   bool m_fde_index_initialized = false; // only scan the section for FDEs once
   std::mutex m_fde_index_mutex; // and isolate the thread that does it
 
   Type m_type;
 
   CIESP
-  ParseCIE(const uint32_t cie_offset);
+  ParseCIE(const dw_offset_t cie_offset);
 
   lldb::RegisterKind GetRegisterKind() const {
     return m_type == EH ? lldb::eRegisterKindEHFrame : lldb::eRegisterKindDWARF;
   }
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_SYMBOL_DWARFCALLFRAMEINFO_H

lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp

DWARFMappedHash::ExtractDIEArray(hash_data,
                                    DIERefCallback(callback, regex.GetText()));
 }
 
 void AppleDWARFIndex::GetGlobalVariables(
     DWARFUnit &cu, llvm::function_ref<bool(DWARFDIE die)> callback) {
   if (!m_apple_names_up)
     return;
 
-  lldbassert(!cu.GetSymbolFileDWARF().GetDwoNum());
   const DWARFUnit &non_skeleton_cu = cu.GetNonSkeletonUnit();
   DWARFMappedHash::DIEInfoArray hash_data;
   m_apple_names_up->AppendAllDIEsInRange(non_skeleton_cu.GetOffset(),
                                          non_skeleton_cu.GetNextUnitOffset(),
                                          hash_data);
   DWARFMappedHash::ExtractDIEArray(hash_data, DIERefCallback(callback));
 }
 

lldb/source/Plugins/SymbolFile/DWARF/DIERef.h

 //===-- DIERef.h ------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DIEREF_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DIEREF_H
 
 #include "lldb/Core/dwarf.h"
-#include "llvm/Support/FormatProviders.h"
+#include "lldb/Utility/LLDBAssert.h"
 #include <cassert>
 #include <optional>
-#include <vector>
 
 /// Identifies a DWARF debug info entry within a given Module. It contains three
 /// "coordinates":
-/// - dwo_num: identifies the dwo file in the Module. If this field is not set,
-///   the DIERef references the main file.
+/// - file_index: identifies the separate stand alone debug info file
+///   that is referred to by the main debug info file. This will be the
+///   index of a DWO file for fission, or the .o file on mac when not
+///   using a dSYM file. If this field is not set, then this references
+///   a DIE inside the original object file.
 /// - section: identifies the section of the debug info entry in the given file:
 ///   debug_info or debug_types.
 /// - die_offset: The offset of the debug info entry as an absolute offset from
 ///   the beginning of the section specified in the section field.
 class DIERef {
 public:
   enum Section : uint8_t { DebugInfo, DebugTypes };
-
-  DIERef(std::optional<uint32_t> dwo_num, Section section,
+  DIERef(std::optional<uint32_t> file_index, Section section,
          dw_offset_t die_offset)
-      : m_dwo_num(dwo_num.value_or(0)), m_dwo_num_valid(bool(dwo_num)),
-        m_section(section), m_die_offset(die_offset) {
-    assert(this->dwo_num() == dwo_num && "Dwo number out of range?");
+      : m_die_offset(die_offset), m_file_index(file_index.value_or(0)),
+        m_file_index_valid(file_index ? true : false), m_section(section) {
+    assert(this->file_index() == file_index && "File Index is out of range?");
+  }
+
+  explicit DIERef(lldb::user_id_t uid) {
+    m_die_offset = uid & k_die_offset_mask;
+    m_file_index_valid = (uid & k_file_index_valid_bit) != 0;
+    m_file_index = m_file_index_valid
+                       ? (uid >> k_die_offset_bit_size) & k_file_index_mask
+                       : 0;
+    m_section =
+        (uid & k_section_bit) != 0 ? Section::DebugTypes : Section::DebugInfo;
   }
 
-  std::optional<uint32_t> dwo_num() const {
-    if (m_dwo_num_valid)
-      return m_dwo_num;
+  lldb::user_id_t get_id() const {
+    if (m_die_offset == k_die_offset_mask)
+      return LLDB_INVALID_UID;
+
+    return lldb::user_id_t(file_index().value_or(0)) << k_die_offset_bit_size |
+           die_offset() | (m_file_index_valid ? k_file_index_valid_bit : 0) |
+           (section() == Section::DebugTypes ? k_section_bit : 0);
+  }
+
+  std::optional<uint32_t> file_index() const {
+    if (m_file_index_valid)
+      return m_file_index;
     return std::nullopt;
   }
 
   Section section() const { return static_cast<Section>(m_section); }
 
   dw_offset_t die_offset() const { return m_die_offset; }
 
   bool operator<(DIERef other) const {
-    if (m_dwo_num_valid != other.m_dwo_num_valid)
-      return m_dwo_num_valid < other.m_dwo_num_valid;
-    if (m_dwo_num_valid && (m_dwo_num != other.m_dwo_num))
-      return m_dwo_num < other.m_dwo_num;
+    if (m_file_index_valid != other.m_file_index_valid)
+      return m_file_index_valid < other.m_file_index_valid;
+    if (m_file_index_valid && (m_file_index != other.m_file_index))
+      return m_file_index < other.m_file_index;
     if (m_section != other.m_section)
       return m_section < other.m_section;
     return m_die_offset < other.m_die_offset;
   }
 
   bool operator==(const DIERef &rhs) const {
-    return dwo_num() == rhs.dwo_num() && m_section == rhs.m_section &&
+    return file_index() == rhs.file_index() && m_section == rhs.m_section &&
            m_die_offset == rhs.m_die_offset;
   }
 
   bool operator!=(const DIERef &rhs) const { return !(*this == rhs); }
 
   /// Decode a serialized version of this object from data.
   ///
   /// \param data
public:
   ///
   /// This allows this object to be serialized to disk.
   ///
   /// \param encoder
   ///   A data encoder object that serialized bytes will be encoded into.
   ///
   void Encode(lldb_private::DataEncoder &encoder) const;
 
+  static constexpr uint64_t k_die_offset_bit_size = DW_DIE_OFFSET_MAX_BITSIZE;
+  static constexpr uint64_t k_file_index_bit_size =
+      64 - DW_DIE_OFFSET_MAX_BITSIZE - /* size of control bits */ 2;
+
+  static constexpr uint64_t k_file_index_valid_bit =
+      (1ull << (k_file_index_bit_size + k_die_offset_bit_size));
+  static constexpr uint64_t k_section_bit =
+      (1ull << (k_file_index_bit_size + k_die_offset_bit_size + 1));
+  static constexpr uint64_t
+      k_file_index_mask = (~0ull) >> (64 - k_file_index_bit_size); // 0x3fffff;
+  static constexpr uint64_t k_die_offset_mask = (~0ull) >>
+                                                (64 - k_die_offset_bit_size);
+
 private:
-  uint32_t m_dwo_num : 30;
-  uint32_t m_dwo_num_valid : 1;
-  uint32_t m_section : 1;
-  dw_offset_t m_die_offset;
+  // Allow 2TB of .debug_info/.debug_types offset
+  dw_offset_t m_die_offset : k_die_offset_bit_size;
+  // Used for DWO index or for .o file index on mac
+  dw_offset_t m_file_index : k_file_index_bit_size;
+  // Set to 1 if m_file_index is a DWO number
+  dw_offset_t m_file_index_valid : 1;
+  // Set to 0 for .debug_info 1 for .debug_types,
+  dw_offset_t m_section : 1;
 };
 static_assert(sizeof(DIERef) == 8);
 
 typedef std::vector<DIERef> DIEArray;
 
 namespace llvm {
 template<> struct format_provider<DIERef> {
   static void format(const DIERef &ref, raw_ostream &OS, StringRef Style);
 };
 } // namespace llvm
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DIEREF_H

lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp

 #include "llvm/Support/Format.h"
 #include <optional>
 
 using namespace lldb;
 using namespace lldb_private;
 
 void llvm::format_provider<DIERef>::format(const DIERef &ref, raw_ostream &OS,
                                            StringRef Style) {
-  if (ref.dwo_num())
-    OS << format_hex_no_prefix(*ref.dwo_num(), 8) << "/";
+  if (ref.file_index())
+    OS << format_hex_no_prefix(*ref.file_index(), 8) << "/";
   OS << (ref.section() == DIERef::DebugInfo ? "INFO" : "TYPE");
   OS << "/" << format_hex_no_prefix(ref.die_offset(), 8);
 }
 
-constexpr uint32_t k_dwo_num_mask = 0x3FFFFFFF;
-constexpr uint32_t k_dwo_num_valid_bitmask = (1u << 30);
-constexpr uint32_t k_section_bitmask = (1u << 31);
-
 std::optional<DIERef> DIERef::Decode(const DataExtractor &data,
                                      lldb::offset_t *offset_ptr) {
-  const uint32_t bitfield_storage = data.GetU32(offset_ptr);
-  uint32_t dwo_num = bitfield_storage & k_dwo_num_mask;
-  bool dwo_num_valid = (bitfield_storage & (k_dwo_num_valid_bitmask)) != 0;
-  Section section = (Section)((bitfield_storage & (k_section_bitmask)) != 0);
+  DIERef die_ref(data.GetU64(offset_ptr));
+
   // DIE offsets can't be zero and if we fail to decode something from data,
   // it will return 0
-  dw_offset_t die_offset = data.GetU32(offset_ptr);
-  if (die_offset == 0)
+  if (!die_ref.die_offset())
     return std::nullopt;
-  if (dwo_num_valid)
-    return DIERef(dwo_num, section, die_offset);
-  else
-    return DIERef(std::nullopt, section, die_offset);
-}
 
-void DIERef::Encode(DataEncoder &encoder) const {
-  uint32_t bitfield_storage = m_dwo_num;
-  if (m_dwo_num_valid)
-    bitfield_storage |= k_dwo_num_valid_bitmask;
-  if (m_section)
-    bitfield_storage |= k_section_bitmask;
-  encoder.AppendU32(bitfield_storage);
-  static_assert(sizeof(m_die_offset) == 4, "m_die_offset must be 4 bytes");
-  encoder.AppendU32(m_die_offset);
+  return die_ref;
 }
+
+void DIERef::Encode(DataEncoder &encoder) const { encoder.AppendU64(get_id()); }

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

if (cu_language == eLanguageTypeObjC ||
             attrs.type.Clear();
             resolve_state = Type::ResolveState::Full;
           }
         }
       }
     }
   }
 
-  type_sp = dwarf->MakeType(
-      die.GetID(), attrs.name, attrs.byte_size, nullptr,
-      dwarf->GetUID(attrs.type.Reference()), encoding_data_type, &attrs.decl,
-      clang_type, resolve_state, TypePayloadClang(GetOwningClangModule(die)));
+  type_sp = dwarf->MakeType(die.GetID(), attrs.name, attrs.byte_size, nullptr,
+                            attrs.type.Reference().GetID(), encoding_data_type,
+                            &attrs.decl, clang_type, resolve_state,
+                            TypePayloadClang(GetOwningClangModule(die)));
 
   dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
   return type_sp;
 }
 
 ConstString
 DWARFASTParserClang::GetDIEClassTemplateParams(const DWARFDIE &die) {
   if (llvm::StringRef(die.GetName()).contains("<"))
clang_type = m_ast.CreateEnumerationType(
         GetOwningClangModule(die), attrs.decl, enumerator_clang_type,
         attrs.is_scoped_enum);
   } else {
     enumerator_clang_type = m_ast.GetEnumerationIntegerType(clang_type);
   }
 
   LinkDeclContextToDIE(TypeSystemClang::GetDeclContextForType(clang_type), die);
 
-  type_sp = dwarf->MakeType(die.GetID(), attrs.name, attrs.byte_size, nullptr,
-                            dwarf->GetUID(attrs.type.Reference()),
-                            Type::eEncodingIsUID, &attrs.decl, clang_type,
-                            Type::ResolveState::Forward,
-                            TypePayloadClang(GetOwningClangModule(die)));
+  type_sp =
+      dwarf->MakeType(die.GetID(), attrs.name, attrs.byte_size, nullptr,
+                      attrs.type.Reference().GetID(), Type::eEncodingIsUID,
+                      &attrs.decl, clang_type, Type::ResolveState::Forward,
 
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
     if (die.HasChildren()) {
       bool is_signed = false;
       enumerator_clang_type.IsIntegerType(is_signed);
       ParseChildEnumerators(clang_type, is_signed,
                             type_sp->GetByteSize(nullptr).value_or(0), die);
     }
if (array_info && array_info->element_orders.size() > 0) {
     }
   } else {
     clang_type =
         m_ast.CreateArrayType(array_element_type, 0, attrs.is_vector);
   }
   ConstString empty_name;
   TypeSP type_sp =
       dwarf->MakeType(die.GetID(), empty_name, array_element_bit_stride / 8,
-                      nullptr, dwarf->GetUID(type_die), Type::eEncodingIsUID,
+                      nullptr, type_die.GetID(), Type::eEncodingIsUID,
                       &attrs.decl, clang_type, Type::ResolveState::Full);
   type_sp->SetEncodingType(element_type);
   const clang::Type *type = ClangUtil::GetQualType(clang_type).getTypePtr();
   m_ast.SetMetadataAsUserID(type, die.GetID());
   return type_sp;
 }
 
 TypeSP DWARFASTParserClang::ParsePointerToMemberType(

lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp

 #include <optional>
 
 using namespace lldb_private;
 
 std::optional<DIERef> DWARFBaseDIE::GetDIERef() const {
   if (!IsValid())
     return std::nullopt;
 
-  return DIERef(m_cu->GetSymbolFileDWARF().GetDwoNum(), m_cu->GetDebugSection(),
-                m_die->GetOffset());
+  return DIERef(m_cu->GetSymbolFileDWARF().GetFileIndex(),
+                m_cu->GetDebugSection(), m_die->GetOffset());
 }
 
 dw_tag_t DWARFBaseDIE::Tag() const {
   if (m_die)
     return m_die->Tag();
   else
     return llvm::dwarf::DW_TAG_null;
 }
uint64_t DWARFBaseDIE::GetAttributeValueAsAddress(const dw_attr_t attr,
                                               uint64_t fail_value) const {
   if (IsValid())
     return m_die->GetAttributeValueAsAddress(GetCU(), attr, fail_value);
   else
     return fail_value;
 }
 
 lldb::user_id_t DWARFBaseDIE::GetID() const {
-  if (IsValid())
-    return GetDWARF()->GetUID(*this);
+  const std::optional<DIERef> &ref = this->GetDIERef();
+  if (ref)
+    return ref->get_id();
+
   return LLDB_INVALID_UID;
 }
 
 const char *DWARFBaseDIE::GetName() const {
   if (IsValid())
     return m_die->GetName(m_cu);
   else
     return nullptr;

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp

uint32_t DWARFDebugInfo::FindUnitIndex(DIERef::Section section,
   auto pos = llvm::upper_bound(
       m_units, std::make_pair(section, offset),
       [](const std::pair<DIERef::Section, dw_offset_t> &lhs,
          const DWARFUnitSP &rhs) {
         return lhs < std::make_pair(rhs->GetDebugSection(), rhs->GetOffset());
       });
   uint32_t idx = std::distance(m_units.begin(), pos);
   if (idx == 0)
-    return DW_INVALID_OFFSET;
+    return DW_INVALID_INDEX;
   return idx - 1;
 }
 
 DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section section,
                                            dw_offset_t cu_offset,
                                            uint32_t *idx_ptr) {
   uint32_t idx = FindUnitIndex(section, cu_offset);
   DWARFUnit *result = GetUnitAtIndex(idx);

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h

 /// std::vector, we cannot delete the copy constructor.
 class DWARFDebugInfoEntry {
 public:
   typedef std::vector<DWARFDebugInfoEntry> collection;
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
   DWARFDebugInfoEntry()
-      : m_offset(DW_INVALID_OFFSET), m_sibling_idx(0), m_has_children(false) {}
+      : m_offset(DW_INVALID_OFFSET), m_parent_idx(0), m_sibling_idx(0),
+        m_has_children(false) {}
 
   explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; }
   bool operator==(const DWARFDebugInfoEntry &rhs) const;
   bool operator!=(const DWARFDebugInfoEntry &rhs) const;
 
   void BuildFunctionAddressRangeTable(DWARFUnit *cu,
                                       DWARFDebugAranges *debug_aranges) const;
 
public:
   // global or (file-static). It will return false for static variables
   // that are local to a function, as they have local scope.
   bool IsGlobalOrStaticScopeVariable() const;
 
 protected:
   static DWARFDeclContext
   GetDWARFDeclContextStatic(const DWARFDebugInfoEntry *die, DWARFUnit *cu);
 
-  dw_offset_t m_offset; // Offset within the .debug_info/.debug_types
-  uint32_t m_parent_idx = 0;   // How many to subtract from "this" to get the
-                               // parent. If zero this die has no parent
-  uint32_t m_sibling_idx : 31, // How many to add to "this" to get the sibling.
-      // If it is zero, then the DIE doesn't have children, or the
-      // DWARF claimed it had children but the DIE only contained
-      // a single NULL terminating child.
-      m_has_children : 1;
+  // Up to 2TB offset within the .debug_info/.debug_types
+  dw_offset_t m_offset : DW_DIE_OFFSET_MAX_BITSIZE;
+  // How many to subtract from "this" to get the parent. If zero this die has no
+  // parent
+  dw_offset_t m_parent_idx : 64 - DW_DIE_OFFSET_MAX_BITSIZE;
+  // How many to add to "this" to get the sibling.
+  // If it is zero, then the DIE doesn't have children,
+  // or the DWARF claimed it had children but the DIE
+  // only contained a single NULL terminating child.
+  uint32_t m_sibling_idx : 31, m_has_children : 1;
   uint16_t m_abbr_idx = 0;
   /// A copy of the DW_TAG value so we don't have to go through the compile
   /// unit abbrev table
   dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;
 
 private:
   size_t GetAttributes(DWARFUnit *cu, DWARFAttributes &attrs, Recurse recurse,
                        uint32_t curr_depth) const;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGINFOENTRY_H

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor &data,
 
   lldb::offset_t offset = *offset_ptr;
   const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
   if (abbrevDecl == nullptr) {
     cu->GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
         "[{0:x16}]: invalid abbreviation code {1}, "
         "please file a bug and "
         "attach the file at the start of this error message",
-        m_offset, (unsigned)abbr_idx);
+        (uint64_t)m_offset, (unsigned)abbr_idx);
     // WE can't parse anymore if the DWARF is borked...
     *offset_ptr = UINT32_MAX;
     return false;
   }
   m_tag = abbrevDecl->Tag();
   m_has_children = abbrevDecl->HasChildren();
   // Skip all data in the .debug_info or .debug_types for the attributes
   const uint32_t numAttributes = abbrevDecl->NumAttributes();
else {
           form_size = 0;
           break;
 
         default:
           cu->GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
               "[{0:x16}]: Unsupported DW_FORM_{1:x}, please file a bug "
               "and "
               "attach the file at the start of this error message",
-              m_offset, (unsigned)form);
+              (uint64_t)m_offset, (unsigned)form);
           *offset_ptr = m_offset;
           return false;
         }
         offset += form_size;
 
       } while (form_is_indirect);
     }
   }

lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

 // may use DW_FORM_strx* forms pointing to its own .debug_str_offsets.dwo and
 // for that case, we should find the offset (skip the section header).
 void DWARFUnit::SetDwoStrOffsetsBase() {
   lldb::offset_t baseOffset = 0;
 
   if (const llvm::DWARFUnitIndex::Entry *entry = m_header.GetIndexEntry()) {
     if (const auto *contribution =
             entry->getContribution(llvm::DW_SECT_STR_OFFSETS))
-      baseOffset = contribution->getOffset32();
+      baseOffset = contribution->getOffset();
     else
       return;
   }
 
   if (GetVersion() >= 5) {
     const DWARFDataExtractor &strOffsets =
         GetSymbolFileDWARF().GetDWARFContext().getOrLoadStrOffsetsData();
     uint64_t length = strOffsets.GetU32(&baseOffset);
if (const llvm::DWARFUnitIndex::Entry *entry = m_header.GetIndexEntry()) {
     const auto *contribution = entry->getContribution(llvm::DW_SECT_LOCLISTS);
     if (!contribution) {
       GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
           "Failed to find location list contribution for CU with DWO Id "
           "{0:x16}",
           *GetDWOId());
       return;
     }
-    offset += contribution->getOffset32();
+    offset += contribution->getOffset();
   }
   m_loclists_base = loclists_base;
 
   uint64_t header_size = llvm::DWARFListTableHeader::getHeaderSize(DWARF32);
   if (loclists_base < header_size)
     return;
 
   m_loclist_table_header.emplace(".debug_loclists", "locations");

 
 DWARFDataExtractor DWARFUnit::GetLocationData() const {
   DWARFContext &Ctx = GetSymbolFileDWARF().GetDWARFContext();
   const DWARFDataExtractor &data =
       GetVersion() >= 5 ? Ctx.getOrLoadLocListsData() : Ctx.getOrLoadLocData();
   if (const llvm::DWARFUnitIndex::Entry *entry = m_header.GetIndexEntry()) {
     if (const auto *contribution = entry->getContribution(
             GetVersion() >= 5 ? llvm::DW_SECT_LOCLISTS : llvm::DW_SECT_EXT_LOC))
-      return DWARFDataExtractor(data, contribution->getOffset32(),
+      return DWARFDataExtractor(data, contribution->getOffset(),
                                 contribution->getLength32());
     return DWARFDataExtractor();
   }
   return data;
 }
 
 DWARFDataExtractor DWARFUnit::GetRnglistData() const {
   DWARFContext &Ctx = GetSymbolFileDWARF().GetDWARFContext();
   const DWARFDataExtractor &data = Ctx.getOrLoadRngListsData();
   if (const llvm::DWARFUnitIndex::Entry *entry = m_header.GetIndexEntry()) {
     if (const auto *contribution =
             entry->getContribution(llvm::DW_SECT_RNGLISTS))
-      return DWARFDataExtractor(data, contribution->getOffset32(),
+      return DWARFDataExtractor(data, contribution->getOffset(),
                                 contribution->getLength32());
     GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
         "Failed to find range list contribution for CU with signature {0:x16}",
         entry->getSignature());
 
     return DWARFDataExtractor();
   }
   return data;
if (header.m_index_entry) {
     }
     auto *abbr_entry =
         header.m_index_entry->getContribution(llvm::DW_SECT_ABBREV);
     if (!abbr_entry) {
       return llvm::createStringError(
           llvm::inconvertibleErrorCode(),
           "DWARF package index missing abbreviation column");
     }
-    header.m_abbr_offset = abbr_entry->getOffset32();
+    header.m_abbr_offset = abbr_entry->getOffset();
   }
 
   bool length_OK = data.ValidOffset(header.GetNextUnitOffset() - 1);
   bool version_OK = SymbolFileDWARF::SupportedVersion(header.m_version);
   bool addr_size_OK = (header.m_addr_size == 4) || (header.m_addr_size == 8);
   bool type_offset_OK =
       !header.IsTypeUnit() || (header.m_type_offset <= header.GetLength());
 

lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

if (!cu_offset)
     return std::nullopt;
 
   DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *cu_offset);
   if (!cu)
     return std::nullopt;
 
   cu = &cu->GetNonSkeletonUnit();
   if (std::optional<uint64_t> die_offset = entry.getDIEUnitOffset())
-    return DIERef(cu->GetSymbolFileDWARF().GetDwoNum(),
+    return DIERef(cu->GetSymbolFileDWARF().GetFileIndex(),
                   DIERef::Section::DebugInfo, cu->GetOffset() + *die_offset);
 
   return std::nullopt;
 }
 
 bool DebugNamesDWARFIndex::ProcessEntry(
     const DebugNames::Entry &entry,
     llvm::function_ref<bool(DWARFDIE die)> callback, llvm::StringRef name) {
for (const DebugNames::NameIndex &ni: *m_debug_names_up) {
     }
   }
 
   m_fallback.GetGlobalVariables(regex, callback);
 }
 
 void DebugNamesDWARFIndex::GetGlobalVariables(
     DWARFUnit &cu, llvm::function_ref<bool(DWARFDIE die)> callback) {
-  lldbassert(!cu.GetSymbolFileDWARF().GetDwoNum());
+  lldbassert(!cu.GetSymbolFileDWARF().GetFileIndex());
   uint64_t cu_offset = cu.GetOffset();
   bool found_entry_for_cu = false;
   for (const DebugNames::NameIndex &ni: *m_debug_names_up) {
     for (DebugNames::NameTableEntry nte: ni) {
       uint64_t entry_offset = nte.getEntryOffset();
       llvm::Expected<DebugNames::Entry> entry_or = ni.getEntry(&entry_offset);
       for (; entry_or; entry_or = ni.getEntry(&entry_offset)) {
         if (entry_or->tag() != DW_TAG_variable)

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

 void ManualDWARFIndex::GetGlobalVariables(
     const RegularExpression &regex,
     llvm::function_ref<bool(DWARFDIE die)> callback) {
   Index();
   m_set.globals.Find(regex, DIERefCallback(callback, regex.GetText()));
 }
 
 void ManualDWARFIndex::GetGlobalVariables(
     DWARFUnit &unit, llvm::function_ref<bool(DWARFDIE die)> callback) {
-  lldbassert(!unit.GetSymbolFileDWARF().GetDwoNum());
   Index();
   m_set.globals.FindAllEntriesForUnit(unit, DIERefCallback(callback));
 }
 
 void ManualDWARFIndex::GetObjCMethods(
     ConstString class_name, llvm::function_ref<bool(DWARFDIE die)> callback) {
   Index();
   m_set.objc_class_selectors.Find(
       class_name, DIERefCallback(callback, class_name.GetStringRef()));
enum DataID {
   kDataIDFunctionSelectors,
   kDataIDFunctionObjcClassSelectors,
   kDataIDGlobals,
   kDataIDTypes,
   kDataIDNamespaces,
   kDataIDEnd = 255u,
 
 };
-constexpr uint32_t CURRENT_CACHE_VERSION = 1;
+
+// Version 2 changes the encoding of DIERef objects used in the DWARF manual
+// index name tables. See DIERef class for details.
+constexpr uint32_t CURRENT_CACHE_VERSION = 2;
 
 bool ManualDWARFIndex::IndexSet::Decode(const DataExtractor &data,
                                         lldb::offset_t *offset_ptr) {
   StringTableReader strtab;
   // We now decode the string table for all strings in the data cache file.
   if (!strtab.Decode(data, offset_ptr))
     return false;
 

lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp

for (const auto &entry : m_map)
     if (regex.Execute(entry.cstring.GetCString())) {
       if (!callback(entry.value))
         return false;
     }
   return true;
 }
 
 void NameToDIE::FindAllEntriesForUnit(
     DWARFUnit &s_unit, llvm::function_ref<bool(DIERef ref)> callback) const {
-  lldbassert(!s_unit.GetSymbolFileDWARF().GetDwoNum());
   const DWARFUnit &ns_unit = s_unit.GetNonSkeletonUnit();
   const uint32_t size = m_map.GetSize();
   for (uint32_t i = 0; i < size; ++i) {
     const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
-    if (ns_unit.GetSymbolFileDWARF().GetDwoNum() == die_ref.dwo_num() &&
+    if (ns_unit.GetSymbolFileDWARF().GetFileIndex() == die_ref.file_index() &&
         ns_unit.GetDebugSection() == die_ref.section() &&
         ns_unit.GetOffset() <= die_ref.die_offset() &&
         die_ref.die_offset() < ns_unit.GetNextUnitOffset()) {
       if (!callback(die_ref))
         return;
     }
   }
 }

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

 class DWARFDebugLine;
 class DWARFDebugRanges;
 class DWARFDeclContext;
 class DWARFFormValue;
 class DWARFTypeUnit;
 class SymbolFileDWARFDebugMap;
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
+class UserID;
 
 #define DIE_IS_BEING_PARSED ((lldb_private::Type *)1)
 
-class SymbolFileDWARF : public lldb_private::SymbolFileCommon,
-                        public lldb_private::UserID {
+class SymbolFileDWARF : public lldb_private::SymbolFileCommon {
   /// LLVM RTTI support.
   static char ID;
 
 public:
   /// LLVM RTTI support.
   /// \{
   bool isA(const void *ClassID) const override {
     return ClassID == &ID || SymbolFileCommon::isA(ClassID);
public:
   const ExternalTypeModuleMap &getExternalTypeModules() const {
     return m_external_type_modules;
   }
 
   virtual DWARFDIE GetDIE(const DIERef &die_ref);
 
   DWARFDIE GetDIE(lldb::user_id_t uid);
 
-  lldb::user_id_t GetUID(const DWARFBaseDIE &die) {
-    return GetUID(die.GetDIERef());
-  }
-
-  lldb::user_id_t GetUID(const std::optional<DIERef> &ref) {
-    return ref ? GetUID(*ref) : LLDB_INVALID_UID;
-  }
-
-  lldb::user_id_t GetUID(DIERef ref);
-
   std::shared_ptr<SymbolFileDWARFDwo>
   GetDwoSymbolFileForCompileUnit(DWARFUnit &dwarf_cu,
                                  const DWARFDebugInfoEntry &cu_die);
 
-  virtual std::optional<uint32_t> GetDwoNum() { return std::nullopt; }
-
   /// If this is a DWARF object with a single CU, return its DW_AT_dwo_id.
   std::optional<uint64_t> GetDWOId();
 
   static bool
   DIEInDeclContext(const lldb_private::CompilerDeclContext &parent_decl_ctx,
                    const DWARFDIE &die);
 
   std::vector<std::unique_ptr<lldb_private::CallEdge>>
-  ParseCallEdgesInFunction(UserID func_id) override;
+  ParseCallEdgesInFunction(lldb_private::UserID func_id) override;
 
   void Dump(lldb_private::Stream &s) override;
 
   void DumpClangAST(lldb_private::Stream &s) override;
 
   lldb_private::DWARFContext &GetDWARFContext() { return m_context; }
 
   const std::shared_ptr<SymbolFileDWARFDwo> &GetDwpSymbolFile();
public:
   ParseVendorDWARFOpcode(uint8_t op, const lldb_private::DataExtractor &opcodes,
                          lldb::offset_t &offset,
                          std::vector<lldb_private::Value> &stack) const {
     return false;
   }
 
   lldb_private::ConstString ConstructFunctionDemangledName(const DWARFDIE &die);
 
+  std::optional<uint64_t> GetFileIndex() const { return m_file_index; }
+  void SetFileIndex(std::optional<uint64_t> file_index) {
+    m_file_index = file_index;
+  }
+
 protected:
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb_private::Type *>
       DIEToTypePtr;
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP>
       DIEToVariableSP;
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *,
                          lldb::opaque_compiler_type_t>
       DIEToClangType;
protected:
 
   virtual ClangTypeToDIE &GetForwardDeclClangTypeToDie() {
     return m_forward_decl_clang_type_to_die;
   }
 
   void BuildCuTranslationTable();
   std::optional<uint32_t> GetDWARFUnitIndex(uint32_t cu_idx);
 
-  struct DecodedUID {
-    SymbolFileDWARF &dwarf;
-    DIERef ref;
-  };
-  std::optional<DecodedUID> DecodeUID(lldb::user_id_t uid);
-
   void FindDwpSymbolFile();
 
   const lldb_private::FileSpecList &GetTypeUnitSupportFiles(DWARFTypeUnit &tu);
 
   void InitializeFirstCodeAddressRecursive(
       const lldb_private::SectionList &section_list);
 
   void InitializeFirstCodeAddress();
protected:
   /// to invalidate debug info describing dead-stripped code. These linkers will
   /// keep the debug info but resolve any addresses referring to such code as
   /// zero (BFD) or a small positive integer (zero + relocation addend -- GOLD).
   /// Try to filter out this debug info by comparing it to the lowest code
   /// address in the module.
   lldb::addr_t m_first_code_address = LLDB_INVALID_ADDRESS;
   lldb_private::StatsDuration m_parse_time;
   std::atomic_flag m_dwo_warning_issued = ATOMIC_FLAG_INIT;
+  /// If this DWARF file a .DWO file or a DWARF .o file on mac when
+  /// no dSYM file is being used, this file index will be set to a
+  /// valid value that can be used in DIERef objects which will contain
+  /// an index that identifies the .DWO or .o file.
+  std::optional<uint64_t> m_file_index = std::nullopt;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARF_H

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

 //===-- SymbolFileDWARF.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolFileDWARF.h"
 
 #include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Threading.h"
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Progress.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Utility/ArchSpec.h"
default:
       break;
     }
   }
   return DWARFDIE();
 }
 
 SymbolFileDWARF::SymbolFileDWARF(ObjectFileSP objfile_sp,
                                  SectionList *dwo_section_list)
-    : SymbolFileCommon(std::move(objfile_sp)),
-      UserID(0x7fffffff00000000), // Used by SymbolFileDWARFDebugMap to
-                                  // when this class parses .o files to
-                                  // contain the .o file index/ID
-      m_debug_map_module_wp(), m_debug_map_symfile(nullptr),
+    : SymbolFileCommon(std::move(objfile_sp)), m_debug_map_module_wp(),
+      m_debug_map_symfile(nullptr),
       m_context(m_objfile_sp->GetModule()->GetSectionList(), dwo_section_list),
       m_fetched_external_modules(false),
       m_supports_DW_AT_APPLE_objc_complete_type(eLazyBoolCalculate) {}
 
 SymbolFileDWARF::~SymbolFileDWARF() = default;
 
 static ConstString GetDWARFMachOSegmentName() {
   static ConstString g_dwarf_section_name("__DWARF");
if (section != nullptr) {
             m_objfile_sp->GetModule()->ReportWarning(
                 "empty dSYM file detected, dSYM was created with an "
                 "executable with no debug info.");
           }
         }
       }
     }
 
+    constexpr uint64_t MaxDebugInfoSize = (1ull) << DW_DIE_OFFSET_MAX_BITSIZE;
+    if (debug_info_file_size >= MaxDebugInfoSize) {
+      m_objfile_sp->GetModule()->ReportWarning(
+          "SymbolFileDWARF can't load this DWARF. It's larger then {0:x+16}",
+          MaxDebugInfoSize);
+      return 0;
+    }
+
     if (debug_abbrev_file_size > 0 && debug_info_file_size > 0)
       abilities |= CompileUnits | Functions | Blocks | GlobalVariables |
                    LocalVariables | VariableTypes;
 
     if (debug_line_file_size > 0)
       abilities |= LineTables;
   }
   return abilities;

 
 void SymbolFileDWARF::ParseDeclsForContext(CompilerDeclContext decl_ctx) {
   auto *type_system = decl_ctx.GetTypeSystem();
   if (type_system != nullptr)
     type_system->GetDWARFParser()->EnsureAllDIEsInDeclContextHaveBeenParsed(
         decl_ctx);
 }
 
-user_id_t SymbolFileDWARF::GetUID(DIERef ref) {
-  if (GetDebugMapSymfile())
-    return GetID() | ref.die_offset();
-
-  lldbassert(GetDwoNum().value_or(0) <= 0x3fffffff);
-  return user_id_t(GetDwoNum().value_or(0)) << 32 | ref.die_offset() |
-         lldb::user_id_t(GetDwoNum().has_value()) << 62 |
-         lldb::user_id_t(ref.section() == DIERef::Section::DebugTypes) << 63;
-}
-
-std::optional<SymbolFileDWARF::DecodedUID>
-SymbolFileDWARF::DecodeUID(lldb::user_id_t uid) {
-  // This method can be called without going through the symbol vendor so we
-  // need to lock the module.
-  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
-  // Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we
-  // must make sure we use the correct DWARF file when resolving things. On
-  // MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple
-  // SymbolFileDWARF classes, one for each .o file. We can often end up with
-  // references to other DWARF objects and we must be ready to receive a
-  // "lldb::user_id_t" that specifies a DIE from another SymbolFileDWARF
-  // instance.
-  if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
-    SymbolFileDWARF *dwarf = debug_map->GetSymbolFileByOSOIndex(
-        debug_map->GetOSOIndexFromUserID(uid));
-    return DecodedUID{
-        *dwarf, {std::nullopt, DIERef::Section::DebugInfo, dw_offset_t(uid)}};
-  }
-  dw_offset_t die_offset = uid;
-  if (die_offset == DW_INVALID_OFFSET)
-    return std::nullopt;
-
-  DIERef::Section section =
-      uid >> 63 ? DIERef::Section::DebugTypes : DIERef::Section::DebugInfo;
-
-  std::optional<uint32_t> dwo_num;
-  bool dwo_valid = uid >> 62 & 1;
-  if (dwo_valid)
-    dwo_num = uid >> 32 & 0x3fffffff;
-
-  return DecodedUID{*this, {dwo_num, section, die_offset}};
-}
-
 DWARFDIE
-SymbolFileDWARF::GetDIE(lldb::user_id_t uid) {
-  // This method can be called without going through the symbol vendor so we
-  // need to lock the module.
-  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
-
-  std::optional<DecodedUID> decoded = DecodeUID(uid);
-
-  if (decoded)
-    return decoded->dwarf.GetDIE(decoded->ref);
-
-  return DWARFDIE();
-}
+SymbolFileDWARF::GetDIE(lldb::user_id_t uid) { return GetDIE(DIERef(uid)); }
 
 CompilerDecl SymbolFileDWARF::GetDeclForUID(lldb::user_id_t type_uid) {
   // This method can be called without going through the symbol vendor so we
   // need to lock the module.
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   // Anytime we have a lldb::user_id_t, we must get the DIE by calling
   // SymbolFileDWARF::GetDIE(). See comments inside the
   // SymbolFileDWARF::GetDIE() for details.
lldb::ModuleSP SymbolFileDWARF::GetExternalModule(ConstString name) {
   if (pos != m_external_type_modules.end())
     return pos->second;
   else
     return lldb::ModuleSP();
 }
 
 DWARFDIE
 SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
-  if (die_ref.dwo_num()) {
-    SymbolFileDWARF *dwarf = *die_ref.dwo_num() == 0x3fffffff
-                                 ? m_dwp_symfile.get()
-                                 : this->DebugInfo()
-                                       .GetUnitAtIndex(*die_ref.dwo_num())
-                                       ->GetDwoSymbolFile();
-    return dwarf->DebugInfo().GetDIE(die_ref);
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
+
+  SymbolFileDWARF *symbol_file = nullptr;
+
+  // Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we
+  // must make sure we use the correct DWARF file when resolving things. On
+  // MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple
+  // SymbolFileDWARF classes, one for each .o file. We can often end up with
+  // references to other DWARF objects and we must be ready to receive a
+  // "lldb::user_id_t" that specifies a DIE from another SymbolFileDWARF
+  // instance.
+  std::optional<uint32_t> file_index = die_ref.file_index();
+  if (file_index) {
+    if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
+      symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO case
+      if (symbol_file)
+        return symbol_file->DebugInfo().GetDIE(die_ref);
+      return DWARFDIE();
+    }
+
+    if (*file_index == DIERef::k_file_index_mask)
+      symbol_file = m_dwp_symfile.get(); // DWP case
+    else
+      symbol_file = this->DebugInfo()
+                        .GetUnitAtIndex(*die_ref.file_index())
+                        ->GetDwoSymbolFile(); // DWO case
+  } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
+    return DWARFDIE();
   }
 
+  if (symbol_file)
+    return symbol_file->GetDIE(die_ref);
+
   return DebugInfo().GetDIE(die_ref);
 }
 
 /// Return the DW_AT_(GNU_)dwo_id.
 static std::optional<uint64_t> GetDWOId(DWARFCompileUnit &dwarf_cu,
                                         const DWARFDebugInfoEntry &cu_die) {
   std::optional<uint64_t> dwo_id =
       cu_die.GetAttributeValueAsOptionalUnsigned(&dwarf_cu, DW_AT_GNU_dwo_id);
   if (dwo_id)
size_t SymbolFileDWARF::ParseBlocksRecursive(Function &func) {
   CompileUnit *comp_unit = func.GetCompileUnit();
   lldbassert(comp_unit);
 
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(comp_unit);
   if (!dwarf_cu)
     return 0;
 
   size_t functions_added = 0;
-  const dw_offset_t function_die_offset = func.GetID();
+  const dw_offset_t function_die_offset = DIERef(func.GetID()).die_offset();
   DWARFDIE function_die =
       dwarf_cu->GetNonSkeletonUnit().GetDIE(function_die_offset);
   if (function_die) {
     ParseBlocksRecursive(*comp_unit, &func.GetBlock(false), function_die,
                          LLDB_INVALID_ADDRESS, 0);
   }
 
   return functions_added;
VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
   if (!symbol_context_scope) {
     // Not ready to parse this variable yet. It might be a global or static
     // variable that is in a function scope and the function in the symbol
     // context wasn't filled in yet
     return nullptr;
   }
 
   auto type_sp = std::make_shared<SymbolFileType>(
-      *this, GetUID(type_die_form.Reference()));
+      *this, type_die_form.Reference().GetID());
 
   if (use_type_size_for_value && type_sp->GetType()) {
     DWARFExpression *location = location_list.GetMutableExpressionAtAddress();
     location->UpdateValue(const_value_form.Unsigned(),
                           type_sp->GetType()->GetByteSize(nullptr).value_or(0),
                           die.GetCU()->GetAddressByteSize());
   }
 
for (DWARFDIE child : function_die.children()) {
     }
 
     call_edges.push_back(std::move(edge));
   }
   return call_edges;
 }
 
 std::vector<std::unique_ptr<lldb_private::CallEdge>>
-SymbolFileDWARF::ParseCallEdgesInFunction(UserID func_id) {
+SymbolFileDWARF::ParseCallEdgesInFunction(lldb_private::UserID func_id) {
   // ParseCallEdgesInFunction must be called at the behest of an exclusively
   // locked lldb::Function instance. Storage for parsed call edges is owned by
   // the lldb::Function instance: locking at the SymbolFile level would be too
   // late, because the act of storing results from ParseCallEdgesInFunction
   // would be racy.
   DWARFDIE func_die = GetDIE(func_id.GetID());
   if (func_die.IsValid())
     return CollectCallEdges(GetObjectFile()->GetModule(), func_die);
if (FileSystem::Instance().Exists(dwp_filespec)) {
       DataBufferSP dwp_file_data_sp;
       lldb::offset_t dwp_file_data_offset = 0;
       ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
           GetObjectFile()->GetModule(), &dwp_filespec, 0,
           FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp,
           dwp_file_data_offset);
       if (!dwp_obj_file)
         return;
-      m_dwp_symfile =
-          std::make_shared<SymbolFileDWARFDwo>(*this, dwp_obj_file, 0x3fffffff);
+      m_dwp_symfile = std::make_shared<SymbolFileDWARFDwo>(
+          *this, dwp_obj_file, DIERef::k_file_index_mask);
     }
   });
   return m_dwp_symfile;
 }
 
 llvm::Expected<lldb::TypeSystemSP>
 SymbolFileDWARF::GetTypeSystem(DWARFUnit &unit) {
   return unit.GetSymbolFileDWARF().GetTypeSystemForLanguage(GetLanguage(unit));

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

 //===-- SymbolFileDWARFDebugMap.h ------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARFDEBUGMAP_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARFDEBUGMAP_H
 
+#include "DIERef.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Utility/RangeMap.h"
 #include "llvm/Support/Chrono.h"
 #include <bitset>
 #include <map>
 #include <optional>
 #include <vector>
 
protected:
   /// upfront.
   uint32_t CalculateNumCompileUnits() override;
 
   /// This function actually returns the first compile unit the object file at
   /// the given index contains.
   lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
 
   static uint32_t GetOSOIndexFromUserID(lldb::user_id_t uid) {
-    return (uint32_t)((uid >> 32ull) - 1ull);
+    std::optional<uint32_t> OsoNum = DIERef(uid).file_index();
+    lldbassert(OsoNum && "Invalid OSO Index");
+    return *OsoNum;
   }
 
   static SymbolFileDWARF *GetSymbolFileAsSymbolFileDWARF(SymbolFile *sym_file);
 
   bool GetFileSpecForSO(uint32_t oso_idx, lldb_private::FileSpec &file_spec);
 
   CompileUnitInfo *GetCompUnitInfo(const lldb_private::SymbolContext &sc);
   CompileUnitInfo *GetCompUnitInfo(const lldb_private::CompileUnit &comp_unit);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

if (exe_module_sp) {
           ObjectFile *exe_objfile = exe_module_sp->GetObjectFile();
           SymbolFile *exe_symfile = exe_module_sp->GetSymbolFile();
 
           if (exe_objfile && exe_symfile) {
             oso_symfile->SetDebugMapModule(exe_module_sp);
             // Set the ID of the symbol file DWARF to the index of the OSO
             // shifted left by 32 bits to provide a unique prefix for any
             // UserID's that get created in the symbol file.
-            oso_symfile->SetID(((uint64_t)m_cu_idx + 1ull) << 32ull);
+            oso_symfile->SetFileIndex((uint64_t)m_cu_idx);
           }
           return symfile;
         }
       }
     }
     return nullptr;
   }
 
if (sc_scope) {
     ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool {
       oso_dwarf->GetTypes(sc_scope, type_mask, type_list);
       return false;
     });
   }
 }
 
 std::vector<std::unique_ptr<lldb_private::CallEdge>>
-SymbolFileDWARFDebugMap::ParseCallEdgesInFunction(UserID func_id) {
+SymbolFileDWARFDebugMap::ParseCallEdgesInFunction(
+    lldb_private::UserID func_id) {
   uint32_t oso_idx = GetOSOIndexFromUserID(func_id.GetID());
   SymbolFileDWARF *oso_dwarf = GetSymbolFileByOSOIndex(oso_idx);
   if (oso_dwarf)
     return oso_dwarf->ParseCallEdgesInFunction(func_id);
   return {};
 }
 
 TypeSP SymbolFileDWARFDebugMap::FindDefinitionTypeForDWARFDeclContext(
SymbolFileDWARFDebugMap::AddOSOARanges(SymbolFileDWARF *dwarf2Data,
   if (debug_aranges && dwarf2Data) {
     CompileUnitInfo *compile_unit_info = GetCompileUnitInfo(dwarf2Data);
     if (compile_unit_info) {
       const FileRangeMap &file_range_map =
           compile_unit_info->GetFileRangeMap(this);
       for (size_t idx = 0; idx < file_range_map.GetSize(); idx++) {
         const FileRangeMap::Entry *entry = file_range_map.GetEntryAtIndex(idx);
         if (entry) {
-          debug_aranges->AppendRange(dwarf2Data->GetID(), entry->GetRangeBase(),
+          debug_aranges->AppendRange(*dwarf2Data->GetFileIndex(),
+                                     entry->GetRangeBase(),
                                      entry->GetRangeEnd());
           num_line_entries_added++;
         }
       }
     }
   }
   return num_line_entries_added;
 }

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

 //===-- SymbolFileDWARFDwo.h ------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARFDWO_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARFDWO_H
 
 #include "SymbolFileDWARF.h"
 #include <optional>
 
 class SymbolFileDWARFDwo : public SymbolFileDWARF {
   /// LLVM RTTI support.
   static char ID;
 
 public:
   /// LLVM RTTI support.
void GetObjCMethods(lldb_private::ConstString class_name,
                       llvm::function_ref<bool(DWARFDIE die)> callback) override;
 
   llvm::Expected<lldb::TypeSystemSP>
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
   DWARFDIE
   GetDIE(const DIERef &die_ref) override;
 
-  std::optional<uint32_t> GetDwoNum() override { return GetID() >> 32; }
-
   lldb::offset_t
   GetVendorDWARFOpcodeSize(const lldb_private::DataExtractor &data,
                            const lldb::offset_t data_offset,
                            const uint8_t op) const override;
 
   bool ParseVendorDWARFOpcode(
       uint8_t op, const lldb_private::DataExtractor &opcodes,
       lldb::offset_t &offset,

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

 //===-- SymbolFileDWARFDwo.cpp --------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolFileDWARFDwo.h"
 
 #include "lldb/Core/Section.h"
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "llvm/Support/Casting.h"
 
 #include "DWARFCompileUnit.h"
 #include "DWARFDebugInfo.h"
 #include "DWARFUnit.h"
 #include <optional>
 
 using namespace lldb;
 using namespace lldb_private;
 
 char SymbolFileDWARFDwo::ID;
 
 SymbolFileDWARFDwo::SymbolFileDWARFDwo(SymbolFileDWARF &base_symbol_file,
                                        ObjectFileSP objfile, uint32_t id)
     : SymbolFileDWARF(objfile, objfile->GetSectionList(
                                    /*update_module_section_list*/ false)),
       m_base_symbol_file(base_symbol_file) {
-  SetID(user_id_t(id) << 32);
+  SetFileIndex(id);
 
   // Parsing of the dwarf unit index is not thread-safe, so we need to prime it
   // to enable subsequent concurrent lookups.
   m_context.GetAsLLVM().getCUIndex();
 }
 
 DWARFCompileUnit *SymbolFileDWARFDwo::GetDWOCompileUnitForHash(uint64_t hash) {
   if (const llvm::DWARFUnitIndex &index = m_context.GetAsLLVM().getCUIndex()) {
     if (const llvm::DWARFUnitIndex::Entry *entry = index.getFromHash(hash)) {
       if (auto *unit_contrib = entry->getContribution())
         return llvm::dyn_cast_or_null<DWARFCompileUnit>(
             DebugInfo().GetUnitAtOffset(DIERef::Section::DebugInfo,
-                                        unit_contrib->getOffset32()));
+                                        unit_contrib->getOffset()));
     }
     return nullptr;
   }
 
   DWARFCompileUnit *cu = FindSingleCompileUnit();
   if (!cu)
     return nullptr;
   std::optional<uint64_t> dwo_id = cu->GetDWOId();

 
 llvm::Expected<lldb::TypeSystemSP>
 SymbolFileDWARFDwo::GetTypeSystemForLanguage(LanguageType language) {
   return GetBaseSymbolFile().GetTypeSystemForLanguage(language);
 }
 
 DWARFDIE
 SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
-  if (die_ref.dwo_num() == GetDwoNum())
+  if (die_ref.file_index() == GetFileIndex())
     return DebugInfo().GetDIE(die_ref);
   return GetBaseSymbolFile().GetDIE(die_ref);
 }

lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s

 # DW_AT_ranges can use DW_FORM_sec_offset (instead of DW_FORM_rnglistx).
 # In such case DW_AT_rnglists_base does not need to be present.
 
 # REQUIRES: x86
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
 # RUN: %lldb %t -o "image lookup -v -s lookup_rnglists" \
 # RUN:   -o exit | FileCheck %s
 
 # Failure was the block range 1..2 was not printed plus:
-# error: DW_AT_range-DW_FORM_sec_offset.s.tmp {0x0000003f}: DIE has DW_AT_ranges(0xc) attribute, but range extraction failed (missing or invalid range list table), please file a bug and attach the file at the start of this error message
+# error: DW_AT_range-DW_FORM_sec_offset.s.tmp {0x000000000000003f}: DIE has DW_AT_ranges(0xc) attribute, but range extraction failed (missing or invalid range list table), please file a bug and attach the file at the start of this error message
 
 # CHECK-LABEL: image lookup -v -s lookup_rnglists
 # CHECK:  Function: id = {0x00000029}, name = "rnglists", range = [0x0000000000000000-0x0000000000000003)
 # CHECK:    Blocks: id = {0x00000029}, range = [0x00000000-0x00000003)
 # CHECK-NEXT:       id = {0x0000003f}, range = [0x00000001-0x00000002)
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj \
 # RUN:   --defsym RNGLISTX=0 %s > %t-rnglistx

lldb/unittests/Expression/DWARFExpressionTest.cpp

)";
   //             - Attribute:       DW_AT_dwo_id
   //               Form:            DW_FORM_data4
   //   debug_info: #.dwo
   //     - Version:         4
   //       AddrSize:        4
   //       Entries:
   //         - AbbrCode:        0x1
   //           Values:
-  //             - Value:           0x01020304
+  //             - Value:           0x0120304
   //         - AbbrCode:        0x0
   const char *dwo_yamldata = R"(
 --- !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_EXEC
   Machine:         EM_386
)";
   auto skeleton_module_sp =
       std::make_shared<Module>(skeleton_file->moduleSpec());
   auto &skeleton_symfile =
       *llvm::cast<CustomSymbolFileDWARF>(skeleton_module_sp->GetSymbolFile());
 
   auto dwo_module_sp = std::make_shared<Module>(dwo_file->moduleSpec());
   SymbolFileDWARFDwo dwo_symfile(
       skeleton_symfile, dwo_module_sp->GetObjectFile()->shared_from_this(),
-      0x01020304);
+      0x0120304);
   auto *dwo_dwarf_unit = dwo_symfile.DebugInfo().GetUnitAtIndex(0);
 
   testExpressionVendorExtensions(dwo_module_sp, *dwo_dwarf_unit);
 }

lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp

 TEST(DWARFIndexCachingTest, DIERefEncodeDecode) {
   // Tests DIERef::Encode(...) and DIERef::Decode(...)
   EncodeDecode(DIERef(std::nullopt, DIERef::Section::DebugInfo, 0x11223344));
   EncodeDecode(DIERef(std::nullopt, DIERef::Section::DebugTypes, 0x11223344));
   EncodeDecode(DIERef(100, DIERef::Section::DebugInfo, 0x11223344));
   EncodeDecode(DIERef(200, DIERef::Section::DebugTypes, 0x11223344));
 }
 
+TEST(DWARFIndexCachingTest, DIERefEncodeDecodeMax) {
+  // Tests DIERef::Encode(...) and DIERef::Decode(...)
+  EncodeDecode(DIERef(std::nullopt, DIERef::Section::DebugInfo,
+                      DIERef::k_die_offset_mask - 1));
+  EncodeDecode(DIERef(std::nullopt, DIERef::Section::DebugTypes,
+                      DIERef::k_die_offset_mask - 1));
+  EncodeDecode(
+      DIERef(100, DIERef::Section::DebugInfo, DIERef::k_die_offset_mask - 1));
+  EncodeDecode(
+      DIERef(200, DIERef::Section::DebugTypes, DIERef::k_die_offset_mask - 1));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugInfo,
+                      DIERef::k_file_index_mask));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugTypes,
+                      DIERef::k_file_index_mask));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugInfo,
+                      0x11223344));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugTypes,
+                      0x11223344));
+}
+
 static void EncodeDecode(const NameToDIE &object, ByteOrder byte_order) {
   const uint8_t addr_size = 8;
   DataEncoder encoder(byte_order, addr_size);
   DataEncoder strtab_encoder(byte_order, addr_size);
   ConstStringTable const_strtab;
 
   object.Encode(encoder, const_strtab);