-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
macho/load_commands: support new macOS 15 dylib use command #625
Conversation
Cursed, thank you Apple 🤦. Reviewing now! |
If you'd like an example binary, I can send you some |
lib/macho/load_commands.rb
Outdated
@@ -528,6 +542,35 @@ class DylibCommand < LoadCommand | |||
# @return [Integer] the library's compatibility version number | |||
field :compatibility_version, :uint32 | |||
|
|||
def self.new_from_bin(view) |
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.
Minor: it'd be nice to have a comment here explaining why we override new_from_bin
-- just a summary of what you put in the PR desc would be great 🙂
Yes please! Checking them in with the other test assets would be IMO ideal. |
}.freeze | ||
|
||
# the marker used to denote a newer style dylib use command. | ||
# the value is the timestamp 24 January 1984 18:12:16 |
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.
Funny choice -- that's the day the Macintosh was released.
I've checked one in. Not actually linked into any test yet but it's there. Would you like a test prior to merging and releasing? Not poked around the tests here yet but can have a look and see which one might fit it best. |
Actually didn't see the Makefile - let me fix that. |
No test before merge necessary, but before release would be ideal -- happy to review in a follow-up if you'd like to get this in first. |
e62d222
to
cdc1636
Compare
Did some tests - let me know if you think I've missed anything. |
:LC_LOAD_DYLIB => "DylibCommand", | ||
:LC_LOAD_DYLIB => "DylibUseCommand", |
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.
Did this and moved the new_from_bin
hack to there to be slightly more strict where this applies - it should not apply to LC_ID_DYLIB
etc.
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.
LGTM, thanks @Bo98!
@Bo98 LMK when you want a release -- I can do one whenever. |
A new release now would be great so I can use this in Homebrew/brew and get a release out for that too. |
Released with 4.1.0! |
Thank you! |
macOS 15 adds a new
flags
field at the end of the load command. The presence of this is checked indyld
by comparing thetimestamp
andnameoff
:https://github.com/apple-oss-distributions/dyld/blob/d552c40cd1de105f0ec95008e0e0c0972de43456/mach_o/Header.cpp#L1175
This requires a bit of a hack in
new_from_bin
to basically load the old command, check it and then load the new command instead if conditions match.