Skip to content

Commit 673e542

Browse files
authored
MC: Fix fragment-in-BSS check
* Handle non-zero fill values for `.fill` and `.org` directives. * Restore the fragment type check (5ee34ff removed a reachable `llvm_unreachable`) to detect unintended API usage. Remove virtual functions `getVirtualSectionKind` (added in https://reviews.llvm.org/D78138) as they are unnecessary in diagnostics. The a.out object file format has the BSS concept, which has been inherited by COFF, XCOFF, Mach-O, and ELF object file formats. Pull Request: #149721
1 parent ba6b705 commit 673e542

File tree

10 files changed

+61
-35
lines changed

10 files changed

+61
-35
lines changed

llvm/include/llvm/MC/MCSection.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class LLVM_ABI MCSection {
112112
StringRef Name;
113113
SectionVariant Variant;
114114

115-
MCSection(SectionVariant V, StringRef Name, bool IsText, bool IsVirtual,
115+
MCSection(SectionVariant V, StringRef Name, bool IsText, bool IsBss,
116116
MCSymbol *Begin);
117117
// Protected non-virtual dtor prevents destroy through a base class pointer.
118118
~MCSection() {}
@@ -175,11 +175,9 @@ class LLVM_ABI MCSection {
175175
/// instead of 0s.
176176
virtual bool useCodeAlign() const = 0;
177177

178-
/// Return true if this is a BSS section (e.g., ELF .bss or .tbss) that does
179-
/// not store content and is typically initialized to zeroes by the runtime.
178+
/// Check whether this section is "virtual", that is has no actual object
179+
/// file contents.
180180
bool isBssSection() const { return IsBss; }
181-
182-
virtual StringRef getVirtualSectionKind() const;
183181
};
184182

185183
// Represents a contiguous piece of code or data within a section. Its size is

llvm/include/llvm/MC/MCSectionCOFF.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ class MCSectionCOFF final : public MCSection {
8282
raw_ostream &OS,
8383
uint32_t Subsection) const override;
8484
bool useCodeAlign() const override;
85-
StringRef getVirtualSectionKind() const override;
8685

8786
unsigned getOrAssignWinCFISectionID(unsigned *NextID) const {
8887
if (WinCFISectionID == ~0U)

llvm/include/llvm/MC/MCSectionELF.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ class MCSectionELF final : public MCSection {
8888
raw_ostream &OS,
8989
uint32_t Subsection) const override;
9090
bool useCodeAlign() const override;
91-
StringRef getVirtualSectionKind() const override;
9291

9392
bool isUnique() const { return UniqueID != NonUniqueID; }
9493
unsigned getUniqueID() const { return UniqueID; }

llvm/lib/MC/MCAssembler.cpp

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -567,15 +567,37 @@ void MCAssembler::writeSectionData(raw_ostream &OS,
567567
// not tracked for efficiency.
568568
auto Fn = [](char c) { return c != 0; };
569569
for (const MCFragment &F : *Sec) {
570-
if (any_of(F.getContents(), Fn) || any_of(F.getVarContents(), Fn)) {
571-
reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
572-
Sec->getName() +
570+
bool HasNonZero = false;
571+
switch (F.getKind()) {
572+
default:
573+
reportFatalInternalError("BSS section '" + Sec->getName() +
574+
"' contains invalid fragment");
575+
break;
576+
case MCFragment::FT_Data:
577+
case MCFragment::FT_Relaxable:
578+
HasNonZero =
579+
any_of(F.getContents(), Fn) || any_of(F.getVarContents(), Fn);
580+
break;
581+
case MCFragment::FT_Align:
582+
// Disallowed for API usage. AsmParser changes non-zero fill values to
583+
// 0.
584+
assert(F.getAlignFill() == 0 && "Invalid align in virtual section!");
585+
break;
586+
case MCFragment::FT_Fill:
587+
HasNonZero = cast<MCFillFragment>(F).getValue() != 0;
588+
break;
589+
case MCFragment::FT_Org:
590+
HasNonZero = cast<MCOrgFragment>(F).getValue() != 0;
591+
break;
592+
}
593+
if (HasNonZero) {
594+
reportError(SMLoc(), "BSS section '" + Sec->getName() +
573595
"' cannot have non-zero bytes");
574596
break;
575597
}
576598
if (F.getFixups().size() || F.getVarFixups().size()) {
577-
reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
578-
Sec->getName() + "' cannot have fixups");
599+
reportError(SMLoc(),
600+
"BSS section '" + Sec->getName() + "' cannot have fixups");
579601
break;
580602
}
581603
}

llvm/lib/MC/MCParser/AsmParser.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3406,9 +3406,8 @@ bool AsmParser::parseDirectiveAlign(bool IsPow2, uint8_t ValueSize) {
34063406

34073407
if (HasFillExpr && FillExpr != 0 && Section->isBssSection()) {
34083408
ReturnVal |=
3409-
Warning(FillExprLoc, "ignoring non-zero fill value in " +
3410-
Section->getVirtualSectionKind() +
3411-
" section '" + Section->getName() + "'");
3409+
Warning(FillExprLoc, "ignoring non-zero fill value in BSS section '" +
3410+
Section->getName() + "'");
34123411
FillExpr = 0;
34133412
}
34143413

llvm/lib/MC/MCSection.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@
1818

1919
using namespace llvm;
2020

21-
MCSection::MCSection(SectionVariant V, StringRef Name, bool IsText,
22-
bool IsVirtual, MCSymbol *Begin)
21+
MCSection::MCSection(SectionVariant V, StringRef Name, bool IsText, bool IsBss,
22+
MCSymbol *Begin)
2323
: Begin(Begin), HasInstructions(false), IsRegistered(false), IsText(IsText),
24-
IsBss(IsVirtual), LinkerRelaxable(false), Name(Name), Variant(V) {
24+
IsBss(IsBss), LinkerRelaxable(false), Name(Name), Variant(V) {
2525
// The initial subsection number is 0. Create a fragment list.
2626
CurFragList = &Subsections.emplace_back(0u, FragList{}).second;
2727
}
@@ -34,8 +34,6 @@ MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
3434

3535
bool MCSection::hasEnded() const { return End && End->isInSection(); }
3636

37-
StringRef MCSection::getVirtualSectionKind() const { return "virtual"; }
38-
3937
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
4038
LLVM_DUMP_METHOD void MCSection::dump(
4139
DenseMap<const MCFragment *, SmallVector<const MCSymbol *, 0>> *FragToSyms)

llvm/lib/MC/MCSectionCOFF.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,3 @@ void MCSectionCOFF::printSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
115115
}
116116

117117
bool MCSectionCOFF::useCodeAlign() const { return isText(); }
118-
119-
StringRef MCSectionCOFF::getVirtualSectionKind() const {
120-
return "IMAGE_SCN_CNT_UNINITIALIZED_DATA";
121-
}

llvm/lib/MC/MCSectionELF.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,5 +215,3 @@ void MCSectionELF::printSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
215215
bool MCSectionELF::useCodeAlign() const {
216216
return getFlags() & ELF::SHF_EXECINSTR;
217217
}
218-
219-
StringRef MCSectionELF::getVirtualSectionKind() const { return "SHT_NOBITS"; }

llvm/test/MC/COFF/bss-text.s

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
# RUN: llvm-mc -triple=x86_64-pc-win32 %s
55

66
.bss
7-
# CHECK: <unknown>:0: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section '.bss' cannot have non-zero bytes
7+
# CHECK: <unknown>:0: error: BSS section '.bss' cannot have non-zero bytes
88
addb %bl,(%rax)
99

1010
.section uninitialized,"b"
11-
# CHECK: <unknown>:0: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section 'uninitialized' cannot have non-zero bytes
11+
# CHECK: <unknown>:0: error: BSS section 'uninitialized' cannot have non-zero bytes
1212
jmp foo
1313

1414
.section bss0,"b"

llvm/test/MC/ELF/nobits-non-zero-value.s

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,37 @@
99
.bss
1010
addb %al,(%rax)
1111

12-
# CHECK: {{.*}}.s:[[#@LINE+1]]:11: warning: ignoring non-zero fill value in SHT_NOBITS section '.bss'
12+
# CHECK: {{.*}}.s:[[#@LINE+1]]:11: warning: ignoring non-zero fill value in BSS section '.bss'
1313
.align 4, 42
1414

15-
.align 4, 0
16-
1715
.long 1
1816

1917
.section .bss0,"aw",%nobits
2018
addb %al,(%rax)
2119

22-
.section .bss1,"aw",%nobits
20+
.section data_fixup,"aw",%nobits
2321
.quad foo
2422

23+
.section fill,"aw",%nobits
24+
.fill b-a,1,1
25+
26+
.section org,"aw",%nobits
27+
.org 1,1
28+
29+
.section ok,"aw",%nobits
30+
.org 1
31+
.fill 1
32+
.fill b-a,1,0
33+
.align 4, 0
34+
.long 0
35+
36+
.text
37+
a: nop
38+
b:
39+
2540
## Location is not tracked for efficiency.
26-
# CHECK: <unknown>:0: error: SHT_NOBITS section '.tbss' cannot have non-zero bytes
27-
# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss' cannot have non-zero bytes
28-
# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss1' cannot have fixups
41+
# CHECK: <unknown>:0: error: BSS section '.tbss' cannot have non-zero bytes
42+
# CHECK: <unknown>:0: error: BSS section '.bss' cannot have non-zero bytes
43+
# CHECK: <unknown>:0: error: BSS section 'data_fixup' cannot have fixups
44+
# CHECK: <unknown>:0: error: BSS section 'fill' cannot have non-zero bytes
45+
# CHECK: <unknown>:0: error: BSS section 'org' cannot have non-zero bytes

0 commit comments

Comments
 (0)