Runtime: System.Runtime.CompilerServices.Unsafe 4.0.6.0 published in debug mode

[wdnijdam]

System.Runtime.CompilerServices.Unsafe, Version=4.0.6.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a: has debug flags Default, IgnoreSymbolStoreSequencePoints

It should be published as a Release branch to prevent debuggers to be kicked in production environments.

Please create a new package in Release mode.

[msftbot[bot]]

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

[ViktorHofer]

cc @joperezr @ericstj

[joperezr]

It seems like the asset we build with net461 configuration is the one causing this which means this is presumably a fallout from the work I did to add net461 assets on our OOB packages recently. I will take a look to see what went wrong here.

[joperezr]

This will most likely have to be fixed on release/3.1 as that is the only place we have ever introduced this asset.

[joperezr]

setting to 5.0 as we can't really set milestone to 3.1

[ViktorHofer]

The pre 5.0 milestones are in the "Closed" section. IRC we use them for such issues.

[wdnijdam]

I can confirm we have the problem using the net471. We did not notice the same issue in core3.1

[joperezr]

The workaround for people hitting this is for now to downgrade the package back to 4.7.0 (if you depend on 4.7.1) or 4.5.2 (if you depend on 4.5.3) version which shouldn't have this issue.

[ericstj]

@brianrob this looks related to https://github.com/dotnet/corefx/pull/40772. I wouldn't expect this to cause it since it wasn't made in servicing, but it is related. I checked the latest builds of Unsafe package (5.0.0-preview.4.20251.6) and now I see that all lib files set [assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints)] so this seems to disagree with the assertion of the original issue, or we've made the bug worse in 5.0.

[joperezr]

I think I found the issue. Problem seems to be on the arguments passed in to IlAsm, specifically this one: https://github.com/dotnet/coreclr/blob/release/3.1/src/.nuget/Microsoft.NET.Sdk.IL/targets/Microsoft.NET.Sdk.IL.targets#L135

      <_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>

The problem seems to be that specifically for the switch -Debug=OPT we are not also ensuring that the configuration might be set to Release. This problem doesn't repro on other configurations of Unsafe because DebugType is set to portable so we don't pass the -DEBUG swtich at all to ilAsm, but for assemblies that target netfx like the one in the package (which targets net461) DebugType is defaulted to PdbOnly which means that the parameters we pass in to ilasm contain -Debug=OPT even when the configuration is set to release. I tried manually to change the DebugType of the project when targeting desktop and that fixed the issue. @ericstj do we want to fix this on the IL SDK on coreclr or should we instead have a tactical fix/workaround on the Unsafe project here?

[joperezr]

cc @tannergooding who originally wrote the IL SDK

[ericstj]

Please fix this in master first with whatever is the best fix.

For servicing: can you describe how this changed over time? It’s not clear to me when this regressed and why it’s important to backport to servicing (yet). I’d like to understand severity here.

[joperezr]

Please fix this in master first with whatever is the best fix.

Master doesn't cross-compile for netfx yet, this issue is a regression that happened on release branches when we added net461 configuration for System.Runtime.CompilerServices.Unsafe

For servicing: can you describe how this changed over time? It’s not clear to me when this regressed and why it’s important to backport to servicing (yet). I’d like to understand severity here.

As said above, this was regresed when I added net461 configurations to Unsafe on both release/2.1 and release/3.1. The reason why it broke, is because the package used to only have assets for netstandard and netcoreapp, and for those the DebugType is set to portable. Because of that, we were not passing -DEBUG=OPT to ilasm. When I added net461 this regressed since in corefx Directory.Build.Props of the root we have:

    <!-- Empty DebugType when building for netfx and in windows so that it is set to full or pdbonly later -->
    <DebugType Condition="'$(TargetsNetFx)' == 'true' AND '$(OS)' == 'Windows_NT'" />

Which sets DebugType to PdbOnly, which was causing us to pass in the extra -DEBUG=OPT which in the end is causing the attribute that people started to notice.

[ericstj]

Master doesn't cross-compile for netfx yet, this issue is a regression that happened on release branches when we added net461 configuration for System.Runtime.CompilerServices.Unsafe

Yes it does. We're building for net45 : https://github.com/dotnet/runtime/blob/master/src/libraries/System.Runtime.CompilerServices.Unsafe/src/System.Runtime.CompilerServices.Unsafe.ilproj#L13

I noticed in master that not only the net45 build but all configurations have the DebuggableAttribute. I believe because of @brianrob's change. @brianrob indicated in his change that this was actually intentional which is what made me wonder which is desired behavior.

[joperezr]

Yes it does.

Oh interesting you are right, looks like that is a new configuration though since if you check Configurations.props on release branch you'll see it wasn't there which is why I had to add it in the first place:

    <PackageConfigurations>
      netcoreapp2.0;
      netstandard1.0;
      netstandard;
      net461;
    </PackageConfigurations>
    <BuildConfigurations>
      $(PackageConfigurations);
      netcoreapp;
      netfx;
    </BuildConfigurations>

Seems like it was actually added by you on November: https://github.com/dotnet/corefx/pull/42297

[brianrob]

The details around this change are a bit fuzzy to me, but I recall that without this, you don't get the expected inlining behavior. The goal of my change was to match what Roslyn does.

[joperezr]

@wdnijdam do you mind explaining a bit more what is the extent of the problem here? It is not clear to me if this new attribute is breaking your application in any way, and if it is, I'm also not understanding in which way. If we understand that better, then we can decide if we have to fix this or not, as since how my colleagues suggest above, seems like for .NET 5 we are intentionally adding that attribute for all configurations.

[wdnijdam]

Running software on production that is compiled in Debug mode is somewhat risky. I a debugger of any kind is running on the same machine the software might halt in the debugger. We have had incidents in the past where our software stopped with a debug pop-up after an exception that would normally be catched.

Since then we verify for running debuggers on the machine but also check all published code for the debug flags. Every once in a while external parties and also Microsoft publishes a new version of package in debug mode.

It might be good practice to prevent any publication of packages in debug mode. There should be a check in the pipeline or nuget server to prevent debug publications.

[tannergooding]

As per the command line help:

/DEBUG          Disable JIT optimization, create PDB file, use sequence points from PDB
/DEBUG=IMPL     Disable JIT optimization, create PDB file, use implicit sequence points
/DEBUG=OPT      Enable JIT optimization, create PDB file, use implicit sequence points
/OPTIMIZE       Optimize long instructions to short

As I understand it, /DEBUG=OPT is basically just a switch which enables optimizations and produces a PDB. This is the default behavior of all of our assemblies and is why it gets set when PDBs are enabled.

If I create a new C# project in VS and build both the Debug and Release configurations using the default settings and targeting netcoreapp3.1 I see:

• Debug: [assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.DisableOptimizations | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints | DebuggableAttribute.DebuggingModes.EnableEditAndContinue)]
• Release: [assembly: Debuggable(DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints)]

So the only difference between ilasm and csc is really that the former is also setting Default. When you go and examine the docs: https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.debuggableattribute.debuggingmodes?view=netcore-3.1

Default:

Instructs the just-in-time (JIT) compiler to use its default behavior, which includes enabling optimizations, disabling Edit and Continue support, and using symbol store sequence points if present. Starting with the .NET Framework version 2.0, JIT tracking information, the Microsoft intermediate language (MSIL) offset to the native-code offset within a method, is always generated.

IgnoreSymbolStoreSequencePoints:

Use the implicit MSIL sequence points, not the program database (PDB) sequence points. The symbolic information normally includes at least one Microsoft intermediate language (MSIL) offset for each source line. When the just-in-time (JIT) compiler is about to compile a method, it asks the profiling services for a list of MSIL offsets that should be preserved. These MSIL offsets are called sequence points.

None:

Starting with the .NET Framework version 2.0, JIT tracking information is always generated, and this flag has the same effect as Default, except that it sets the IsJITTrackingEnabled property to false. However, because JIT tracking is always enabled, the property value is ignored in version 2.0 or later.
Note that, unlike the DisableOptimizations flag, the None flag cannot be used to disable JIT optimizations.

So I don't see any issues that would arise from this.