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.

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

🍒[release/6.0][llvm][MachO] Fix integer truncation in rebase/bind parsing (#89337) #8832

Merged
merged 1 commit into from
Jun 7, 2024

Conversation

zixu-w
Copy link

@zixu-w zixu-w commented Jun 3, 2024

Count and Skip should use uint64_t as they are encoded/decoded using 64-bit ULEB128.

In *_OPCODE_DO_*_ULEB_TIMES_SKIPPING_ULEB, Skip could be encoded as a two's complement for moving SegmentOffset backwards. Having a 32-bit Skip truncates the encoded value and leads to a malformed AdvanceAmount
and invalid SegmentOffset that extends past valid sections.

(cherry picked from commit b910beb)

`Count` and `Skip` should use `uint64_t` as they are encoded/decoded
using 64-bit ULEB128.

In `*_OPCODE_DO_*_ULEB_TIMES_SKIPPING_ULEB`, `Skip` could be encoded as
a two's complement for moving `SegmentOffset` backwards. Having a 32-bit
`Skip` truncates the encoded value and leads to a malformed
`AdvanceAmount`
and invalid `SegmentOffset` that extends past valid sections.

(cherry picked from commit b910beb)
@zixu-w zixu-w requested review from fredriss and cyndyishida June 3, 2024 21:28
Copy link

@cyndyishida cyndyishida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fredriss Could you review? We need this for tapi fixes, but not necessarily for the (swift or clang) compiler. Same goes for #8833
I don't have merge privileges.

@zixu-w
Copy link
Author

zixu-w commented Jun 3, 2024

@swift-ci please test

@zixu-w
Copy link
Author

zixu-w commented Jun 5, 2024

Hi @fredriss could you help merge this PR? Thanks!

@fredriss fredriss merged commit fa2926b into swift/release/6.0 Jun 7, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants