[PATCH] D54924: [WebAssembly] Check if the section order is correct

Fri Dec 14 15:54:09 PST 2018

aheejin marked 6 inline comments as done.
aheejin added inline comments.

================
Comment at: include/llvm/Object/Wasm.h:311
+    // Must come after "linking" section in order to validate reloc indexes.
+    WASM_SEC_ORDER_RELOC = 101,
+    // "name" section must appear after DATA. Comes after "linking" to allow
----------------
aheejin wrote:
> aheejin wrote:
> > I copied this comments on orders (linking section should be after data section and reloc sections should be after linking section) from D44024. Are these still valid? If so, do you think we should specify them in [Linking spec](https://github.com/WebAssembly/tool-conventions/blob/master/Linking.md)?
> Oh after deleting some code the comment location got weird. What I meant was the comments about relationship of data - "reloc" and "reloc" - "linking".
Are these explanations about data - "reloc" and "reloc" - "linking" ordering still correct?

================
Comment at: lib/Object/WasmObjectFile.cpp:278
+    uint32_t SecType = peakUint8(Ctx);
+    if (!Checker.isValidSectionOrder(SecType)) {
+      Err = make_error<StringError>("Out of order section type: " +
----------------
sbc100 wrote:
> If this is only passing the section type, not the section name, I can't see how it can check the ordering for custom sections?
> 
> Should we move this change down in "parseSection" so it can access the name and ID of custom sections?
Oh right, I forgot that... Moved the checking code into `readSection`. And also fixed it in yaml2wasm side too, in which case I didn't need to move the checking code into a function though.

================
Comment at: tools/yaml2obj/CMakeLists.txt:2
 set(LLVM_LINK_COMPONENTS
+  BinaryFormat
   DebugInfoCodeView
----------------
sbc100 wrote:
> Is this change still needed?
Nope. Deleted it, thanks.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54924/new/

https://reviews.llvm.org/D54924