[llvm] 508aa69 - [WebAssembly] Fix assert in lookup of section symbols

[Wouter van Oortmerssen via llvm-commits]

Author: Wouter van Oortmerssen
Date: 2021-02-18T11:50:14-08:00
New Revision: 508aa69e9dbcf4b4de1876cb1b0e9d2c1dbc176f

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

LOG: [WebAssembly] Fix assert in lookup of section symbols

Fixes assert in: https://bugs.llvm.org/show_bug.cgi?id=49036

getWasmSection creates sections if they don't exist, but doesn't add them to the Symbols table. This may cause problems in subsequent calls to getOrCreateSymbol which checks this table, the calls createSymbol assuming it doesn't exist, which then checks UsedNames and finds out it does exist, causing an assert on trying to rename a non-temp symbol.

I tried also fixing the somewhat unintuitive forced suffixing (adding `0`), but it turns out that WasmObjectWriter currently assumes these section symbols are unique, so that may have to be a separate fix: https://bugs.llvm.org/show_bug.cgi?id=49252

Also worth noting is that getWasmSection calling createSymbol may not be correct to start with, given that createSymbol seems to assume it is creating non-section symbols. But again, for a future fix.

Related: where some of this was introduced: https://github.com/llvm/llvm-project/commit/8d396acac3bc21f688ac707bb42e4698dbdcab7e

Differential Revision: https://reviews.llvm.org/D96473

Added: 
    llvm/test/MC/WebAssembly/section-symbol.s

Modified: 
    llvm/lib/MC/MCContext.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
index 7d5a1db04358..0902a15979cd 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -656,6 +656,7 @@ MCSectionWasm *MCContext::getWasmSection(const Twine &Section, SectionKind Kind,
   StringRef CachedName = Entry.first.SectionName;
 
   MCSymbol *Begin = createSymbol(CachedName, true, false);
+  Symbols[Begin->getName()] = Begin;
   cast<MCSymbolWasm>(Begin)->setType(wasm::WASM_SYMBOL_TYPE_SECTION);
 
   MCSectionWasm *Result = new (WasmAllocator.Allocate())

diff  --git a/llvm/test/MC/WebAssembly/section-symbol.s b/llvm/test/MC/WebAssembly/section-symbol.s
new file mode 100644
index 000000000000..b55221c0947f
--- /dev/null
+++ b/llvm/test/MC/WebAssembly/section-symbol.s
@@ -0,0 +1,16 @@
+# RUN: llvm-mc -triple=wasm32 < %s | FileCheck %s
+
+# check that we can refer to section symbols of other sections.
+# getWasmSection currently forces the section symbol to have a suffix.
+
+# TODO: fix the 0-suffix: https://bugs.llvm.org/show_bug.cgi?id=49252
+
+    .section    .debug_abbrev,"",@
+    .int8       1
+    .section    .debug_info,"",@
+    .int32      .debug_abbrev0
+
+# CHECK:         .section    .debug_abbrev,"",@
+# CHECK-NEXT:    .int8       1
+# CHECK-NEXT:    .section    .debug_info,"",@
+# CHECK-NEXT:    .int32      .debug_abbrev0