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 Undefined Behavior in check_len #28

Closed
Alexhuszagh opened this issue Jun 20, 2021 · 1 comment
Closed

Fix Undefined Behavior in check_len #28

Alexhuszagh opened this issue Jun 20, 2021 · 1 comment

Comments

@Alexhuszagh
Copy link
Contributor

Comparison between pointers that do no reference the same array (or 1-past the end of the array) is undefined behavior. Quoting the Rust documentation:

If any of the following conditions are violated, the result is Undefined Behavior:

  • Both the starting and resulting pointer must be either in bounds or one byte past the end of the same allocated object.
  • The computed offset, in bytes, cannot overflow an isize.
  • The offset being in bounds cannot rely on “wrapping around” the address space. That is, the infinite-precision sum must fit in a usize.

Likewise, quoting the LLVM Language Reference:

This value only has defined behavior when used as an operand to the ‘indirectbr’ or ‘callbr’instruction, or for comparisons against null. Pointer equality tests between labels addresses results in undefined behavior — though, again, comparison against null is ok, and no label is equal to the null pointer. This may be passed around as an opaque pointer sized value as long as the bits are not inspected. This allows ptrtoint and arithmetic to be performed on these values so long as the original value is reconstituted before the indirectbr or callbr instruction.

Therefore, the following code is undefined behavior. This should likely justify a new version release once this is published.

#[inline]
pub fn check_len(&self, n: usize) -> bool {
unsafe { self.ptr.add(n) <= self.end }
}

Alexhuszagh added a commit to Alexhuszagh/fast-float-rust that referenced this issue Jun 20, 2021
Alexhuszagh added a commit to Alexhuszagh/fast-float-rust that referenced this issue Jun 20, 2021
Alexhuszagh added a commit to Alexhuszagh/fast-float-rust that referenced this issue Jun 20, 2021
@aldanor aldanor closed this as completed in 5d086d1 Jul 3, 2021
aldanor added a commit that referenced this issue Jul 3, 2021
@aldanor
Copy link
Owner

aldanor commented Jul 3, 2021

Thanks for detailed investigation and the fix - this was UB indeed (perhaps not immediately obvious).

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

No branches or pull requests

2 participants