[WebAssembly] Initial Disassembler.

[aardappel]

Summary

This implements a new table-gen emitter to create tables for
a wasm disassembler, and a dissassembler to use them.

Comes with 2 tests, that tests a few instructions manually. Is also able to
disassemble large .wasm files with objdump reasonably.

Not working so well, to be addressed in followups:

• objdump appears to be passing an incorrect starting point.
• since the disassembler works an instruction at a time, and it is disassembling stack instruction, it has no idea of pseudo register assignments. These registers are required for the instruction printing code that follows. For now, all such registers appear in the output as $0.

[aardappel]

This actually implements the disassembler, some parts not working
ideally, see comments.

[sbc100]

Is this ready to be submitted now?

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

71

Mark these internal functions as static?

71

Does it make sense to pass an arrayref by ref?

77

I general I think llvm style says not to use auto unless is immediately obvious what the type is (i.g. if there is a cast on the right hand side which already names the type).

I also wouldn't use auto for start above.

84

decodeULEB128 functions already detect malformed LEBs. I think you can avoid the entire readByte loop and just pass the error argument to decodeULEB128, along with the actual end of the Bytes array.

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

202

Formatting looks strange. git clang-format origin/master?

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.h

46

It looks like none of these functions use this new arg. Is it needed for some other reason?

lib/Target/WebAssembly/WebAssembly.td

87

Can this be 0 and avoid the new STI argument?

unittests/MC/Disassembler.cpp

66 ↗

(On Diff #145519)

Probably want a better if you want to add this new test :)

utils/TableGen/DisassemblerEmitter.cpp

133 ↗

