Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[package] Many packages missing pdbs...strategy? #1982

Open
rconde01 opened this issue Jun 20, 2020 · 48 comments
Open

[package] Many packages missing pdbs...strategy? #1982

rconde01 opened this issue Jun 20, 2020 · 48 comments
Labels
question Further information is requested

Comments

@rconde01
Copy link
Contributor

Package and Environment Details (include every applicable attribute)

  • Package Name/Version: e.g. openssl/1.1.1g, gdal/3.1.0, cspice/0066
  • Operating System+version: Windows 10 x64
  • Compiler+version: Visual Studio 2019
  • Conan version: conan 1.26.0
  • Python version: Python 3.7.2

Steps to reproduce (Include if Applicable)

Doing a debug build on windows I'm getting warnings for many missing pdbs (would also apply to RelWithDebInfo). I can create issues and potentially PRs for each one, but since it appears systemic I thought it justified further discussion. I'm also not sure if the additional size is a problem, and thus maybe there should be an option 'with-pdbs' so that cci doesn't need to manage them. Although, in that case better not produce them at all.

@rconde01 rconde01 added the bug Something isn't working label Jun 20, 2020
@sourcedelica
Copy link
Contributor

sourcedelica commented Jun 20, 2020

I am in favor of having PDBs in the debug Windows packages. That would put them on par with the Linux packages. Our developers expect debug packages to have symbols.

@rconde01
Copy link
Contributor Author

Ouch - that seems fairly problematic. I would really like to see then a move towards using a 'with-pdbs' option which defaults to false as standard practice so you can build them locally with the same recipe.

@madebr
Copy link
Contributor

madebr commented Jun 20, 2020

@rconde01
Copy link
Contributor Author

At way level are you suggesting to use Z7? For the packages it would be ok for debug builds, but no good for relwithdebinfo. At that point maybe it’s better to be consistent.

@madebr
Copy link
Contributor

madebr commented Jun 20, 2020

What /Z7,'/Zi and /ZI does is adding information for the debugger.
So it's perfectly fine to add it for all build types except Release, which one expects to not contain any debug overhead, and MinSizeRel.
The difference in these flags is where/how the compiler stores this information.

The/a difference between RelWithDebInfo and Debug, is that the former defines the NDEBUGmacro during compilation and the latter DEBUG.
And maybe different runtime (compiler.runtime)

@rconde01
Copy link
Contributor Author

Well I think you want debug information for your delivered binaries, so when your customers give you a crash dump you can get symbols. So this is RelWithDebInfo, but you want this as pdbs because you don’t want the bigger binary sizes (nor deliver your symbols to the customer generally).

@madebr
Copy link
Contributor

madebr commented Jun 20, 2020

Indeed.
Unless it's GPL'ed code 😄

@sourcedelica
Copy link
Contributor

If there's a need for some to not deliver symbols to an external customer (ours are internal) then it seems like an option to generate PDBs (or /Z7) is the best choice. We really don't want to fork every third party recipe that we use from Conan Center just to get symbols for Windows.

@rconde01
Copy link
Contributor Author

Did you mean Zi? I’m not sure the exact point you’re making. My current idea is not to fork, but to have a “with-pdb” option on windows that you could specify but which would default to false so that the cci rules are still followed. Although I think it might be desirable to use Z7 for static libraries...I’ve had issues with pdbs being located when using Zi with static libraries...but IIRC I had trouble reproducing it in a simple config so 🤷.

@rconde01
Copy link
Contributor Author

An additional problem here is that even apart from the pdb source location not being valid, generally a package will not include the source. This should be equally true on all platforms. OTTOMH I’m not sure if there’s an easy way to ensure you have the sources for all requirements...if not that’s something to consider as well.

@madebr
Copy link
Contributor

madebr commented Jun 21, 2020

It would be nice to use the cci recipes to build your program in RelWithDebInfo/Debug and have pdb's for all dependencies available.
As written by @rconde01 , this would make debugging releases super convenient.

I feel this issue touches a similar problem as #1783 .

We should separate two things here: what we want the recipe to provide and what we want to/can provide on bintray.

The recipe should be as general as possible. This means providing pdb's for all libraries/executables.
The disk space constraint, imposed by bintray, can be solved by adding a with_pdb for each recipe buildable on MSVC. As proposed up here.

