Fix linker PDB behavior and add DebuggerSupport property

[sbomer]

(Edited)

Linker symbol behavior

Based on discussion with @eerhardt, @marek-safar, @vitek-karas, we have decided that there should be two properties, one to control debugger support all-up, and one to control linker symbol behavior. The latter should get defaults matching the debugger support property, but it should be possible to override the defaults by setting it explicitly. We also agreed that these settings should be independent of the compiler options (DebugSymbols/DebugType).

Ideally we would not have a linker symbol knob, and instead it would always fix up symbols in the input - and the SDK would have options to publish with or without symbols (or with symbols in a separate output directory). Lacking that ability, we expect the linker option to be useful especially for cases where the inputs contain embedded symbols that would be published otherwise.

Implementation

This change introduces two properties, DebuggerSupport and TrimmerRemoveSymbols. DebuggerSupport will eventually enable RuntimeHostConfigurationOption to remove debugger support (see dotnet/runtime#37288). In this change, it only influences the defaults for TrimmerRemoveSymbols:

• If TrimmerRemoveSymbols is set explicitly, this will be respected. If not set explicitly:
• When DebuggerSupport is empty or true, TrimmerRemoveSymbols defaults to false.
• When DebuggerSupport is false, TrimmerRemoveSymbols defaults to true.

TrimmerRemoveSymbols will cause the linker to drop symbols from all inputs. If it is not true, the linker will modify input symbols the same way that it modifies the corresponding IL (barring bugs).

@samsp-msft could you please review the proposed naming of the options DebuggerSupport and TrimmerRemoveSymbols? Rationale was:

• DebuggerSupport matches existing feature flags like InvariantGlobalization.
• TrimmerRemoveSymbols avoids the ambiguity of TrimSymbols (see the naming notes below).

Below are the original notes, which are out of date.

---

Outdated notes

• Filter out pre-link PDBs from the publish output
• Introduce TrimSymbols public option to allow removing symbols (replaces _LinkSymbols)

Naming:

• TrimSymbols matches the "trim" terminology we've been using, but is odd in that it suggests selective trimming, but actually means complete removal. Symbols are either "trimmed" with the same granularity as the assembly (defined by the new TrimMode option), or not at all.
• Remove/Strip/Drop might make more sense for the verb, but if we want consistency with the other options, maybe it should be called TrimmerRemoveSymbols for example. This is descriptive, but a bit verbose.
• Another option is to go with the opposite behavior, like _LinkSymbols. true means the "trimmed" symbols are kept, and false means they are discarded. Link makes some sense in this context, but isn't accurate either - maybe a better name would be TrimmerPreserveSymbols if we went this route.

Defaults:

• The defaults before this change are buggy. Symbols would be removed by the linker, but the SDK would keep pre-link symbols. As a result, embedded PDBs would be stripped, while separate PDB files would be placed unmodified into the publish folder.
• With this change, the default behavior will be to remove symbols, under the assumption that people using the linker are optimizing for size over debuggability. TrimSymbols can be set to false to keep the PDBs (which are modified to match the trimmed IL).

@marek-safar @vitek-karas @eerhardt @samsp-msft I would appreciate feedback on the defaults and naming. Initially I thought TrimSymbols made sense, but after writing this TrimmerRemoveSymbols seems less confusing.

[marek-safar]

I'm wondering if we need this option at all. I can think of two major scenarios where debug symbols play a role.

Linker link/copy/copyused managed assembly

I think it should not matter what TrimMode is used but the illinker should always do the right thing and produce PDBs, if they exist, which are correct. The user should not be asked to set up anything in this mode.

Debugging is disabled

We already have feature-switch for that and we should fold the logic of removing any debug symbols into the same option instead of adding another combination.

[vitek-karas]

I agree with @marek-safar for the most part. The only additional question is if we should have an option to not copy symbols to the output - as a separate option from "Debugging disabled". Maybe the answer is no - we don't need it.

Basically:

• If I disable debugging support - I probably want maximum size saving and thus I don't want PDBs (and they would not help making the debugging experience much better anyway).
• If I enable debugging support
  • by default I probably want full debugging support -> I want PDBs
  • I think there is a scenario where I want to have debugging enabled, but I don't want to ship PDBs to customers. In this case the toolchain should still produce the PDBs. I may choose to move them somewhere else after build to separate them from the shipping app. (For example single-file will by default not bundle PDBs - which is equivalent to moving them someplace else).

[eerhardt]

Debugging is disabled
We already have feature-switch for that

What is the feature switch for that? I know the linker .exe has a command line option, but how does a user toggle it?

[vitek-karas]

@eerhardt Isn't dotnet/runtime#37288 basically introduce a feature switch for that? It's runtime only for now, but we should also expose it do projects as an MSBuild property. The linker changes here won't need the property though, as they can adapt the behavior based on the item group with all of the feature switches which is passed to the linker.

[sbomer]

it should not matter what TrimMode is used but the illinker should always do the right thing and produce PDBs, if they exist

I take this to mean that the default should be equivalent to TrimmerRemoveSymbols = false.

We already have feature-switch for that and we should fold the logic of removing any debug symbols into the same option

I take this to mean that when the feature switch dotnet/runtime#37288 is ready, we should add an msbuild property, say, DebuggerSupport, that is empty by default, but:

• when set will place an item System.Diagnostics.Debugger.IsSupported with the same Value into RuntimeHostConfiguratinOption - so that the linker will be able to remove debugger support if it is false
• when set to false, will imply the equivalent of TrimmerRemoveSymbols = true.

I don't think we should wait for the feature switch to be ready to fix the PDB behavior. It seems to me we'll need something like TrimmerRemoveSymbols either way, so we could add it now (maybe with a _ prefix if we don't want people to be able to set it independently of DebuggerSupport?) and add the RuntimeHostConfigurationOption in a separate change.

[marek-safar]

I take this to mean that the default should be equivalent to TrimmerRemoveSymbols = false.

It depends on what TrimmerRemoveSymbols should do. The intention is to by default ignore PDBs if there are none, fix them if they need fixing, copy them if they don't need fixing.

I take this to mean that when the feature switch dotnet/runtime#37288 is ready, we should add an msbuild property, say, DebuggerSupport, that is empty by default, but:

No, let's add correct MSBuild property now and link it to the feature switch when is available but hook it up correctly to debug symbols removal linker options

[vitek-karas]

No, let's add correct MSBuild property now and link it to the feature switch when is available but hook it up correctly to debug symbols removal linker options

I think the better solution would be to make the behavior of the linker depend solely on the RuntimeHostConfigurationOption - the easy to use property for enabling/disabling debugger support should be just a sugar coat of the item group. Basically - one source of truth. If we use two different things in the SDK we're asking for hard to debug corner cases where something sets just the item group and not the property... and so on. That is assuming that there's a way in MSBuild to ask if an item group contains a certain item... which I don't know.

[sbomer]

let's add correct MSBuild property now

I see. Thinking more about a property like DebuggerSupport, should DebuggerSupport = false imply DebugSymbols = false? DebugSymbols gets a default value in a pretty early import so it seems we would need to either:

• Require developers to explicitly set DebugSymbols = false in addition to DebuggerSupport = false, or
• Override DebugSymbols settings so that they have no effect when DebuggerSupport = false. In this case we wouldn't be able to distinguish between a DebugSymbols set by the SDK defaults vs explicitly by the developer - ideally it would be an error to set DebugSymbols = true together with DebuggerSupport = false.

[marek-safar]

Basically - one source of truth

Totally agree and that's what mean by introducing MSBuild property. It should be similar to how InvariantGlobalization property works.

should DebuggerSupport = false imply DebugSymbols = false

I see where you are heading but I'm not sure we should make that automatic, especially if there are more combinations (like DebugType=None). I don't know what is the default for DebugSymbols when the property is not specified but changing it to false when DebuggerSupport=false is used makes sense to me.

ideally it would be an error to set DebugSymbols = true together with DebuggerSupport = false.

I'd go with a warning only

[sbomer]

I've added a DebuggerSupport property that implies _TrimmerLinkSymbols. I have left it independent of DebugSymbols and DebugType.

[sbomer]

make the behavior of the linker depend solely on the RuntimeHostConfigurationOption
one source of truth. If we use two different things in the SDK we're asking for hard to debug corner cases

@vitek-karas does the DebuggerSupport property which sets the private _TrimmerLinkSymbols (which passes -b to the linker) satisfy this? Or were you suggesting that we replace the linker command-line argument -b with the ability to specify an action on symbols in the feature substitution files (similar to what we do for resources)?

[vitek-karas]

I guess it depends how we want to treat RuntimeHostConfigurationOption:

• We assume that the values we define in it are really only populated by our SDK and thus the DebuggerSupport is the primary source of truth.
• We expect others (outside of the SDK) to potentially add values we define - in which case DebuggerSupport is only one of the ways the debugger support feature switch can be turned on.

If it's the former, then using DebuggerSupport is perfectly OK. If it's the latter we might want to consider reading the value from the RuntimeHostConfigurationOption.

Basically the question is how do we tie the link task options to feature switches...

I "think" it's ok to go with the first option - in which case all we need to make sure of is that there's really only one MSBuild property which controls both the link task behavior and the feature switch. (The advanced scenario which Eric mentions that one wants debug symbols but with the feature switch off - we can probably say it's currently unsupported and let people mock with the internal properties if they really need it).

[sbomer]

DebuggerSupport works similarly to InvariantGlobalization in that respect - we expect people to use these options instead of RuntimeHostConfigurationOption, but RuntimeHostConfigurationOption is public and supported.

The difference is that DebuggerSupport has a broader meaning than RuntimeHostConfigurationOption since it not only adds a feature switch, but sets an independent linker flag.

I "think" it's ok to go with the first option

Agreed. If someone sets RuntimeHostConfigurationOption but not DebuggerSupport, I think it's fine to remove runtime debugger logic via the runtimeconfig/feature switch but still keep PDBs.

how do we tie the link task options to feature switches...

Currently there's a 1-to-1 mapping for feature switch arguments: RuntimeHostConfigurationOption with trim <-> FeatureSettings task argument <-> --feature command-line argument. If we did want to tie the behaviors together, I would put it behind the --feature switch, for example by controlling PDB stripping under a feature="DebuggerSupport" element in the XML - instead of having a RuntimeHostConfigurationOption feature switch map to a non-feature-switch argument on the linker.

[vitek-karas]

Makes sense - let's do that.

[marek-safar]

Do we actually need _TrimmerLinkSymbols property at all?

[sbomer]

We need some way to set the linker option to true when DebuggerSupport is empty. We could do that by defaulting DebuggerSupport itself to true, but I don't want to commit to making that default available at a particular place in the import chain - it would have to happen before RuntimeHostConfigurationOption, but if it happens too early, then SDK components won't be able to distinguish between the default true and an explicit true set by the user.

Aside from that, this provides an "escape hatch" for people who want to tweak just the linker settings and are ok with relying on an unsupported property to experiment with things - see @eerhardt's comment for example.

[sbomer]

I've updated this to include the two properties we talked about (DebuggerSupport/TrimmerRemoveSymbols). See the updated notes in the top post. @samsp-msft could you please review the names we picked?

[vitek-karas]

Do we need the _TrimmerLinkSymbols property anymore?

[eerhardt]

Since the naming is inverted, it is probably easier to read this way. Unless we wanted to invert the ILLink Task option.

[sbomer]

It's necessary because the existing task option has the opposite polarity of the public property. (I don't think you can pass ="! $(TrimmerRemoveSymbols)"). We could remove this by changing the task option (and maybe the command-line arguments). edit: @eerhardt beat me to it. :)

[eerhardt]

[marek-safar]

TrimmerRemoveSymbols hooking up into linker will be done separately, right?

[sbomer]

It's already hooked up - TrimmerRemoveSymbols sets _TrimmerLinkSymbols (inverted) which is already consumed by the linker (LinkSymbols on the task -> -b). If you would like, I can change the linker task option (in a separate change) to RemoveSymbols so that we can simplify this.

[marek-safar]

Simplifications in follow up PR sounds great