[Webassembly] consider invalid CALL_S in removeRegisterOperands

[HerrCai0907]

Summary

CALL_S at least need one operand as call func index. This case needs to
be identifer during removing register operands. Otherwise incorrect CALL_S
instruction will crash other part.
Invalid CALL_S will be replaced with UNREACHABLE

[HerrCai0907]

llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

99

I use arc diff to submit patch. It seems to format the whole file and cause this change.

[HerrCai0907]

format

[aheejin]

I think the problem is, the call is instruction-selected as CALL, when it is supposed to be selected as CALL_INDIRECT. FastIsel selects it with CALL_INDIRECT; you can see the test passes with -fast-isel. I tried to fix this in ISel in D147033. PTAL.

[HerrCai0907]

detail fix see https://reviews.llvm.org/D147033, only add assert in this patch to avoid further spread of errors

[aheejin]

In D146781#4227396, @HerrCai0907 wrote:

detail fix see https://reviews.llvm.org/D147033, only add assert in this patch to avoid further spread of errors

I'm not sure if it makes much sense to only check about CALLs condition at the end of the pipeline. If we want to do that, we should be doing check for all instructions, which is equivalent to running a checker. We have a type checker, but this is to ensure the validity of assembly files we parse. If we emit an invalid instruction and something crashes at the end, I think that means we should be fixing the bug that emitted the invalid instruction, rather than asserting for a single instruction here.

llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

 }
 
 MCOperand WebAssemblyMCInstLower::lowerSymbolOperand(const MachineOperand &MO,
                                                      MCSymbol *Sym) const {
   MCSymbolRefExpr::VariantKind Kind = MCSymbolRefExpr::VK_None;
   unsigned TargetFlags = MO.getTargetFlags();
 
   switch (TargetFlags) {
     case WebAssemblyII::MO_NO_FLAG:
       break;
     case WebAssemblyII::MO_GOT_TLS:
       Kind = MCSymbolRefExpr::VK_WASM_GOT_TLS;
       break;
     case WebAssemblyII::MO_GOT:
       Kind = MCSymbolRefExpr::VK_GOT;
       break;
     case WebAssemblyII::MO_MEMORY_BASE_REL:
static void removeRegisterOperands(const MachineInstr *MI, MCInst &OutMI) {
 
   // Remove register operands.
   for (auto I = OutMI.getNumOperands(); I; --I) {
     auto &MO = OutMI.getOperand(I - 1);
     if (MO.isReg()) {
       OutMI.erase(&MO);
     }
   }
+  // CALL_S should at least have 1 operand, otherwise this code is invalid.
+  assert(!(StackOpcode == WebAssembly::CALL_S && OutMI.getNumOperands() == 0));
 }