danielmarjamaki abandoned this revision.
danielmarjamaki added a comment.
Herald added subscribers: llvm-commits, a.sidorin, rnkovacs.
I will not continue working on this. Feel free to take over the patch or write
a new patch.
Repository:
rL LLVM
https://reviews.llvm.org/D39049
__
xazax.hun requested changes to this revision.
xazax.hun added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D39049#928620, @danielmarjamaki wrote:
> > So what are the arguments that are passed to getSimplifiedOffset() in
> that case? 0? That does not see
danielmarjamaki added a comment.
> So what are the arguments that are passed to getSimplifiedOffset() in that
> case? 0? That does not seem to be correct.
yes.
so the conclusion is:
- this code does not work
- this code is untested
- this code is not even used in the use cases it was intended
xazax.hun added a comment.
In https://reviews.llvm.org/D39049#926597, @danielmarjamaki wrote:
> > Could you do a similar analysis that I did above to check why does this not
> > work for the multidimensional case? (I.e.: checking what constraints are
> > generated and what the analyzer does wit
danielmarjamaki added a comment.
> Could you do a similar analysis that I did above to check why does this not
> work for the multidimensional case? (I.e.: checking what constraints are
> generated and what the analyzer does with them.)
the "location.dump()" will just say "x".
the ProgramState
xazax.hun added a comment.
In https://reviews.llvm.org/D39049#910482, @NoQ wrote:
> // TODO: once the constraint manager is smart enough to handle non
> simplified
> // symbolic expressions remove this function. Note that this can not be
> used in
> // the constraint manager as is, since
NoQ added a comment.
// TODO: once the constraint manager is smart enough to handle non simplified
// symbolic expressions remove this function. Note that this can not be used
in
// the constraint manager as is, since this does not handle overflows. It is
// safe to assume, however, that
danielmarjamaki added a comment.
> Do you mind writing some tests with multidimensional arrays to check what do
> we lose if we remove that code?
I have spent a few hours trying to write a test case that shows there is false
negatives caused by this change. And fail.
I see lots of false negati
xazax.hun added a comment.
I checked what happens:
The checker would like to solve the following (I inspect the branch when x == 0
):
`((reg_$1) + 1) * 4 <= 0`
The `getSimplifiedOffsets` function kicks in and simplifies the expression
above to the following:
`(reg_$1) <= -1`
The analyzer a
danielmarjamaki created this revision.
Herald added a subscriber: szepet.
Example code:
void test3_simplified_offset(int x, unsigned long long y) {
int buf[100];
if (x < 0)
x = 0;
for (int i = y - x; i > 0 && i < 100; i++)
buf[i] = 0; // no-warning
}
Without this patc
10 matches
Mail list logo
