================
@@ -0,0 +1,39 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -std=c++17 -verify %s
+
+// https://github.com/llvm/llvm-project/issues/210183
+// Don't crash when binding a value that isn't a CompoundVal (e.g. a pointer
+// reinterpreted as an integer via a template-induced round trip) to a region
+// of array type.
+
+// expected-no-diagnostics
+
+template <typename T>
+class Span {
+public:
+  template <int N>
+  Span(T (&arr)[N]) : ptr_(arr) {}
+  T *data() { return ptr_; }
+  T *ptr_;
+};
+
+class Decoder {
+public:
+  template <typename TInput, typename TOutput>
+  auto decodeOne(TInput input, TOutput output) {
+    using InPtr_t = decltype(input.data());
+    InPtr_t inPtr = input.data();
+    using OutPtr_t = decltype(output.data());
+    OutPtr_t outPtr = output.data();
+    int scratch[1];
+    scratch[0] = *inPtr;
+    int value = scratch[0];
+    *outPtr = value;
+  }
+};
+
+void test() {
+  char buffer[1];
+  Span<char> span(buffer);
+  Decoder decoder;
+  decoder.decodeOne(span, span);
+}
----------------
andyames-a11y wrote:

Confirmed both — thanks for digging in.

`Init` is indeed `nonloc::LocAsInteger`. Ran your reduced repro with 
`debug.ExprInspection` against a build with this patch applied:

```
clang_analyzer_dump(buffer): &Element{buffer,0 S64b,char}
clang_analyzer_dump(p):      &buffer
clang_analyzer_dump(v):      &SymRegion{conj_$3{char *, LC1, S2103, #1}} [as 32 
bit integer]
*p = v;                      -- no crash
```

So yes — `v` is a pointer symbol reinterpreted as a 32-bit integer via the 
`(int)(long)ptr()` round-trip, and that's exactly the shape that reaches 
`bindArray` as a non-`CompoundVal` `Init` in the original crash too.

The last dump, with this patch, is `Unknown` — not the 
`&SymRegion{conj_$4{...}} [as 8 bit integer] you flagged as the ideal result. 
So you're right that there's more to the story: this patch's fallback 
(`bindAggregate(B, R, UnknownVal())`) just discards the value rather than 
reinterpreting it, so we trade away precision for safety rather than actually 
modeling the truncation.

Agreed a more correct fix would special-case `LocAsInteger` in `bindArray`: 
extract the underlying symbol and rebind it as a `SymbolCast` to the element's 
type/width (8-bit here) instead of falling back to `UnknownVal()`, preserving 
whatever the analyzer already knew about that pointer symbol.

Happy to take a pass at that if you think it's the right direction, or if you'd 
rather sketch the approach given you know this code better than I do — either 
way works for me.

https://github.com/llvm/llvm-project/pull/210200
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to