dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.21k stars 1.35k forks source link

DebugSymbols is ignored if DebugType is set. #2169

Open distantcam opened 7 years ago

distantcam commented 7 years ago

The Fody project recently had an issue where debug symbols were not being processed properly in release mode.

https://github.com/Fody/Fody/issues/360

It turns out that the value of DebugSymbols is completely ignored if DebugType is set.

https://github.com/Microsoft/msbuild/blob/master/src/Tasks/Microsoft.Common.CurrentVersion.targets#L145-L151

<_DebugSymbolsProduced>false</_DebugSymbolsProduced>
<_DebugSymbolsProduced Condition="'$(DebugSymbols)'=='true'">true</_DebugSymbolsProduced>
<_DebugSymbolsProduced Condition="'$(DebugType)'=='none'">false</_DebugSymbolsProduced>
<_DebugSymbolsProduced Condition="'$(DebugType)'=='pdbonly'">true</_DebugSymbolsProduced>
<_DebugSymbolsProduced Condition="'$(DebugType)'=='full'">true</_DebugSymbolsProduced>
<_DebugSymbolsProduced Condition="'$(DebugType)'=='portable'">true</_DebugSymbolsProduced>
<_DebugSymbolsProduced Condition="'$(DebugType)'=='embedded'">false</_DebugSymbolsProduced>

But according to the documentation DebugSymbol should control the output of the symbol file.

https://msdn.microsoft.com/en-us/library/bb629394.aspx

DebugSymbols A boolean value that indicates whether symbols are generated by the build. Setting /p:DebugSymbols=false on the command line disables generation of program database (.pdb) symbol files.

DebugType Defines the level of debug information that you want generated. Valid values are "full," "pdbonly," and "none."

Is this a bug?

SimonCropp commented 7 years ago

related https://github.com/Microsoft/msbuild/issues/2170

mikebeaton commented 4 years ago

If you modify "VS/project properties/Build/Advanced/Debugging information" then it creates (if necessary) and sets the value of <DebugType> in the .csproj file, but does not set or add a <DebugSymbols> property.

So, if I've understood correctly, it's probably worth noting here that project files generated by Visual Studio rely on the behaviour under discussion.

daiplusplus commented 7 months ago

As of Feburary 2024, long after the introduction of /debug:embedded and release-debug-plus optimization level, the actual behaviour of the <DebugSymbols> and <DebugType> MSBuild properties does not match the current documentation at all - (while @distantcam linked to the VS2015 docs, the current docs for VS2022 has the same incorrect definitions):

<DebugSymbols> A boolean value that indicates whether symbols are generated by the build.

<DebugType> Defines the level of debug information that you want generated. Valid values are "full," "pdbonly," "portable", "embedded", and "none."

<DefineDebug> A boolean value that indicates whether you want the DEBUG constant defined.

