[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:522
+      ArraySize = Array->getSize().getLimitedValue();
+  if (const auto *String = dyn_cast<StringLiteral>(Source)) {
+    StrLen = String->getLength();
----------------
Nesting this if- inside the previous one would simplify the outer scope and let you assign to `ArraySize` at declaration size.


================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:526
+  }
+  if (StrLenFound && ArraySize >= StrLen + 1)
+    return;
----------------
Why not put this if-expression into the one above where `StrLen` is found?
That would eliminate `StrLenFound` and remove the potential error surface of uninitialized read from `StrLen` (the declaration for which should probably be inside this block as well)


================
Comment at: test/Analysis/security-syntax-checks.m:151
+  char x[5];
+  strcpy(x, "abcd");
+}
----------------
I think it would make sense to add another test case where the warning is expected, even though string length and array size are known at compile time.


https://reviews.llvm.org/D41384