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

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 11 13:48:00 PST 2018


NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:517
+    if (const auto *Array = dyn_cast<ConstantArrayType>(
+            DeclRef->getDecl()->getType().getTypePtr())) {
+      unsigned long long ArraySize = Array->getSize().getLimitedValue();
----------------
This can be simplified to `const auto *Array = DeclRef->getType()->getAs<ConstantArrayType>()`.
`.getTypePtr()` is almost always redundant because of the fancy `operator->()` on `QualType`.


================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:518
+            DeclRef->getDecl()->getType().getTypePtr())) {
+      unsigned long long ArraySize = Array->getSize().getLimitedValue();
+      if (const auto *String = dyn_cast<StringLiteral>(Source)) {
----------------
Hmm, actually, `ArraySize` is the number of elements in the array, not its byte size, so you may want (if you like) to also suppress the warning when the array is not a `char` array (even if it's a weird code anyway) by instead taking `ASTContext.getTypeSize()` here instead.

Also i guess it's nice to use `uint64_t` because that's the return type of `.getLimitedValue()` and that's what we usually use all over the place when we have to deal with those values.


https://reviews.llvm.org/D41384





More information about the cfe-commits mailing list