This would require (at least) the following actions:

  • For a particular recipe:
    • Agree on how we should name this new option. pdb? with_pdb? with_pdbs?
    • Add a with_pdb option to msvc capable recipes, with default value False. (assuming that name is chosen)
    • The with_pdb options should be kept when shared=False. So the final binary has these symbols available.
    • Change whatever needed to build and package to add pdb support.
    • Add the following code to config_options:
if self.settings.compiler != "Visual Studio":
    del self.options.with_pdb
    # question: Should this options also be available when self.settings.build_type == "Release"?
  • For conan hooks:
    • The hooks should check, when options.get_safe("with_pdb", False) is True'ish, whether pdb's are available in the package. This would override KB-017.
      Also pass when self.options.get_safe("with_pdb", False) and options.get_safe("shared", None) == False and len(pdb_files) == 0
  • Conan: I'm not sure about this
    • The /Zi option should be added to the C/CXXFLAGS when with_pdb == True
      I'm not sure whether this should be handled by the build helpers or by the recipe.
      It would be nice if conan would support this as it does for fPIC.
      See this piece of code how it is done right now.

Opinions?

@sourcedelica
Copy link
Contributor

sourcedelica commented Jun 21, 2020

I'm assuming the developer would get the source from Github or wherever it came from - though it sounds like there may be an issue with the VS debugger mapping the source references in the PDB to the downloaded source (easy to do with gcc)? Actually for our developers just having the symbols + file names + line numbers would be enough to let them follow the source in the Github web UI.

@rconde01
Copy link
Contributor Author

@sourcedelica IMO that’s way too manual and giving up a lot. Normally you can move around the callstack and jump to the corresponding source locations, inspect variables etc. You wouldn’t want to give that up.

@rconde01
Copy link
Contributor Author

@madebr i need to think about it more thoroughly but I think you have hit most of the main points. A few thoughts:

  • As I said previously I think you may actually want Z7 for static libs. In this case mentioning “pdb” seems inappropriate...so maybe something more generic like “keep_symbols”.
  • Note that many recipes already generate the pdbs, and then actively delete them.

@madebr
Copy link
Contributor

madebr commented Jun 21, 2020

@rconde01

* As I said previously I think you may actually want Z7 for static libs. In this case mentioning “pdb” seems inappropriate...so maybe something more generic like “keep_symbols”.

This should be investigated. I think the /Zi option should be kept. So the linker will create a pdb when linking. No pdb will be created in that particular package, but the consuming packages can make use of its pdb capability.

* Note that many recipes already generate the pdbs, and then actively delete them.

This is fine, but only when with_pdb is Falseish.

@sourcedelica
Copy link
Contributor

@rconde Definitely but that's what they are doing right now :). The other options are mapping source from the PDB to a cloned Git repo or building the recipe from source.

@jeremy-coulon
Copy link
Contributor

What is the point of having VS debug binaries on Bintray without debug symbols? I would prefer that recipes give me debug symbols by default when I build it locally.

@rconde01
Copy link
Contributor Author

What is the point of having VS debug binaries on Bintray without debug symbols? I would prefer that recipes give me debug symbols by default when I build it locally.

It's a good question. I think at the least it will be linked to a consistent runtime. Of course, I think we would all prefer symbols by default, but we also understand there are space constraints on conan-center. I guess an alternative is with-pdb = true by default, and disable it for all cci packages.

@rconde01
Copy link
Contributor Author

@memsharded I would be good to get some thoughts from the conan team on this. Any ideas on how this could move forward?

@uilianries
Copy link
Member

@rconde01 Conan team will discuss it during this week, please, be patient.

@rconde01
Copy link
Contributor Author

I apologize, I just didn't know if it "scrolled off the feed" as the discussion happened over the weekend.

@uilianries
Copy link
Member

np, PDB has been requested many times, but as described in the wiki, Conan has a reason for not adopting it. Indeed for remote debug is very useful.

@rconde01
Copy link
Contributor Author

bump - it's been a few weeks, any thoughts?

@uilianries
Copy link
Member

Yes! Conan team just discussed more than once about this topics, the last meetings was few hours ago. Sorry the delay!

