[clang] [llvm] [clang-tools-extra] [analyzer] Trust base to derived casts for dynamic types (PR #69057)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 29 08:41:44 PST 2023


================
@@ -492,11 +492,13 @@ void check_required_cast() {
 
 void check_cast_behavior(OSObject *obj) {
   OSArray *arr1 = OSDynamicCast(OSArray, obj);
-  clang_analyzer_eval(arr1 == obj); // expected-warning{{TRUE}}
-                                    // expected-note at -1{{TRUE}}
-                                    // expected-note at -2{{Assuming 'arr1' is not equal to 'obj'}}
-                                    // expected-warning at -3{{FALSE}}
-                                    // expected-note at -4   {{FALSE}}
+  clang_analyzer_eval(arr1 == obj); // #check_cast_behavior_1
+  // expected-warning@#check_cast_behavior_1 {{TRUE}}
+  // expected-note@#check_cast_behavior_1    {{TRUE}}
+  // expected-note@#check_cast_behavior_1{{Assuming 'arr1' is equal to 'obj'}}
----------------
steakhal wrote:

I went back to this comment to double check, and I can confirm what's happening.
On the baseline revision, the node for `clang_analyzer_eval(arr2 == obj)` has two parents:
1) Where `arr2` is known to be `0 (loc)`, by an eager assumption by `clang_analyzer_eval(arr1 == obj)`.
2) Where `arr2` is known to be non-null, by the same eager assumption. However, for this we already had a message as `Assuming 'arr1' is not equal to 'obj'` (see the baseline test).

After this PR, the exploded graph would no longer have two parents for the given bug-report, so it can't couldn't chose the same parent node in the exploded graph when constructing the bug report. This means that on that path, it will see an eager assumption to `true`, and craft a piece message accordingly.

This means, that the bug report construction could have chosen the other parent for constructing the bug report, and have the same behavior as demonstrated after the PR.

The next question could be, why are the following exploded nodes not merged after this PR?
![image](https://github.com/llvm/llvm-project/assets/6280485/9cbd2c7c-e3fe-4186-b09e-8aa5d12dbf50)
![image](https://github.com/llvm/llvm-project/assets/6280485/b999a938-82fc-485d-9780-ffb9e1594539)


On the baseline it looks like this, (notice the two parents):
![image](https://github.com/llvm/llvm-project/assets/6280485/eff9d0f0-4f17-4e83-9404-320fa50ed6fa)


---

Anyways, this debunks the theory that this PR changes this test in any meaningful/practical ways - as far as I can see it.

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


More information about the cfe-commits mailing list