[cfe-dev] [analyzer] Should we invalidate the `this` pointer?

[Artem Dergachev via cfe-dev]

Yeah, this looks pretty broken. One does not simply overwrite his 
this-pointer using valid C++. Feel free to fix :)

On 4/2/18 12:47 AM, Henry Wong wrote:
> Hi Artem,
>
> Thank you for your explanation1 You are right, invalidation of the region
> contents of the class object is correct and common. However `this` 
> pointer i
> s no-lvalue and it's a `prvalue expression` in c++17. IMHO, 
> invalidation of `CXXThisObjectRegion` is incorrect and violates the 
> C++ standard.
>
> Given the code below:
> --------------------------------------------------------------------------------
>    // $ clang -cc1 -analyze -analyzer-checker=core,debug.ExprInspection
>    //    -analyzer-config widen-loops=true test.cpp
>
>   1 void clang_analyzer_eval(int);
>   2
>   3 struct A {
>   4     int num;
>   5     void func(int i) {
>   6         int sum = 0;
>   7         clang_analyzer_eval(sum == 0); // should be true
>   8         for (i = 0; i < 100; ++i) { sum++; }
>   9         num = 0;
>  10     }
>  11 };
>  12
>  13 int main() {
>  14     A a;
>  15     a.num = 10;
>  16     a.func(10);
>  17     clang_analyzer_eval(a.num == 0); // UNKNOWN, should be true.
>  18 }
> --------------------------------------------------------------------------------
>
> Before invalidation,
> --------------------------------------------------------------------------------
> Store (direct and default bindings), 0x7f9de8014d90 :
> (a,0,direct) : 10 S32b
>
> (i,0,direct) : 3 S32b
>
> (this,0,direct) : &a
>
> (sum,0,direct) : 3 S32b
> --------------------------------------------------------------------------------
>
> After invalidation,
> --------------------------------------------------------------------------------
> Store (direct and default bindings), 0x7f9de8015828 :
> (a,0,default) : conj_$2{int}
>
> (i,0,direct) : conj_$3{int}
>
> (this,0,direct) : &SymRegion{conj_$1{struct A *}}
>
> (sum,0,direct) : conj_$0{int}
> --------------------------------------------------------------------------------
>
> `(this,0,direct) : &a` -> ` (this,0,direct) : 
> &SymRegion{conj_$1{struct A *}}`
> is  inaccurate and too conservative. The more serious problem is that the
> corresponding relationship between `this` pointer and its corresponding
> Object-Region has been broken. Modifications to data member do not affect
> the actual Object-Region because at this time `this` pointer is 
> pointing to a
> `SymbolicRegion`. For example, there should emit `TRUE` at the line 17 in
> the sample code, but emitted `UNKNOWN` instead.
>
> Henry Wong
> Qihoo 360 Codesafe Team
> ------------------------------------------------------------------------
> *From:* Artem Dergachev <noqnoqneo at gmail.com>
> *Sent:* Monday, April 2, 2018 8:31
> *To:* Henry Wong; cfe-dev at lists.llvm.org
> *Cc:* Péter Szécsi
> *Subject:* Re: [cfe-dev] [analyzer] Should we invalidate the `this` 
> pointer?
> This assertion is pretty fundamental. Invalidation, if done correctly,
> should not have triggered it - after all, invalidation could occur for
> any other reason, not necessarily because of loop widening.
>
> Invalidation of this-region contents (that is, not of the
> CXXThisObjectRegion of the current stack frame, but of the actual
> this-region which is a pointee of the CXXThisObjectRegion) sounds
> reasonable if the region is modified within the loop - which is going to
> often be the case.
>
> On 3/31/18 2:02 AM, Henry Wong via cfe-dev wrote:
> > Hi all,
> >
> > I recently encountered a assertion failure as shown below.
> >
> > `Assertion `!InitValWithAdjustments.getAs<Loc>() ||
> > Loc::isLocType(Result->getType()) ||
> > Result->getType()->isMemberPointerType()' failed`
> >
> > The code that will trigger this assertion failed.
> > 
> ----------------------------------------------------------------------------------------------------
> > struct BlockId {
> > BlockId();
> > };
> >
> > void goo(BlockId id);
> >
> > BlockId::BlockId() {
> > int count = 10;
> > do {
> >
> > } while (count--);
> > }
> >
> > int main() {
> > goo(BlockId());
> > }
> > 
> ----------------------------------------------------------------------------------------------------
> >
> > The reason is that the analyzer invalidate the `this` pointer
> > at loop-widen. The more essential question is "Should we invalidate
> > the `this` pointer?"
> >
> > Thanks in advance!
> >
> > Henry Wong
> > Qihoo 360 Codesafe Team
> >
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180402/004bb82e/attachment.html>