caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[DWARFLinker] mark odr candidates inside the same object file.

Browse Source 
This patch is extracted from D86539.

Current implementation of lookForDIEsToKeep() function skips types
duplications basing on the getCanonicalDIEOffset() data:

```
if (AttrSpec.Form != dwarf::DW_FORM_ref_addr && (UseOdr || IsModuleRef) &&
    Info.Ctxt &&
    Info.Ctxt != ReferencedCU->getInfo(Info.ParentIdx).Ctxt &&
    Info.Ctxt->getCanonicalDIEOffset() && isODRAttribute(AttrSpec.Attr))  <<<<<
  continue;
```

But that field is set after all compile units inside object file are processed:

```
for (auto &CurrentUnit : OptContext.CompileUnits)
  lookForDIEsToKeep(.., &CurrentUnit, ..);  // check CanonicalDIEOffset

DIECloner.cloneAllCompileUnits(); // set CanonicalDIEOffset
```

Thus, if the object file contains several compilation units - types would
not be deduplicated. The above solution works well for the case when the object file
contains only one compilation unit. But if the object file contains several compilation
units then types would not be deduplicated between these compilation units.

This patch changes the algorithm so that types were deduplicated between
compilation units from the same object file.

It produces binary incompatible output for the cases when several compilation units
are located inside the same object file.

Reviewed By: aprantl

Differential Revision: https://reviews.llvm.org/D125469
This commit is contained in:
Alexey Lapshin
2022-06-27 18:40:10 +03:00
parent 6901607822
commit 2b747241a6
6 changed files with 279 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
llvm/include/llvm/DWARFLinker/DWARFLinker.h
View File

@@ -365,6 +365,8 @@ private:
/// Given a DIE, update its incompleteness based on whether the DIEs it
/// references are incomplete.
UpdateRefIncompleteness,
/// Given a DIE, mark it as ODR Canonical if applicable.
MarkODRCanonicalDie,
};
/// This class represents an item in the work list. The type defines what kind
@@ -464,6 +466,10 @@ private:
const DWARFFile &File,
SmallVectorImpl<WorklistItem> &Worklist);
/// Mark context corresponding to the specified \p Die as having canonical
/// die, if applicable.
void markODRCanonicalDie(const DWARFDie &Die, CompileUnit &CU);
/// \defgroup FindRootDIEs Find DIEs corresponding to Address map entries.
///
/// @{

6
llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
View File

