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

Check alignment of raw pointers in debug mode #54915

Open
RalfJung opened this issue Oct 8, 2018 · 7 comments

Comments

@RalfJung
Copy link
Member

commented Oct 8, 2018

It is UB to *r a raw pointer ("dereference", but this includes &*r) when that pointer is not aligned to its type. I propose that when compiling in debug mode, we insert a check for this to emit a panic when the pointer is not sufficiently aligned. That would essentially be the "big brother" of #52972, and the cousin of #53871.

This would have caught #54908.

However, I guess before someone starts implementing this, we should get @rust-lang/compiler to say that they think this is a good idea. It's in debug mode only, and it's a rather cheap check (as alignment is always a power of two), so I expect the perf impact to be acceptable.

@leonardo-m

This comment has been minimized.

Copy link

commented Oct 9, 2018

See also Zig, that exposes such alignment information in the type system itself:
https://andrewkelley.me/post/unsafe-zig-safer-than-unsafe-rust.html

Using syntax like:

var array align(@alignOf(Foo)) = []u8{1} ** 1024;

The run-time check perf impact is probably acceptable because currently debug builds are often 1000%+ slower.

I suggest to add a switch like -Z force-overflow-checks that could be switched on in "efficient experiment" release builds too if desired, because sometimes debug builds are so slow you can't use them well to test some code.

More generally, what other basic invariants like that could rustc verify automatically (in debug builds)?

@RalfJung

This comment has been minimized.

Copy link
Member Author

commented Oct 9, 2018

I suggest to add a switch like -Z force-overflow-checks that could be switched on in "efficient experiment" release builds too if desired, because sometimes debug builds are so slow you can't use them well to test some code.

Good point. We have something similar for integer overflow, don't we?

More generally, what other basic invariants like that could rustc verify automatically (in debug builds)?

Tons of them, but which have a good perf-benefit trade-off? Also note that this wouldn't reliably verify the alignment invariant, one can still mem::transmute or use a union or cast pointer types around to get an underaligned (or NULL) reference. It's not a perfect check (unlike what we can do e.g. in miri), but it's useful.

Some other things coming to my mind are:

  • Instead of triggering UB when having an uninhabited local variable, trigger a llvm.trap.
  • When doing a match on an enum or bool, verify that the discriminant is valid.
  • Go over the intrinsics and see which could check some of their preconditions -- things like unchecked_div or unchecked_shl. Would it be better to check this is the stable wrappers around the intrinsics, or change the code generated by the intrinsics?
  • Go over the unsafe functions in libstd and see which of these could get debug_assert! checking (some of) their preconditions. Things coming to my mind are from_utf8_unchecked and get_unchecked. Here's an incomplete list.

For the last (or the last two) to be viable, however, we need a libstd compiled with debug assertion to be used when compiling programs in debug mode.

Everything else I can think of involves some kind of type-based recursion on e.g. the return value of mem::transmute, which is much more work and much less useful (as mentioned above, there are many other ways to transmute data).


Finally, we'd probably have to do some communication work to get people to run their test suites in debug mode, so that this actually reaches all the projects out there. Is there any way to get debug assertions, but also get optimizations, so that this doesn't make everything quite so much slower? It does seem useful to me to have code with extra checks optimized; that means the compiler can assume fewer things (less UB) but it can still e.g. optimize away the checks if it can prove they always succeed, and it can do all the other optimizations (inlining and whatnot).

@arielb1

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2018

Is there any way to get debug assertions, but also get optimizations, so that this doesn't make everything quite so much slower?

Sure enough. -O -C debug-assertions=on. Not sure how to do it via cargo.

@RalfJung

This comment has been minimized.

Copy link
Member Author

commented Oct 9, 2018

Ah, nice! And that will also affect integer overflow checks?

@arielb1

This comment has been minimized.

Copy link
Contributor

commented Oct 13, 2018

Ah, nice! And that will also affect integer overflow checks?

Integer overflow checks will be on (they are controlled by debug-assertions, not by -O).

@memoryruins

This comment has been minimized.

Copy link
Contributor

commented Oct 14, 2018

With Cargo, the flags are placed in [profile.*] sections. The following are two of the defaults for dev builds

# The development profile, used for `cargo build`.
[profile.dev]
opt-level = 0      # controls the `--opt-level` the compiler builds with.
                   # 0-1 is good for debugging. 2 is well-optimized. Max is 3.
                   # 's' attempts to reduce size, 'z' reduces size even more.
debug-assertions = true # controls whether debug assertions are enabled
                   # (e.g. debug_assert!() and arithmetic overflow checks)

Finally, we'd probably have to do some communication work to get people to run their test suites in debug mode, so that this actually reaches all the projects out there

Fortunately, the default travis-ci rust set-up runs cargo build --verbose and cargo test --verbose, and the samples for travis and gitlab in the Cargo book do similarly. Both build and test use [profile.dev] by default, so it would definitely reach many out there without even mentioning, at least for unoptimized debug builds and tests.

@RalfJung

This comment has been minimized.

Copy link
Member Author

commented Apr 21, 2019

Cc #51713

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.