Skip to content

Commit

Permalink
[lld][WebAssembly] Do not emit initialization for .bss segments
Browse files Browse the repository at this point in the history
Summary:
This patch fixes a bug where initialization code for .bss segments was
emitted in the memory initialization function even though the .bss
segments were discounted in the datacount section and omitted in the
data section. This was producing invalid binaries due to out-of-bounds
segment indices on the memory.init and data.drop instructions that
were trying to operate on the nonexistent .bss segments.

Reviewers: sbc100

Subscribers: dschuff, jgravelle-google, aheejin, sunfish, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D80354
  • Loading branch information
tlively committed May 21, 2020
1 parent abf4957 commit d851fce
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 10 deletions.
5 changes: 4 additions & 1 deletion lld/test/wasm/data-segments.ll
Expand Up @@ -52,14 +52,17 @@ target triple = "wasm32-unknown-unknown"

; PASSIVE-LABEL: - Type: START
; PASSIVE-NEXT: StartFunction: 1
; PASSIVE-LABEL: - Type: DATACOUNT
; PASSIVE-NEXT: Count: 2
; PASSIVE-LABEL: - Type: CODE
; PASSIVE-NEXT: Functions:
; PASSIVE-NEXT: - Index: 0
; PASSIVE-NEXT: Locals: []
; PASSIVE-NEXT: Body: 0B
; PASSIVE-NEXT: - Index: 1
; PASSIVE-NEXT: Locals: []
; PASSIVE-NEXT: Body: 41B4D60041004101FE480200044041B4D6004101427FFE0102001A054180084100410DFC08000041900841004114FC08010041A40841004190CE00FC08020041B4D6004102FE17020041B4D600417FFE0002001A0BFC0900FC0901FC09020B
; PASSIVE-NEXT: Body: 41B4D60041004101FE480200044041B4D6004101427FFE0102001A054180084100410DFC08000041900841004114FC08010041B4D6004102FE17020041B4D600417FFE0002001A0BFC0900FC09010B

; PASSIVE-NEXT: - Index: 2
; PASSIVE-NEXT: Locals: []
; PASSIVE-NEXT: Body: 0B
Expand Down
2 changes: 1 addition & 1 deletion lld/wasm/SyntheticSections.cpp
Expand Up @@ -326,7 +326,7 @@ void ExportSection::writeBody() {
}

bool StartSection::isNeeded() const {
return !config->relocatable && numSegments && config->sharedMemory;
return !config->relocatable && hasInitializedSegments && config->sharedMemory;
}

void StartSection::writeBody() {
Expand Down
8 changes: 4 additions & 4 deletions lld/wasm/SyntheticSections.h
Expand Up @@ -225,14 +225,14 @@ class ExportSection : public SyntheticSection {

class StartSection : public SyntheticSection {
public:
StartSection(uint32_t numSegments)
: SyntheticSection(llvm::wasm::WASM_SEC_START), numSegments(numSegments) {
}
StartSection(bool hasInitializedSegments)
: SyntheticSection(llvm::wasm::WASM_SEC_START),
hasInitializedSegments(hasInitializedSegments) {}
bool isNeeded() const override;
void writeBody() override;

protected:
uint32_t numSegments;
bool hasInitializedSegments;
};

class ElemSection : public SyntheticSection {
Expand Down
23 changes: 19 additions & 4 deletions lld/wasm/Writer.cpp
Expand Up @@ -54,6 +54,9 @@ class Writer {
private:
void openFile();

bool needsPassiveInitialization(const OutputSegment *segment);
bool hasPassiveInitializedSegments();

void createInitMemoryFunction();
void createApplyRelocationsFunction();
void createCallCtorsFunction();
Expand Down Expand Up @@ -729,6 +732,18 @@ static void createFunction(DefinedFunction *func, StringRef bodyContent) {
cast<SyntheticFunction>(func->function)->setBody(body);
}

bool Writer::needsPassiveInitialization(const OutputSegment *segment) {
return segment->initFlags & WASM_SEGMENT_IS_PASSIVE &&
segment->name != ".tdata" && !segment->isBss;
}

bool Writer::hasPassiveInitializedSegments() {
return std::find_if(segments.begin(), segments.end(),
[this](const OutputSegment *s) {
return this->needsPassiveInitialization(s);
}) != segments.end();
}

void Writer::createInitMemoryFunction() {
LLVM_DEBUG(dbgs() << "createInitMemoryFunction\n");
assert(WasmSym::initMemoryFlag);
Expand All @@ -738,7 +753,7 @@ void Writer::createInitMemoryFunction() {
raw_string_ostream os(bodyContent);
writeUleb128(os, 0, "num locals");

if (segments.size()) {
if (hasPassiveInitializedSegments()) {
// Initialize memory in a thread-safe manner. The thread that successfully
// increments the flag from 0 to 1 is is responsible for performing the
// memory initialization. Other threads go sleep on the flag until the
Expand Down Expand Up @@ -804,7 +819,7 @@ void Writer::createInitMemoryFunction() {

// Did increment 0, so conditionally initialize passive data segments
for (const OutputSegment *s : segments) {
if (s->initFlags & WASM_SEGMENT_IS_PASSIVE && s->name != ".tdata") {
if (needsPassiveInitialization(s)) {
// destination address
writeI32Const(os, s->startVA, "destination address");
// source segment offset
Expand Down Expand Up @@ -838,7 +853,7 @@ void Writer::createInitMemoryFunction() {

// Unconditionally drop passive data segments
for (const OutputSegment *s : segments) {
if (s->initFlags & WASM_SEGMENT_IS_PASSIVE && s->name != ".tdata") {
if (needsPassiveInitialization(s)) {
// data.drop instruction
writeU8(os, WASM_OPCODE_MISC_PREFIX, "bulk-memory prefix");
writeUleb128(os, WASM_OPCODE_DATA_DROP, "data.drop");
Expand Down Expand Up @@ -983,7 +998,7 @@ void Writer::createSyntheticSections() {
out.eventSec = make<EventSection>();
out.globalSec = make<GlobalSection>();
out.exportSec = make<ExportSection>();
out.startSec = make<StartSection>(segments.size());
out.startSec = make<StartSection>(hasPassiveInitializedSegments());
out.elemSec = make<ElemSection>();
out.dataCountSec = make<DataCountSection>(segments);
out.linkingSec = make<LinkingSection>(initFunctions, segments);
Expand Down

0 comments on commit d851fce

Please sign in to comment.