ASan/MSan should call __sanitizer_unaligned_{load,store} functions when performing unaligned reads/writes (TSan already calls equivalents)

[thomcc]

There are special functions exposed by several of the sanitizers for performing unaligned loads and stores. The functions have names like __sanitizer_unaligned_{load,store}{16,32,64}¹, and come with the runtimes of at least MSan/ASan/TSan, but not LSan.

The documentation for them notes:

Some of the sanitizers (for example ASan/TSan) could miss bugs that happen in unaligned loads/stores. To find such bugs reliably, you need to replace plain unaligned loads/stores with these calls.

Under -Zsanitize=thread we end up calling functions like __tsan_unaligned_read4 which turns out to be what tsan's implementations for these do anyway: https://github.com/llvm/llvm-project/blob/350fafabe9d3bda75e80bf077303eb5a09130b53/compiler-rt/lib/tsan/rtl/tsan_interface.cpp

However, we don't do this for any other sanitizer. ASan and MSan both provide implementations of these. I don't know how beneficial this is for MSan, but for ASan at least is mentioned in the header.

Unless I'm mistaken, we should be doing this for MSan and ASan. The fact that we do it for TSan gives me hope that it can be added without too much effort. Alternatively, maybe the current behavior is intentional, and these are unneeded for our case for a reason that I'm not aware of.

For clarity, I haven't seen ASan miss issues because of this (and in truth have never gotten MSan working well), I just stumbled across it.

Anyway, Godbolt for convenience: https://rust.godbolt.org/z/Ed4arK. Change -Zsanitize=thread to use -Zsanitize=address or -Zsanitize=memory to see what I mean.

---

¹ This is included by the (more commonly used) headers <sanitizer/[atml]san_interface.h>. These are definitely public even though they begin with underscores.

[tmiasko]

This is expected, ASAN and MSAN instrumentation usually interacts with shadow memory directly rather than calling into the runtime.

[thomcc]

Is the documentation of those functions incorrect then?

[tmiasko]

The instrumentation is performed under assumption that operations will have required alignment. It would be best combined with separate detection of misaligned loads & stores, because otherwise errors caused by such operations might not be detected.

[thomcc]

I'm not sure I follow. I'm just saying to do it using the times the compiler statically knows the loads are misaligned. Most loads wouldn't go through this, only ones in the cases of:

• Explicit calls to ptr::read_unaligned and equivalent.
• Access to unaligned #[repr(packed)] structs.

These are legal misaligned accesses (when done properly), and the compiler knows about them statically. E.g, I'm not sure there needs to be detection of unaligned ops, the compiler already handles this correctly for tsan AFAICT.

Or do you mean something like: that we insert checks if the address is in-face unaligned at runtime, and only call the unaligned function if actually unaligned? That would be pretty odd, if I'm being honest.

[tmiasko]

I'm just saying to do it using the times the compiler statically knows the loads are misaligned.

This is exactly how it works (btw, I wouldn't call loads in read_unaligned misaligned). If you compare ASAN instrumentation you will see that read_unaligned consults the shadow memory twice to take into account that load is unaligned, while read consults shadow memory only once.