[PATCH] D27355: [WebAssembly] Add wasm support for llvm-readobj

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 24 07:41:07 PST 2017


davide added inline comments.


================
Comment at: test/tools/llvm-readobj/file-headers.test:29-31
+RUN: llvm-readobj -h %p/Inputs/trivial.obj.wasm \
+RUN:   | FileCheck %s -check-prefix WASM
 
----------------
Please add a comment exaplining how you created the source file. That is:
-> compiler/linker/any tool used invocation & source code. Thanks!
This is useful in case we need to regenerate the input object file.


================
Comment at: tools/llvm-readobj/WasmDumper.cpp:1
+//===-- WasmDumper.cpp - Object file dumping utility for llvm -------------===//
+//
----------------
You may want to mention `WASM Object file dumping utility` ?


================
Comment at: tools/llvm-readobj/WasmDumper.cpp:85-88
+  const WasmObjectFile *WasmObj = dyn_cast<WasmObjectFile>(Obj);
+  if (!WasmObj)
+    return readobj_error::unsupported_obj_file_format;
+
----------------
Can this always happen? You enter here under `Obj->isWasm()`.
If there's a legitimate where this can happen, I would add a test (if you don't mind).
Also, not entirely sure if `unsupported_obj_file_format` is the best diagnostic we can emit. Maybe `broken_file`? (I suspect this is consistent with what other dumpers are doing, if so, leave it as is but please add a test to exercise this code).


https://reviews.llvm.org/D27355





More information about the llvm-commits mailing list