This is an archive of the discontinued LLVM Phabricator instance.

Differential D151348

[DWARFLinker] Set size of location expression of DW_FORM_block1 properly.
ClosedPublic

Authored by avl on May 24 2023, 9:59 AM.

Details

Reviewers
JDevlieghere
aprantl
Commits
rGce2631d14727: [DWARFLinker] Set size of location expression of DW_FORM_block1 properly.
Summary

This patch fixes the problem introduced by D147066. As D147066 may change
the contents of location expression, it started to calculate final attribute
size. This patch uses more correct way to calculate size: DIEValue::sizeOf().

Diff Detail

Event Timeline

avl created this revision.May 24 2023, 9:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2023, 9:59 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
avl requested review of this revision.May 24 2023, 9:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2023, 9:59 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B234246: Diff 525234.May 24 2023, 11:08 AM
avl added a comment.May 24 2023, 2:10 PM

the github issue https://github.com/llvm/llvm-project/issues/62874

jryans added a project: debug-info.May 24 2023, 4:36 PM
JDevlieghere accepted this revision.May 24 2023, 4:39 PM

LGTM

llvm/lib/DWARFLinker/DWARFLinker.cpp
1313–1315
This revision is now accepted and ready to land.May 24 2023, 4:39 PM
avl updated this revision to Diff 525489.May 25 2023, 2:07 AM

rebased.

Harbormaster completed remote builds in B234428: Diff 525489.May 25 2023, 3:31 AM
avl updated this revision to Diff 525532.May 25 2023, 3:58 AM

rebased.

Harbormaster completed remote builds in B234452: Diff 525532.May 25 2023, 5:03 AM
avl added a comment.May 25 2023, 5:14 AM

Thank you for the review!

Closed by commit rGce2631d14727: [DWARFLinker] Set size of location expression of DW_FORM_block1 properly. (authored by avl). · Explain WhyMay 25 2023, 5:15 AM
This revision was automatically updated to reflect the committed changes.
avl added a commit: rGce2631d14727: [DWARFLinker] Set size of location expression of DW_FORM_block1 properly..

Revision Contents

PathSize
llvm/
lib/
DWARFLinker/
29 lines
test/
tools/
dsymutil/
X86/
458 lines

Diff 525553

llvm/lib/DWARFLinker/DWARFLinker.cpp

