clang generates incorrect wasm object code from .s input

[TerrorJack]

Minimal repro:

foo:
.functype foo () -> (i32)
i32.const 420
if i32
  i32.const 421
  return
else
  i32.const 423
end_if
end_function

Compile it with clang --target=wasm32-wasi -c test.s -o test.o, then disassemble with wasm-objdump -d test.o:

test.o:	file format wasm 0x1

Code Disassembly:

000041 func[0] <foo>:
 000042: 41 a4 03                   | i32.const 420
 000045: 04 7f                      | if i32
 000047: 41 a5 03                   |   i32.const 421
 00004a: 0f                         |   return
 00004b: 41 a7 03                   |   i32.const 423
 00004e: 0b                         | end
 00004f: 0b                         | end

else has been incorrectly dropped.

This is confirmed against both 14.0.6 and trunk.

[TerrorJack]

OK, if I use llvm-mc --triple=wasm32-wasi --filetype=obj test.s -o test.o -no-type-check to generate test.o then the error is gone.

The crime scene seems to be around

llvm-project/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

Line 1020 in 49d2071

if (!SkipTypeCheck && TC.typeCheck(IDLoc, Inst, Operands))

, when emitting else, asm type checker fails, so the else opcode is omitted, without failing or warning.

@aardappel Sorry for the poke, since I believe you're most familiar with this part of codebase, do you think this particular case can be fixed? Or at least, is there a way I can pass -no-type-check to clang?

[llvmbot]

@llvm/issue-subscribers-backend-webassembly

[sbc100]

In answer to your question about passing no-type-check to clang it looks like it is not yet possible. The only way to set MCNoTypeCheck seems to be from llvm/lib/MC/MCTargetOptionsCommandFlags.cpp and not from clang/lib/CodeGen/BackendUtil.cpp. I think it would need to be piped from -mno-type-check through to clang/lib/CodeGen/BackendUtil.cpp.

[aardappel]

I'm sure it can be fixed.. first step would be to see a) why that call to typeCheck fails on else, and b) make sure it doesn't do so without emitting an error for it.

I am guessing you're not getting errors because of --filetype=obj maybe? It probably is saying something like i32 expected on the stack or similar. I am not sure what causes there a .o be generated when there are errors, that should probably be fixed first. @sbc100 @dschuff

Looking at the code, the processing of else calls checkEnd which wants to see an i32 on the stack as indicated by the signature of if, but that was likely already removed by return. Looks like the type checker needs better support for dealing with this unreachable code type state, maybe by having return push an "unreachable" value, and the popping code accepting that instead of erroring.

[sbc100]

Work in process CL to fix the passing of -Wa,-no-type-check to clang: https://reviews.llvm.org/D131217

[aardappel]

Wouldn't it be better to fix the underlying issue instead? Turning it off can make it really easy for broken code to generated that is hard to debug.

[sbc100]

I think we need to have all 3 fixed in place:

1. make sure errors are not ignored
2. have a way to ignore errors if needed
3. fix the underlying bug

I'm just started with (2) because its fairly simple and prevents the need for such workarounds as WebAssembly/wasi-sdk#248

[sbc100]

I think its fairly inevitable that there will be more bugs in the type checker as we add more features, so having a way to turn it off for users that run into such bugs (until they can be fixed) seems like it will be useful going fowards.

[TerrorJack]

It would be very nice to get at least D131217 merged; llvm-mc turns out to be an imperfect workaround given lack of CPP logic, we still need to invoke clang to process assembly files with #include directives.

[TerrorJack]

@sbc100 Thanks for the work on https://reviews.llvm.org/D131217, would it be possible to backport it to the 15.x branch and make it into the next 15.x release? I have a working patch that resolved the git conflict at https://github.com/tweag/wasi-sdk/blob/main/0001-clang-WebAssembly-Pass-Wa-no-type-check-through-to-t.patch, but I've no idea how to submit a patch for non-trunk branches.

[sbc100]

@TerrorJack, while it is possible to backport patched to release branched the bar for doing so is quite high. I think if we could fix the underlying bug in the assembly that might be more suitable backport, but I haven't gotten around to that fix yet.

BTW, can you remind my what you using the assembly format for? Are you generating it or hand writing it? Just out of interest.

[TerrorJack]

BTW, can you remind my what you using the assembly format for?

The GHC compiler's wasm32 backend will emit assembly code.