In actuality, in the .NET SDK version 7.0 (and also likely 5.0, 6.0, and 8.0, I haven't thoroughly checked those) the actual behavior is:

So, if I were writing the documentation, I'd change the text to something like:

<DebugSymbols>

  • When true then:
    • PDB debugging information will be included in the output (either as a separate .pdb file or embedded within the output .exe/.dll).
    • Enables debugPlus optimization mode in both Release and Debug configurations . This debugPlus Disables certain IL-level optimizations, even in Release builds - or (aka debugPlus).
  • When false then:
    • Disables PDB output (including embedded PDB data) - but only when <DebugType> is not specified.
    • Prevents debugPlus IL output (thus forcing either Release or Debug depending on other options).
  • When undefined then:
    • PDB output is then controlled by <DebugType>.
    • Prevents debugPlus IL output (thus forcing either Release or Debug depending on other options).

<DebugType>

  • When none then:
    • Any <DebugSymbols> property value becomes undefined (thus debugPlus-level optimization will never be applied).
    • Disables PDB output (whether separate-file or embedded)
  • When full, pdbonly, or portable then:
    • Enables PDB output as a separate file. The exact file-format of the .pdb file depends on your OS platform when using full or pdbonly.
    • As of the .NET 7 SDK. provided you are not targeting any legacy platforms then the full and pdbonly values are both redundant and superfluous, instead use only portable or embedded.
  • When embedded then:
    • Enables PDB output as extra data embedded directly in the output .dll/.exe file. The same exact format is used regardless of OS platform.
  • When undefined then:
    • PDB output will be disabled unless <DebugSymbols>true is explicitly set.
    • PDB exact file format will depend on your OS platform.

<DefineDebug>

  • This property is not respected by the C# build system. Instead set an explicit property like so <DefineConstants>$(DefineConstants);DEBUG;</DefineConstants>

So yes - quite the difference...

Furthermore, the documentation on Optimization is incorrect, it says DebugPlusMode is ignored when OptimizationLevel.Release is set, but in reality it's the opposite: debugPlus == true with Release mode gives us ILEmitStyle.DebugFriendlyRelease, while debugPlus has no effect when CompilationOptions.Optimizationlevel == OptimizationLevel.Debug (c.f. Microsoft.CodeAnalysis.CSharp.MethodCompiler.GenerateMethodBody).

So currently, my default Directory.Build.props looks like this:

    <PropertyGroup Condition="$(Configuration) = 'Debug'">
        <!-- This combination of properties causes CSC to compile without IL optimizations, and to emit a very debuggable embedded PDB.  -->
        <DebugType>embedded</DebugType>
        <DebugSymbols>true</DebugSymbols>
        <DefineConstants>$(DefineConstants);DEBUG;</DefineConstants>
    </PropertyGroup>
    <PropertyGroup Condition="$(Configuration) = 'Release'">
        <!-- This combination of properties causes CSC to compile with "release-debug-plus" optimizations. The evidence suggests this has an absolutely negligible performance impact, while significantly improving the debugging experience. -->
        <DebugType>embedded</DebugType>
        <DebugSymbols>true</DebugSymbols>
        <Optimize>true</Optimize>
        <DefineConstants>$(DefineConstants);RELEASE;</DefineConstants>
    </PropertyGroup>

If it helps, I've put together this table:

<DebugType> <DebugSymbols> <Optimize> Effective csc.exe options Effective CompilationOptions Debugability
embedded false false /debug- /debug:embedded /optimize- DebugPlusMode=false, Optimization.Debug, emitPdb=true, emitPdbFile=false High (Debug mode; DebugPlusMode = false)
embedded false true /debug- /debug:embedded /optimize+ DebugPlusMode=false, Optimization.Release, emitPdb=true, emitPdbFile=false Low (Release mode, DebugPlusMode = false)
embedded true false /debug+ /debug:embedded /optimize- DebugPlusMode=true, Optimization.Debug, emitPdb=true, emitPdbFile=false High (Debug mode; DebugPlusMode = true but is ignored)
embedded true true /debug+ /debug:embedded /optimize+ DebugPlusMode=true, Optimization.Release, emitPdb=true, emitPdbFile=false Medium (Release mode, DebugPlusMode = true)
Rabadash8820 commented 4 months ago

@daiplusplus Many thanks for the deep dive!

These properties have been confusing me lately as well. Out of curiosity, in your "default Directory.Build.props", why do you still use embedded debug symbols in the Release configuration? I suppose it might not be a big deal for a server-side app, but for desktop/mobile apps, doesn't that just make it easier for end users to reverse engineer your assemblies? I always thought portable was preferrable in Release.

daiplusplus commented 4 months ago

@Rabadash8820 but for desktop/mobile apps, doesn't that just make it easier for end users to reverse engineer your assemblies?

I always thought portable was preferrable in Release.

Whatever is "preferable" for any given situation is subjective; in my case, it is demonstrably not preferable (c.f.: arguments above), but I appreciate that in yours it might be.

Rabadash8820 commented 3 months ago

@daiplusplus Thanks for the thoughtful response. I guess the reverse engineering concern is a bit overblown; I too would be flattered if somebody actually cared about my software enough to try that lol, and security through obscurity is not real security anyway, plus most of what I write is open-source anyway. 🤷‍♂️

I guess a bigger concern about embedded PDBs in Release is file size. I know assemblies and PDBs are typically not huge (on the order of 10s of MB maybe), but as the number of types and assemblies grows, the savings of not shipping a PDB becomes non-trivial. In particular, do you know if assemblies contain private/protected member/type names as well, or only public ones? If it's only public, then I could see PDBs being quite large by providing all those extra symbols. These files can be hosted elsewhere as you suggested to keep the shipped app smaller. In fact, several SaaS systems for mobile already provide hosting for symbolicating exception call stacks from crashes (Firebase Crashlytics and Unity Cloud Diagnostics do anyway). But for server-side code I guess you're right, there's no big reason against embedded symbols.

slang25 commented 3 months ago

I don't think that's the right way of thinking about it, regardless of public/private types and methods, the assembly contains all of the definitions. The PDB contains local variable names, info about scopes, and metadata about methods, importantly the IL offsets to source code mapping.

So it's generally proportional to how big the assembly is, and that's around 15%.