-
Notifications
You must be signed in to change notification settings - Fork 132
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
ilc doesn't match expectations from SourceBuiltSdkContainsNativeDebugSymbols. #4472
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
1 similar comment
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
What is the raw output of |
Running on my system against a source-built
|
Naot publish strip symbols by default. Can you try to set |
I'll give that a try. |
This makes things better, and I'll open up a PR for it. The test is still failing for |
What is the output of readelf on the unstripped ilc? The line is looking for |
|
Is that |
Oh, never mind, the test runs both -s and -S. I'm surprised FILE is part of -s output. |
cc @NikolaMilosavljevic who is familiar with these tests. |
The FILE symbols are dropped because we pass This switch is a 5% size saving on an unstripped hello world. My question would be why we don't specify |
I don't know what the effects are of dropping the FILE symbols, and why the test goes looking for those specifically. Perhaps it would make sense to use |
cc @am11 who did a lot of the work on StripSymbols. |
Yup, it is also required by lld (llvm linker) 13+ regardless of symbol stripping. @tmds, does the test pass with |
I think the test is not very useful. We should export a symbol in test (via |
Is it required to add
I don't know what uses the @omajid you authored this test, any thoughts on omitting the |
Targets has I think we should not base these decisions off of FILE symbol test (alone), which is not a tangible way to validate the functionality of AOT applications. |
The test is trying to ensure we have the debugging symbols in the executable ( |
|
I think the key is understanding what is the FILE check protecting from. I would think checking for .debug_info and .debug_abbrev sections is sufficient, but there must have been a reason for the extra check. |
I don't really have more context than the comment in the test:
The comment pretty much summarizes what a gdb developer explained to me ~10 years ago. The test is just trying to making sure nothing has stripped symbols or otherwise manipulated debug-info for unmanaged binaries. In other words, that the debuginfo is generated by the original (C/C++) compiler and has not be modified by any other tool. This seemed like a good invariant and has held up true for .NET Core pretty much since 1.0. If this is not a good invariant for managed/NativeAOT generated code, I would be okay with disabling it for those cases (which is just ilc for now?). |
After some experiments, I haven't found FILE symbol being exported on Ubuntu or Alpine with .NET 8 and .NET 9 P6 with/without symbol stripping, which suggests we don't export it and yet gdb and lldb debugging is working fine. |
It's not a good invariant for anything compiled with You can try with:
and run |
ChatGPT:
and with DWARF format (which we are using), it captures the filename and line number info in standard format, rendering |
With dotnet/sdk#41198, we've added the
ilc
ILCompiler as part of the .NET installation.ilc
is .NET application that is NativeAOT compiled.The
Microsoft.DotNet.SourceBuild.SmokeTests
include a test,SourceBuiltSdkContainsNativeDebugSymbols
, which checks the native binaries in the .NET installation.The test is failing for the
ilc
binary with:We've updated the test to skip
ilc
: https://github.com/dotnet/sdk/blob/01aab3448c9185f2a5b7e66e7e327fe4b7841408/src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/DebugTests.cs#L77-L78Are there things we can/should do to make
ilc
pass the test?cc @MichalStrehovsky @filipnavara @jkotas @omajid @ViktorHofer @MichaelSimons
The text was updated successfully, but these errors were encountered: