[analyzer] Track indices of arrays

[Szelethus]

Summary

Observe the test file before this patch:

I think this patch really improves on this case. The problem however, that some subexpressions of expressions should be tracked (like n in arr[n] here) is a broader one, and there may be other some cases I could add.

[NoQ]

Whoa, this looks like a much needed improvement, i'm glad that you found it!

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

1681 ↗

(On Diff #203830)

Mmm, dunno about null fp suppression. We're, like, talking about integers. Integers are more often zero than null. We generally do support some FP suppressions for integers as well (i.e., core.DivideZero uses them), but in this case it doesn't sound as if 0 is anyhow special.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

1681 ↗

(On Diff #203830)

I agree here. Forwarding EnableNUllFPSuppression does not really make sense for the array index even if it was true for the array element.

[Szelethus]

One more test just for good measure, don't enable null fp suppression.

[NoQ]

Great, thanks!!

[baloghadamsoftware]

LGTM!

cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
   ProgramStateRef LVState = LVNode->getState();
 
   // The message send could be nil due to the receiver being nil.
   // At this point in the path, the receiver should be live since we are at the
   // message send expr. If it is nil, start tracking it.
   if (const Expr *Receiver = NilReceiverBRVisitor::getNilReceiver(Inner, LVNode))
     trackExpressionValue(LVNode, Receiver, report, EnableNullFPSuppression);
 
+  if (const auto *Arr = dyn_cast<ArraySubscriptExpr>(Inner))
+    trackExpressionValue(
+        LVNode, Arr->getIdx(), report, EnableNullFPSuppression);
+
   // See if the expression we're interested refers to a variable.
   // If so, we can track both its contents and constraints on its value.
   if (ExplodedGraph::isInterestingLValueExpr(Inner)) {
     SVal LVal = LVNode->getSVal(Inner);
 
     const MemRegion *RR = getLocationRegionIfReference(Inner, LVNode);
     bool LVIsNull = LVState->isNull(LVal).isConstrainedTrue();
 

cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp

 
 void shift_by_undefined_value() {
   uint8_t shift_amount = get_uint8_max(); // expected-note{{'shift_amount' initialized to 255}}
                                 // expected-note@-1{{Calling 'get_uint8_max'}}
                                 // expected-note@-2{{Returning from 'get_uint8_max'}}
   (void)(TCP_MAXWIN << shift_amount); // expected-warning{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
                                       // expected-note@-1{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
 }
+
+namespace array_index_tracking {
+void consume(int);
+
+int getIndex(int x) {
+  int a;
+  if (x > 0)
+    a = 3;
+  else
+    a = 2;
+  return a;
+}
+
+int getInt();
+
+void testArrayIndexTracking() {
+  int arr[10];
+
+  for (int i = 0; i < 3; ++i)
+    arr[i] = 0;
+  int x = getInt();
+  int n = getIndex(x);
+  consume(arr[n]);
+}
+} // end of namespace array_index_tracking