-
Notifications
You must be signed in to change notification settings - Fork 8
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
Sync to LLVM 18.1.3 #14
Conversation
de378ad
to
4425a36
Compare
src/archive_writer.rs
Outdated
|
||
struct SymbolicFile<'a> { | ||
data: &'a [u8], | ||
parsed: File<'a>, |
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.
ar_archive_writer intentionally doesn't parse object files. When used as part of rustc symbols we need to get symbols from LLVM bitcode files, which only LLVM itself can. This is why the get_symbols
field is used for getting symbols instead. Rustc overrides it with a function which calls into LLVM to get all symbols.
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 should be able to avoid parsing in most cases, but I'm not sure what to do about AIX symbol tables: we need to know if the obj is 32 or 64 bit to place the symbols in the correct table, and we need to get the alignment. Should I add new callbacks for both of those?
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.
Ok, I've moved everything that was using the object
crate into a new object_reader
module and replaced the get_symbol
function pointer with an ObjectReader
struct that has function pointers for each of the functions that need to parse object files.
src/archive_writer.rs
Outdated
match &obj.parsed { | ||
File::Coff(coff_obj) => { | ||
coff_obj.architecture() != Architecture::Aarch64 | ||
|| coff_obj.sub_architecture() == Some(SubArchitecture::Arm64EC) |
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.
Does this cover import files?
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.
And it doesn't handle llvm bitcode.
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.
Added in support for import files, but not for bitcode - we can either try to add this later, or have the LLVM backend provide an implementation that does check for bitcode.
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.
Support for reading bitcode is currently done by the LLVM backend of rustc by overriding get_symbols
(now object_reader
).
I'm done with this review round. |
Thanks a lot for this! |
Anything else to do before I publish it to crates.io? |
Nope, I think this is good to go! |
Done. The rustc update to 0.2.0 got approved earlier today. |
Syncs to LLVM 18.1.3:
Other changes:
reference
directory, to make it clear that they aren't built.object_reader
module, and replaced the singleget_symbol
callback with anObjectReader
struct that has all the callbacks we require for reading object files.object
to latest.object
.Fixes #9