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

Fix improper unaligned reads in value_from_ptr #277

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RealKC
Copy link

@RealKC RealKC commented Dec 1, 2024

copy_nonoverlapping documents that its pointer must be aligned for a valid read, otherwise the call will be UB.

This patch will fix that, and I believe it will fix #269. (I tried @igor-petruk's oatbar at commit igor-petruk/oatbar@baf1627 which is what he linked in the issue, changed the Cargo.toml to depend on my local copy that had this change and did cargo run, and I didn't get a crash.)

This was only somewhat recently noticed because Rust started checking these preconditions in Rust 1.78.

As some notes:

  • should we add debug_assert!(!ptr.is_null()) to this function?
  • is there a point in having this function now vs modifying the code generator to call read_unaligned?

@rtbo
Copy link
Collaborator

rtbo commented Dec 25, 2024

Thanks for that.
I think indeed it is correct.

Regarding your notes:

  • The pointer passed to value_from_ptr are the one returned by the libxcb API. Assertion should not be needed.
  • Calling read_unaligned directly from code generator is also fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants