Added DEBUG switches for roundtrip ILASM

[TIHan]

Description

Should resolve: #72127 - this was failing because ilasm.exe was not being invoked with the /DEBUG flag when the corresponding project was.

Acceptance Criteria

• ILASM Roundtrip CI does not have a failure on poison.sh
• No additional ILASM roundtrip failures

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[TIHan]

/azp run runtime-coreclr ilasm

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[TIHan]

/azp run runtime-coreclr ilasm

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Should resolve: #72127 - this was failing because ilasm.exe was not being invoked with the /DEBUG flag when the corresponding project was.

Acceptance Criteria

• ILASM Roundtrip CI does not have a failure on poison.sh
• No additional ILASM roundtrip failures

Author:	TIHan
Assignees:	TIHan
Labels:	area-ILTools-coreclr
Milestone:	-

[TIHan]

/azp run runtime-coreclr ilasm

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[TIHan]

Triggered manually: https://dev.azure.com/dnceng/public/_build/results?buildId=1888046&view=results

[TIHan]

@dotnet/jit-contrib This is ready. I'm running the ilasm roundtrip tests in CI, there are known failures, but we should not see the poison in the list. I tested this manually and adding the <_IlasmSwitches>$(_IlasmSwitches) -DEBUG</_IlasmSwitches> fixes the roundtrip test.

[TIHan]

Pinging @dotnet/jit-contrib again - should be a quick review

[BruceForstall]

This change doesn't seem quite right; it's not general.

Why is this specific to the poison test?

There is already code that uses the various project file configurations to set the correct ilasm flags. Is this not getting used somehow? See:

runtime/src/coreclr/.nuget/Microsoft.NET.Sdk.IL/targets/Microsoft.NET.Sdk.IL.targets

Lines 126 to 133 in 1f0582e

	<_IlasmSwitches>-QUIET -NOLOGO</_IlasmSwitches>
	<_IlasmSwitches Condition="'$(FoldIdenticalMethods)' == 'True'">$(_IlasmSwitches) -FOLD</_IlasmSwitches>
	<_IlasmSwitches Condition="'$(SizeOfStackReserve)' != ''">$(_IlasmSwitches) -STACK=$(SizeOfStackReserve)</_IlasmSwitches>
	<_IlasmSwitches Condition="'$(DebugType)' == 'Full'">$(_IlasmSwitches) -DEBUG</_IlasmSwitches>
	<_IlasmSwitches Condition="'$(DebugType)' == 'Impl'">$(_IlasmSwitches) -DEBUG=IMPL</_IlasmSwitches>
	<_IlasmSwitches Condition="'$(DebugType)' == 'PdbOnly'">$(_IlasmSwitches) -DEBUG=OPT</_IlasmSwitches>
	<_IlasmSwitches Condition="'$(Optimize)' == 'True'">$(_IlasmSwitches) -OPTIMIZE</_IlasmSwitches>
	<_IlasmSwitches Condition="'$(IlasmResourceFile)' != ''">$(_IlasmSwitches) -RESOURCES="$(IlasmResourceFile)"</_IlasmSwitches>

[TIHan]

@BruceForstall - I did see those targets, but I believe the build uses this if you are compiling an .ilproj right? I think that is the case which is why it never picks up those flags for _IlasmSwitches. Our test targets just happen to use the same name _IlasmSwitches.

Why is this specific to the poison test?

In order to get the correct poisoned value, 0xCD, the assembly must be marked as debuggable, otherwise the runtime will not output the poisoned value, 0xCD, which is why the roundtrip test was failing.

Here is the code in the runtime that checks to poison the variable:

    // Returns true if address-exposed user variables should be poisoned with a recognizable value
    bool compShouldPoisonFrame()
    {
#ifdef FEATURE_ON_STACK_REPLACEMENT
        if (opts.IsOSR())
            return false;
#endif
        return !info.compInitMem && opts.compDbgCode;
    }

opts.compDbgCode would return false if we did not compile the .il with the -DEBUG flag.

it's not general

Before, I did try to generalize this in the test targets if we notice the DebugType as PdbOnly/Full/etc., - see this commit: 564dbfd#diff-7a4e5f3ff147c4363c6363eb39d726682c301afae8ab8e71518eba2dba1bb505R38
But it was not working; the conditions never returned true. So, I went for setting the switches directly in the specific project and that worked. I think it's ok that we set it like this for this one specific test.

[BruceForstall]

I did see those targets, but I believe the build uses this if you are compiling an .ilproj right?

I believe that's right; I realized that last night, too. So the .csproj round-trip testing wouldn't see it.

I did try to generalize this in the test targets if we notice the DebugType as PdbOnly/Full/etc., - see this commit: 564dbfd#diff-7a4e5f3ff147c4363c6363eb39d726682c301afae8ab8e71518eba2dba1bb505R38

That's the fix I was thinking. I tried a slight variant, main...BruceForstall:TryFixIlasmRoundTripDebugFlag, and it seemed to work. I assumed for .ilproj it would add the switches twice, but that doesn't seem to be true (and I don't know why).

Anyway, I'll go ahead and approve.

[TIHan]

I'll update this with your variant. I bet mine didn't work because I didn't put it in a property group...

[TIHan]

CI failure is not related, merging.