[Clang] Add release note for pointer overflow optimization change #122462
Conversation
|
@llvm/pr-subscribers-clang Author: Nikita Popov (nikic) ChangesAdd a release note for optimization change related to pointer overflow checks. I've put this in the breaking changes section to give it the best chance of being seen. Full diff: https://github.com/llvm/llvm-project/pull/122462.diff 1 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 511a28c5554bbb..aea5eb2a04ac63 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -58,6 +58,26 @@ code bases.
containing strict-aliasing violations. The new default behavior can be
disabled using ``-fno-pointer-tbaa``.
+- Clang will now more aggressively use undefined behavior on pointer addition
+ overflow for optimization purposes. For example, a check like
+ ``ptr + unsigned_offset < ptr`` will now optimize to ``false``, because
+ ``ptr + unsigned_offset`` will cause undefined behavior if it overflows (or
+ advances past the end of the object).
+
+ Previously, ``ptr + unsigned_offset < ptr`` was optimized (by both Clang and
+ GCC) to ``(ssize_t)unsigned_offset < 0``. This also results in an incorrect
+ overflow check, but in a way that is less apparent when only testing with
+ pointers in the low half of the address space.
+
+ To avoid pointer addition overflow, it is necessary to perform the addition
+ on integers, for example using
+ ``(uintptr_t)ptr + unsigned_offset < (uintptr_t)ptr``.
+
+ Undefined behavior due to pointer addition overflow can be reliably detected
+ using ``-fsanitize=pointer-overflow``. It is also possible to use
+ ``-fno-strict-overflow`` to opt-in to a language dialect where signed integer
+ and pointer overflow are well-defined.
+
C/C++ Language Potentially Breaking Changes
-------------------------------------------
|
clang/docs/ReleaseNotes.rst
Outdated
| To avoid pointer addition overflow, it is necessary to perform the addition | ||
| on integers, for example using | ||
| ``(uintptr_t)ptr + unsigned_offset < (uintptr_t)ptr``. |
There was a problem hiding this comment.
In many cases it's possible to rewrite the check in the integer domain instead, checking that unsigned_offset is less then some ptr diff. Should we mention something like that here, so that people don't immediately just add the uintptr_t casts?
There was a problem hiding this comment.
Is the case you have in mind here where people have a ptr and end_ptr and write something like ptr + offset < end_ptr, which can be rewritten as offset < end_ptr - ptr?
There was a problem hiding this comment.
Yes, I was thinking of code like if (ptr + offset >= end_ptr || ptr + offset < ptr)
There was a problem hiding this comment.
I've added an extra example on how to rewrite the check. Does that look good?
| ``-fno-strict-overflow`` to opt-in to a language dialect where signed integer | ||
| and pointer overflow are well-defined. |
There was a problem hiding this comment.
I'm confused by this flag vs. -fwrapv (which we went with in Chromium). Is there a difference?
There was a problem hiding this comment.
With Clang, -fno-strict-overflow and -fwrapv are the same. With GCC, -fwrapv only controls signed integer overflow, while pointer overflow uses a separate -fwrapv-pointer flag. That's why I'm recommending -fno-strict-overflow here, as it will work on both compilers with the same semantics.
I'm also considering adding -fwrapv-pointer to Clang, in which case we could recommend -fwrapv-pointer as the minimally intrusive option here.
There was a problem hiding this comment.
Makes sense. Thanks for explaining!
There was a problem hiding this comment.
And here's the PR for -fwrapv-pointer: #122486
| @@ -58,6 +58,26 @@ code bases. | |||
| containing strict-aliasing violations. The new default behavior can be | |||
| disabled using ``-fno-pointer-tbaa``. | |||
|
|
|||
| - Clang will now more aggressively use undefined behavior on pointer addition | |||
There was a problem hiding this comment.
Could we add a motivating example to show this is not about pedantry but is useful for removing redundant checks?
There was a problem hiding this comment.
I think it's hard to add an example that is small and simple enough for a release note. After all, the distinguishing feature between a desirable and an undesirable optimization in this context is just how many layers of abstraction there are. (This is Schrödinger's programmer: Doesn't want the compiler to optimize trivial cases because its "obviously wrong". Does want it to optimize complex cases.)
E.g. one of the motivating cases I have lying around is https://godbolt.org/z/3Y5bTxzqq, and that is both too large for a release note and too reduced to be really meaningful :)
…vm#122462) Add a release note for optimization change related to pointer overflow checks. I've put this in the breaking changes section to give it the best chance of being seen.
Add a release note for optimization change related to pointer overflow checks. I've put this in the breaking changes section to give it the best chance of being seen.