[WebAssembly] Better support for WASM Object format

[patcheng]

Summary

Addressed https://bugs.llvm.org/show_bug.cgi?id=35928

WasmObjectFile::section() now exposes WasmDataSegments as sections
so that when Clang iterates through the sections, it wil find the
WasmDataSegments.

When writing WasmDataSegments, if the segment name is "__clangast", now make sure the WasmDataSegment is aligned by adding padding to the integer encoding.

[patcheng]

This commit is not ready. wasm2yaml is relying on WasmObjectFile::sections() to return only 1 Data Section. Now, we are returning each DataSegment. As a result, some of the tests are failing currently.

[sbc100]

lib/Object/WasmObjectFile.cpp

1058

This seems reasonable, and could even be a separate change that could land right away if your prefer.

1149

Should do this for section_end() too? and symbol_begin()/end() too?

Is there a particular motivation for this? I someone reading this value somewhere?

[patcheng]

re: wasm2yaml

Is it only used for unit tests?
Before I process, just want to make sure I am causing issues. Basically, I am thinking changing the output from one block for entire Data section

- Type:            DATA
  Segments:        
    - SectionOffset:   6
      MemoryIndex:     0
      Offset:          
        Opcode:          I32_CONS
        Value:           0
      Content:         '00000000'
...

to a Data Segment.

- Type:            DATA
  SectionOffset:   6
  MemoryIndex:     0
  Offset:          
    Opcode:          I32_CONS
    Value:           0
  Content:         '00000000'
...

lib/Object/WasmObjectFile.cpp

1058

Looks like it's used by clang when -fembed-bitcode is specified:

https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/BackendUtil.cpp#L1246

Probably makes more sense to separate it.

1149

See changes to moveSectionNext() above.

Now when iterating through sections(), we are using Ref.d.a and Ref.d.b.

Don't think it's needed in other methods.

Actually, looking into a bit more, looks like DataRefImpl has a constructor that memset() everything. Maybe it's not strictly necessary (?)

[patcheng]

Updated llvm-readobj and obj2yaml so that output in similar format as previously.

[sbc100]

As far as I know yes the yaml<->obj tools are mostly used for testing.

[sbc100]

include/llvm/Object/Wasm.h

213

This method seems like it could use a better name since "DetaRef" is can refer to many things, not just segment. Also, what is wrong with using the existing getSectionContents method?

lib/MC/WasmObjectWriter.cpp

856

Unlike other formats that wasm file formats is not designed to be mapped into memory, so the file alignment should not be relevent. If it is particularly important for bitcode data perhaps you could just align those segments, but I don't think we want padding for data segments in general.

[patcheng]

Now only align when the segment name is "__clangast"

[sbc100]

lib/MC/WasmObjectWriter.cpp

848

drop the the { } ? (and maybe move the comment out side the if if it makes it more readable?)

867

The indentation here looks funny. Can you run clang-format on this patch ?

lib/Object/WasmObjectFile.cpp

1121

Can this line move to the else of the if below?

test/MC/WebAssembly/bss.ll

72 ↗