Show First 20 Lines • Show All 1,272 Lines • ▼ Show 20 Lines if (AttrSpec.Form == dwarf::DW_FORM_exprloc) {
Linker.DIELocs.push_back(Loc); Linker.DIELocs.push_back(Loc);
} else { } else {
Block = new (DIEAlloc) DIEBlock; Block = new (DIEAlloc) DIEBlock;
Linker.DIEBlocks.push_back(Block); Linker.DIEBlocks.push_back(Block);
} }
Attr = Loc ? static_cast<DIEValueList *>(Loc) Attr = Loc ? static_cast<DIEValueList *>(Loc)
: static_cast<DIEValueList *>(Block); : static_cast<DIEValueList *>(Block);
if (Loc) DWARFUnit &OrigUnit = Unit.getOrigUnit();
Value = DIEValue(dwarf::Attribute(AttrSpec.Attr),
dwarf::Form(AttrSpec.Form), Loc);
else
Value = DIEValue(dwarf::Attribute(AttrSpec.Attr),
dwarf::Form(AttrSpec.Form), Block);
// If the block is a DWARF Expression, clone it into the temporary // If the block is a DWARF Expression, clone it into the temporary
// buffer using cloneExpression(), otherwise copy the data directly. // buffer using cloneExpression(), otherwise copy the data directly.
SmallVector<uint8_t, 32> Buffer; SmallVector<uint8_t, 32> Buffer;
ArrayRef<uint8_t> Bytes = *Val.getAsBlock(); ArrayRef<uint8_t> Bytes = *Val.getAsBlock();
if (DWARFAttribute::mayHaveLocationExpr(AttrSpec.Attr) && if (DWARFAttribute::mayHaveLocationExpr(AttrSpec.Attr) &&
(Val.isFormClass(DWARFFormValue::FC_Block) || (Val.isFormClass(DWARFFormValue::FC_Block) ||
Val.isFormClass(DWARFFormValue::FC_Exprloc))) { Val.isFormClass(DWARFFormValue::FC_Exprloc))) {
DWARFUnit &OrigUnit = Unit.getOrigUnit();
DataExtractor Data(StringRef((const char *)Bytes.data(), Bytes.size()), DataExtractor Data(StringRef((const char *)Bytes.data(), Bytes.size()),
IsLittleEndian, OrigUnit.getAddressByteSize()); IsLittleEndian, OrigUnit.getAddressByteSize());
DWARFExpression Expr(Data, OrigUnit.getAddressByteSize(), DWARFExpression Expr(Data, OrigUnit.getAddressByteSize(),
OrigUnit.getFormParams().Format); OrigUnit.getFormParams().Format);
cloneExpression(Data, Expr, File, Unit, Buffer, cloneExpression(Data, Expr, File, Unit, Buffer,
Unit.getInfo(InputDIE).AddrAdjust, IsLittleEndian); Unit.getInfo(InputDIE).AddrAdjust, IsLittleEndian);
Bytes = Buffer; Bytes = Buffer;
} }
for (auto Byte : Bytes) for (auto Byte : Bytes)
Attr->addValue(DIEAlloc, static_cast<dwarf::Attribute>(0), Attr->addValue(DIEAlloc, static_cast<dwarf::Attribute>(0),
dwarf::DW_FORM_data1, DIEInteger(Byte)); dwarf::DW_FORM_data1, DIEInteger(Byte));
// FIXME: If DIEBlock and DIELoc just reuses the Size field of // FIXME: If DIEBlock and DIELoc just reuses the Size field of
// the DIE class, this "if" could be replaced by // the DIE class, this "if" could be replaced by
// Attr->setSize(Bytes.size()). // Attr->setSize(Bytes.size()).
if (Loc) if (Loc)
Loc->setSize(Bytes.size()); Loc->setSize(Bytes.size());
else else
Block->setSize(Bytes.size()); Block->setSize(Bytes.size());
Die.addValue(DIEAlloc, Value); if (Loc)
return getULEB128Size(Bytes.size()) + Bytes.size(); Value = DIEValue(dwarf::Attribute(AttrSpec.Attr),
dwarf::Form(AttrSpec.Form), Loc);
else {
// The expression location data might be updated and exceed the original
// size. Check whether the new data fits into the original form.
if ((AttrSpec.Form == dwarf::DW_FORM_block1 &&
JDevlieghereUnsubmitted
Not Done
ReplyInline Actions
else {
- // Expression location data might be updated so that it may become longer.
- // Original form may be insufficient to keep new data. Check whether new
- // size fits into the original form
+ // The expression location data might be updated and exceed the original size.
+ // Check whether the new data fits into the original form.
if ((AttrSpec.Form == dwarf::DW_FORM_block1 &&
JDevlieghere:
(Bytes.size() > UINT8_MAX)) ||
(AttrSpec.Form == dwarf::DW_FORM_block2 &&
(Bytes.size() > UINT16_MAX)) ||
(AttrSpec.Form == dwarf::DW_FORM_block4 && (Bytes.size() > UINT32_MAX)))
AttrSpec.Form = dwarf::DW_FORM_block;
Value = DIEValue(dwarf::Attribute(AttrSpec.Attr),
dwarf::Form(AttrSpec.Form), Block);
}
return Die.addValue(DIEAlloc, Value)->sizeOf(OrigUnit.getFormParams());
} }
unsigned DWARFLinker::DIECloner::cloneAddressAttribute( unsigned DWARFLinker::DIECloner::cloneAddressAttribute(
DIE &Die, const DWARFDie &InputDIE, AttributeSpec AttrSpec, DIE &Die, const DWARFDie &InputDIE, AttributeSpec AttrSpec,
unsigned AttrSize, const DWARFFormValue &Val, const CompileUnit &Unit, unsigned AttrSize, const DWARFFormValue &Val, const CompileUnit &Unit,
AttributesInfo &Info) { AttributesInfo &Info) {
if (AttrSpec.Attr == dwarf::DW_AT_low_pc) if (AttrSpec.Attr == dwarf::DW_AT_low_pc)
Info.HasLowPc = true; Info.HasLowPc = true;
▲ Show 20 Lines • Show All 1,726 Lines • Show Last 20 Lines

llvm/test/tools/dsymutil/X86/location-expression.test

  • This file was added.
## This test checks that size of DW_AT_location attribute
## of the DW_FORM_block1 form is correctly written to the
## output.
# 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 --verbose - | 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: 0x0000001b: DW_TAG_variable
# CHECK: DW_AT_name {{.*}}"var1"
# CHECK: DW_AT_type {{.*}}"class1"
# CHECK: DW_AT_location [DW_FORM_block1] (DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address)
# CHECK: 0x000000ab: DW_TAG_variable
# CHECK: DW_AT_name {{.*}}"var2"
# CHECK: DW_AT_type {{.*}}"class1"
# CHECK: DW_AT_location [DW_FORM_block] (DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address, DW_OP_const8u 0x2000, DW_OP_form_tls_address)
# CHECK: 0x00000146: DW_TAG_variable
# CHECK: DW_AT_name {{.*}}"var3"
# CHECK: DW_AT_type {{.*}}"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: 0x38
offset: 0x00000380
align: 0
reloff: 0x00000000
nreloc: 0
flags: 0x02000000
reserved1: 0x00000000
reserved2: 0x00000000
reserved3: 0x00000000
- sectname: __debug_info
segname: __DWARF
addr: 0x000000000000100
size: 0x15e
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
- Attribute: DW_AT_location
Form: DW_FORM_block1
- 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
- Attribute: DW_AT_location
Form: DW_FORM_block
- 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
- BlockData:
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- AbbrCode: 4
Values:
- CStr: var2
- Value: 0x00000000
- Value: 0x0000001a
- BlockData:
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- 0x0e
- 0x00
- 0x20
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x00
- 0x9b
- AbbrCode: 5
Values:
- CStr: var3
- Value: 0x00000000
- Value: 0x0000001a
- AbbrCode: 0
...