[cfe-commits] r152440 - in /cfe/trunk: include/clang/Frontend/AnalyzerOptions.h include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/Frontend/CompilerInvocation.cpp lib/StaticAnalyzer/Core/CoreEngine.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Jordan Rose jediknil at belkadan.com
Fri Mar 9 16:47:11 PST 2012


On Mar 9, 2012, at 16:13, Anna Zaks wrote:

> 
> On Mar 9, 2012, at 1:50 PM, Jordan Rose wrote:
> 
>> On Mar 9, 2012, at 13:14, Anna Zaks wrote:
>> 
>>> [analyzer] Add support for NoRedundancy inlining mode.
>>> 
>>> We do not reanalyze a function, which has already been analyzed as an
>>> inlined callee. As per PRELIMINARY testing, this gives over
>>> 50% run time reduction on some benchmarks without decreasing of the
>>> number of bugs found.
>> 
>> 
>> I might be missing something, but isn't this straight-up incorrect? Example:
>> 
>> void doSomethingStupid(bool shouldCrash);
>> 
>> void analyzeMeFirst() {
>>  doSomethingStupid(false);
>> }
>> 
>> void doSomethingStupid(bool shouldCrash) {
>>  if (!shouldCrash) return;
>>  void *p;
>>  free(p); // this branch is pruned when doSomethingStupid is inlined
>> }
>> 
>> 
>> (Even if this particular example works, I feel like there's a similar problem here somewhere.)
>> 
> 
> Actually, when only considering this example (and the like), the optimization might actually be viewed as a desired feature for false positive suppression (assuming we always analyze externally available functions as top level functions). If all the callers of this function always pass in 'false', one might argue that we should not report the error. This is the appeal of inter-procedural context sensitive analyses. For example, we often report a buffer overflow with the CString checker under the assumption that the length parameter can be negative, when the programmer is sure it's always positive or is within a specific range.

Okay, but that's only a fair assumption if the inlined function is static. Maybe we can have a mode flag for this: none, static, all.


> On the other hand, my commit message might be somewhat misleading. This is an optimization which can result in loss of precision. Here is one example where we could loose precision:
>  -  We are always analyzing the inlined function with a lower stack depth then that of the top-level function. Since we also bound the stack depth, the inlined function does not get as much inlining as the top level.
> 
> We still need to evaluate this optimization on a lot of code and see if that's the best option we have for improving performance under inlining. I'll be experimenting with other approaches as well (suggestions are welcome). Currently, analyzing with inlining can be an order of magnitude slower than without on some benchmarks, so we have to do something to bring that down.

Makes sense. Hard problems. Thanks for the explanation!
Jordy





More information about the cfe-commits mailing list