There were some options, pros and cons ... anyway, there will be a way to provide PDB files through an extra package folder, which will be optional. This feature will cover more cases, like when users want to package benchmarks or sanitizer results. It need to be studied, but can sure there will be an option for PDB distribution. Thanks for pushing it, this kind of discussion and community engagement it's really important to find new solutions!

@rconde01
Copy link
Contributor Author

intriguing...looking forward to further details.

@SSE4
Copy link
Contributor

SSE4 commented Jul 16, 2020

just in case someone interested, here the summary of approaches discussed with their pros and cons:

  1. do nothing
  • cons: users need to build locally (from sources) and modify exported conanfile or fork it, not that simple
  1. add new configurations (build types), such as DebugWithDebugInfo
  • cons: configuration names are really weird, obscure and confusing (most people expect debug to contain debug info by default)
  • cons: increases matrix of supported configurations
  • cons: tooling problem, need to match them with IDE and build system configurations
  1. add new options like with_pdb or without_pdb
  • cons: doesn't scale, options have to be added into each recipe, plus code handling them
  • cons: PDB is not only debug information format, the same story actually applies for .dwarf files on macOS, for instance
  • cons: option actually has no effect on ABI or package ID, thus it doesn't seem it should be an option at all
  1. remove .pdb files in conan center hook as post-process (pre-upload, post-package)
  • pros: single place, elegant implementation, keeps recipes clean and simple
  • cons: locally generated packages aren't the same as packages provided on conan center, might be source of confusion for users (e.g. why am I getting 2 Gb build locally but 100 Mb build from bintray?), no longer truly reproducible
  1. add auxiliary archive(s) in addition to conan_package.tgz
  • pros: aligns with other package managers (e.g. debian allows to have optional openssl-dev, openssl-doc, openssl-src and openssl-dbg in addition to the openssl main package)
  • pros: allows to handle supplemental files for various use cases besides debug information (e.g. to package benchmark results)
  • pros: doesn't affect on main conan package and leaves it untouched (same revision, same hash)
  • cons: requires significant development effort on conan side, probably also on server side (bintray)

some summary:

  • right now we're not going to store debug information on conan center, neither as PDBs, nor as binaries compiled with /Z7 - they contain lots of space and will slow down downloads and consume additional disk space for users, although very few users will actually try to debug 3rd-party libraries
  • we're not likely to going with approaches 1-3 as they introduce more headache than benefit

@rconde01
Copy link
Contributor Author

btw if the same issue applies to MacOS I think you could go with "symbols" rather than "pdb"...and technically you can strip symbols on linux and do the same.

@rconde01
Copy link
Contributor Author

One issue I see with the archive solution is that the compiler needs to find the pdbs which IIRC generally means they need to be located next to the dlls (at least with msvc). This requires a capability to overlay an archive on top of the normal package, rather than in a separate folder.

@SSE4
Copy link
Contributor

SSE4 commented Jul 16, 2020

@rconde01 not really, PDBs could be loaded from anywhere. you may locate them manually with load symbols dialog, or add to the symbols file path. you may even upload em to the symbols server and use from there.

@rconde01
Copy link
Contributor Author

@SSE4 That's on the debugging side...I'm talking about at compilation time e.g. for static libraries.

@rconde01
Copy link
Contributor Author

