steakhal added a comment.
In D81254#2122449 <https://reviews.llvm.org/D81254#2122449>, @ASDenysPetrov
wrote:
> @NoQ, thanks for the examples.
>
> I didn't get the first one. How do you get to the "//In reality we don't
> know//", if we don't touch `a[index1]`:
If `index1` and `index2` has the **same** value, we should not be confident
that the `x == y` holds.
> void foo(int *a); // Potentially modifies elements of 'a'.
> void fooRef(int *&a); // Potentially modifies elements of 'a'.
>
> void test2(int *a) {
> // a - &SymRegion{reg_$6<int * a>}
> foo(a); // after this, CSA doesn't change a.
> // a - &SymRegion{reg_$6<int * a>}
> fooRef(a); // after this, CSA changes a.
> // a - &SymRegion{conj_$10{int *, LC1, S925, #1}}
> }
I don't think this is the right way expressing this.
AFAIK this means that we //don't// know where the resulting pointer points to -
which is not true.
We know where it points to (aka. the pointer's value is conjured), but we don't
know what the value if there.
Unfortunately, I don't know the solution to your problem, but I'm pretty
confident that this is something else.
However, I'm really interested in where this discussion goes.
---
I feel like this problem should be handled by some sort of invalidation
mechanism.
Take my comments on these with a pinch of salt though - I'm really not
experienced in this :|
================
Comment at: clang/test/Analysis/PR9289.cpp:13-21
+int index_int(const int *a, int index) {
+ int var;
+ int ret = 0;
+ if (a[42] < 2)
+ var = 1;
+ if (a[42] < 2)
+ ret = var; // no warning about garbage value
----------------
Why do we test this?
Here we can //reason// about the index.
This patch preserves the current behavior relating to this.
================
Comment at: clang/test/Analysis/PR9289.cpp:36-40
+ if (a[index] < 2)
+ var = 1;
+ index = index2;
+ if (a[index] < 2)
+ ret = var; // expected-warning{{Assigned value is garbage or undefined
[core.uninitialized.Assign]}}
----------------
I'm not sure why do you overwrite the `index` if you could simply index with
`index2` instead in the following expression. That would make no difference,
only make the test more readable IMO.
Same comment for all the`*_change` test cases.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81254/new/
https://reviews.llvm.org/D81254
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
