[lld] d851fce - [lld][WebAssembly] Do not emit initialization for .bss segments

Thomas Lively via llvm-commits llvm-commits at lists.llvm.org
Thu May 21 11:33:35 PDT 2020


Author: Thomas Lively
Date: 2020-05-21T11:33:25-07:00
New Revision: d851fce4cb238d5fe85ce71002721dfc2330fa46

URL: https://github.com/llvm/llvm-project/commit/d851fce4cb238d5fe85ce71002721dfc2330fa46
DIFF: https://github.com/llvm/llvm-project/commit/d851fce4cb238d5fe85ce71002721dfc2330fa46.diff

LOG: [lld][WebAssembly] Do not emit initialization for .bss segments

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

Added: 
    

Modified: 
    lld/test/wasm/data-segments.ll
    lld/wasm/SyntheticSections.cpp
    lld/wasm/SyntheticSections.h
    lld/wasm/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/test/wasm/data-segments.ll b/lld/test/wasm/data-segments.ll
index 7c5af8f74305..eb419ba07f30 100644
--- a/lld/test/wasm/data-segments.ll
+++ b/lld/test/wasm/data-segments.ll
@@ -52,6 +52,8 @@ 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
@@ -59,7 +61,8 @@ target triple = "wasm32-unknown-unknown"
 ; 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

diff  --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp
index 87e4251bffd4..4e974c8e4c9d 100644
--- a/lld/wasm/SyntheticSections.cpp
+++ b/lld/wasm/SyntheticSections.cpp
@@ -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() {

diff  --git a/lld/wasm/SyntheticSections.h b/lld/wasm/SyntheticSections.h
index ceb6d604da87..6cf593cf016c 100644
--- a/lld/wasm/SyntheticSections.h
+++ b/lld/wasm/SyntheticSections.h
@@ -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 {

diff  --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index a1b5a463a530..4947c3581467 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -54,6 +54,9 @@ class Writer {
 private:
   void openFile();
 
+  bool needsPassiveInitialization(const OutputSegment *segment);
+  bool hasPassiveInitializedSegments();
+
   void createInitMemoryFunction();
   void createApplyRelocationsFunction();
   void createCallCtorsFunction();
@@ -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);
@@ -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
@@ -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
@@ -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");
@@ -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);


        


More information about the llvm-commits mailing list