[lld][WebAssembly] Fix segfault in map file support

[tlively]

Summary

The code previously assumed that getChunk would return a non-null pointer for
every symbol, but in fact it only returns non-null pointers for DefinedFunction
and DefinedData symbols. This patch fixes the segfault by checking whether
getChunk returns a null for each symbol and skipping the mapping output for
any symbols for which it does.

[sbc100]

Can you update the mapfile test to tickle this?

[tlively]

I'm not actually sure how to do that in the .s format, since I found this on a real-world project. Do you know what I should add to the test?

[sbc100]

Do you know that symbol type that causes the crash?

[sbc100]

Maybe a DefinedGlobal would do it? You can define wasm globals in .s files (see lld/test/wasm/globals.s).

[sbc100]

lld/wasm/MapFile.cpp

84

How about if we just have fileOffset default to 0 if chunk is null? I think we still want to include all symbols in the map.

[sbc100]

Same issue? https://bugs.llvm.org/show_bug.cgi?id=48972

[tlively]

Yes, almost certainly. I actually ran into another null pointer dereference and had to add another workaround to my local branch as well. I haven't pursued this patch upstream because I still have not been able to create a test case :(

[sbc100]

Can we get this landed? Did adding a defined global to a .s file work? (I think maybe you can just add a defined global to lld/test/wasm/map-file.s?

[tlively]

Trying again now, but last time I tried that (a while ago) I couldn't reproduce the issue.

[tlively]

• Add test with wasm global, try to print it

[tlively]

Ok, I was able to reproduce one of the issues by using a wasm global this time. However, I'm having trouble figuring out how to fit printing the global to the map file in with the current infrastructure. Comments would be very appreciated.

[sbc100]

Seems reasonable to me. Our map file format is relatively arbitrary AFAICT. Its mostly about conveying the final contents of the linked binary IIUC.

[sbc100]

Is this good to go now?

[tlively]

• Clean up

[tlively]

Yes, I think so. I changed this back to skipping symbols without chunks in the because we don't need to demangle global symbols.

lld/test/wasm/map-file.s

 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %s -o %t1.o
 # RUN: wasm-ld %t1.o -o %t -M | FileCheck --match-full-lines --strict-whitespace %s
 # RUN: wasm-ld %t1.o -o %t -print-map | FileCheck --match-full-lines --strict-whitespace %s
 # RUN: wasm-ld %t1.o -o %t -Map=%t.map
 # RUN: FileCheck --match-full-lines --strict-whitespace %s < %t.map
 
+.globaltype wasm_global, i32, immutable
+wasm_global:
+
 bar:
     .functype bar () -> ()
     i32.const   somedata
     end_function
 
+write_global:
+    .functype write_global (i32) -> ()
+    local.get 0
+    global.set wasm_global
+    end_function
+
     .globl _start
 _start:
     .functype _start () -> ()
     call bar
+    call write_global
     end_function
 
 .section .data.somedata,"",@
 somedata:
     .int32 123
 .size somedata, 4
 
 .section .debug_info,"",@
     .int32 bar
 
 #      CHECK:    Addr      Off     Size Out     In      Symbol
-# CHECK-NEXT:       -        8        6 TYPE
-# CHECK-NEXT:       -        e        5 FUNCTION
-# CHECK-NEXT:       -       13        7 TABLE
-# CHECK-NEXT:       -       1a        5 MEMORY
-# CHECK-NEXT:       -       1f        a GLOBAL
-# CHECK-NEXT:       -       29       15 EXPORT
-# CHECK-NEXT:       -       3e       15 CODE
-# CHECK-NEXT:       -       3f        9         {{.*}}{{/|\\}}map-file.s.tmp1.o:(bar)
-# CHECK-NEXT:       -       3f        9                 bar
-# CHECK-NEXT:       -       48        9         {{.*}}{{/|\\}}map-file.s.tmp1.o:(_start)
-# CHECK-NEXT:       -       48        9                 _start
-# CHECK-NEXT:       -       53        d DATA
-# CHECK-NEXT:     400       54        4 .data
-# CHECK-NEXT:     400       5a        4         {{.*}}{{/|\\}}map-file.s.tmp1.o:(.data.somedata)
-# CHECK-NEXT:     400       5a        4                 somedata
-# CHECK-NEXT:       -       60       12 CUSTOM(.debug_info)
-# CHECK-NEXT:       -       72       35 CUSTOM(name)
+# CHECK-NEXT:       -        8        a TYPE
+# CHECK-NEXT:       -       12        6 FUNCTION
+# CHECK-NEXT:       -       18        7 TABLE
+# CHECK-NEXT:       -       1f        5 MEMORY
+# CHECK-NEXT:       -       24        f GLOBAL
+# CHECK-NEXT:       0        0        0         __stack_pointer
+# CHECK-NEXT:       1        0        0         wasm_global
+# CHECK-NEXT:       -       33       15 EXPORT
+# CHECK-NEXT:       -       48       26 CODE
+# CHECK-NEXT:       -       49        9         {{.*}}{{/|\\}}map-file.s.tmp1.o:(bar)
+# CHECK-NEXT:       -       49        9                 bar
+# CHECK-NEXT:       -       52        b         {{.*}}{{/|\\}}map-file.s.tmp1.o:(write_global)
+# CHECK-NEXT:       -       52        b                 write_global
+# CHECK-NEXT:       -       5d        f         {{.*}}{{/|\\}}map-file.s.tmp1.o:(_start)
+# CHECK-NEXT:       -       5d        f                 _start
+# CHECK-NEXT:       -       6e        d DATA
+# CHECK-NEXT:     400       6f        4 .data
+# CHECK-NEXT:     400       75        4         {{.*}}{{/|\\}}map-file.s.tmp1.o:(.data.somedata)
+# CHECK-NEXT:     400       75        4                 somedata
+# CHECK-NEXT:       -       7b       12 CUSTOM(.debug_info)
+# CHECK-NEXT:       -       8d       50 CUSTOM(name)
 
 # RUN: not wasm-ld %t1.o -o /dev/null -Map=/ 2>&1 \
 # RUN:  | FileCheck -check-prefix=FAIL %s
 # FAIL: wasm-ld: error: cannot open map file /

lld/wasm/MapFile.cpp

 //          - 0000000e     10            test.o:(.text)
 //          - 00000000      5                    local
 //          - 00000000      5                    f(int)
 //
 //===----------------------------------------------------------------------===//
 
 #include "MapFile.h"
 #include "InputFiles.h"
+#include "InputGlobal.h"
 #include "OutputSections.h"
 #include "OutputSegment.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "SyntheticSections.h"
 #include "lld/Common/Strings.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SetVector.h"

 // Construct a map from symbols to their stringified representations.
 // Demangling symbols (which is what toString() does) is slow, so
 // we do that in batch using parallel-for.
 static DenseMap<Symbol *, std::string>
 getSymbolStrings(ArrayRef<Symbol *> syms) {
   std::vector<std::string> str(syms.size());
   parallelForEachN(0, syms.size(), [&](size_t i) {
     raw_string_ostream os(str[i]);
-    auto &chunk = *syms[i]->getChunk();
-    uint64_t fileOffset = chunk.outputSec->getOffset() + chunk.outputOffset;
+    auto *chunk = syms[i]->getChunk();
+    if (chunk == nullptr)
+      return;
+    uint64_t fileOffset = chunk->outputSec->getOffset() + chunk->outputOffset;
     uint64_t vma = -1;
     uint64_t size = 0;
     if (auto *DD = dyn_cast<DefinedData>(syms[i])) {
       vma = DD->getVirtualAddress();
       size = DD->getSize();
       fileOffset += DD->offset;
     }
     if (auto *DF = dyn_cast<DefinedFunction>(syms[i])) {
       size = DF->function->getSize();
if (auto *code = dyn_cast<CodeSection>(osec)) {
           writeHeader(os, oseg->startVA + chunk->outputSegmentOffset,
                       chunk->outputSec->getOffset() + chunk->outputOffset,
                       chunk->getSize());
           os.indent(8) << toString(chunk) << '\n';
           for (Symbol *sym : sectionSyms[chunk])
             os << symStr[sym] << '\n';
         }
       }
+    } else if (auto *globals = dyn_cast<GlobalSection>(osec)) {
+      for (auto *global : globals->inputGlobals) {
+        writeHeader(os, global->getGlobalIndex(), 0, 0);
+        os.indent(8) << global->getName() << '\n';
+      }
     }
+    // TODO: other section/symbol types
   }
 }

lld/wasm/SyntheticSections.h

public:
   void addEvent(InputEvent *event);
 
   std::vector<InputEvent *> inputEvents;
 };
 
 class GlobalSection : public SyntheticSection {
 public:
   GlobalSection() : SyntheticSection(llvm::wasm::WASM_SEC_GLOBAL) {}
+
+  static bool classof(const OutputSection *sec) {
+    return sec->type == llvm::wasm::WASM_SEC_GLOBAL;
+  }
+
   uint32_t numGlobals() const {
     assert(isSealed);
     return inputGlobals.size() + dataAddressGlobals.size() +
            internalGotSymbols.size();
   }
   bool isNeeded() const override { return numGlobals() > 0; }
   void assignIndexes() override;
   void writeBody() override;