Obsolete SuppressIldasmAttribute and remove ildasm.exe support for it

[GrabYourPitchforks]

Resolves #13341.
Resolves #40622.

The obsoletion hasn't been officially approved yet, but based on email conversations we expect approval is imminent (maybe with some wording changes). I'm opening this PR optimistically to get a head start on CI runs and make sure there's no functional regression.

Temporarily marked as no merge until API signoff is finalized.

@BruceForstall Do you anticipate any problem with me deleting the existing resource string and reassigning the resource ids to be gapless? I didn't see any functional regressions but wanted to pick your brain on this. If this is risky, I can restore the original resource ids and leave the gap.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[GrabYourPitchforks]

/azp run runtime-coreclr ilasm

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[BruceForstall]

cc @jkotas in case you have opinions here

[GrabYourPitchforks]

There was some email discussion about what hardcoded message we should put in the attribute. IMO we should keep the hardcoded message short and sweet, and if we want to give further information on "here's why you can't rely on it for IP protection," that belongs in the aka.ms link that accompanies these attributes.

[stephentoub]

While you're here, should we change the message for SYSLIB0015 to be consistent, e.g. drop "applications"?

[briansull]

I see some ilasm testing failures:

  Starting:    Regressions.coreclr.XUnitWrapper
    Regressions/coreclr/GitHub_49982/test49982/test49982.sh [FAIL]
      usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file
             cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory
      rm: /private/tmp/helix/working/AF640960/w/AF7D096D/e/Regressions/coreclr/GitHub_49982/test49982/composite-r2r.dll.rsp: No such file or directory
      Unhandled exception. System.IO.FileLoadException: Could not load file or assembly '/private/tmp/helix/working/AF640960/w/AF7D096D/e/Regressions/coreclr/GitHub_49982/test49982/test49982.asm.dll'. The located assembly's manifest definition does not match the assembly reference. (0x80131040 (FUSION_E_REF_DEF_MISMATCH))

[BruceForstall]

ildasm changes LGTM

@BruceForstall Do you anticipate any problem with me deleting the existing resource string and reassigning the resource ids to be gapless? I didn't see any functional regressions but wanted to pick your brain on this. If this is risky, I can restore the original resource ids and leave the gap.

I can't think of any reason why we would need to worry about this (but I don't have a lot of experience in this code)

[GrabYourPitchforks]

The failure reported by Brian (full logs here) seems related to #50364. That's a newly-introduced crossgen regression test. But I don't know why this PR would've introduced a regression in that code path. @trylek, thoughts?

I'm rerunning the test just in case it's flaky.

[trylek]

Hmm, I must admit this is the first time I'm seeing an ilasm test failure. This was certainly not the purpose of the new test, I suppose I should just mark it as ilasm-incompatible for now until the problem is understood; can someone please give me some pointers on how to run ilasm testing locally so that I can investigate what's wrong with the test? I guess that most likely the problem is that this is a "forced Crossgen2" test and if the test does something like ildasm followed by ilasm, applying that to a R2R executable certainly won't produce an identical result.

[BruceForstall]

To run the round-trip testing, set RunningIlasmRoundTrip=1 before executing the test wrapper script. Note there are other environment variables set, e.g., COMPlus_TieredCompilation=0

[trylek]

Thanks Bruce for clarifying! It can easily be the case that it's just about wrong ordering of the steps in the CLRTest.*.targets scripts under src\tests\Common - if we make sure to run the ildasm - ilasm roundtrip before executing Crossgen2 (or Crossgen1, for that matter), I bet it would work just fine.

[jeffhandley]

SYSLIB0016 has been claimed by another obsoletion. I'm going to update this to be SYSLIB0024.

[GrabYourPitchforks]

I'll resolve the conflicts and get this in shortly.

[GrabYourPitchforks]

Changes since Jeff's latest iteration:

• Bumped diagnostic code to SYSLIB0025.
• Fixed a stray whitespace issue in the .builds file.

Otherwise, no functional changes from previous iteration. Since PR is already signed off I'll merge once CI goes green.

[GrabYourPitchforks]

Test failure is known issue #48851.

[jeffhandley]

Breaking change documentation will be handled by dotnet/docs#23856. /cc @gewarren