[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 ↗

(On Diff #131542)

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 ↗

(On Diff #130773)

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 ↗

(On Diff #130773)

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

1046

Just use getSectionContents() here?

1052

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

1129

remove extra newline

tools/llvm-readobj/WasmDumper.cpp

148 ↗

(On Diff #131732)

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 ↗

(On Diff #130773)

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

public:
   std::error_code getSectionContents(DataRefImpl Sec,
                                      StringRef &Res) const override;
   uint64_t getSectionAlignment(DataRefImpl Sec) const override;
   bool isSectionCompressed(DataRefImpl Sec) const override;
   bool isSectionText(DataRefImpl Sec) const override;
   bool isSectionData(DataRefImpl Sec) const override;
   bool isSectionBSS(DataRefImpl Sec) const override;
   bool isSectionVirtual(DataRefImpl Sec) const override;
-  bool isSectionBitcode(DataRefImpl Sec) const override;
   relocation_iterator section_rel_begin(DataRefImpl Sec) const override;
   relocation_iterator section_rel_end(DataRefImpl Sec) const override;
 
   // Overrides from RelocationRef.
   void moveRelocationNext(DataRefImpl &Rel) const override;
   uint64_t getRelocationOffset(DataRefImpl Rel) const override;
   symbol_iterator getRelocationSymbol(DataRefImpl Rel) const override;
   uint64_t getRelocationType(DataRefImpl Rel) const override;
   void getRelocationTypeName(DataRefImpl Rel,
                              SmallVectorImpl<char> &Result) const override;
 
   section_iterator section_begin() const override;
   section_iterator section_end() const override;
   uint8_t getBytesInAddress() const override;
   StringRef getFileFormatName() const override;
   Triple::ArchType getArch() const override;
   SubtargetFeatures getFeatures() const override;
   bool isRelocatableObject() const override;
 
 private:
   bool isValidFunctionIndex(uint32_t Index) const;
   const WasmSection &getWasmSection(DataRefImpl Ref) const;
   const wasm::WasmRelocation &getWasmRelocation(DataRefImpl Ref) const;
 
+  const wasm::WasmDataSegment &getWasmDataSegment(DataRefImpl Ref) const;
+
+  const ArrayRef<uint8_t> &getDataRefContent(DataRefImpl Ref) const;
+
   WasmSection* findCustomSectionByName(StringRef Name);
   WasmSection* findSectionByType(uint32_t Type);
 
   const uint8_t *getPtr(size_t Offset) const;
   Error parseSection(WasmSection &Sec);
   Error parseCustomSection(WasmSection &Sec, const uint8_t *Ptr,
                            const uint8_t *End);
 
private:
   uint32_t StartFunction = -1;
   bool HasLinkingSection = false;
   wasm::WasmLinkingData LinkingData;
   uint32_t NumImportedGlobals = 0;
   uint32_t NumImportedFunctions = 0;
   uint32_t ImportSection = 0;
   uint32_t ExportSection = 0;
 
-  StringMap<uint32_t> SymbolMap;
+  StringMap<uint32_t> SymbolMap;  
 };
 
 } // end namespace object
 
 inline raw_ostream &operator<<(raw_ostream &OS,
                                const object::WasmSymbol &Sym) {
   Sym.print(OS);
   return OS;
 }
 
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_WASM_H

lib/MC/WasmObjectWriter.cpp

void WasmObjectWriter::writeDataSection(ArrayRef<WasmDataSegment> Segments) {
 
   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
+
+    uint64_t Padding = OffsetToAlignment(getStream().tell() + 5, Segment.Alignment);
+    unsigned SizeLen = encodeULEB128(Segment.Data.size() + Padding, getStream(), 5); // size
+    assert(SizeLen == 5);
+    
+    WriteZeros(Padding);
+    assert(OffsetToAlignment(getStream().tell(), Segment.Alignment) == 0 && "DataSegments is not aligned");
     Segment.Section->setSectionOffset(getStream().tell() - Section.ContentsOffset);
     writeBytes(Segment.Data); // data
   }
 
   // Apply fixups.
   applyRelocations(DataRelocations, Section.ContentsOffset);
 
   endSection(Section);
 }
 
 void WasmObjectWriter::writeNameSection(ArrayRef<WasmFunction> Functions,
                                         ArrayRef<WasmImport> Imports) {
   uint32_t TotalFunctions = NumFunctionImports + Functions.size();
   if (TotalFunctions == 0)
     return;
 
   SectionBookkeeping Section;
   startSection(Section, wasm::WASM_SEC_CUSTOM, "name");
   SectionBookkeeping SubSection;
   startSection(SubSection, wasm::WASM_NAMES_FUNCTION);
 
   encodeULEB128(TotalFunctions, getStream());
   uint32_t Index = 0;
   for (const WasmImport &Import : Imports) {

lib/Object/WasmObjectFile.cpp

 #include "llvm/Object/Error.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/SymbolicFile.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/LEB128.h"
+#include "llvm/Support/MathExtras.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <cstring>
 #include <system_error>
 
 #define DEBUG_TYPE "wasm-object"
 

 
 Expected<section_iterator>
 WasmObjectFile::getSymbolSection(DataRefImpl Symb) const {
   DataRefImpl Ref;
   Ref.d.a = getWasmSymbol(Symb).Section;
   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 {
   const WasmSection &S = Sections[Sec.d.a];
 #define ECase(X)                                                               \
   case wasm::WASM_SEC_##X:                                                     \
     Res = #X;                                                                  \
     break
   switch (S.Type) {
     ECase(TYPE);
     ECase(IMPORT);
     ECase(FUNCTION);
     ECase(TABLE);
     ECase(MEMORY);
     ECase(GLOBAL);
     ECase(EXPORT);
     ECase(START);
     ECase(ELEM);
     ECase(CODE);
-    ECase(DATA);
+  case wasm::WASM_SEC_DATA: {
+    const WasmSegment &Seg = DataSegments[Sec.d.b];
+    Res = Seg.Data.Name;
+    break;
+  }
   case wasm::WASM_SEC_CUSTOM:
     Res = S.Name;
     break;
   default:
     return object_error::invalid_section_index;
   }
 #undef ECase
   return std::error_code();
 }
 
 uint64_t WasmObjectFile::getSectionAddress(DataRefImpl Sec) const { return 0; }
 
 uint64_t WasmObjectFile::getSectionIndex(DataRefImpl Sec) const {
   return Sec.d.a;
 }
 
 uint64_t WasmObjectFile::getSectionSize(DataRefImpl Sec) const {
-  const WasmSection &S = Sections[Sec.d.a];
-  return S.Content.size();
+  return getDataRefContent(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 = getDataRefContent(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 = getWasmDataSegment(Sec);
+    return Data.Alignment;
+  }
+
+  return 1;  
 }
 
+const wasm::WasmDataSegment &WasmObjectFile::getWasmDataSegment(DataRefImpl Sec) const {
+  assert(isSectionData(Sec) && "Section should be Data");
+  assert(Sec.d.b < DataSegments.size());
+  return DataSegments[Sec.d.b].Data;
+}
+
+const ArrayRef<uint8_t> &WasmObjectFile::getDataRefContent(DataRefImpl Sec) const {
+  if (isSectionData(Sec)) {
+    const wasm::WasmDataSegment &Data = getWasmDataSegment(Sec);
+    size_t Padding = alignmentAdjustment(Data.Content.data(), Data.Alignment);
+
+    ArrayRef<uint8_t> AdjustedContent(Data.Content.data() + Padding, Data.Content.size() - Padding);
+    return std::move(AdjustedContent);
+  }
+  // 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 {
   return false;
 }
 
 bool WasmObjectFile::isSectionText(DataRefImpl Sec) const {
   return getWasmSection(Sec).Type == wasm::WASM_SEC_CODE;
 }
 
 bool WasmObjectFile::isSectionData(DataRefImpl Sec) const {
   return getWasmSection(Sec).Type == wasm::WASM_SEC_DATA;
 }
 
 bool WasmObjectFile::isSectionBSS(DataRefImpl Sec) const { return false; }
 
 bool WasmObjectFile::isSectionVirtual(DataRefImpl Sec) const { return false; }
 
-bool WasmObjectFile::isSectionBitcode(DataRefImpl Sec) const { return false; }
-
 relocation_iterator WasmObjectFile::section_rel_begin(DataRefImpl Ref) const {
   DataRefImpl RelocRef;
   RelocRef.d.a = Ref.d.a;
   RelocRef.d.b = 0;
   return relocation_iterator(RelocationRef(RelocRef, this));
 }
 
 relocation_iterator WasmObjectFile::section_rel_end(DataRefImpl Ref) const {
uint64_t WasmObjectFile::getRelocationOffset(DataRefImpl Ref) const {
   return Rel.Offset;
 }
 
 symbol_iterator WasmObjectFile::getRelocationSymbol(DataRefImpl Rel) const {
   llvm_unreachable("not yet implemented");
   SymbolRef Ref;
   return symbol_iterator(Ref);
 }
 
 uint64_t WasmObjectFile::getRelocationType(DataRefImpl Ref) const {
   const wasm::WasmRelocation &Rel = getWasmRelocation(Ref);
   return Rel.Type;
 }
 
 void WasmObjectFile::getRelocationTypeName(
     DataRefImpl Ref, SmallVectorImpl<char> &Result) const {
   const wasm::WasmRelocation& Rel = getWasmRelocation(Ref);
   StringRef Res = "Unknown";
 
 #define WASM_RELOC(name, value)  \
   case wasm::name:              \
     Res = #name;               \
     break;
 
   switch (Rel.Type) {
 #include "llvm/BinaryFormat/WasmRelocs.def"
   }
 
 #undef WASM_RELOC
 
   Result.append(Res.begin(), Res.end());
 }
 
 section_iterator WasmObjectFile::section_begin() const {
   DataRefImpl Ref;
   Ref.d.a = 0;
+  Ref.d.b = 0;
   return section_iterator(SectionRef(Ref, this));
 }
 
 section_iterator WasmObjectFile::section_end() const {
   DataRefImpl Ref;
   Ref.d.a = Sections.size();
   return section_iterator(SectionRef(Ref, this));
 }