And now that i think about it, maybe i'm just dumb but i've found pdbs with static libraries to be problematic (although it's possible that was just from a package?)...but in any case, the archive approach doesn't play nice with Z7 I think.

@rconde01
Copy link
Contributor Author

Not that i'm married to it, but thoughts on 3.

3. add new options like with_pdb or without_pdb
* cons: doesn't scale, options have to be added into each recipe, plus code handling them

yes - you need to add the option, and propagate it down to dependencies. But you'll need code the handle the archive too right? And regardless you're gonna need the code to ensure they're generated for debug/releasewithdebinfo

* cons: PDB is not only debug information format, the same story actually applies for .dwarf files on macOS, for instance

Yeah - as mentioned in my other comment..."with-symbols" or "with-debug-info" would work.

* cons: option actually has no effect on ABI or package ID, thus it doesn't seem it should be an option at all

Maybe not ABI, but it potentially has an effect on the binary. The pdb name is embedded in the dll. I don't think symbols are the same as docs like some "independent extra thing".

So - i don't find those cons particular strong. Also you didn't list any pros:

  • Relatively simple
  • Doesn't require any additional conan features
  • Works for static libraries
  • Can put it in your conan profile (I think)

@uilianries
Copy link
Member

It's a not good idea, today is PDB, tomorrow will be with_benchmark, after, with_sanitizer ... It's simple, but also is a good excuse for adding new options in the future. I also considered option 3 at beginning, but in long term it will be a big problem, I don't want to maintain one more option, juse like fPIC, where we need manage when to remove or not.

Following option 5, you can add anything there, which will work not only for PDB, but also for any other file that users have requested for a long time, benchmark, for example. So we can hit two birds with a single rock.

@rconde01
Copy link
Contributor Author

Ok - forget about option 3...what about the problems I mentioned with option 5?

@rconde01
Copy link
Contributor Author

rconde01 commented Oct 2, 2020

Any progress on this issue?

@boussaffawalid
Copy link
Contributor

We are also very interested in having pdb files. Option 5 sounds like the most reasonable approach. Is there any decision so far ?

@k0zmo
Copy link

k0zmo commented Feb 2, 2021

Let's compare boost package in terms of size and "debug functionality" you get from them:

  1. Boost, gcc, debug + shared : 112MB .so
  2. Boost, msvc, debug + shared: 14MB .dll and 3.26MB import .libs

If we strip the first configuration we end up with 13MB of .so files which is very close to the MSVC configuration without .pdb files packaged. Instead of stripping the debug data altogether we can separate the debug sections into new file [1] - let's call it a .debug file. This ends up taking up 30MB of .so files and 99MB for .debug files - a bit more than original.
If compared to MSVC .pdb files which occupy 193MB it looks to me they are in the same ballpark.

If you look what you can do with the package the GCC is clearly the winner. For 112MB we get binaries with complete debugging data embedded inside them. On the other hand MSVC users get unoptimized and undebuggable binaries.

What about static variants?

  1. Boost, gcc, debug + static: 318MB .a
  2. Boost, msvc, debug + static: 612MB .lib

Here, the situation is quite alike. Both packages has the same full debugging capabilities. That's because b2 by default for MSVC uses /Z7 which makes the ".pdb contents" embedded inside .lib files. That's why these static libraries are so huge. You can force b2 to spit out .pdb files along .lib [2] and you'd end up with 400MB of .lib and 85.9MB of .pdb [3].

Lastly, let's look at a smaller package, namely thrift:

  1. thrift, gcc, debug + static: 18.8MB .a
  2. thrift, msvc, debug + static: 24.2MB .lib

In 7) .pdb files would take only 4.85MB which is a fraction of the total size of static libraries. Like previously, with GCC for 18.8MB we get a package with a great debugging capabilities. For MSVC our libraries cannot be debugged because of missing 5MB of .pdb files.

So the question shouldn't be if we should package .pdb files or not - we've been already doing the equivalence for most of the time - always with GCC-family compilers or with /Z7 for MSVC if that's the settings the package is built with.
IMHO, the 5th approach you presented looks most flexible. Have you considered doing that also for GCC as it would bring feature parity?

[1] It mimics the .pdb files in terms of distribution. You store your .debug files aside and hand out stripped executables saving tons of storage four your end-users.
[2] Requires debug-symbols=on debug-store=database and also cflags=/FS pch=off, boostorg/build#269
[3] boostorg/build#492

@uilianries uilianries added question Further information is requested and removed bug Something isn't working labels Jul 8, 2021
@tsondergaard
Copy link
Contributor

tsondergaard commented Feb 20, 2022

I’m currently evaluating Conan and the stance on debugging symbols is an issue for us. I would not ship binaries that I do not have debuginfo for. If the program crashes, that debuginfo is needed to analyze the minidump/coredump. I saw a comment higher up saying that most people wont debug third party libraries, but that is not the only reason we need debuginfo for thirdparty modules. A crash will very often happen inside third-party libraries, and if that happens you need the debugging symbols to be able to reliably walk the stack, which you need to diagnose the crash. Storing debuginfo symbols is therefore an unnegotiable requirement. Equally for debug and release builds.

