AnalysisBasedWarnings Sema layer and out of the Analysis library itself.
This returns the uninitialized values analysis to a more pure form,
allowing its original logic to correctly detect some categories of
definitely uninitialized values. Fixes PR10358 (again).
Thanks to Ted for reviewing and updating this patch after his rewrite of
several portions of this analysis.
llvm-svn: 135748
This is accomplished by forcing the needed expressions for -Wuninitialized to always be CFGElements in the CFG.
This allows us to remove a fair amount of the code for -Wuninitialized.
Some fallout:
- AnalysisBasedWarnings.cpp now specifically toggles the CFGBuilder to create a CFG that is suitable for -Wuninitialized. This
is a layering violation, since the logic for -Wuninitialized is in libAnalysis. This can be fixed with the proper refactoring.
- Some of the source locations for -Wunreachable-code warnings have shifted. While not ideal, this is okay because that analysis
already needs some serious reworking.
llvm-svn: 135480
patch, we actually move the state-machine for the value set backwards
one step. This can pretty easily lead to infinite loops where we
continually try to propagate a bit, succeed for one iteration, but then
back up because we find an uninitialized use.
A reduced test case from PR10379 is included.
llvm-svn: 135359
Previously, despite the names 'enqueue' and 'dequeue', it behaved as
a stack and visited blocks in a LIFO fashion. This interacts badly with
extremely broad CFGs *inside* of a loop (such as a large switch inside
a state machine) where every block updates a different variable.
When encountering such a CFG, the checker visited blocks in essentially
a "depth first" order due to the stack-like behavior of the work list.
Combined with each block updating a different variable, the saturation
logic of the checker caused it to re-traverse blocks [1,N-1] of the
broad CFG inside the loop after traversing block N. These re-traversals
were to propagate the variable values derived from block N. Assuming
approximately the same number of variables as inner blocks exist, the
end result is O(N^2) updates. By making this a queue, we also make the
traversal essentially "breadth-first" across each of the N inner blocks
of the loop. Then all of this state is propagated around to all N inner
blocks of the loop. The result is O(N) updates.
The truth is in the numbers:
Before, gcc.c: 96409 block visits (max: 61546, avg: 591)
After, gcc.c: 69958 block visits (max: 33090, avg: 429)
Before, PR10183: 2540494 block vists (max: 2536495, avg: 37360)
After, PR10183: 137803 block visits (max: 134406, avg: 2026)
The nearly 20x reduction in work for PR10183 corresponds to a roughly
100x speedup in compile time.
I've tested it on all the code I can get my hands on, and I've seen no
slowdowns due to this change. Where I've collected stats, the ammount of
work done is on average less. I'll also commit shortly some synthetic
test cases useful in analyzing the performance of CFG-based warnings.
Submitting this based on Doug's feedback that post-commit review should
be good. Ted, please review! Hopefully this helps compile times until
then.
llvm-svn: 134697
Special detail is added for uninitialized variable analysis as this has
serious performance problems than need to be tracked.
Computing some of this data is expensive, for example walking the CFG to
determine its size. To avoid doing that unless the stats data is going
to be used, we thread a bit into the Sema object to track whether
detailed stats should be collected or not. This bit is used to avoid
computations whereever the computations are likely to be more expensive
than checking the state of the flag. Thus, counters are in some cases
unconditionally updated, but the more expensive (and less frequent)
aggregation steps are skipped.
With this patch, we're able to see that for 'gcc.c':
*** Analysis Based Warnings Stats:
232 functions analyzed (0 w/o CFGs).
7151 CFG blocks built.
30 average CFG blocks per function.
1167 max CFG blocks per function.
163 functions analyzed for uninitialiazed variables
640 variables analyzed.
3 average variables per function.
94 max variables per function.
96409 block visits.
591 average block visits per function.
61546 max block visits per function.
And for the reduced testcase in PR10183:
*** Analysis Based Warnings Stats:
98 functions analyzed (0 w/o CFGs).
8526 CFG blocks built.
87 average CFG blocks per function.
7277 max CFG blocks per function.
68 functions analyzed for uninitialiazed variables
1359 variables analyzed.
19 average variables per function.
1196 max variables per function.
2540494 block visits.
37360 average block visits per function.
2536495 max block visits per function.
That last number is the somewhat scary one that indicates the problem in
PR10183.
llvm-svn: 134494
instantiation), be sure to add the transformed declaration into the
current DeclContext. Also, remove the -Wuninitialized hack that works
around this bug. Fixes <rdar://problem/9200676>.
llvm-svn: 129544
evaluated and unevaluated contexts. Add some testing of sizeof and
typeid.
Both of the typeid tests added here were triggering warnings previously.
Now the one false positive is suppressed without suppressing the warning
on actually buggy code.
llvm-svn: 129431
marked explicitly as uninitialized through direct self initialization:
int x = x;
With r128894 we prevented warnings about this code, and this patch
teaches the analysis engine to continue analyzing subsequent uses of
'x'. This should wrap up PR9624.
There is still an open question of whether we should suppress the
maybe-uninitialized warnings resulting from variables initialized in
this fashion. The definitely-uninitialized uses should always be warned.
llvm-svn: 128932
1) Change the CFG to include the DeclStmt for conditional variables, instead of using the condition itself as a faux DeclStmt.
2) Update ExprEngine (the static analyzer) to understand (1), so not to regress.
3) Update UninitializedValues.cpp to initialize all tracked variables to Uninitialized at the start of the function/method.
4) Only use the SelfReferenceChecker (SemaDecl.cpp) on global variables, leaving the dataflow analysis to handle other cases.
The combination of (1) and (3) allows the dataflow-based -Wuninitialized to find self-init problems when the initializer
contained control-flow.
llvm-svn: 128858
Note this can potentially be enhanced to detect if the __block variable
is actually written by the block, or only when the block "escapes" or
is actually used, but that requires more analysis than it is probably worth
for this simple check.
llvm-svn: 128681
my expertise on the template instantiation logic isn't good enough to fix this problem for real. This patch worksaround the
problem in -Wuninitialized, but we should fix it for real later.
llvm-svn: 128443
class and to bind the shared value using OpaqueValueExpr. This fixes an
unnoticed problem with deserialization of these expressions where the
deserialized form would lose the vital pointer-equality trait; or rather,
it fixes it because this patch also does the right thing for deserializing
OVEs.
Change OVEs to not be a "temporary object" in the sense that copy elision is
permitted.
This new representation is not totally unawkward to work with, but I think
that's really part and parcel with the semantics we're modelling here. In
particular, it's much easier to fix things like the copy elision bug and to
make the CFG look right.
I've tried to update the analyzer to deal with this in at least some
obvious cases, and I think we get a much better CFG out, but the printing
of OpaqueValueExprs probably needs some work.
llvm-svn: 125744
After discussion with Zhongxing, don't force the initializer of DeclStmts to be
block-level expressions.
This led to some interesting fallout:
[UninitializedValues]
Always visit the initializer of DeclStmts (do not assume they are block-level expressions).
[BasicStore]
With initializers of DeclStmts no longer block-level expressions, this causes self-referencing initializers (e.g. 'int x = x') to no longer cause the initialized variable to be live before the DeclStmt. While this is correct, it caused BasicStore::RemoveDeadBindings() to prune off the values of these variables from the initial store (where they are set to uninitialized). The fix is to back-port some (and only some) of the lazy-binding logic from RegionStore to
BasicStore. Now the default values of local variables are determined lazily as opposed
to explicitly initialized.
llvm-svn: 97591
(1) libAnalysis is a generic analysis library that can be used by
Sema. It defines the CFG, basic dataflow analysis primitives, and
inexpensive flow-sensitive analyses (e.g. LiveVariables).
(2) libChecker contains the guts of the static analyzer, incuding the
path-sensitive analysis engine and domain-specific checks.
Now any clients that want to use the frontend to build their own tools
don't need to link in the entire static analyzer.
This change exposes various obvious cleanups that can be made to the
layout of files and headers in libChecker. More changes pending. :)
This change also exposed a layering violation between AnalysisContext
and MemRegion. BlockInvocationContext shouldn't explicitly know about
BlockDataRegions. For now I've removed the BlockDataRegion* from
BlockInvocationContext (removing context-sensitivity; although this
wasn't used yet). We need to have a better way to extend
BlockInvocationContext (and any LocationContext) to add
context-sensitivty.
llvm-svn: 94406
Ted Kremenek