Fix debug symbol generation #34154
Fix debug symbol generation #34154
Conversation
The debug symbol generation got recently broken. For most of the shared libraries, the debug symbols were stripped twice due to the fact that install_clr for them was invoked twice - once for the default install location and once for the sharedFramework location and the stripping was executed in both of them. First stripping stripped the symbols off the target binary and set so called debuglink in the binary to point to the symbol file. This debuglink includes a crc32 of the dbg symbols file. The second stripping tried to strip symbols from the already stripped binary. That resulted in a small dbg symbols file that didn't actually contain any useful symbols. Moreover, it is not possible to set a debuglink in a binary if it is already set there. So the second attempt failed and the crc was left set to the crc of the previous debug. Thus when debugger loads such a binary, it cannot find the debug symbols file, as the crc doesn't match. And even if it matched, the data would have no value. The fix is to modify install_clr so that it has an extra optional argument to specify the secondary install location and use just one install_clr per target. The function then does the stripping just once and the actual installation once or twice depending on the secondary location argumenbt presence.
|
cc: @am11 |
| if(CLR_CMAKE_TARGET_WIN32) | ||
| install(FILES ${CMAKE_CURRENT_BINARY_DIR}/$<CONFIG>/${targetName}.pdb DESTINATION ${destination_path}/PDB) | ||
| install(FILES ${symbol_file} DESTINATION ${destination_path}/PDB) |
There was a problem hiding this comment.
I think it is better, if we do not modify this (Windows) part at this stage, as it is working out fine. Otherwise we would need to update libraries and installer as well.
There was a problem hiding this comment.
I'll rather look into fixing the libraries and installer. I have forgotten that they use these now too.
There was a problem hiding this comment.
Sure, I remember doing similar cleanup in libraries, it was something like touching one file (src/libraries/Native/Unix/CMakeLists.txt) vs. multiple other CMakeLists.txt under libraries, therefore, ended up favoring the common pattern so to touch fewer files. :)
|
Thanks @janvorli! 👍 Failure is due to libraries symbols, which I think can be fixed by reverting the changes in the common, kitchen sink strip function. |
| install_symbols(${targetName} .) | ||
| set(install_source_file $<TARGET_FILE:${targetName}>) | ||
| install(PROGRAMS ${install_source_file} DESTINATION .) | ||
| install_with_stripped_symbols(${targetName} .) | ||
| endfunction() |
There was a problem hiding this comment.
A nit: since there is not much left in this function, would it make sense to inline this at call sites by introducing a variable in this file:
set(CLR_LIBRARIES_UNIX_ROOT ${CMAKE_CURRENT_LIST_DIR})and at call sites: install_with_stripped_symbols(System.Native.XYZ ${CLR_LIBRARIES_UNIX_ROOT})?
There was a problem hiding this comment.
The . doesn't represent the CMAKE_CURRENT_LIST_DIR, but rather the installer output root (CMAKE_INSTALL_PREFIX), as the destination argument of the install is relative to that.
But it seems to make sense to replace this function with call to install_with_stripped_symbols(System.Native.XYZ .) to make it more consistent with the other installation commands that strip symbols (except for the coreclr one that's special).
There was a problem hiding this comment.
replace this function with call to
install_with_stripped_symbols(System.Native.XYZ .)
Makes sense, I think that will be a bit more work to get packaging pick it up. I tried that previous but the packaging step was failing, IIRC.
There was a problem hiding this comment.
I am not sure why should there be a difference. It should generate exactly the same files at the same places as before my change.
There was a problem hiding this comment.
I can see the problem - the install_with_stripped_symbols uses install(PROGRAMS ...) while the installer was using install(TARGETS ...) and I've missed that difference. That causes the .lib for .dll files to not to be installed.
|
@dotnet/dnceng some of the test legs are failing due to the inability to download .NET Core SDK. For example: It uses the following URL and when I open it in a web browser, I get the response below. |
|
You hit #34015. The 404 isn't the right URL to check, |
|
The failures in the CI are due to #28553 |
|
@jkoritzinsky can you please take a look at this PR? |
ghost
locked as resolved and limited conversation to collaborators
The debug symbol generation got recently broken. For most of the
shared libraries, the debug symbols were stripped twice due to
the fact that install_clr for them was invoked twice - once for
the default install location and once for the sharedFramework
location and the stripping was executed in both of them. First
stripping stripped the symbols off the target binary and set
so called debuglink in the binary to point to the symbol file.
This debuglink includes a crc32 of the dbg symbols file.
The second stripping tried to strip symbols from the already
stripped binary. That resulted in a small dbg symbols file
that didn't actually contain any useful symbols. Moreover,
it is not possible to set a debuglink in a binary if it is
already set there. So the second attempt failed and the crc
was left set to the crc of the previous debug. Thus when
debugger loads such a binary, it cannot find the debug symbols
file, as the crc doesn't match. And even if it matched, the
data would have no value.
The fix is to modify install_clr so that it has an extra
optional argument to specify the secondary install location and
use just one install_clr per target. The function then does the
stripping just once and the actual installation once or twice
depending on the secondary location argumenbt presence.
Close #34108