Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

[dsymutil] Add support for mergeable libraries
ClosedPublic

Authored by Alpha on Aug 16 2023, 3:19 PM.

Details

Summary

This adds support in dsymutil for mergeable libraries.

dsymutil reads a new stab emitted by ld, allowing it to operate on dynamic libraries instead of object files. It also now loads the Dwarf files associated to the libraries, and build the debug map for each binary from the list of symbols exported by the library.
For each Debug Map Object, there is a new associated Relocation Map which is serialized from the information retrieved in the original debug_info (or debug_addr) section of the .o file.

The final DWARF file has multiple compile units, so the offsets information of the relocations are adjusted relatively to the compile unit they will end up belonging to, inside the final linked DWARF file.

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Alpha updated this revision to Diff 553568.Aug 25 2023, 12:03 PM

Properly add binary files

avl added a comment.Aug 28 2023, 10:20 AM

Thank you for the update! Please consider inline comments.

llvm/include/llvm/DWARFLinker/DWARFLinker.h
17

Looks like nothing from this file is used in DWARFLinker. So it(include and file itself) might be removed.

37

AttributeLinkedOffsetFixup is local to DWARFLinker.cpp no need to place it in a header. It would be OK to declare it inside DWARFLinker.cpp before DWARFLinker::cloneDIE.

66

Can we use more descriptive comment and name here, please? Like getMergedLibraryName() or getLibraryInstallName() or getAppleOriginName() or something else?

80

it looks like it is not used in DWARFLinker, probably we can remove it from DWARFLinker.h.

llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
151 ↗(On Diff #553568)

AddedOriginObject used for offset calculation previously. Now it looks unused and could be removed completely.

llvm/include/llvm/DWARFLinkerParallel/AddressesMap.h
68
69

probably relinkAndSaveValidRelocs would be more presize naming?

llvm/lib/DWARFLinker/DWARFLinker.cpp
603

If I understood correctly, no need to store RelocMgr.getFileName() into MyInfo.FileName. It could be used directly from RelocMgr.

1771

we can use RelocMgr.getFileName() here(instead of Info.FileName), correct?

1863

probably move Unit.getStartOffset() from this place into the point where F.LinkedOffsetFixupVal is calculated. That looks a bit unclear why it is added here:

CurAttrFixup.LinkedOffsetFixupVal =
        Unit.getStartOffset() + OutOffset - CurAttrFixup.InputAttrStartOffset;
avl added a comment.Aug 28 2023, 3:18 PM

I also tried to guess how it could be used with DWARFLinkerParallel. The differences are:

  1. DWARFLinkerParallel handles compile units parallelly. That means ObjFile.Addresses->saveValidRelocs() can be called asynchronously.
  1. Unit.getStartOffset() is not known at the moment ObjFile.Addresses->saveValidRelocs() is called.
  1. DWARFLinkerParallel tries to release resources as soon as they are not needed anymore. It deletes AddressesMap once all compile units are cloned using Addresses.reset(); So when current patch would try to call emitRelocations all data would already be deleted.

I do not say that we need to implement the DWARFLinkerParallel part with this patch. It would be better
to do it with a separate patch. But, probably it would be good to make the interface part compatible with
DWARFLinkerParallel in this patch. To do this we could probably change the interface in this way:

  1. Add the offset of the original compile unit to the method which relinks relocations.
class DwarfLinkerForBinary::AddressManager<AddressesMapBase> {
  virtual void saveValidRelocs(uint64_t OriginalUnitOffset, int64_t LinkedOffsetFixupVal, uint64_t InputAttrStartOffset,
                               uint64_t InputAttrEndOffset) {
    ...
    std::vector<ValidReloc>& StoredValidDebugInfoRelocs = StoredValidDebugInfoRelocsMap[OriginalUnitOffset];
    for (ValidReloc R : Relocs) {
       StoredValidDebugInfoRelocs.emplace_back(R.Offset + LinkedOffsetFixupVal, R.Size,
                                            R.Addend, R.SymbolName,
                                            R.SymbolMapping);
    }
    ...
  }

It allows storing relocations for compile units separately and then could be done in parallel.

  1. When the AddressesMap object is created - build a map for units relocations:
DenseMap<uint64_t Offset, std::vector<ValidReloc>> StoredValidDebugInfoRelocsMap;

DwarfLinkerForBinary::AddressManager<AddressesMapBase> (DWARFContext& Context) {

  for (const std::unique_ptr<DWARFUnit> &CU : Context.compile_units()) {
    StoredValidDebugInfoRelocs.insert(std::make_pair(CU->getOffset(), std::vector<ValidReloc>>));
  }
}
  1. Add a method which allows updating relocations offset with outputUnit Offset:
class DwarfLinkerForBinary::AddressManager<AddressesMapBase> {
   
   void updateRelinkedRelocationsWithUnitOffset (uint64_t OriginalUnitOffset, uint64_t OutputUnitOffset) {
    std::vector<ValidReloc>& StoredValidDebugInfoRelocs = StoredValidDebugInfoRelocsMap[OriginalUnitOffset];
    for (ValidReloc R : Relocs) {
      R.Offset += OutputUnitOffset;
    }
   }
}

that methods would be called by DWARFLinkerParallel when output unit offsets would be known.

  1. Add the cleanupInputData() method to the AddressesMap which will release all resources except StoredValidDebugInfoRelocsMap. that allows calling "Addresses.cleanupInputData()" instead of "Addresses.reset();" and then to keep StoredValidDebugInfoRelocsMap for further processing.

What do you think? Could we implement these changes also?

Alpha updated this revision to Diff 554820.Aug 30 2023, 1:41 PM
Alpha marked 10 inline comments as done.

Addressed inline comments

Alpha added inline comments.Aug 30 2023, 1:42 PM
llvm/include/llvm/DWARFLinker/DWARFLinker.h
17

We need the RelocMap type for addValidRelocs(RelocMap *RM)

80

addValidRelocs is called in DwarfLinkerForBinary::emitRelocations to add the relocations to the RelocationMap

llvm/include/llvm/DWARFLinkerParallel/AddressesMap.h
68

Updating the comment, but just to clarify, saveValidRelocs doesn't "relink" anything, it merely finds relocations that are applied to the object, store them, so they can be serialized later.

69

Addressed in the comment above

llvm/lib/DWARFLinker/DWARFLinker.cpp
603

We need the name to be accessible within cloneDie, which is why we pass it via DIEInfo, to add the APPLE_origin attribute

1771

Unless I'm mistaken, we do not have access to the AddressManager from within cloneDIE.
The other alternative would be adding the libInstallName (which right now is specific to dsymutil and its AddressManager) to the parent AddressMap, but is this something we really want to do?

Alpha added a comment.Aug 30 2023, 3:25 PM

I have nothing necessarily against the suggestion to help the parallel implementation, only a few points:

  1. When the AddressesMap object is created - build a map for units relocations:
DenseMap<uint64_t Offset, std::vector<ValidReloc>> StoredValidDebugInfoRelocsMap;

DwarfLinkerForBinary::AddressManager<AddressesMapBase> (DWARFContext& Context) {

  for (const std::unique_ptr<DWARFUnit> &CU : Context.compile_units()) {
    StoredValidDebugInfoRelocs.insert(std::make_pair(CU->getOffset(), std::vector<ValidReloc>>));
  }

In dsymutil, the DWARFContext is not passed to the AddressManager.
I could break down the DwarfFile constructor within DwarfLinkerForBinary::loadObject, and pass the second DwrafContext argument also to the AddressesMap as a new parameter, if we feel that is reasonable architecturally and would not lead to unknown race and/or runtime dependency issues (?)

  1. Add the cleanupInputData() method to the AddressesMap which will release all resources except StoredValidDebugInfoRelocsMap. that allows calling "Addresses.cleanupInputData()" instead of "Addresses.reset();" and then to keep StoredValidDebugInfoRelocsMap for further processing.

Isn't this already the purpose of clear() ? Like the following from DwarfLinkerForBinary.h

void clear() override {
  ValidDebugInfoRelocs.clear();
  ValidDebugAddrRelocs.clear();
}
avl added a comment.Aug 31 2023, 5:39 AM

In dsymutil, the DWARFContext is not passed to the AddressManager.
I could break down the DwarfFile constructor within DwarfLinkerForBinary::loadObject, and pass the second DwrafContext argument also to the AddressesMap as a new parameter, if we feel that is reasonable architecturally and would not lead to unknown race and/or runtime dependency issues (?)

It would be OK, though we should not change DWARFLinker::AddressesMap. With new class DwarfLinkerForBinaryRelocMap(described in comments) things would look like this: when DwarfLinkerForBinary::AddressManager would be created it needs to get reference to the DwarfLinkerForBinaryRelocMap object. So that calls to saveValidRelocs()/updateRelinkedRelocationsWithUnitOffset() were forwarded to DwarfLinkerForBinaryRelocMap by DwarfLinkerForBinary::AddressManager. Also, DwarfLinkerForBinaryRelocMap should be initialized with DwarfContext to build units relocation map.

As to the DWARFLinker::AddressesMap, it should have only these three methods : getLibraryInstallName(), saveValidRelocs(), updateRelinkedRelocationsWithUnitOffset().

Isn't this already the purpose of clear() ? Like the following from DwarfLinkerForBinary.h

void clear() override {

ValidDebugInfoRelocs.clear();
ValidDebugAddrRelocs.clear();

}

you are right it might be used for resources cleanup. Though with the design using DwarfLinkerForBinaryRelocMap things may be left as it currently is. Because relocations would be kept separately and then DwarfLinkerForBinary::AddressManager may be safely deleted.

llvm/include/llvm/DWARFLinker/DWARFLinker.h
17

yes, but addValidRelocs is not used(and is not supposed to be used) anywhere inside lib/DWARFLinker/*. It is used inside dsymutil/DwarfLinkerForBinary. So this method could be declared and defined in DwarfLinkerForBinary::AddressManager. No neccessary to declare it in base class DWARFLinker::AddressesMap.

80

That means that addValidRelocs not needed in base class DWARFLinker::AddressesMap and it would be better to move it into the DwarfLinkerForBinary::AddressManager. There are no users of that method inside lib/DWARFLinker/*.

methods declared in DWARFLinker::AddressesMap could be used by DWARFLinker library. If they are not supposed to be used by DWARFLinker library then we should not add them into DWARFLinker::AddressesMap.

In this patch addValidRelocs is called through DWARFLinker::AddressesMap interface:

Obj->Addresses->addValidRelocs(static_cast<RelocMap *>(&RM));

instead DwarfLinkerForBinary::AddressManager may get reference to separate DwarfLinkerForBinaryRelocMap object. When saveValidRelocs is called, the relocations would be stored by that separate object. Which allows to store content of StoredValidDebugInfoRelocsMap not using base interface DWARFLinker::AddressesMap.

class DwarfLinkerForBinaryRelocMap {

    void addValidRelocs(RelocMap *RM);

    void updateRelinkedRelocationsWithUnitOffset (uint64_t OriginalUnitOffset, uint64_t OutputUnitOffset);

    void saveValidRelocs(uint64_t OriginalUnitOffset, int64_t LinkedOffsetFixupVal, uint64_t InputAttrStartOffset,  uint64_t InputAttrEndOffset);

    void init ( DwarfContext& );

    DenseMap<uint64_t Offset, std::vector<ValidReloc>> StoredValidDebugInfoRelocsMap;
};
  
struct ObjectWithRelocMap {
  std::unique_ptr<OutDwarfFile> Object;
  DwarfLinkerForBinaryRelocMap OutRelocs;
};
std::vector<ObjectWithRelocMap> ObjectsForLinking;

for (const auto &Obj : ObjectsForLinking) {
  Obj.OutRelocs.addValidRelocs(static_cast<RelocMap *>(&RM));
}
llvm/include/llvm/DWARFLinkerParallel/AddressesMap.h
68

The saved relocations are not the same as original, correct? DWARFLinker links debug info, when saveValidRelocs is called the offsets of relocations are updated with proper offsets inside linked debug info. It would be good if method name would reflect the fact that new relocations have new offsets. If "relink" is not a good word for it probably we could use some another.

llvm/lib/DWARFLinker/DWARFLinker.cpp
603

cloneDIE is the method of DIECloner class:

DIE *DWARFLinker::DIECloner::cloneDIE()

DIECloner has a member DWARFFile &ObjFile:

class DIECloner {
 ...
  DWARFFile &ObjFile;
 ...
 }

So we can call ObjFile.Addresses->getLibraryInstallName(); within cloneDIE():

DIE *DWARFLinker::DIECloner::cloneDIE() {
...
   ObjFile.Addresses->getLibraryInstallName();
...
}
1771

addressed in another comment.

Alpha updated this revision to Diff 555191.EditedAug 31 2023, 4:11 PM
Alpha marked 3 inline comments as done.

Address inline comments.
However, regarding all the DwarfLinkerForBinaryRelocMap and the discussion about wether or not some logic should be in DwrafLinker (as they're all related).
I agree in the sense that my initial incline was to limit all that logic to AddressManager.
The issue here is that with DwarfLinkerForBinary, we have access to Obj->Addresses, which is the AddressesMap, and there is no reasonable way do downcast it to AddressManager (as dyn_cast doesn't work here), which is why I had to resort to have it being part of the AddressesMap map interface and call via virtual dispatch

avl added a comment.Sep 1 2023, 4:27 AM

Address inline comments.
However, regarding all the DwarfLinkerForBinaryRelocMap and the discussion about wether or not some logic should be in DwrafLinker (as they're all related).
I agree in the sense that my initial incline was to limit all that logic to AddressManager.
The issue here is that with DwarfLinkerForBinary, we have access to Obj->Addresses, which is the AddressesMap, and there is no reasonable way do downcast it to AddressManager (as dyn_cast doesn't work here), which is why I had to resort to have it being part of the AddressesMap map interface and call via virtual dispatch

As you said, to make possible to call addValidRelocs through Obj->Addresses it is neccessary to put some unrelated data into the base interface DWARFLinker::AddressesMap. This is not a good thing. DWARFLinker library does not call to addValidRelocs(static_cast<RelocMap *>(&RM)),
it does not use "class Relocation" and "class RelocMap". We should not make these classes known to DWARFLinker library.

We can hide all RelocMap specifics without downcasting DWARFLinker::AddressesMap to DwarfLinkerForBinary::AddressManager.
DwarfLinkerForBinary::AddressManager can delegate handling to the separate class(f.e. DwarfLinkerForBinaryRelocMap from previous comments).
This allows to not put specific classes into the DWARFLinker interface and to not use downcasting.

llvm/lib/DWARFLinker/DWARFLinker.cpp
620

no need to add break here.

1762

it would be good to add check for ObjFile.Addresses->getLibraryInstallName() here to not collect AttributeLinkedOffsetFixup if not necessary:

std::optional<StringRef> LibraryInstallName = ObjFile.Addresses->getLibraryInstallName();
.....
if (FinalAttrSize != 0 && LibraryInstallName.has_value()) {
Alpha marked 4 inline comments as done.Sep 26 2023, 1:38 PM
Alpha added inline comments.
llvm/lib/DWARFLinker/DWARFLinker.cpp
1762

Hmm, we also need t have the fixups/relocs saved, even if we're dealing with regular object files, as we still need to always save the relocations, regardless of the type of input file.

Alpha updated this revision to Diff 557374.Sep 26 2023, 1:40 PM
Alpha marked an inline comment as done.

Addressed feedback.

Alpha updated this revision to Diff 557377.Sep 26 2023, 3:27 PM

Rebase against main

Alpha updated this revision to Diff 557378.Sep 26 2023, 3:44 PM

Update doc and remove dead code

avl added a comment.EditedSep 28 2023, 7:57 AM

Thank you for the update!

I have a general question - Could you measure difference with run-time memory requirements(f.e. for clang binary) with and without this patch, please?

llvm/lib/DWARFLinker/DWARFLinker.cpp
1762

Before this patch dsymutil did not store fixup/relocs for regular files. Is that correct that this information would stored for usual files(not mergeable libraries) after this patch?

Alpha marked an inline comment as done.Sep 28 2023, 11:01 AM

I have a general question - Could you measure difference with run-time memory requirements(f.e. for clang binary) with and without this patch, please?

Is there a specific test suite/benchmark/tool somewhere that you want to be run here, and can point to?

llvm/lib/DWARFLinker/DWARFLinker.cpp
1762

A mergeable library is a file a regular dylib that contains extra metadata that fundamentally just represents a the original regular object files.
So when let's say an executable links against a mergeable library, in addition to the regular N_OSO stab to tell were the symbol might have originated from, there is an additional N_LIB that indicates the library where the symbol is from (so both stabs are present). This is the "only" moment when dsymutil detects the presence of mergeable libraries, and takes can take a different code path.
So in order for dsymutil to be able to work in this case, the .dSYM files used as input need the original relocations to be present. So yes, when we emit .dSYM files, we need to serialize the relocation info.

avl added a comment.Sep 28 2023, 12:29 PM

I have a general question - Could you measure difference with run-time memory requirements(f.e. for clang binary) with and without this patch, please?

Is there a specific test suite/benchmark/tool somewhere that you want to be run here, and can point to?

/usr/bin/time -l would be enough:

/usr/bin/time -l dsymutil clang
  19635666944  maximum resident set size
  ...
llvm/lib/DWARFLinker/DWARFLinker.cpp
1762

In that case we probably need to add a flag indicating whether DWARFLinker needs to gather and save relocations info. Searching for relocations and keeping them in some array requires additional resources. Other tools(f.e. llvm-dwarfutil) may save run-time&memory by not doing this thing:

class AddressesMap {
  bool needToSaveRelocs();
}

and then here:

if (FinalAttrSize != 0 && ObjFile.Addresses->needToSaveRelocs()) {
Alpha marked 2 inline comments as done.Sep 29 2023, 12:00 PM

/usr/bin/time -l would be enough:

$ /usr/bin/time -l /ninja-main/bin/dsymutil ninja-main/bin/clang
[...]
          7394394112  maximum resident set size
[...]
         16408924544  peak memory footprint

$ /usr/bin/time -l ninja-patch/bin/dsymutil ninja-main/bin/clang
[...]
          9706700800  maximum resident set size
[...]
         17250589760  peak memory footprint
Alpha updated this revision to Diff 557494.Sep 29 2023, 12:02 PM

Conditionalize relocations saving

avl added a comment.Sep 30 2023, 11:12 AM

It looks like patch is updated with wrong changes.

avl added a comment.Sep 30 2023, 12:19 PM

Thank you for the update and measurements!

The DWARFLinker changes look generally OK. Though, I still have two concerns, probably, they could be resolved by separate patches.

First thing is that increasing binary RSS requirements for more than 2G is quite a big change. I think, this increasing is coused mostly by two things:
symbol name and mapping now kept inside ValidReloc structure(previously ValidReloc had only pointer to mapping). StoredValidDebugInfoReloc contains copy of original
relocation which differ with only Offset field(other fields have the same data). Probably the most memory saving solution would be to keep original ValidReloc structure with new field added. In this case, only single uint64_t for each relocation would be added. No additional containers would be required:

struct ValidReloc {
   uint64_t Offset;
   uint64_t LinkedOffset;
   uint32_t Size;
   uint64_t Addend;
   const DebugMapObject::DebugMapEntry *Mapping;
}

Second thing is that interface of AddressesMap looks a bit weak. It does not have updateRelocationsWithUnitOffset() method for DWARFLinkerParallel. The updateAndSaveValidRelocs() method does not have a RelocationKind parameter which would be important when support of .debug_addr would be added. Probably, these changes might be done with separate patches for DWARFLinkerParallel and support of .debug_addr.

So, changes for DWARFLinker is LGTM, assuming above concerns would be addressed by separate patches.

Please, wait approve for the rest of the patch from dsymutil owners.

Alpha updated this revision to Diff 557512.Sep 30 2023, 1:28 PM

Address AddressesMap interface comments:

  • updateRelocationsWithUnitOffset was added,
  • added a bool parameter to updateAndSaveValidRelocs (as it's more straightforward than adding a whole new type that can be made available to both DWARFLinker and DWARFLinkerParallel)
avl added a comment.Oct 2 2023, 6:50 AM

DWARFLinker part LGTM.

JDevlieghere added inline comments.Oct 2 2023, 9:02 AM
llvm/docs/CommandGuide/dsymutil.rst
37

This doesn't really explain anything as much as rephrase the command option. What is the build variant suffix?

50

Can you add a little bit more detail, such as that it's used with relinkable libraries?

llvm/include/llvm/DWARFLinker/DWARFLinker.h
776

HasAppleOrigin would be more in line with the rest of the code base.

llvm/lib/DWARFLinker/DWARFLinker.cpp
1762–1764

No braces around single-line if blocks.

1769

This seems incomplete looking at the code below. Should it say something like "Add origin attribute to Compile Unit die if we have an install name and the DWARF doesn't have the DW_AT_APPLE_origin attribute yet".

1770–1780

Instead of 3 levels of nesting, how about

const bool NeedsAppleOrigin = Tag == dwarf::DW_TAG_compile_unit && LibraryInstallName.has_value() && AttrInfo.AttrAppleOriginSeen;
if (NeedsAppleOrigin) {
  ...
}

or even just inline the condition.

1857–1859

Nit: No braces

1861–1865

No braces

1863

Hardcoding this to not be DWARF5 seems suspicious. If this is the right thing to do, this definitely needs a comment explaining why.

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
1231–1234

Nit: braces. Same below.

llvm/tools/dsymutil/DwarfLinkerForBinary.h
49–51

Doxygen group

llvm/tools/dsymutil/MachODebugMapParser.cpp
219–221

Nit: reflow?

Alpha updated this revision to Diff 557578.Oct 3 2023, 3:48 PM
Alpha marked 12 inline comments as done.

Addressed comments

This revision was not accepted when it landed; it landed in state Needs Review.Oct 24 2023, 10:39 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

Breaks bbot:

839.901 [400/32/3613] Building CXX object tools/dsymutil/CMakeFiles/dsymutil.dir/DebugMap.cpp.o
FAILED: tools/dsymutil/CMakeFiles/dsymutil.dir/DebugMap.cpp.o 
ccache /usr/bin/c++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/tools/dsymutil -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/include -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/dsymutil/CMakeFiles/dsymutil.dir/DebugMap.cpp.o -MF tools/dsymutil/CMakeFiles/dsymutil.dir/DebugMap.cpp.o.d -o tools/dsymutil/CMakeFiles/dsymutil.dir/DebugMap.cpp.o -c /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/DebugMap.cpp
In file included from /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/DebugMap.h:24,
                 from /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/DebugMap.cpp:9:
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/RelocationMap.h:60:17: error: declaration of ‘llvm::dsymutil::SymbolMapping llvm::dsymutil::ValidReloc::SymbolMapping’ changes meaning of ‘SymbolMapping’ [-fpermissive]
   60 |   SymbolMapping SymbolMapping;
      |                 ^~~~~~~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/RelocationMap.h:36:8: note: ‘SymbolMapping’ declared here as ‘struct llvm::dsymutil::SymbolMapping’
   36 | struct SymbolMapping {
      |        ^~~~~~~~~~~~~
In file included from /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/DebugMap.cpp:9:
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/DebugMap.h:198:32: error: declaration of ‘std::optional<llvm::dsymutil::RelocationMap> llvm::dsymutil::DebugMapObject::RelocationMap’ changes meaning of ‘RelocationMap’ [-fpermissive]
  198 |   std::optional<RelocationMap> RelocationMap;
      |                                ^~~~~~~~~~~~~
In file included from /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/DebugMap.h:24,
                 from /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/DebugMap.cpp:9:
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/tools/dsymutil/RelocationMap.h:76:7: note: ‘RelocationMap’ declared here as ‘class llvm::dsymutil::RelocationMap’
   76 | class RelocationMap {
      |       ^~~~~~~~~~~~~

This revision has not been accepted it seems.