[clang] -fsanitize=alignment: check memcpy/memmove arguments (PR #67766)

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 29 14:03:04 PDT 2023


zygoloid wrote:

> Uh, why are we allowed to assume that memcpy pointer arguments are aligned? This looks like a miscompile to me.

This is definitely a bit weird, but...

> A plain `int *` pointer is not required to be aligned, and memcpy works on `void *` pointers, so I'm not sure where an alignment requirement would appear from.

... a plain `int *` pointer *is* sort of required to be aligned. Per https://eel.is/c++draft/expr.static.cast#14.sentence-2 (and similar wording exists in C), you get an unspecified pointer value if you cast an unaligned pointer to `int *`, which is allowed to be an invalid pointer value, which we are then permitted to give whatever semantics we like, such as disallowing it being passed to `memcpy`.

Clang generally chooses to define the semantics of misaligned pointer values as doing the "obvious" thing (@rjmccall circulated an RFC on this a few years ago). In short: we allow them to be formed, and we allow pointer arithmetic on them, but don't allow them to be used to actually access misaligned memory. But `memcpy` is a memory access, so if you directly pass an `int *` to it, I think we're following our rules if we require it to be aligned. And it seems like a valuable thing to assume: in the vast majority of cases, it is reasonable to assume `T`'s alignment when a `T*` is passed to `memcpy`.

I think the choice we're making here is probably worth it, though we should probably document it better. I think you can remove the alignment assumption by explicitly casting the operands to `char*` before passing them to `memcpy`; if you can't, I'd be more worried that we're doing something problematic here. Also, it'd seem like a good idea to make the sanitizer message as clear as possible for this case, because Clang's behavior here is surprising.

https://github.com/llvm/llvm-project/pull/67766


More information about the cfe-commits mailing list