Skip to content

Commit 12cbc8c

Browse files
committed
[analyzer] Fix property access kind detection inside parentheses.
'(self.prop)' produces a surprising AST where ParenExpr resides inside `PseudoObjectExpr. This breaks ObjCMethodCall::getMessageKind() which in turn causes us to perform unnecessary dynamic dispatch bifurcation when evaluating body-farmed property accessors, which in turn causes us to explore infeasible paths.
1 parent effbf0b commit 12cbc8c

File tree

2 files changed

+35
-4
lines changed

2 files changed

+35
-4
lines changed

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,12 +1039,12 @@ const PseudoObjectExpr *ObjCMethodCall::getContainingPseudoObjectExpr() const {
10391039

10401040
static const Expr *
10411041
getSyntacticFromForPseudoObjectExpr(const PseudoObjectExpr *POE) {
1042-
const Expr *Syntactic = POE->getSyntacticForm();
1042+
const Expr *Syntactic = POE->getSyntacticForm()->IgnoreParens();
10431043

10441044
// This handles the funny case of assigning to the result of a getter.
10451045
// This can happen if the getter returns a non-const reference.
10461046
if (const auto *BO = dyn_cast<BinaryOperator>(Syntactic))
1047-
Syntactic = BO->getLHS();
1047+
Syntactic = BO->getLHS()->IgnoreParens();
10481048

10491049
return Syntactic;
10501050
}

clang/test/Analysis/ObjCProperties.m

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,35 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -analyzer-store=region -Wno-objc-root-class %s -verify
2-
// expected-no-diagnostics
1+
// RUN: %clang_analyze_cc1 -w %s -verify \
2+
// RUN: -analyzer-checker=core,alpha.core,debug.ExprInspection
3+
4+
#ifdef HEADER // A clever trick to avoid splitting up the test.
5+
6+
@interface NSObject
7+
@end
8+
9+
@interface HeaderClass : NSObject
10+
@property NSObject *prop;
11+
@end
12+
13+
#else
14+
#define HEADER
15+
#include "ObjCProperties.m"
16+
17+
@implementation HeaderClass
18+
- (void)foo {
19+
if ((self.prop)) {
20+
}
21+
22+
// This test tests that no dynamic bifurcation is performed on the property.
23+
// The TRUE/FALSE dilemma correctly arises from eagerly-assume behavior
24+
// inside the if-statement. The dynamic bifurcation at (self.prop) inside
25+
// the if-statement was causing an UNKNOWN to show up as well due to
26+
// extra parentheses being caught inside PseudoObjectExpr.
27+
// This should not be UNKNOWN.
28+
clang_analyzer_eval(self.prop); // expected-warning{{TRUE}}
29+
// expected-warning@-1{{FALSE}}
30+
}
31+
@end
32+
333

434
// The point of this test cases is to exercise properties in the static
535
// analyzer
@@ -19,3 +49,4 @@ - (id)initWithY:(id)Y {
1949
return self;
2050
}
2151
@end
52+
#endif

0 commit comments

Comments
 (0)