Use BOOL (vs. bool) in event pipe qcall signatures

[VSadov]

BOOL is preferred in QCALL signatures since it has well defined size (int32) while c++ bool is implementation specific and is typically 1 byte.
It looks like we expect int32 (UnmanagedType.Bool) on the managed side as well.

runtime/src/coreclr/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipe.CoreCLR.cs

Line 57 in e67e4da

[return: MarshalAs(UnmanagedType.Bool)]

This fixes #62145 for me.

I am not sure why this is not causing failures in non-singlefile case. It should though.
Perhaps some differences in initialization or how calls are bound have a mitigating effect on non-singlefile.

[AaronRobinsonMSFT]

FYI @jkoritzinsky @elinor-fung

[VSadov]

Thanks!!

[VSadov]

/backport to release/7.0

[github-actions]

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/2917168012

[nr-ahemsath]

Is there any chance this fix will be backported to .NET 6?

[AaronRobinsonMSFT]

@nr-ahemsath We would need to do a bar check, but given the size of this change, I don't think it would get much pushback. Are you sure this is the same issue?

[nr-ahemsath]

@AaronRobinsonMSFT Thanks for responding so quickly! I'm not 100% sure the issue I'm seeing is the same as this, but it seems pretty likely, based on the following:

1. The stack trace from the exception we get is very similar to the one in issue AccessViolationException with PublishSingleFile #62145:

Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at System.Diagnostics.Tracing.EventPipePayloadDecoder.DecodePayload(EventMetadata ByRef, System.ReadOnlySpan`1<Byte>)
   at System.Diagnostics.Tracing.EventPipeEventDispatcher.DispatchEventsToEventListeners()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(System.Threading.Tasks.Task ByRef, System.Threading.Thread)

2. Our .NET APM agent does use custom EventListeners to get resource usage data from the CLR of the applications we are instrumenting, e.g.: https://github.com/newrelic/newrelic-dotnet-agent/blob/580657f772f75cad1c1ec78d62bf2db956b7da36/src/Agent/NewRelic/Agent/Core/Samplers/GCSamplerNetCore.cs#L137
3. I am able to reproduce the exception 100% (3/3) when I try to use our agent to profile a .NET 6.0 application published with the PublishSingleFile option. It apparently can still occur without that option, just very sporadically.

Do you have any suggestions for how I can positively confirm that this is the same issue? Is there an overnight dev build of .NET 7.0 with this fix that I could grab (maybe in a docker image?), or would I have to build the runtime from source? Sorry, I should have taken 30 seconds to read the README.md at the top level of this repo. I'll test with the latest .NET 7 by following the instructions here: https://github.com/dotnet/runtime/blob/main/docs/project/dogfooding.md

[AaronRobinsonMSFT]

@nr-ahemsath The "nightly" builds can be found at https://github.com/dotnet/installer/#table, but it seems this fixed hasn't made it into that build yet. Probably next week? If you can confirm this fixes the problem, please let me know and I can start the process.

[AaronRobinsonMSFT]

Also, I'd recommend creating another issue and tagging this one if a port to .NET 6 is requested.

[nr-ahemsath]

@AaronRobinsonMSFT Okay, thanks. Yeah, I tested with the latest RC build and it still crashes, so thanks for confirming that the fix hasn't made it in yet. I will create another issue and tag this one if I can confirm that this PR fixes our issue.

[VSadov]

/backport to release/6.0

[github-actions]

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/4009666237

[github-actions]

@VSadov backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Use BOOL (vs. bool) in event pipe qcall signatures
Using index info to reconstruct a base tree...
M	src/coreclr/vm/eventpipeinternal.cpp
M	src/coreclr/vm/eventpipeinternal.h
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/vm/eventpipeinternal.h
CONFLICT (content): Merge conflict in src/coreclr/vm/eventpipeinternal.h
Auto-merging src/coreclr/vm/eventpipeinternal.cpp
CONFLICT (content): Merge conflict in src/coreclr/vm/eventpipeinternal.cpp
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Use BOOL (vs. bool) in event pipe qcall signatures
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

[github-actions]

@VSadov an error occurred while backporting to release/6.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.