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

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 30 14:15:33 PST 2017


sbc100 marked an inline comment as done.
sbc100 added inline comments.


================
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;
+
----------------
davide wrote:
> 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).
This is exactly what the MachO and COFF dumpers do yes, and basically the same as the ELF dumper.

You are correct that in practice there is no way this can happen, at least not by running the tool itself.   In light of this I don't see how I can write a test to tickle this, and I think perhaps an assert would be more appropriate here?  


https://reviews.llvm.org/D27355





More information about the llvm-commits mailing list