(On Diff #145519)

This is a standalone function so it should start with capital letter I think

[aardappel]

It can be ready to submit if we agree that for the moment having non-sense registers in the output is ok (looks like this will need works elsewhere before that can be properly fixed).
It could also probably need more tests. I haven't run it on a larger binary yet.

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

71

No, it doesn't.. it was originally mutable, forgot to change it.

84

You're totally right.. I had initially assumed there was a possibility of it reading past the end.

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

202

did git clang-format HEAD^ since I am maybe behind on master..

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.h

46

Because of the changes I made in the .td files, the generated code that interfaces with this code now expects these extra arguments. No idea why.

lib/Target/WebAssembly/WebAssembly.td

87

Ahh that's where that came from, thanks :)

unittests/MC/Disassembler.cpp

66 ↗

(On Diff #145519)

I presume you mean a better name.. fixed.

utils/TableGen/DisassemblerEmitter.cpp

133 ↗

(On Diff #145519)

I don't see any mention of special treatmeant of stand-alone functions in https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
In fact, it has an example that happens to be a stand-alone function which starts with lowercase.

[aardappel]

Fixes due to sbc100 review + clang-format.

[sbc100]

I don't think there is a problem with landing a partial implementation. Up to you I guess.
I'm wondering if this change will be enough for me to use it in some of the existing tests where are force to use obj2yaml.

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

120

At least to my eye this is overuse of auto, but i can't see what the type of OT is here.

See: https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

[aardappel]

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

120

I'm an "maximally auto" kind of person so I may miss these on occasion :)

[aardappel]

Forgot test file (wasm.txt) + 1 more review fix.

[dschuff]

haven't looked at most of the code, but thought about the register operand issue.

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

151

I think we will need to generate the register assignments, either here or in a post-processing step. I don't think it currently makes sense to have a second version of all the instructions with implicit operands (even though they wouldn't be needed by MC in explicit-locals mode).

You'd think it ought to be as simple as running a "pass" over the MCInsts, but last I checked there wasn't really any facility for running passes over MC; it's structured much more like a stream. (and looking at the use of getInstruction in e.g. LLVMDisasmInstruction in MCDisassembler or in llvm-objdump, it looks like the returned MCInsts just get passed directly through to the printer. We could maybe hang some state off of the WebAssemblyDisassembler?

[aardappel]

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

151

Yes, we can easily keep a stack to track assignment + types in WebAssemblyDisassembler, but we'd be making the assumption that instructions are always disassembled in order. To accomodate this code being called for disassembling single instructions, this code would have to be "robust" and still function if the stack is empty etc.

More-over to get correct types we'd need access to locals/args, which we don't have. We currently don't even know where a function starts and stops.

Then, in table-gen there is currently one instruction per return type, i.e. i32.call vs i64.call, which are actually the same instruction. We'd either need to strip these types in a post-process, or we'd need to output more complicated tables that contain all of them, so the dissassembler can pick the right one based on stack type information, if it has any.

Since this isn't quite trivial, I suggest I land this basic disassembler (which will at least allow objdump etc to show something useful), then experiment with the above in follow-ups.

[dschuff]

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

151

More-over to get correct types we'd need access to locals/args, which we don't have. We currently don't even know where a function starts and stops.

Ah yeah, that's the really unfortunate bit that I hadn't thought about.

Then, in table-gen there is currently one instruction per return type, i.e. i32.call vs i64.call, which are actually the same instruction.

This is mostly just for call, right? It looks all the flavors have the same mnemonic, so maybe we can just fake it or something.

Since this isn't quite trivial, I suggest I land this basic disassembler (which will at least allow objdump etc to show something useful), then experiment with the above in follow-ups.

Yeah I think that makes sense. This seems like a reasonable unit of code to put in, do you want to remove the DO NOT MERGE and consider it ready then? I like that this is one of the few areas of LLVM that can get proper unit tests. We should maybe also add at least a basic mc test as well though to ensure things can be hooked up end-to-end. See e.g. test/MC/Disassembler, but we can probably put the most coverage in the unit tests.

[sbc100]

test/MC/Disassembler/WebAssembly/wasm.txt

1 ↗

(On Diff #145578)

Can you drop the -elf? Its hopefully not long for this world anyway.

[aardappel]

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

151

You're right, it is actually few instructions: call, call_indirect, select. The first two already have a shorter version in the tables, so if I on purpose select the one with the least operands, only select remains? So maybe we can add an instruction for that one.. then we can largely ignore types.

I have a bit more testing I want to do with objdump before we can merge this, I will let y'all know when its ready.

There are already 2 tests here, one in unittests/MC/Disassembler.cpp and one in test/MC/Disassembler/WebAssembly/wasm.txt, see below. The latter in particular could use more tests I am sure.

[aardappel]

Last review fix.

lib/Target/WebAssembly/CMakeLists.txt

 set(LLVM_TARGET_DEFINITIONS WebAssembly.td)
 
 tablegen(LLVM WebAssemblyGenAsmMatcher.inc -gen-asm-matcher)
 tablegen(LLVM WebAssemblyGenAsmWriter.inc -gen-asm-writer)
 tablegen(LLVM WebAssemblyGenDAGISel.inc -gen-dag-isel)
+tablegen(LLVM WebAssemblyGenDisassemblerTables.inc -gen-disassembler)
 tablegen(LLVM WebAssemblyGenFastISel.inc -gen-fast-isel)
 tablegen(LLVM WebAssemblyGenInstrInfo.inc -gen-instr-info)
 tablegen(LLVM WebAssemblyGenMCCodeEmitter.inc -gen-emitter)
 tablegen(LLVM WebAssemblyGenRegisterInfo.inc -gen-register-info)
 tablegen(LLVM WebAssemblyGenSubtargetInfo.inc -gen-subtarget)
 
 add_public_tablegen_target(WebAssemblyCommonTableGen)
 

lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp

 /// MCInsts.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/WebAssemblyMCTargetDesc.h"
 #include "WebAssembly.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
+#include "llvm/MC/MCFixedLenDisassembler.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/TargetRegistry.h"
+
 using namespace llvm;
 
 #define DEBUG_TYPE "wasm-disassembler"
 
+using DecodeStatus = MCDisassembler::DecodeStatus;
+
+#include "WebAssemblyGenDisassemblerTables.inc"
+
 namespace {
 class WebAssemblyDisassembler final : public MCDisassembler {
   std::unique_ptr<const MCInstrInfo> MCII;
 
   DecodeStatus getInstruction(MCInst &Instr, uint64_t &Size,
                               ArrayRef<uint8_t> Bytes, uint64_t Address,
                               raw_ostream &VStream,
                               raw_ostream &CStream) const override;
extern "C" void LLVMInitializeWebAssemblyDisassembler() {
   // Register the disassembler for each target.
   TargetRegistry::RegisterMCDisassembler(getTheWebAssemblyTarget32(),
                                          createWebAssemblyDisassembler);
   TargetRegistry::RegisterMCDisassembler(getTheWebAssemblyTarget64(),
                                          createWebAssemblyDisassembler);
 }
 
 MCDisassembler::DecodeStatus WebAssemblyDisassembler::getInstruction(
-    MCInst &MI, uint64_t &Size, ArrayRef<uint8_t> Bytes, uint64_t /*Address*/,
+    MCInst &MI, uint64_t &Size, ArrayRef<uint8_t> Bytes, uint64_t Address,
     raw_ostream &OS, raw_ostream &CS) const {
-
-  // TODO: Implement disassembly.
+  CommentStream = &CS;
+  // We want to read at least 1 byte of data.
+  if (Bytes.size() < 1) {
+    Size = 0;
+    return MCDisassembler::Fail;
+  }
+  uint32_t Insn = Bytes[0];
+  Size = 1;  // FIXME
+  // Calling the auto-generated decoder function.
+  DecodeStatus Result =
+      decodeInstruction(DecoderTable8, MI, Insn, Address, this, STI);
+  if (Result != MCDisassembler::Fail) {
+    return checkDecodedInstruction(MI, Size, Address, OS, CS, Insn, Result);
+  }
 
   return MCDisassembler::Fail;
 }
+

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.h

public:
   WebAssemblyInstPrinter(const MCAsmInfo &MAI, const MCInstrInfo &MII,
                          const MCRegisterInfo &MRI);
 
   void printRegName(raw_ostream &OS, unsigned RegNo) const override;
   void printInst(const MCInst *MI, raw_ostream &OS, StringRef Annot,
                  const MCSubtargetInfo &STI) override;
 
   // Used by tblegen code.
-  void printOperand(const MCInst *MI, unsigned OpNo, raw_ostream &O);
+  void printOperand(const MCInst *MI, unsigned OpNo, const MCSubtargetInfo &STI,
+                    raw_ostream &O);
   void printWebAssemblyP2AlignOperand(const MCInst *MI, unsigned OpNo,
+                                      const MCSubtargetInfo &STI,
                                       raw_ostream &O);
   void printWebAssemblySignatureOperand(const MCInst *MI, unsigned OpNo,
+                                        const MCSubtargetInfo &STI,
                                         raw_ostream &O);
 
   // Autogenerated by tblgen.
-  void printInstruction(const MCInst *MI, raw_ostream &O);
+  void printInstruction(const MCInst *MI, const MCSubtargetInfo &STI,
+                        raw_ostream &O);
   static const char *getRegisterName(unsigned RegNo);
 };
 
 namespace WebAssembly {
 
 const char *TypeToString(MVT Ty);
 const char *TypeToString(wasm::ValType Type);
 
 } // end namespace WebAssembly
 
 } // end namespace llvm
 
 #endif

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

void WebAssemblyInstPrinter::printRegName(raw_ostream &OS,
                                           unsigned RegNo) const {
   assert(RegNo != WebAssemblyFunctionInfo::UnusedReg);
   // Note that there's an implicit get_local/set_local here!
   OS << "$" << RegNo;
 }
 
 void WebAssemblyInstPrinter::printInst(const MCInst *MI, raw_ostream &OS,
                                        StringRef Annot,
-                                       const MCSubtargetInfo & /*STI*/) {
+                                       const MCSubtargetInfo &STI) {
   // Print the instruction (this uses the AsmStrings from the .td files).
-  printInstruction(MI, OS);
+  printInstruction(MI, STI, OS);
 
   // Print any additional variadic operands.
   const MCInstrDesc &Desc = MII.get(MI->getOpcode());
   if (Desc.isVariadic())
     for (auto i = Desc.getNumOperands(), e = MI->getNumOperands(); i < e; ++i) {
       // FIXME: For CALL_INDIRECT_VOID, don't print a leading comma, because
       // we have an extra flags operand which is not currently printed, for
       // compatiblity reasons.
       if (i != 0 &&
           (MI->getOpcode() != WebAssembly::CALL_INDIRECT_VOID ||
            i != Desc.getNumOperands()))
         OS << ", ";
-      printOperand(MI, i, OS);
+      printOperand(MI, i, STI, OS);
     }
 
   // Print any added annotation.
   printAnnotation(OS, Annot);
 
   if (CommentStream) {
     // Observe any effects on the control flow stack, for use in annotating
     // control flow label references.
auto Written = FP.convertToHexString(
       buf, /*hexDigits=*/0, /*upperCase=*/false, APFloat::rmNearestTiesToEven);
   (void)Written;
   assert(Written != 0);
   assert(Written < BufBytes);
   return buf;
 }
 
 void WebAssemblyInstPrinter::printOperand(const MCInst *MI, unsigned OpNo,
+                                          const MCSubtargetInfo &/*STI*/,
                                           raw_ostream &O) {
   const MCOperand &Op = MI->getOperand(OpNo);
   if (Op.isReg()) {
     assert((OpNo < MII.get(MI->getOpcode()).getNumOperands() ||
             MII.get(MI->getOpcode()).TSFlags == 0) &&
            "WebAssembly variable_ops register ops don't use TSFlags");
     unsigned WAReg = Op.getReg();
     if (int(WAReg) >= 0)
if (Op.isReg()) {
     assert(Op.isExpr() && "unknown operand kind in printOperand");
     Op.getExpr()->print(O, &MAI);
   }
 }
 
 void
 WebAssemblyInstPrinter::printWebAssemblyP2AlignOperand(const MCInst *MI,
                                                        unsigned OpNo,
+                                                       const MCSubtargetInfo &
+                                                         STI,
                                                        raw_ostream &O) {
   int64_t Imm = MI->getOperand(OpNo).getImm();
   if (Imm == WebAssembly::GetDefaultP2Align(MI->getOpcode()))
     return;
   O << ":p2align=" << Imm;
 }
 
 void
 WebAssemblyInstPrinter::printWebAssemblySignatureOperand(const MCInst *MI,
                                                          unsigned OpNo,
+                                                         const MCSubtargetInfo &
+                                                           STI,
                                                          raw_ostream &O) {
   int64_t Imm = MI->getOperand(OpNo).getImm();
   switch (WebAssembly::ExprType(Imm)) {
   case WebAssembly::ExprType::Void: break;
   case WebAssembly::ExprType::I32: O << "i32"; break;
   case WebAssembly::ExprType::I64: O << "i64"; break;
   case WebAssembly::ExprType::F32: O << "f32"; break;
   case WebAssembly::ExprType::F64: O << "f64"; break;

lib/Target/WebAssembly/WebAssembly.td

 // Target Declaration
 //===----------------------------------------------------------------------===//
 
 def WebAssemblyAsmParser : AsmParser {
   // The physical register names are not in the binary format or asm text
   let ShouldEmitMatchRegisterName = 0;
 }
 
+def WebAssemblyAsmWriter : AsmWriter {
+  string AsmWriterClassName  = "InstPrinter";
+  int PassSubtarget = 1;
+  int Variant = 0;
+  bit isMCAsmWriter = 1;
+}
+
 def WebAssembly : Target {
   let InstructionSet = WebAssemblyInstrInfo;
   let AssemblyParsers  = [WebAssemblyAsmParser];
+  let AssemblyWriters = [WebAssemblyAsmWriter];
 }

lib/Target/WebAssembly/WebAssemblyInstrFormats.td

 /// \file
 /// \brief WebAssembly instruction format definitions.
 ///
 //===----------------------------------------------------------------------===//
 
 // WebAssembly Instruction Format.
 class WebAssemblyInst<bits<32> inst, string asmstr> : Instruction {
   field bits<32> Inst = inst; // Instruction encoding.
+  field bits<32> SoftFail = 0;  // Needed for disassembler?
+  let Size        = 1;  // Needed for disassembler?
   let Namespace   = "WebAssembly";
   let Pattern     = [];
   let AsmString   = asmstr;
 }
 
 // Normal instructions.
 class I<dag oops, dag iops, list<dag> pattern, string asmstr = "", bits<32> inst = -1>
     : WebAssemblyInst<inst, asmstr> {