This is an archive of the discontinued LLVM Phabricator instance.

Differential D85079

[lld-macho] Ensure .tbss sections are also considered as ZeroFilled
ClosedPublic

Authored by int3 on Aug 1 2020, 3:21 PM.

Details

Reviewers
compnerd
MaskRay
Group Reviewers
Restricted Project
Commits
rG4e43f1804817: [lld-macho] Ensure .tbss sections are also considered as ZeroFilled
Summary

This diff makes the behavior in D80859: [lld-macho] Ensure __bss sections we output have file offset of zero and D81888: [lld-macho] Make sure ZeroFill sections are at the end of their segments apply to
thread-local ZeroFill sections too. I realized this was necessary whie
trying to implement thread-local variables.

Diff Detail

Event Timeline

int3 created this revision.Aug 1 2020, 3:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2020, 3:21 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
int3 requested review of this revision.Aug 1 2020, 3:21 PM
Harbormaster completed remote builds in B66672: Diff 282417.Aug 1 2020, 3:35 PM
compnerd accepted this revision.Aug 3 2020, 9:24 AM
This revision is now accepted and ready to land.Aug 3 2020, 9:24 AM
MaskRay accepted this revision.Aug 3 2020, 9:30 AM
MaskRay added a subscriber: MaskRay.

Looks great!

lld/MachO/InputSection.h
39–40

If you using namespace llvm then you can omit llvm::

int3 added inline comments.Aug 3 2020, 10:12 AM
lld/MachO/InputSection.h
39–40

I think we should avoid using namespace inside header files...

MaskRay added inline comments.Aug 3 2020, 10:19 AM
lld/MachO/InputSection.h
39–40

You're right. Did not notice this is a .h......... my bad

This revision was landed with ongoing or failed builds.Aug 7 2020, 11:05 AM
Closed by commit rG4e43f1804817: [lld-macho] Ensure .tbss sections are also considered as ZeroFilled (authored by int3). · Explain Why
This revision was automatically updated to reflect the committed changes.
int3 added a commit: rG4e43f1804817: [lld-macho] Ensure .tbss sections are also considered as ZeroFilled.

Revision Contents

PathSize
lld/
MachO/
3 lines
test/
MachO/
23 lines

Diff 283954

lld/MachO/InputSection.h

Show All 30 Linesstruct Reloc {
uint32_t offset; uint32_t offset;
// Adding this offset to the address of the target symbol or subsection gives // Adding this offset to the address of the target symbol or subsection gives
// the destination that this relocation refers to. // the destination that this relocation refers to.
uint64_t addend; uint64_t addend;
llvm::PointerUnion<Symbol *, InputSection *> target; llvm::PointerUnion<Symbol *, InputSection *> target;
}; };
inline bool isZeroFill(uint8_t flags) { inline bool isZeroFill(uint8_t flags) {
return (flags & llvm::MachO::SECTION_TYPE) == llvm::MachO::S_ZEROFILL; return llvm::MachO::isVirtualSection(flags & llvm::MachO::SECTION_TYPE);
}
MaskRayUnsubmitted
Not Done
ReplyInline Actions

If you using namespace llvm then you can omit llvm::

MaskRay: If you `using namespace llvm` then you can omit `llvm::`
int3AuthorUnsubmitted
Done
ReplyInline Actions

I think we should avoid using namespace inside header files...

int3: I think we should avoid `using namespace` inside header files...
MaskRayUnsubmitted
Not Done
ReplyInline Actions

You're right. Did not notice this is a .h......... my bad

MaskRay: You're right. Did not notice this is a .h......... my bad
} }
class InputSection { class InputSection {
public: public:
virtual ~InputSection() = default; virtual ~InputSection() = default;
virtual uint64_t getSize() const { return data.size(); } virtual uint64_t getSize() const { return data.size(); }
virtual uint64_t getFileSize() const { virtual uint64_t getFileSize() const {
return isZeroFill(flags) ? 0 : getSize(); return isZeroFill(flags) ? 0 : getSize();
Show All 27 Lines

lld/test/MachO/bss.s

Show All 20 Lines
# CHECK-NEXT: Reserved1: 0x0 # CHECK-NEXT: Reserved1: 0x0
# CHECK-NEXT: Reserved2: 0x0 # CHECK-NEXT: Reserved2: 0x0
# CHECK-NEXT: Reserved3: 0x0 # CHECK-NEXT: Reserved3: 0x0
# CHECK: Index: 2 # CHECK: Index: 2
# CHECK-NEXT: Name: __bss # CHECK-NEXT: Name: __bss
# CHECK-NEXT: Segment: __DATA # CHECK-NEXT: Segment: __DATA
# CHECK-NEXT: Address: # CHECK-NEXT: Address:
# CHECK-NEXT: Size: 0x4 # CHECK-NEXT: Size: 0x8
# CHECK-NEXT: Offset: 0 # CHECK-NEXT: Offset: 0
# CHECK-NEXT: Alignment: 0 # CHECK-NEXT: Alignment: 0
# CHECK-NEXT: RelocationOffset: 0x0 # CHECK-NEXT: RelocationOffset: 0x0
# CHECK-NEXT: RelocationCount: 0 # CHECK-NEXT: RelocationCount: 0
# CHECK-NEXT: Type: ZeroFill (0x1) # CHECK-NEXT: Type: ZeroFill (0x1)
# CHECK-NEXT: Attributes [ (0x0) # CHECK-NEXT: Attributes [ (0x0)
# CHECK-NEXT: ] # CHECK-NEXT: ]
# CHECK-NEXT: Reserved1: 0x0 # CHECK-NEXT: Reserved1: 0x0
# CHECK-NEXT: Reserved2: 0x0 # CHECK-NEXT: Reserved2: 0x0
# CHECK-NEXT: Reserved3: 0x0 # CHECK-NEXT: Reserved3: 0x0
# CHECK: Index: 3
# CHECK-NEXT: Name: __thread_bss
# CHECK-NEXT: Segment: __DATA
# CHECK-NEXT: Address: 0x100001010
# CHECK-NEXT: Size: 0x4
# CHECK-NEXT: Offset: 0
# CHECK-NEXT: Alignment: 0
# CHECK-NEXT: RelocationOffset: 0x0
# CHECK-NEXT: RelocationCount: 0
# CHECK-NEXT: Type: ThreadLocalZerofill (0x12)
# CHECK-NEXT: Attributes [ (0x0)
# CHECK-NEXT: ]
# CHECK-NEXT: Reserved1: 0x0
# CHECK-NEXT: Reserved2: 0x0
# CHECK-NEXT: Reserved3: 0x0
# CHECK: Name: __DATA # CHECK: Name: __DATA
# CHECK-NEXT: Size: # CHECK-NEXT: Size:
# CHECK-NEXT: vmaddr: # CHECK-NEXT: vmaddr:
# CHECK-NEXT: vmsize: 0xC # CHECK-NEXT: vmsize: 0x14
# CHECK-NEXT: fileoff: # CHECK-NEXT: fileoff:
# CHECK-NEXT: filesize: 8 # CHECK-NEXT: filesize: 8
.globl _main .globl _main
.text .text
_main: _main:
movq $0, %rax movq $0, %rax
retq retq
.bss .bss
.zero 4 .zero 4
.tbss _foo, 4
.zero 4
.data .data
.quad 0x1234 .quad 0x1234