(On Diff #131542)

Revert this change?

test/tools/llvm-objdump/wasm.txt

14

We should probably give this section (actually its the one and only segment, right?) a name.

Also, perhaps we should omit the data section itself from the list, now that its sub-segments are themselves lists?

[patcheng]

test/tools/llvm-objdump/wasm.txt

13

Now Section.Name comes from DataSegment's Name, which is comes from linking.

In trivial.obj.wasm, there is no names for the DataSegment, so it is currently returning nothing.

Should it default to "DATA"?

[sbc100]

test/tools/llvm-objdump/wasm.txt

13

Maybe we should regenerate trivial.obj.wasm? See test/tools/llvm-readobj/Inputs/trivial.ll for how to do this

[patcheng]

Changes suggested by sbc100

[sbc100]

lib/Object/WasmObjectFile.cpp

995

Remove extra brackets around DataSegments.size()

1025

Use if/else here

1045–1047

Just use getSectionContents() here?

1059

No need for getSectionContentArrayRef method I think, just put that logic right here.

1124

remove extra newline

tools/llvm-readobj/WasmDumper.cpp

148

Move this up before the declaration of WasmSec

[patcheng]

Hopefully, the code should be good now.

Unfortunately, I need help adding the newly generated trivial.obj.wasm here

trivial.obj.wasm346 BDownload

to the diff.

Thanks

test/tools/llvm-objdump/wasm.txt

13

Thanks for the tip.

I was able to regenerate trivial.obj.wasm file, and updated the test result, but I was not able to include the new .wasm as part of the patch without getting an error from Phabricator.

[sbc100]

Sorry to flip flop on this but I think we are going in the direction of using custom sections for this kind of this (debug info, lto stuff, etc). Anything that doesn't need to be in the address space at runtime.

Hopefully this should simply this change.

[sbc100]

Are you able to generate and consume the __clangast now? It should show up as a custom section

[patcheng]

I haven't had the time to working on this project recently.

But looks like zhuowei was able to access "_clangast" without my changes. Here are the changes that was needed: https://github.com/apple/swift-llvm/pull/155

Maybe this can be closed?

[sbc100]

I think this can be closed?

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;
+
   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: {
+    if (isSectionWasmDataSegment(Sec))
+      Res = getSectionWasmDataSegment(Sec).Name;
+    else 
+      Res = "DATA";    
+    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();
+  StringRef Res;
+  getSectionContents(Sec, Res);
+  return Res.size();
 }
 
 std::error_code WasmObjectFile::getSectionContents(DataRefImpl Sec,
                                                    StringRef &Res) const {
-  const WasmSection &S = Sections[Sec.d.a];
+  const auto &Content = isSectionWasmDataSegment(Sec) 
+                            ? getSectionWasmDataSegment(Sec).Content 
+                            : getWasmSection(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).
-  Res = StringRef(reinterpret_cast<const char *>(S.Content.data()),
-                  S.Content.size());
+  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;
 }
 
 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/WebAssembly/symbol-table.test

 CHECK-NEXT: 00000000 g     F IMPORT puts
 CHECK-NEXT: 00000000 g     F IMPORT SomeOtherFunction
 CHECK-NEXT: 00000002 g     F EXPORT main
+CHECK-NEXT: 00000000 l       EXPORT .L.str
 CHECK-NEXT: 00000010 g       EXPORT var
 

test/tools/llvm-objdump/wasm.txt

 
 # CHECK:      Sections:
 # CHECK-NEXT: Idx Name          Size      Address          Type
-# CHECK-NEXT:  0 TYPE          0000000e 0000000000000000 
-# CHECK-NEXT:  1 IMPORT        00000024 0000000000000000 
-# CHECK-NEXT:  2 FUNCTION      00000002 0000000000000000 
-# CHECK-NEXT:  3 TABLE         00000004 0000000000000000 
-# CHECK-NEXT:  4 MEMORY        00000003 0000000000000000 
-# CHECK-NEXT:  5 GLOBAL        0000000b 0000000000000000 
-# 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:  0 TYPE           0000000e 0000000000000000 
+# CHECK-NEXT:  1 IMPORT         0000005d 0000000000000000 
+# CHECK-NEXT:  2 FUNCTION       00000002 0000000000000000 
+# CHECK-NEXT:  3 GLOBAL         0000000b 0000000000000000 
+# CHECK-NEXT:  4 EXPORT         00000017 0000000000000000 
+# CHECK-NEXT:  5 CODE           00000019 0000000000000000 TEXT 
+# CHECK-NEXT:  6 DATA           0000001c 0000000000000000 DATA 
+# CHECK-NEXT:  7 .rodata..L.str 0000000d 0000000000000000 DATA 
+# CHECK-NEXT:  8 .bss.var       00000004 0000000000000000 
+# CHECK-NEXT:  9 reloc.CODE     00000017 0000000000000000 
+# CHECK-NEXT: 10 linking        00000041 0000000000000000 
 
 # RUN: llvm-objdump -p %p/Inputs/trivial.obj.wasm | FileCheck %s -check-prefix CHECK-HEADER
 

tools/llvm-readobj/WasmDumper.cpp

 void WasmDumper::printSections() {
   ListScope Group(W, "Sections");
   for (const SectionRef &Section : Obj->sections()) {
+    if (Obj->isSectionWasmDataSegment(Section))
+      continue;    
     const WasmSection &WasmSec = Obj->getWasmSection(Section);
     DictScope SectionD(W, "Section");
     W.printEnum("Type", WasmSec.Type, makeArrayRef(WasmSectionTypes));

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;