Implement break opcode as llvm.debugtrap

[Blealtan]

Implemented break opcode as llvm.debugtrap. Not adding any test cases since it seems that no C# codes generates break opcode directly.

[hippiehunter]

There is an IL section (cpobj) of the hellowasm test that you can bolt on a test for this.

I'm more than a little curious what the experience is like when running this instruction in WASM.

[morganbr]

A good way to both test this and make it useful could be to make Debugger.Break() work. It calls into Debugger.DebugBreak, which is an intrinsic that gets a generated break opcode.

The logic currently checks IsDebuggerPresent, which is hardcoded to false in the Unix PAL

corert/src/Native/Runtime/unix/PalRedhawkUnix.cpp

Lines 1045 to 1048 in 2aaccdf

	extern "C" UInt32_BOOL IsDebuggerPresent()
	{
	// UNIXTODO: Implement this function
	return UInt32_FALSE;

If that were changed to true for WASM, I think there wouldn't be any other problems. To include a test for this in HelloWasm, we'd have to be sure it wouldn't break if a debugger isn't attached so we don't lock up the test.

[Blealtan]

@hippiehunter As I've mentioned in #4511 , in Emscripten it's implemented as a round-trip call to statement debugger; in JS, and in LLVM it's simply implemented as WASM instruction unreachable. For now we're using Emscripten, and browser will pause at the debugger; statement if devtools window is opened (I've tested in Firefox and Chrome, my Edge DevTools is broken unexpectedly).

@morganbr Testing if a debugger is present by our self seems to be unnecessary: both Firefox and Chrome only pauses at debugger; statement when devtools window is opened. We can always assume that there is a debugger attached, since if not, it'll neither lock up the test. I'll try on that.

[Blealtan]

@morganbr It works, Debugger.Break() compiles and only pauses the execution with developer windows opened once changed the line you mentioned. However, this is also used by the Unix port which has not implementated debugger, right? Maybe we'd add a conditional compilation for WASM?

[Blealtan]

Another problem is, how will we deal with Debugger._isDebuggerAttached? It's said that the debugger is responsible to set it true, but WASM debugger is obviously not implemented with CoreRT in mind, and we have no way to know whether a debugger is attached (that is, devtools window opened). Maybe always set it to true since debugger; in JS itself doesn't function without debugger attached?

[morganbr]

Good question. on further thought, maybe Debugger.Break should just conditionally not call IsDebuggerPresent. I think you should be able to use #if WASM. We might be able to come back to Debugger._isDebuggerAttached by writing some Javascript for it, but from a quick search, I get the impression it varies by browser and might be a bit fragile.

[Blealtan]

It seems that Debugger.Break assumes Debugger._isDebuggerAttached is always true in case of IsDebuggerPresent returns true according to the comments there. Anyway I'll test the #if WASM and if okay, commit it here.

[Blealtan]

#if WASM doesn't work, neither does #ifdef WASM. Is this the correct macro name? Or similar macro by now doesn't exist?

[jkotas]

It is #ifdef _WASM_

[morganbr]

Looks good. I've tested it in Edge, Firefox and Chrome and it stops under their debuggers and not without them.

[morganbr]

Thanks, @Blealtan! I can already tell this will be incredibly useful for debugging until we have more complete debugger support. In particular, this makes the Chrome debugger really useful since it has the best WASM function output, but no search. Using this, we can put Debugger.Break() in interesting places without having to figure out what number every function ended up with.

Can I interest you in picking up another issue next?