-
Notifications
You must be signed in to change notification settings - Fork 5k
Generate .comment
section for ELF executables
#113218
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
Conversation
This is the standard way to emit compiler version info on ELF. No standard way for other formats.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR adds support for emitting a .comment section in ELF executables to provide compiler version information.
- Adds a new using directive for System.Reflection.
- Introduces a CommentSection static field and adds handling for it in CreateSection.
- Implements writing of the .comment section using the assembly's informational version in EmitSectionsAndLayout.
Reviewed Changes
File | Description |
---|---|
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ElfObjectWriter.cs | Adds .comment section handling to embed compiler version info for ELF executables |
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ElfObjectWriter.cs:578
- Accessing InformationalVersion directly may throw a NullReferenceException if AssemblyInformationalVersionAttribute is missing. Consider adding a null-check or providing a default value.
commentSectionWriter.WriteUtf8String($".NET AOT {Assembly.GetExecutingAssembly().GetCustomAttribute<AssemblyInformationalVersionAttribute>().InformationalVersion}");
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
|
Mach-O actually has somewhat standard way, if we want to populate it. The information is also present in the DWARF debugging info IIRC. |
@@ -567,6 +574,9 @@ private void EmitRelocationsRiscV64(int sectionIndex, List<SymbolicRelocation> r | |||
|
|||
private protected override void EmitSectionsAndLayout() | |||
{ | |||
SectionWriter commentSectionWriter = GetOrCreateSection(CommentSection); | |||
commentSectionWriter.WriteUtf8String($".NET AOT {Assembly.GetExecutingAssembly().GetCustomAttribute<AssemblyInformationalVersionAttribute>().InformationalVersion}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think shorter string is better, we just need something reasonably unique.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense this is for technical use for crash investigations etc. BTW, with Native
, it'll be 94 characters. These two will still be the longest (117 and 115 respectively):
[ 16] clang version 19.1.7 (https://github.com/ziglang/zig-bootstrap 1c3c59435891bc9caf8cd1d3783773369d191c5f)
[ 7f] Linker: LLD 19.1.7 (https://github.com/ziglang/zig-bootstrap 1c3c59435891bc9caf8cd1d3783773369d191c5f)
If this is a concern for someone, they can drop the .comment section from the binary (using tools like objcopy).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.NET Native AOT, the proper brand name
.NET Native AOT is not a proper brand name since it is not its own product. The docs talk about native AOT without .NET prefix, same as how they talk about any other .NET feature. There should be zero hits for ".NET Native AOT" in docs and dotnet-api-docs
".NET AOT" is pushing it too.
My preference would be one of the following strings:
.NET 10.0.0-preview3...
.NET: ilc 10.0.0-preview3...
.NET: Native AOT 10.0.0-preview3...
.NET: PublishAot 10.0.0-preview3...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a preference; rolled a dice and went with one of these in #113255.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 sounds good. ilc
is, relatable, name of the tool.
@@ -567,6 +574,9 @@ private void EmitRelocationsRiscV64(int sectionIndex, List<SymbolicRelocation> r | |||
|
|||
private protected override void EmitSectionsAndLayout() | |||
{ | |||
SectionWriter commentSectionWriter = GetOrCreateSection(CommentSection); | |||
commentSectionWriter.WriteUtf8String($".NET AOT {Assembly.GetExecutingAssembly().GetCustomAttribute<AssemblyInformationalVersionAttribute>().InformationalVersion}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the informational version is what we want here. I don’t know what else we would pick. It might be nice to include the git sha, but I’m not sure if we put that anywhere but the PDB. Jeremy do you know offhand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InformationVersion is a good choice. It returns semver with 'semver metadata', like e.g. 10.0.0-preview.3.25152.4+088d199063dd1bf7fa00a445d23f93f915f84b31
. That's what differenciates it from Environment.Version
(which strips away the metadata).
public partial class ProductVersionInfoGenerator : IIncrementalGenerator |
cc @akoeplinger who worked on similar topic recently (related to servicing versions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The informational version has the commit sha (as @am11 points out), so I think it's the right choice for the scenario.
I'm not a fan of hijacking values. PE also has the Rich header that we might be able to get into but it has the same problems (need to assign a new number on every major release, which is really only possible for things that live in-tree with link.exe sources). |
This is the standard way to emit compiler version info on ELF. No standard way for other formats.
Cc @dotnet/ilc-contrib