[cfe-commits] r145575 - in /cfe/trunk: lib/StaticAnalyzer/Core/ExprEngine.cpp test/Analysis/misc-ps-region-store.cpp

[Jordy Rose]

Hi, Ted. Just wanted to note that it's technically legal for 'this' to be null if the method is non-virtual, which I'm sure you know already. Maybe someday we can make this a setting?

I'm also confused about the test. I thought that testing 'p' was supposed to bifurcate the state for 'this', but only the "non-null" state would be vaild. That already feels hacky to me, relying on the analyzer treating the test of a member as a bifurcation point for the base.

Then I glanced at the C++ standard and realized that &field may not be defined. It's short for &(this->field), right? &(this->field) could be compiled to (this+offsetof(field)), and if offsetof(field) is nonzero (say, because of RTTI info being inserted), the test could actually come out true even if 'this' is null.

I'm not saying Clang /will/ do this, but it'd be nice to have a test that depended less on the compiler settings. I'd prefer making this a deadcode test: "if (!this) doSomethingStupid();" or something. 

(I realize this is at odds with what I pointed out first about 'this' possibly being 'null', but this will notice if we ever put that behavior under a flag, anyway. I feel like negative tests can silently break more easily than positive tests.)

Sorry to get to this one a week later,
Jordy


On Dec 1, 2011, at 12:29, Ted Kremenek wrote:

> Modified: cfe/trunk/test/Analysis/misc-ps-region-store.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.cpp?rev=145575&r1=145574&r2=145575&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/misc-ps-region-store.cpp (original)
> +++ cfe/trunk/test/Analysis/misc-ps-region-store.cpp Wed Nov 30 23:29:42 2011
> @@ -492,3 +492,17 @@
>   return NaN;
> }
> 
> +// Test that 'this' is assumed null upon analyzing the entry to a "top-level"
> +// function (i.e., when not analyzing from a specific caller).
> +struct TestNullThis {
> +  int field;
> +  void test();
> +};
> +
> +void TestNullThis::test() {
> +  int *p = &field;
> +  if (p)
> +    return;
> +  field = 2; // no-warning
> +}
> +
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits