[WebAssembly] ignore non-SymbolRef MCExpr.

[patcheng]

Summary

instead of static casting a MCExpr to MCSymbolRefExpr directly, use dyn_cast and added null check.

Also removed an unnecessary (?) assertion

[sunfish]

Is this fixing a bug? It would help me to see what kind of code triggers this bug.

[sbc100]

lib/MC/WasmObjectWriter.cpp

446

Why is this unnecessary? Were you hitting this case? Perhaps we need a test that triggers this? Also, if this is unrelated to the MCSymbolRefExpr change than perhaps make it a separate change?

[patcheng]

Changes suggested by sbc100

[patcheng]

I messed up the diff. I will upload a correct one soon. hopefully with detail on the case that were hitting the code in the original diff.

[patcheng]

I rolled back the changes to ignore non-MCSymbolExpr.

Here is a dump of the MCExpr that I was encountering around line 1260 (in the for loop that's handling weak aliases.

$Ss10_ArrayBodyVN : isExternal: true isInSection: true isRegistered: true isUsed: true isVariable: true
	dumpMCExpr: 	Binary: opCode: Add
	LHS
		dumpMCExpr: 		Symbol: $Ss10_ArrayBodyVMf
	RHS
		dumpMCExpr: 		Constant: 4

[patcheng]

https://github.com/apple/swift-llvm/pull/155/files has an alternative fix.

include/llvm/Object/Wasm.h

   SubtargetFeatures getFeatures() const override;
   bool isRelocatableObject() const override;
 
+  bool isSectionWasmDataSegment(const SectionRef &Section) const;
+
 private:
   bool isValidFunctionIndex(uint32_t Index) const;
   bool isDefinedFunctionIndex(uint32_t Index) const;

   const WasmSection &getWasmSection(DataRefImpl Ref) const;
   const wasm::WasmRelocation &getWasmRelocation(DataRefImpl Ref) const;
 
+  bool isSectionWasmDataSegment(DataRefImpl Sec) const;
+
+  const wasm::WasmDataSegment &getSectionWasmDataSegment(DataRefImpl Ref) const;
+  const ArrayRef<uint8_t> &getSectionContentArrayRef(DataRefImpl Ref) const;
+
   WasmSection* findCustomSectionByName(StringRef Name);
   WasmSection* findSectionByType(uint32_t Type);
 

lib/MC/WasmObjectWriter.cpp

   void writeCodeSection(const MCAssembler &Asm, const MCAsmLayout &Layout,
                         ArrayRef<WasmFunction> Functions);
   void writeDataSection(ArrayRef<WasmDataSegment> Segments);
+
+  void writeAlignedDataSegment(SectionBookkeeping &Section, const WasmDataSegment & Segment);
+  void writeDataSegment(SectionBookkeeping &Section, const WasmDataSegment & Segment, 
+                        unsigned MemIndexPadTo = 0, unsigned OffsetPadTo = 0, 
+                        unsigned SizePadTo = 0);
+  
   void writeCodeRelocSection();
   void writeDataRelocSection();
   void writeLinkingMetaDataSection(

   encodeULEB128(Segments.size(), getStream()); // count
 
   for (const WasmDataSegment & Segment : Segments) {
-    encodeULEB128(0, getStream()); // memory index
-    write8(wasm::WASM_OPCODE_I32_CONST);
-    encodeSLEB128(Segment.Offset, getStream()); // offset
-    write8(wasm::WASM_OPCODE_END);
-    encodeULEB128(Segment.Data.size(), getStream()); // size
-    Segment.Section->setSectionOffset(getStream().tell() - Section.ContentsOffset);
-    writeBytes(Segment.Data); // data
+
+    // although WASM format is not supposed to designed to be mapped into memory
+    // some certain sections are expected to aligned. 
+    if (Segment.Name == "__clangast")
+      writeAlignedDataSegment(Section, Segment);
+    else
+      writeDataSegment(Section, Segment);
   }
 
   // Apply fixups.

   endSection(Section);
 }
 
+void WasmObjectWriter::writeAlignedDataSegment(SectionBookkeeping &Section,
+                                               const WasmDataSegment &Segment) {
+  // alignment is done by padding extra 0 in the ULEB encoded integers
+  // namely memoryIndex, offset and size
+
+  // ULEB128 encoding is limited to 10 bytes.
+#define MAX_PAD_TO (10)  // 64-bit integer is encoded as 10 7-bit elements
+
+  raw_null_ostream NullStream;
+  unsigned MemIndexLen = encodeULEB128(0, NullStream);
+  unsigned OffsetLen = encodeSLEB128(Segment.Offset, NullStream);
+  unsigned SizeLen = encodeULEB128(Segment.Data.size(), NullStream);
+
+  // 2 = the 2-bytes for WASM_OPCODE_I32_CONST and WASM_OPCODE_END
+  uint64_t SegmentStart =
+      (getStream().tell() + MemIndexLen + OffsetLen + SizeLen + 2);
+  uint64_t RemainingPadding =
+      OffsetToAlignment(SegmentStart, Segment.Alignment);
+
+  // now divide Padding between MemIndex, Offset and Size
+  unsigned MemIndexPadTo = MemIndexLen + RemainingPadding;
+  if (MemIndexPadTo > MAX_PAD_TO) MemIndexPadTo = MAX_PAD_TO;
+  RemainingPadding = RemainingPadding - (MemIndexPadTo - MemIndexLen);
+  unsigned OffsetPadTo = OffsetLen + RemainingPadding;
+  if (OffsetPadTo > MAX_PAD_TO) OffsetPadTo = MAX_PAD_TO;
+  RemainingPadding = RemainingPadding - (OffsetPadTo - OffsetLen);
+  unsigned SizePadTo = SizeLen + RemainingPadding;
+  assert(SizePadTo < MAX_PAD_TO && "Unable to align data segment");
+
+  writeDataSegment(Section, Segment, MemIndexPadTo, OffsetPadTo, SizePadTo);
+}
+
+void WasmObjectWriter::writeDataSegment(SectionBookkeeping &Section,
+                                        const WasmDataSegment &Segment,
+                                        unsigned MemIndexPadTo,
+                                        unsigned OffsetPadTo,
+                                        unsigned SizePadTo) {
+  encodeULEB128(0, getStream(), MemIndexPadTo);  // memory index
+  write8(wasm::WASM_OPCODE_I32_CONST);
+  encodeSLEB128(Segment.Offset, getStream(), OffsetPadTo);  // offset
+  write8(wasm::WASM_OPCODE_END);
+
+  encodeULEB128(Segment.Data.size(), getStream(), SizePadTo);  // size
+
+  Segment.Section->setSectionOffset(getStream().tell() -
+                                    Section.ContentsOffset);
+  writeBytes(Segment.Data);  // data
+}
+
 void WasmObjectWriter::writeCodeRelocSection() {
   // See: https://github.com/WebAssembly/tool-conventions/blob/master/Linking.md
   // for descriptions of the reloc sections.
 
-  if (CodeRelocations.empty())
+  if (CodeRelocations.empty()) 
     return;
 
   SectionBookkeeping Section;

   // See: https://github.com/WebAssembly/tool-conventions/blob/master/Linking.md
   // for descriptions of the reloc sections.
 
-  if (DataRelocations.empty())
+  if (DataRelocations.empty()) 
     return;
 
   SectionBookkeeping Section;

lib/Object/WasmObjectFile.cpp

   return section_iterator(SectionRef(Ref, this));
 }
 
-void WasmObjectFile::moveSectionNext(DataRefImpl &Sec) const { Sec.d.a++; }
+void WasmObjectFile::moveSectionNext(DataRefImpl &Sec) const { 
+  if (isSectionData(Sec) && (Sec.d.b < (DataSegments.size()))) {
+    Sec.d.b++;
+  } else {    
+    Sec.d.a++;
+    Sec.d.b = 0;
+  }
+}
 
 std::error_code WasmObjectFile::getSectionName(DataRefImpl Sec,
                                                StringRef &Res) const {

     ECase(START);
     ECase(ELEM);
     ECase(CODE);
-    ECase(DATA);
+  case wasm::WASM_SEC_DATA: {
+    Res = "DATA";
+
+    if (isSectionWasmDataSegment(Sec))
+      Res = getSectionWasmDataSegment(Sec).Name;
+    
+    break;
+  }
   case wasm::WASM_SEC_CUSTOM:
     Res = S.Name;
     break;

 }
 
 uint64_t WasmObjectFile::getSectionSize(DataRefImpl Sec) const {
-  const WasmSection &S = Sections[Sec.d.a];
-  return S.Content.size();
+  return getSectionContentArrayRef(Sec).size();
 }
 
 std::error_code WasmObjectFile::getSectionContents(DataRefImpl Sec,
                                                    StringRef &Res) const {
-  const WasmSection &S = Sections[Sec.d.a];
-  // This will never fail since wasm sections can never be empty (user-sections
-  // must have a name and non-user sections each have a defined structure).
-  Res = StringRef(reinterpret_cast<const char *>(S.Content.data()),
-                  S.Content.size());
+
+  const ArrayRef<uint8_t> &Content = getSectionContentArrayRef(Sec); 
+
+  Res = StringRef(reinterpret_cast<const char *>(Content.data()),
+                  Content.size());
   return std::error_code();
 }
 
 uint64_t WasmObjectFile::getSectionAlignment(DataRefImpl Sec) const {
-  return 1;
+  if (isSectionData(Sec)) {
+    const wasm::WasmDataSegment &Data = getSectionWasmDataSegment(Sec);
+    return Data.Alignment;
+  }
+
+  return 1;  
+}
+
+bool WasmObjectFile::isSectionWasmDataSegment(DataRefImpl Sec) const {
+  return isSectionData(Sec) && Sec.d.b != 0;
+}
+
+bool WasmObjectFile::isSectionWasmDataSegment(const SectionRef &Section) const {
+  return isSectionWasmDataSegment(Section.getRawDataRefImpl());
+}
+
+const wasm::WasmDataSegment &WasmObjectFile::getSectionWasmDataSegment(DataRefImpl Sec) const {
+  assert(isSectionData(Sec) && "Section should be Data");
+  assert(Sec.d.b > 0);
+  assert(Sec.d.b <= DataSegments.size());
+  return DataSegments[Sec.d.b-1].Data;
+}
+
+const ArrayRef<uint8_t> &WasmObjectFile::getSectionContentArrayRef(DataRefImpl Sec) const {
+  if (isSectionWasmDataSegment(Sec))
+    return getSectionWasmDataSegment(Sec).Content;
+
+  // This will never fail since wasm sections can never be empty (user-sections
+  // must have a name and non-user sections each have a defined structure).
+  return getWasmSection(Sec).Content;
 }
 
 bool WasmObjectFile::isSectionCompressed(DataRefImpl Sec) const {

   const WasmSection &Sec = getWasmSection(Ref);
   DataRefImpl RelocRef;
   RelocRef.d.a = Ref.d.a;
-  RelocRef.d.b = Sec.Relocations.size();
+
+  // if Ref is pointing to a "Section" that proxy to a Data Segment, 
+  // don't expose the relocation table
+  if (isSectionWasmDataSegment(Ref))
+    RelocRef.d.b = 0;
+  else
+    RelocRef.d.b = Sec.Relocations.size();
+
+
   return relocation_iterator(RelocationRef(RelocRef, this));
 }
 

test/tools/llvm-objdump/wasm.txt

 # CHECK-NEXT:  6 EXPORT        0000000e 0000000000000000 
 # CHECK-NEXT:  7 CODE          00000019 0000000000000000 TEXT 
 # CHECK-NEXT:  8 DATA          0000001a 0000000000000000 DATA 
-# CHECK-NEXT:  9 name          0000002b 0000000000000000 
-# CHECK-NEXT: 10 reloc.CODE    00000017 0000000000000000 
-# CHECK-NEXT: 11 linking       00000016 0000000000000000 
+# CHECK-NEXT:  9               00000014 0000000000000000 DATA 
+# CHECK-NEXT: 10 name          0000002b 0000000000000000 
+# CHECK-NEXT: 11 reloc.CODE    00000017 0000000000000000 
+# CHECK-NEXT: 12 linking       00000016 0000000000000000 
 
 # RUN: llvm-objdump -p %p/Inputs/trivial.obj.wasm | FileCheck %s -check-prefix CHECK-HEADER
 

tools/llvm-readobj/WasmDumper.cpp

   ListScope Group(W, "Sections");
   for (const SectionRef &Section : Obj->sections()) {
     const WasmSection &WasmSec = Obj->getWasmSection(Section);
+    if (Obj->isSectionWasmDataSegment(Section))
+      continue;
+
     DictScope SectionD(W, "Section");
     W.printEnum("Type", WasmSec.Type, makeArrayRef(WasmSectionTypes));
     W.printNumber("Size", static_cast<uint64_t>(WasmSec.Content.size()));

tools/obj2yaml/wasm2yaml.cpp

       break;
     }
     case wasm::WASM_SEC_DATA: {
+      if (Obj.isSectionWasmDataSegment(Sec))
+        continue;
       auto DataSec = make_unique<WasmYAML::DataSection>();
       for (const object::WasmSegment &Segment : Obj.dataSegments()) {
         WasmYAML::DataSegment Seg;