[clang] 12cbc8c - [analyzer] Fix property access kind detection inside parentheses.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2021-10-14T21:07:19-07:00
New Revision: 12cbc8cbf071901686b36e192a6d4da19deb6ec6

URL: https://github.com/llvm/llvm-project/commit/12cbc8cbf071901686b36e192a6d4da19deb6ec6
DIFF: https://github.com/llvm/llvm-project/commit/12cbc8cbf071901686b36e192a6d4da19deb6ec6.diff

LOG: [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.

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Core/CallEvent.cpp
    clang/test/Analysis/ObjCProperties.m

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index e035db8ef35a7..9a95921b64c3b 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -1039,12 +1039,12 @@ const PseudoObjectExpr *ObjCMethodCall::getContainingPseudoObjectExpr() const {
 
 static const Expr *
 getSyntacticFromForPseudoObjectExpr(const PseudoObjectExpr *POE) {
-  const Expr *Syntactic = POE->getSyntacticForm();
+  const Expr *Syntactic = POE->getSyntacticForm()->IgnoreParens();
 
   // This handles the funny case of assigning to the result of a getter.
   // This can happen if the getter returns a non-const reference.
   if (const auto *BO = dyn_cast<BinaryOperator>(Syntactic))
-    Syntactic = BO->getLHS();
+    Syntactic = BO->getLHS()->IgnoreParens();
 
   return Syntactic;
 }

diff  --git a/clang/test/Analysis/ObjCProperties.m b/clang/test/Analysis/ObjCProperties.m
index 1a112ec2b1289..c4d264471fd16 100644
--- a/clang/test/Analysis/ObjCProperties.m
+++ b/clang/test/Analysis/ObjCProperties.m
@@ -1,5 +1,35 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -analyzer-store=region -Wno-objc-root-class %s -verify
-// expected-no-diagnostics
+// RUN: %clang_analyze_cc1 -w %s -verify \
+// RUN:     -analyzer-checker=core,alpha.core,debug.ExprInspection
+
+#ifdef HEADER // A clever trick to avoid splitting up the test.
+
+ at interface NSObject
+ at end
+
+ at interface HeaderClass : NSObject
+ at property NSObject *prop;
+ at end
+
+#else
+#define HEADER
+#include "ObjCProperties.m"
+
+ at implementation HeaderClass
+- (void)foo {
+  if ((self.prop)) {
+  }
+
+  // This test tests that no dynamic bifurcation is performed on the property.
+  // The TRUE/FALSE dilemma correctly arises from eagerly-assume behavior
+  // inside the if-statement. The dynamic bifurcation at (self.prop) inside
+  // the if-statement was causing an UNKNOWN to show up as well due to
+  // extra parentheses being caught inside PseudoObjectExpr.
+  // This should not be UNKNOWN.
+  clang_analyzer_eval(self.prop); // expected-warning{{TRUE}}
+                                  // expected-warning at -1{{FALSE}}
+}
+ at end
+
 
 // The point of this test cases is to exercise properties in the static
 // analyzer
@@ -19,3 +49,4 @@ - (id)initWithY:(id)Y {
   return self;
 }
 @end
+#endif