[llvm] [llvm] Fix behavior of llvm.objectsize in presence of negative / large offset (PR #115504)

via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 12 22:41:46 PST 2024


================
@@ -580,6 +585,11 @@ bool llvm::getObjectSize(const Value *Ptr, uint64_t &Size, const DataLayout &DL,
   if (!Data.bothKnown())
     return false;
 
+  // We could compute an over-approximation in that situation, may be if
+  // Opts.EvalMode == Max, but let's bee conservative and bail out.
+  if (Data.Offset.isNegative())
+    return false;
----------------
serge-sans-paille wrote:

I disagree :-)
Let's consider the following fortified access:

```
inline int access(int* ptr, int n) {
    long bos = __builtin_object_size(ptr, 0);
    if (/*fail to perform static check */ bos < 0 || /*in bound access */ n < bos)
        return ptr[n];
    else
        abort(); // we're sure the access is invalid
}
```

which looks like a decent usage of  ``__builtin_object_size``.

Then what happens with the following call site?

```
int caller() {
    int a[2] = {-1, -1};
    return access(a - 2, 3);
}
```

1. this behavior is undefined (forming a pointer that's previous the actual object. `-fsanitize=undefined` correctly catches it (see https://godbolt.org/z/6Ejojc7Y5 for an equivalent scenario).
2. the ``access`` call will incorrectly abort, throwing a false positive. It can be argued that it's an UB so it's ok (and it is!) but returning -1 looks like the path of least surprise to me. See https://godbolt.org/z/nG5voPco5 for an equivalent scenario.

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


More information about the llvm-commits mailing list