Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MachO::Sections function flag? implementation is wrong #423

Closed
iOS-Developer-Sun opened this issue Jan 7, 2022 · 5 comments · Fixed by #424
Closed

MachO::Sections function flag? implementation is wrong #423

iOS-Developer-Sun opened this issue Jan 7, 2022 · 5 comments · Fixed by #424
Assignees
@woodruffw
Labels
bug outdated

Comments

@iOS-Developer-Sun
Copy link

iOS-Developer-Sun commented Jan 7, 2022 •
edited
Loading

The SECTION_TYPE 0xff should be considered like

flags & SECTION_TYPE == flag
@woodruffw
Copy link
Member

Thanks for catching that. I'll make a fix PR in a moment.

@woodruffw woodruffw added the bug label Jan 7, 2022
@woodruffw woodruffw self-assigned this Jan 7, 2022
@woodruffw
Copy link
Member

Oh, I think I see why this is -- we don't currently distinguish between the SECTION_TYPE masked area and the SECTION_ATTRIBUTES masked area within flags. Those should really be separate predicates, like type? and attribute?.

woodruffw added a commit that referenced this issue Jan 7, 2022
@woodruffw
sections: add Section#type? and Section#attribute?
421f4d0 
This two predicates replace the `Section#flag?` predicate, which
has unintuitive behavior and is now deprecated.

Closes #423.
@woodruffw woodruffw mentioned this issue Jan 7, 2022
Merged
@woodruffw
Copy link
Member

woodruffw commented Jan 7, 2022 •
edited
Loading

#424 will address this. It'll also do the same for segments, since SegmentCommand#flag? suffers from the same bug. Misspoke, that flag? impl is fine.

@iOS-Developer-Sun
Copy link
Author

flag? is used in function low_fileoff
next if sect.flag?(:S_ZEROFILL) next if sect.flag?(:S_THREAD_LOCAL_ZEROFILL)
It is not right although most of the time it returns the right section since the flags of the first section like __text, __got are usually even number. I think these two call references should be changed to type?(:S_ZEROFILL). flags? is meaningless.

@woodruffw
Copy link
Member

Yep, that's on the list of callsites to update. And yeah, I've marked flags? as deprecated and it'll be removed in an upcoming major release.

woodruffw added a commit that referenced this issue Jan 11, 2022
@woodruffw
sections: add Section#type? and Section#attribute? (#424)
59020eb 
* sections: add `Section#type?` and `Section#attribute?`

This two predicates replace the `Section#flag?` predicate, which
has unintuitive behavior and is now deprecated.

Closes #423.

* macho_file: fix `MachOFile#low_fileoff`

Previously, the offset returned by this method could be incidentally
wrong, depending on unrelated flags in each section. This changeset fixes
the method so that only the section's type is checked, rather than
the entire composite flag field.

* load_commands: add SG_READ_ONLY

Introduced by Mach-O at some point.

* test: add another segment flags test

* sections: rename constants

Clarify that these are masks.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@woodruffw woodruffw

Labels
bug outdated
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sections: add Section#type? and Section#attribute? Homebrew/ruby-macho
2 participants
@woodruffw @iOS-Developer-Sun