Microsoft has a public symbol server and symbol archive for this reason. So does nvidia. On the linux side all major distributions provide debuginfo packages. That is so core/minidumps can be analyzed.

@michkrom
Copy link

What is the resolution on this?

This question are actually two separate issues:

  1. Ability to build my app in Release mode but with debug information (with-symbols) and preferably for all its dependencies. This is necessary when debugging a Release build either locally or as post-mortem crash of a deployed application.
  2. Weather the conan repositories storing binary builds should include symbols and if so, for which build types? This is a convenience feature if 1) as available unless there are binary-only packages (ie not buildable from source).

@uilianries uilianries added wontfix This will not be worked on and removed wontfix This will not be worked on labels Mar 23, 2022
@puetzk
Copy link
Contributor

puetzk commented Apr 29, 2022

IMO the a great option would be RTFACT-18260 and conan-io/conan#4047, so that the pdb files could be built and peeled off into the symbol server. That way the package download (for CI, etc) would not be unnecessarily bloated, but the symbols for CCI packages could be easily available for debugging (by adding the appropriate CCI symbol server to Visual Studio's debugger).

I'm sure I'm not the first to think of this since there were already issues filed, but it's (surprisingly) not linked here, so I'll at least add it.

@tsondergaard
Copy link
Contributor

I could not find anything in https://github.com/conan-io/tribe. Is this a thing that should be addressed as part of Conan 2.0?

@rconde01
Copy link
Contributor Author

rconde01 commented May 9, 2022

One idea i'm considering here is for public releases is to:

  1. start with an empty cache
  2. Force rebuild all dependencies
  3. Zip up the entire conan cache and archive with the release artifacts

This ensures:

  • I have all pdbs
  • I have all the sources for all dependencies

Of course it's also a big storage and time overhead, but theoretically straightforward and robust

@Ext3h
Copy link

Ext3h commented Jun 15, 2022

Compile vs link time PDBs

There appears to be some mixup in this stread about symbols for a static library and symbols for a shared library.

/Z7 ist pretty much without choice for static libraries, if the build scripts for the library hasn't been explicitly patched to install the compile time debug archives. (E.g. CMake-based builds of static libraries which haven't modified COMPILE_PDB_NAME and haven't installed the file explicitly? /Z7 is the only working option!)

In terms of file sizes, /Z7 ist less favorable compared to a properly configured /Zi with COMPILE_PDB_NAME (/Fd). Every duplicated symbol across multiple object files remains duplicated with /Z7 until linked, /Zi deduplicates early. For header-heavy libraries such as boost, this makes a huge difference.

Using a single compile time PDB for an entire package can actually provide even more savings, even though it can also trigger some exotic compiler bugs when compiler flags differ too much for different targets. (And requires a flat list of static libs in the same folder once packaged!)

Unfortunately, there doesn't appear to be any (documented) option to convert /Z7 based object files to PDB format post-compilation. If there was, that would be pretty much the preferred path for any post-processing based solution as this would be usable on a per-directory base after packaging.

The consumer of the package does need the compile time PDBs to have the linker build the full link time PDB from it.
There is no deferred loading mechanism applicable here, everything missing at link time will be missing for good.

For shared libraries and executables stripping the linker generated PDBs is very much an option. These are not required for the correct build of dependent libraries, but can be deferred loaded during debugging as required. With as little as the minimal dump, both the image, and from that the associated symbol file, can be identified.

/DEBUG:FASTLINK while linking is a trap. While it would reduce the size of the debug symbols for the final, linked images a lot, it's impossible to use with a symbol server. It extends the chain for resolving a symbol from Dump -> Image -> PDB to Dump -> Image -> Object-File / Lib -> PDB - but neither the object-file nor a static lib can never be served by a symbol server, rendering this broken. So the default of /DEBUG:FULL is really the only link type working.

About the build types...

Even Release type builds should have compile time debug symbols. This goes against CMakes definition of Release builds but is a perfect match with Microsofts definition of Release builds.

Release-builds don't mean that you do not want debug capabilities. It only means that you will permit optimizations which might hinder debugging (such as allowing the folding of redundant code sections or aggressive inlining!), but it does not mean that you can categorically discard debug information.