@@ -74,6 +74,12 @@ public:
/// Does DIE transitively refer an incomplete decl?
bool Incomplete : 1;
/// Is DIE in the clang module scope?
bool InModuleScope : 1;
/// Is ODR marking done?
bool ODRMarkingDone : 1;
};
CompileUnit(DWARFUnit &OrigUnit, unsigned ID, bool CanUseODR,

8
llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h
View File

@@ -18,6 +18,7 @@
#include "llvm/DebugInfo/DWARF/DWARFDie.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include <atomic>
namespace llvm {
@@ -91,6 +92,10 @@ public:
bool setLastSeenDIE(CompileUnit &U, const DWARFDie &Die);
void setHasCanonicalDIE() { HasCanonicalDIE = true; }
bool hasCanonicalDIE() const { return HasCanonicalDIE; }
uint32_t getCanonicalDIEOffset() const { return CanonicalDIEOffset; }
void setCanonicalDIEOffset(uint32_t Offset) { CanonicalDIEOffset = Offset; }
@@ -112,7 +117,8 @@ private:
const DeclContext &Parent;
DWARFDie LastSeenDIE;
uint32_t LastSeenCompileUnitID = 0;
uint32_t CanonicalDIEOffset = 0;
std::atomic<uint32_t> CanonicalDIEOffset = {0};
bool HasCanonicalDIE = false;
};
/// This class gives a tree-like API to the DenseMap that stores the

82
llvm/lib/DWARFLinker/DWARFLinker.cpp
View File

@@ -361,16 +361,16 @@ static bool analyzeContextInfo(
}
Info.ParentIdx = Current.ParentIdx;
bool InClangModule = CU.isClangModule() || Current.InImportedModule;
if (CU.hasODR() || InClangModule) {
Info.InModuleScope = CU.isClangModule() || Current.InImportedModule;
if (CU.hasODR() || Info.InModuleScope) {
if (Current.Context) {
auto PtrInvalidPair = Contexts.getChildDeclContext(
*Current.Context, Current.Die, CU, InClangModule);
*Current.Context, Current.Die, CU, Info.InModuleScope);
Current.Context = PtrInvalidPair.getPointer();
Info.Ctxt =
PtrInvalidPair.getInt() ? nullptr : PtrInvalidPair.getPointer();
if (Info.Ctxt)
Info.Ctxt->setDefinedInClangModule(InClangModule);
Info.Ctxt->setDefinedInClangModule(Info.InModuleScope);
} else
Info.Ctxt = Current.Context = nullptr;
}
@@ -616,6 +616,27 @@ void DWARFLinker::lookForChildDIEsToKeep(
}
}
static bool isODRCanonicalCandidate(const DWARFDie &Die, CompileUnit &CU) {
CompileUnit::DIEInfo &Info = CU.getInfo(Die);
if (!Info.Ctxt || (Die.getTag() == dwarf::DW_TAG_namespace))
return false;
if (!CU.hasODR() && !Info.InModuleScope)
return false;
return !Info.Incomplete && Info.Ctxt != CU.getInfo(Info.ParentIdx).Ctxt;
}
void DWARFLinker::markODRCanonicalDie(const DWARFDie &Die, CompileUnit &CU) {
CompileUnit::DIEInfo &Info = CU.getInfo(Die);
Info.ODRMarkingDone = true;
if (Info.Keep && isODRCanonicalCandidate(Die, CU) &&
!Info.Ctxt->hasCanonicalDIE())
Info.Ctxt->setHasCanonicalDIE();
}
/// Look at DIEs referenced by the given DIE and decide whether they should be
/// kept. All DIEs referenced though attributes should be kept.
void DWARFLinker::lookForRefDIEsToKeep(
@@ -645,8 +666,6 @@ void DWARFLinker::lookForRefDIEsToKeep(
if (auto RefDie =
resolveDIEReference(File, Units, Val, Die, ReferencedCU)) {
CompileUnit::DIEInfo &Info = ReferencedCU->getInfo(RefDie);
bool IsModuleRef = Info.Ctxt && Info.Ctxt->getCanonicalDIEOffset() &&
Info.Ctxt->isDefinedInClangModule();
// If the referenced DIE has a DeclContext that has already been
// emitted, then do not keep the one in this CU. We'll link to
// the canonical DIE in cloneDieReferenceAttribute.
@@ -657,15 +676,14 @@ void DWARFLinker::lookForRefDIEsToKeep(
//
// FIXME: compatibility with dsymutil-classic. There is no
// reason not to unique ref_addr references.
if (AttrSpec.Form != dwarf::DW_FORM_ref_addr && (UseOdr || IsModuleRef) &&
Info.Ctxt &&
Info.Ctxt != ReferencedCU->getInfo(Info.ParentIdx).Ctxt &&
Info.Ctxt->getCanonicalDIEOffset() && isODRAttribute(AttrSpec.Attr))
if (AttrSpec.Form != dwarf::DW_FORM_ref_addr &&
isODRAttribute(AttrSpec.Attr) && Info.Ctxt &&
Info.Ctxt->hasCanonicalDIE())
continue;
// Keep a module forward declaration if there is no definition.
if (!(isODRAttribute(AttrSpec.Attr) && Info.Ctxt &&
Info.Ctxt->getCanonicalDIEOffset()))
Info.Ctxt->hasCanonicalDIE()))
Info.Prune = false;
ReferencedDIEs.emplace_back(RefDie, *ReferencedCU);
}
@@ -756,6 +774,9 @@ void DWARFLinker::lookForDIEsToKeep(AddressesMap &AddressesMap,
lookForParentDIEsToKeep(Current.AncestorIdx, Current.CU, Current.Flags,
Worklist);
continue;
case WorklistItemType::MarkODRCanonicalDie:
markODRCanonicalDie(Current.Die, Current.CU);
continue;
case WorklistItemType::LookForDIEsToKeep:
break;
}
@@ -778,6 +799,16 @@ void DWARFLinker::lookForDIEsToKeep(AddressesMap &AddressesMap,
Current.Flags = shouldKeepDIE(AddressesMap, Ranges, Current.Die, File,
Current.CU, MyInfo, Current.Flags);
// We need to mark context for the canonical die in the end of normal
// traversing(not TF_DependencyWalk) or after normal traversing if die
// was not marked as kept.
if (!(Current.Flags & TF_DependencyWalk) ||
(MyInfo.ODRMarkingDone && !MyInfo.Keep)) {
if (Current.CU.hasODR() || MyInfo.InModuleScope)
Worklist.emplace_back(Current.Die, Current.CU,
WorklistItemType::MarkODRCanonicalDie);
}
// Finish by looking for child DIEs. Because of the LIFO worklist we need
// to schedule that work before any subsequent items are added to the
// worklist.
@@ -845,7 +876,7 @@ void DWARFLinker::assignAbbrev(DIEAbbrev &Abbrev) {
unsigned DWARFLinker::DIECloner::cloneStringAttribute(
DIE &Die, AttributeSpec AttrSpec, const DWARFFormValue &Val,
const DWARFUnit &U, OffsetsStringPool &StringPool, AttributesInfo &Info) {
const DWARFUnit &, OffsetsStringPool &StringPool, AttributesInfo &Info) {
Optional<const char *> String = dwarf::toString(Val);
if (!String)
return 0;
@@ -875,7 +906,6 @@ unsigned DWARFLinker::DIECloner::cloneDieReferenceAttribute(
DIE *NewRefDie = nullptr;
CompileUnit *RefUnit = nullptr;
DeclContext *Ctxt = nullptr;
DWARFDie RefDie =
Linker.resolveDIEReference(File, CompileUnits, Val, InputDIE, RefUnit);
@@ -888,14 +918,14 @@ unsigned DWARFLinker::DIECloner::cloneDieReferenceAttribute(
// If we already have emitted an equivalent DeclContext, just point
// at it.
if (isODRAttribute(AttrSpec.Attr)) {
Ctxt = RefInfo.Ctxt;
if (Ctxt && Ctxt->getCanonicalDIEOffset()) {
DIEInteger Attr(Ctxt->getCanonicalDIEOffset());
Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::DW_FORM_ref_addr, Attr);
return U.getRefAddrByteSize();
}
if (isODRAttribute(AttrSpec.Attr) && RefInfo.Ctxt &&
RefInfo.Ctxt->getCanonicalDIEOffset()) {
assert(RefInfo.Ctxt->hasCanonicalDIE() &&
"Offset to canonical die is set, but context is not marked");
DIEInteger Attr(RefInfo.Ctxt->getCanonicalDIEOffset());
Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::DW_FORM_ref_addr, Attr);
return U.getRefAddrByteSize();
}
if (!RefInfo.Clone) {
@@ -925,7 +955,7 @@ unsigned DWARFLinker::DIECloner::cloneDieReferenceAttribute(
// A forward reference. Note and fixup later.
Attr = 0xBADDEF;
Unit.noteForwardReference(
NewRefDie, RefUnit, Ctxt,
NewRefDie, RefUnit, RefInfo.Ctxt,
Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::DW_FORM_ref_addr, DIEInteger(Attr)));
}
@@ -1356,10 +1386,10 @@ DIE *DWARFLinker::DIECloner::cloneDIE(const DWARFDie &InputDIE,
assert(Die->getTag() == InputDIE.getTag());
Die->setOffset(OutOffset);
if ((Unit.hasODR() || Unit.isClangModule()) && !Info.Incomplete &&
Die->getTag() != dwarf::DW_TAG_namespace && Info.Ctxt &&
Info.Ctxt != Unit.getInfo(Info.ParentIdx).Ctxt &&
!Info.Ctxt->getCanonicalDIEOffset()) {
if (isODRCanonicalCandidate(InputDIE, Unit) && Info.Ctxt &&
(Info.Ctxt->getCanonicalDIEOffset() == 0)) {
if (!Info.Ctxt->hasCanonicalDIE())
Info.Ctxt->setHasCanonicalDIE();
// We are about to emit a DIE that is the root of its own valid
// DeclContext tree. Make the current offset the canonical offset
// for this context.

6
llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
View File

@@ -90,9 +90,11 @@ void CompileUnit::fixupForwardReferences() {
PatchLocation Attr;
DeclContext *Ctxt;
std::tie(RefDie, RefUnit, Ctxt, Attr) = Ref;
if (Ctxt && Ctxt->getCanonicalDIEOffset())
if (Ctxt && Ctxt->hasCanonicalDIE()) {
assert(Ctxt->getCanonicalDIEOffset() &&
"Canonical die offset is not set");
Attr.set(Ctxt->getCanonicalDIEOffset());
else
} else
Attr.set(RefDie->getOffset() + RefUnit->getStartOffset());
}
}

200
llvm/test/tools/dsymutil/X86/odr-two-units-in-single-file.test Normal file
View File

@@ -0,0 +1,200 @@
## This test checks debug info for the types located into the
## different compilation units from the single object file.
## Type definition for the "class1" should be removed from the
## second compilation unit.
# RUN: yaml2obj %s -o %t.o
# RUN: echo '---' > %t2.map
# RUN: echo "triple: 'x86_64-apple-darwin'" >> %t2.map
# RUN: echo 'objects:' >> %t2.map
# RUN: echo " - filename: '%t.o'" >> %t2.map
# RUN: echo ' symbols:' >> %t2.map
# RUN: echo ' - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10000, size: 0x10 }' >> %t2.map
# RUN: echo '...' >> %t2.map
# RUN: dsymutil -y %t2.map -f -o - | llvm-dwarfdump -a - | FileCheck %s
# CHECK: file format Mach-O 64-bit x86-64
# CHECK: .debug_info contents:
# CHECK: Compile Unit:
# CHECK: DW_TAG_compile_unit
# CHECK: DW_AT_name{{.*}}"CU1"
# CHECK: DW_TAG_class_type{{.*[[:space:]].*}}DW_AT_name{{.*}}"class1"
# CHECK: DW_TAG_variable
# CHECK: DW_AT_name{{.*}}"var1"
# CHECK: DW_AT_const_value
# CHECK: DW_AT_type (0x00000000[[CLASS1:[0-9a-f]*]]
# CHECK: Compile Unit:
# CHECK: DW_TAG_compile_unit
# CHECK: DW_AT_name{{.*}}"CU2"
# CHECK-NOT: DW_TAG_class_type
# CHECK-NOT: "class1"
# CHECK: DW_TAG_variable
# CHECK: DW_AT_name{{.*}}"var2"
# CHECK: DW_AT_const_value
# CHECK: DW_AT_type (0x00000000[[CLASS1]]
--- !mach-o
FileHeader:
magic: 0xFEEDFACF
cputype: 0x01000007
cpusubtype: 0x00000003
filetype: 0x00000001
ncmds: 2
sizeofcmds: 376
flags: 0x00002000
reserved: 0x00000000
LoadCommands:
- cmd: LC_SEGMENT_64
cmdsize: 232
segname: ''
vmaddr: 0x00
vmsize: 0x300
fileoff: 0x300
filesize: 0x300
maxprot: 7
initprot: 7
nsects: 2
flags: 0
Sections:
- sectname: __debug_abbrev
segname: __DWARF
addr: 0x000000000000000F
size: 0x3c
offset: 0x00000380
align: 0
reloff: 0x00000000
nreloc: 0
flags: 0x02000000
reserved1: 0x00000000
reserved2: 0x00000000
reserved3: 0x00000000
- sectname: __debug_info
segname: __DWARF
addr: 0x000000000000100
size: 0x62
offset: 0x00000410
align: 0
reloff: 0x00000600
nreloc: 1
flags: 0x02000000
reserved1: 0x00000000
reserved2: 0x00000000
reserved3: 0x00000000
relocations:
- address: 0x1FC
symbolnum: 1
pcrel: true
length: 3
extern: true
type: 0
scattered: false
value: 0
- cmd: LC_SYMTAB
cmdsize: 24
symoff: 0x700
nsyms: 2
stroff: 0x720
strsize: 10
LinkEditData:
NameList:
- n_strx: 1
n_type: 0x0F
n_sect: 1
n_desc: 0
n_value: 0
- n_strx: 1
n_type: 0x0F
n_sect: 1
n_desc: 0
n_value: 0
StringTable:
- ''
- '__Z3foov'
- ''
DWARF:
debug_abbrev:
- Table:
- Tag: DW_TAG_compile_unit
Children: DW_CHILDREN_yes
Attributes:
- Attribute: DW_AT_producer
Form: DW_FORM_string
- Attribute: DW_AT_language
Form: DW_FORM_data2
- Attribute: DW_AT_name
Form: DW_FORM_string
- Tag: DW_TAG_class_type
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_name
Form: DW_FORM_string
- Tag: DW_TAG_variable
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_name
Form: DW_FORM_string
- Attribute: DW_AT_const_value
Form: DW_FORM_data4
- Attribute: DW_AT_type
Form: DW_FORM_ref4
- Table:
- Tag: DW_TAG_compile_unit
Children: DW_CHILDREN_yes
Attributes:
- Attribute: DW_AT_producer
Form: DW_FORM_string
- Attribute: DW_AT_language
Form: DW_FORM_data2
- Attribute: DW_AT_name
Form: DW_FORM_string
- Tag: DW_TAG_class_type
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_name
Form: DW_FORM_string
- Tag: DW_TAG_variable
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_name
Form: DW_FORM_string
- Attribute: DW_AT_const_value
Form: DW_FORM_data4
- Attribute: DW_AT_type
Form: DW_FORM_ref4
debug_info:
- Version: 4
Entries:
- AbbrCode: 1
Values:
- CStr: by_hand
- Value: 0x04
- CStr: CU1
- AbbrCode: 2
Values:
- CStr: class1
- AbbrCode: 3
Values:
- CStr: var1
- Value: 0x00000000
- Value: 0x0000001a
- AbbrCode: 0
- Version: 4
Entries:
- AbbrCode: 1
Values:
- CStr: by_hand
- Value: 0x04
- CStr: CU2
- AbbrCode: 2
Values:
- CStr: class1
- AbbrCode: 3
Values:
- CStr: var2
- Value: 0x00000000
- Value: 0x0000001a
- AbbrCode: 0
...