[WebAssembly] MC: Include unnamed data when writing wasm files

[sbc100]

Summary

Also, include global entries for all data symbols, not
just external ones, since these are referenced by the
relocation records.

Add a test case that includes unnamed data.

[sbc100]

• cleanup

[sbc100]

• add more test checks

[dschuff]

lib/MC/WasmObjectWriter.cpp

698 ↗

(On Diff #98559)

How are we hitting the line you took out, when this is still here?

[dschuff]

lib/MC/WasmObjectWriter.cpp

698 ↗

(On Diff #98559)

Oh, wait this are unnamed too. So we still want to ignore unnamed temporaries.

[sbc100]

lib/MC/WasmObjectWriter.cpp

698 ↗

(On Diff #98559)

If the comment is correct then yes.

[sbc100]

@sunfish can you take a look?

[sbc100]

test/MC/WebAssembly/unnamed_data.ll

53 ↗

(On Diff #98559)

I guess this type of test is too brittle and I should simply not check for the Content here or the exact offsets of the relocs above?

[sunfish]

lgtm

[alekseyshl]

Could this change be the reason of MC related build failures? Please check http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/1418/steps/check-llvm%20msan/logs/stdio

[sbc100]

I don't think this change can effect anything other than the wasm target. The failures you link to look like they are occurring with other targets such as -mtriple arm-apple-darwin

llvm/trunk/lib/MC/WasmObjectWriter.cpp

static void ApplyRelocations(
     uint64_t ContentsOffset)
 {
   for (const WasmRelocationEntry &RelEntry : Relocations) {
     uint64_t Offset = ContentsOffset +
                       RelEntry.FixupSection->getSectionOffset() +
                       RelEntry.Offset;
     switch (RelEntry.Type) {
     case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB: {
+      assert(SymbolIndices.count(RelEntry.Symbol));
       uint32_t Index = SymbolIndices[RelEntry.Symbol];
       assert(RelEntry.Addend == 0);
 
       WritePatchableLEB(Stream, Index, Offset);
       break;
     }
     case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB: {
+      assert(SymbolIndices.count(RelEntry.Symbol));
       uint32_t Index = SymbolIndices[RelEntry.Symbol];
       assert(RelEntry.Addend == 0);
 
       WritePatchableSLEB(Stream, Index, Offset);
       break;
     }
     case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_SLEB: {
       uint32_t Value = ProvisionalValue(RelEntry);
 
       WritePatchableSLEB(Stream, Value, Offset);
       break;
     }
     case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_LEB: {
       uint32_t Value = ProvisionalValue(RelEntry);
 
       WritePatchableLEB(Stream, Value, Offset);
       break;
     }
     case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32: {
+      assert(SymbolIndices.count(RelEntry.Symbol));
       uint32_t Index = SymbolIndices[RelEntry.Symbol];
       assert(RelEntry.Addend == 0);
 
       WriteI32(Stream, Index, Offset);
       break;
     }
     case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_I32: {
       uint32_t Value = ProvisionalValue(RelEntry);
WriteRelocations(ArrayRef<WasmRelocationEntry> Relocations,
                  raw_pwrite_stream &Stream,
                  DenseMap<const MCSymbolWasm *, uint32_t> &SymbolIndices,
                  uint64_t HeaderSize) {
   for (const WasmRelocationEntry RelEntry : Relocations) {
     encodeULEB128(RelEntry.Type, Stream);
 
     uint64_t Offset = RelEntry.Offset +
                       RelEntry.FixupSection->getSectionOffset() + HeaderSize;
+    assert(SymbolIndices.count(RelEntry.Symbol));
     uint32_t Index = SymbolIndices[RelEntry.Symbol];
     int64_t Addend = RelEntry.Addend;
 
     switch (RelEntry.Type) {
     case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
     case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB:
     case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32:
       encodeULEB128(Offset, Stream);
if (WS.isFunction()) {
         // An import; the index was assigned above.
         Index = SymbolIndices.find(&WS)->second;
       }
 
       // If needed, prepare the function to be called indirectly.
       if (IsAddressTaken.count(&WS))
         TableElems.push_back(Index);
     } else {
-      // For now, ignore temporary non-function symbols.
-      if (S.isTemporary())
-        continue;
-
       if (WS.getOffset() != 0)
         report_fatal_error("data sections must contain one variable each");
       if (!WS.getSize())
         report_fatal_error("data symbols must have a size set with .size");
 
       int64_t Size = 0;
       if (!WS.getSize()->evaluateAsAbsolute(Size, Layout))
         report_fatal_error(".size expression must be evaluatable");
if (WS.isFunction()) {
           } else {
             const MCDataFragment &DataFrag = cast<MCDataFragment>(Frag);
             const SmallVectorImpl<char> &Contents = DataFrag.getContents();
 
             DataBytes.insert(DataBytes.end(), Contents.begin(), Contents.end());
           }
         }
 
-        // For each external global, prepare a corresponding wasm global
-        // holding its address.
-        if (WS.isExternal()) {
-          Index = NumGlobalImports + Globals.size();
+        // For each global, prepare a corresponding wasm global holding its
+        // address.  For externals these will also be named exports.
 
-          WasmGlobal Global;
-          Global.Type = PtrType;
-          Global.IsMutable = false;
-          Global.HasImport = false;
-          Global.InitialValue = DataSection.getSectionOffset();
-          Global.ImportIndex = 0;
-          SymbolIndices[&WS] = Index;
-          Globals.push_back(Global);
-        }
-      }
-    }
 
     // If the symbol is visible outside this translation unit, export it.
     if (WS.isExternal()) {
       assert(WS.isDefined(false));
       WasmExport Export;
       Export.FieldName = WS.getName();
       Export.Index = Index;
 

llvm/trunk/test/MC/WebAssembly/unnamed-data.ll

+; RUN: llc -mtriple wasm32-unknown-unknown-wasm -filetype=obj %s -o - | obj2yaml | FileCheck %s
+
+@.str1 = private unnamed_addr constant [6 x i8] c"hello\00", align 1
+@.str2 = private unnamed_addr constant [6 x i8] c"world\00", align 1
+
+@a = global i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.str1, i32 0, i32 0), align 8
+@b = global i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.str2, i32 0, i32 0), align 8
+
+
+; CHECK:   - Type:            GLOBAL
+; CHECK:     Globals:         
+; CHECK:       - Type:            I32
+; CHECK:         Mutable:         false
+; CHECK:         InitExpr:        
+; CHECK:           Opcode:          I32_CONST
+; CHECK:           Value:           0
+; CHECK:       - Type:            I32
+; CHECK:         Mutable:         false
+; CHECK:         InitExpr:        
+; CHECK:           Opcode:          I32_CONST
+; CHECK:           Value:           6
+; CHECK:       - Type:            I32
+; CHECK:         Mutable:         false
+; CHECK:         InitExpr:        
+; CHECK:           Opcode:          I32_CONST
+; CHECK:           Value:           16
+; CHECK:       - Type:            I32
+; CHECK:         Mutable:         false
+; CHECK:         InitExpr:        
+; CHECK:           Opcode:          I32_CONST
+; CHECK:           Value:           24
+; CHECK:   - Type:            EXPORT
+; CHECK:     Exports:         
+; CHECK:       - Name:            a
+; CHECK:         Kind:            GLOBAL
+; CHECK:         Index:           2
+; CHECK:       - Name:            b
+; CHECK:         Kind:            GLOBAL
+; CHECK:         Index:           3
+; CHECK:   - Type:            DATA
+; CHECK:     Relocations:     
+; CHECK:       - Type:            R_WEBASSEMBLY_GLOBAL_ADDR_I32
+; CHECK:         Index:           0
+; CHECK:         Offset:          0x00000016
+; CHECK:       - Type:            R_WEBASSEMBLY_GLOBAL_ADDR_I32
+; CHECK:         Index:           1
+; CHECK:         Offset:          0x0000001E
+; CHECK:     Segments:        
+; CHECK:       - Index:           0
+; CHECK:         Offset:          
+; CHECK:           Opcode:          I32_CONST
+; CHECK:           Value:           0
+; CHECK:         Content:         68656C6C6F00776F726C640000000000000000000000000006000000