[PATCH] improve clang's static analysis of __builtin_sprintf_chk()

[Richard Trieu]

================
Comment at: lib/Sema/SemaChecking.cpp:4168
@@ +4167,3 @@
+
+  if (isa<StringLiteral>(Data)) {
+    auto SL = cast<StringLiteral>(Data);
----------------
This seems backwards.  You using the type of expression to determine how to calculate the value.  You should use the argument type along with a switch statement to handle all the different specifiers.

================
Comment at: lib/Sema/SemaChecking.cpp:4172
@@ +4171,3 @@
+  }
+  else if (isa<IntegerLiteral>(Data)) {
+    // Default to decimal representation.
----------------
Another problem of trying to match expressions is that there are plenty of strange cases, for instance, negative numbers are actually positive numbers wrapped in a unary negative expression.  Also, constants won't show up, or 1+1.  Instead, use the Evaluate* methods in Expr.  If it is a compile time constant, then Evaulate* will provide the value.

================
Comment at: lib/Sema/SemaChecking.cpp:4226-4228
@@ +4225,5 @@
+
+  // Pointer address have 'p' prepended
+  if (CSKind == ConversionSpecifier::pArg)
+    ++StorageRequired;
+
----------------
This is kind of tricky.  Pointers have an implementation defined printing style.  We should make sure that this takes into account how different implementations print pointers.

================
Comment at: lib/Sema/SemaChecking.cpp:4236
@@ +4235,3 @@
+    if (Precision.getHowSpecified() == OptionalAmount::Constant) {
+      if (StorageRequired.ule(Precision.getConstantAmount()))
+        StorageRequired = llvm::APInt(64, Precision.getConstantAmount());
----------------
StorageRequired is the total size needed for the entire string, not just the space required for this specifier.  A local size is needed so that the extra width is added on correctly.

http://reviews.llvm.org/D9665

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/