Tbh.: Releasing software built with CMakes definition of Release into the world is pretty much suicidal, as you are declaring already at compile time that you will never be able to analyze any crashes at all. Consider that almost all commercial software out there comes equipped with either integrated crash dump handling, or is registered with Windows integrated functionalities for a good reason. You actively prevent yourself from applying any text-book defensive programming techniques such as fail-fast.

At the same time CMakes RelWithDebInfo is equally a bad choice as you have been disabling about half of the possible compiler optimizations.

It turns pretty much into madness once you realize that with RelWithDebInfo and Release differing also in optimization level, you are quite likely to have both undefined behavior and compiler bugs showing different effects in those build types, with absolutely no chance of tracing the incorrect behavior at all in the Release build.

Always keep in mind that CMake support for Windows was an afterthought, and the concept of splitting symbols from the implementation by default is a first class concept on Windows. So it's perfectly expected that some of the defaults in the integration are just not sane at all.

How to index symbols

What you should realize about the linker created PDBs:

  1. They compress extremely well. They are mostly text-like or nullbytes.
  2. You need to be able to serve requests individually.
  3. They are requested by hash, not in the context of any package ID.
  4. You need to serve both the binary and the debug symbol from a symbol server.

The 2nd requirement renders .tar.gz actually a bad fit for storing the debug symbols, simply because you can not efficiently extract an file from the middle of a gz compressed stream. .zip works perfectly fine for debug symbol archives though.

The 4th requirement in combination with the perfomance constraints means that you do have to make a trade-off between having to store the binaries twice, or serving from an inefficient container! Conan packages - once installed into the cache - can behave different, but that's unrelated to how the conan center as a web-service can handle this.

Building a Windows world compatible symbol index over either dedicated symbol packages or conan archives with embedded link time debug symbols is actually surprisingly simple.

Just a simple POC: https://gist.github.com/Ext3h/1c2125ba838b8cb4ac88b1555ba78ba9
In case you can utilize python, that should already cover half of your development efforts on the PDB related side ;)
The other half is the data model to hold the index.

The only tricky part is to keep the archive itself, as well as the symbol index in sync with any possible de-published binary package. So some database backend integration is mandatory.

There is no pure conan equivalent here, but it integrates really well with downstream Artifactory instances too. Said ahead - the Artifactory integrated PDB support is purely nuget focused so far so we can't use it at all, yet.

However, implementing a dedicated symbol indexing service which maps Symbol-Hashes from https://your.symbolserver/<file_name>/<hash>/<file_name> to a proxy request to https://your.artifactory/repo/sideloaded_debug_package.zip!bin/<file_name> is almost trivial. Just take above POC, crawl your Artifactory via API, download to temp files, index once, persist your index, and start serving. Works fine with authentication or forwarded API keys too!

Have your developers set up their _NT_SYMBOL_PATH environment variable once, and have pretty much every symbol aware debugger or profiler work out of the box. It's all cached by default, so the server load is pretty acceptable once local cache are building up.

Well, almost all is cached. Beware that cache misses are not cached. So a symbol not served by anyone is searched over and over again. Site local caches still recommended.

What about Linux?

That's actually a lot more tricky. The biggest constraint here is that dear old GDB only has support for locating debug symbols in local paths, but is completely missing the ability to specify an upstream server and to cache anything from it.

Of course you can also just side-load a symbol package, but you need to merge it into a local symbol index. And you need to deal with collisions on file names, which only works in the first place when the build-id section was filled in during compilation. Otherwise you are forced to mirror the file system structure which you can't do for a centralized service. gnu_debuglink extension work too, but you will need to manually prefix the paths with package-dependent unique hashes.

I had explored the approach of using WebDAV with davfs to mount a symbol server, but you are reliant on the user configuring aggressive caching policies. And you can't even force the file system to cache file metadata beyond a reboot, so there is no way of providing a symbol server suitable for mounting as a public Internet service. You would be forced to run a site-local caching proxy in front of it, which explicitly needs to cache misses and PROPFIND requests as well.

@tsondergaard
Copy link
Contributor

tsondergaard commented Jun 15, 2022

@Ext3h thanks for the extensive update which I largely agree with. A couple of comments:

Tbh.: Releasing software built with CMakes definition of Release into the world is pretty much suicidal, as you are declaring already at compile time that you will never be able to analyze any crashes at all. Consider that almost all commercial software out there comes equipped with either integrated crash dump handling, or is registered with Windows integrated functionalities for a good reason. You actively prevent yourself from applying any text-book defensive programming techniques such as fail-fast.

I agree 💯!

Always keep in mind that CMake support for Windows was an afterthought

Not fair. Visual Studio has been supported by CMake for more than 20 years. Windows is a first class platform in CMake.

Even Release type builds should have compile time debug symbols. This goes against CMakes definition of Release builds but is a perfect match with Microsofts definition of Release builds.

I believe CMake took the Debug, Release, and RelWithDebInfo names from Visual Studio and if you go back far enough, perhaps to Visual Studio 2005 or thereabouts then the Visual studio "Release" build did not include debug-info.

At the same time CMakes RelWithDebInfo is equally a bad choice as you have been disabling about half of the possible compiler optimizations.

That sounds very bad and also not correct to me. I looked and found https://github.com/Kitware/CMake/blob/master/Modules/Platform/Windows-MSVC.cmake#L456 where the only difference I see is /Ob1 vs /Ob2. I was very surprised to see this difference and have no idea why there is that difference. More restrictive inlining could have a significant impact on performance.

That's actually a lot more tricky. The biggest constraint here is that dear old GDB only has support for locating debug symbols in local paths, but is completely missing the ability to specify an upstream server and to cache anything from it.

This is no longer true. debuginfod exists and gdb in e.g. Fedora 36 has it enabled and will use it to pull debug symbols for distro libraries - it is no longer necessary to yum/dnf install debuginfo packages.

@rconde01
Copy link
Contributor Author

At the same time CMakes RelWithDebInfo is equally a bad choice as you have been disabling about half of the possible compiler optimizations.

That sounds very bad and also not correct to me. I looked and found https://github.com/Kitware/CMake/blob/master/Modules/Platform/Windows-MSVC.cmake#L456 where the only difference I see is /Ob1 vs /Ob2. I was very surprised to see this difference and have no idea why there is that difference. More restrictive inlining could have a significant impact on performance.

See https://gitlab.kitware.com/cmake/cmake/-/issues/20812.

I do the following in my cmake root:

if(MSVC)
   # ---- Fix RelWithDebInfo flags ----
   # * https://gitlab.kitware.com/cmake/cmake/-/issues/20812
   # * Note O2 implies Gy so it is not added here
   # * https://docs.microsoft.com/en-us/cpp/build/reference/o1-o2-minimize-size-maximize-speed?view=msvc-160
   # * Note Ob3 gives more aggressive inlining and is an optimization, and not really a
   #   fix.
   set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "/DNDEBUG /O2 /Ob3 /Zi")
   set(CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO
       "/DEBUG /INCREMENTAL:NO /OPT:REF /OPT:ICF"
   )
   set(CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO "/DEBUG /INCREMENTAL:NO /OPT:REF /OPT:ICF")

But as I was reading this thread I realized I should be trying to handle this for packages as well :(

@Ext3h
Copy link

Ext3h commented Jun 15, 2022

This is no longer true. debuginfod exists and gdb in e.g. Fedora 36 has it enabled and will use it to pull debug symbols for distro libraries - it is no longer necessary to yum/dnf install debuginfo packages.

Thank you very much, I completely missed out on that development. Looks like setting build ID tags is the way to go then, and the web facing API is simple enough to implement 👍

bdegreve added a commit to cocamware/lass that referenced this issue Nov 8, 2023
By default, CMake Release builds are built without debug symbols. The
idea is that you use RelWithDebInfo if you want them. However,
RelWithDebInfo generates less optimized code, only inlining __inline
code. More specifically, RelWithDebInfo builds use the /Ob1 option, and
Release builds use /Ob2 [1,2].

In the end, we want to use Release for production code, but we also need
debug symbols for post-mortem analysis.

[1] conan-io/conan-center-index#1982 (comment)
[2] https://gitlab.kitware.com/cmake/cmake/-/blob/d7741457861816d50114801abc84d1d78afdc754/Modules/Platform/Windows-MSVC.cmake#L486-490
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests