Address code review comments for Wstrncat-size warning (r161440).
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161527 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 7b3cd0f..357a63b 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -373,9 +373,11 @@
def warn_strncat_large_size : Warning<
"the value of the size argument in 'strncat' is too large, might lead to a "
- "buffer overflow">, InGroup<StrncatSize>, DefaultWarnNoWerror;
+ "buffer overflow">, InGroup<StrncatSize>;
def warn_strncat_src_size : Warning<"size argument in 'strncat' call appears "
- "to be size of the source">, InGroup<StrncatSize>, DefaultWarnNoWerror;
+ "to be size of the source">, InGroup<StrncatSize>;
+def warn_strncat_wrong_size : Warning<
+ "the value of the size argument to 'strncat' is wrong">, InGroup<StrncatSize>;
def note_strncat_wrong_size : Note<
"change the argument to be the free space in the destination buffer minus "
"the terminating null byte">;
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 5e43091..a58e904 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -3101,6 +3101,19 @@
return Ex;
}
+static bool isConstantSizeArrayWithMoreThanOneElement(QualType Ty,
+ ASTContext &Context) {
+ // Only handle constant-sized or VLAs, but not flexible members.
+ if (const ConstantArrayType *CAT = Context.getAsConstantArrayType(Ty)) {
+ // Only issue the FIXIT for arrays of size > 1.
+ if (CAT->getSize().getSExtValue() <= 1)
+ return false;
+ } else if (!Ty->isVariableArrayType()) {
+ return false;
+ }
+ return true;
+}
+
// Warn if the user has made the 'size' argument to strlcpy or strlcat
// be the size of the source, instead of the destination.
void Sema::CheckStrlcpycatArguments(const CallExpr *Call,
@@ -3151,16 +3164,8 @@
// pointers if we know the actual size, like if DstArg is 'array+2'
// we could say 'sizeof(array)-2'.
const Expr *DstArg = Call->getArg(0)->IgnoreParenImpCasts();
- QualType DstArgTy = DstArg->getType();
-
- // Only handle constant-sized or VLAs, but not flexible members.
- if (const ConstantArrayType *CAT = Context.getAsConstantArrayType(DstArgTy)) {
- // Only issue the FIXIT for arrays of size > 1.
- if (CAT->getSize().getSExtValue() <= 1)
- return;
- } else if (!DstArgTy->isVariableArrayType()) {
+ if (!isConstantSizeArrayWithMoreThanOneElement(DstArg->getType(), Context))
return;
- }
SmallString<128> sizeString;
llvm::raw_svector_ostream OS(sizeString);
@@ -3242,26 +3247,23 @@
SM.getSpellingLoc(SR.getEnd()));
}
+ // Check if the destination is an array (rather than a pointer to an array).
+ QualType DstTy = DstArg->getType();
+ bool isKnownSizeArray = isConstantSizeArrayWithMoreThanOneElement(DstTy,
+ Context);
+ if (!isKnownSizeArray) {
+ if (PatternType == 1)
+ Diag(SL, diag::warn_strncat_wrong_size) << SR;
+ else
+ Diag(SL, diag::warn_strncat_src_size) << SR;
+ return;
+ }
+
if (PatternType == 1)
Diag(SL, diag::warn_strncat_large_size) << SR;
else
Diag(SL, diag::warn_strncat_src_size) << SR;
- // Output a FIXIT hint if the destination is an array (rather than a
- // pointer to an array). This could be enhanced to handle some
- // pointers if we know the actual size, like if DstArg is 'array+2'
- // we could say 'sizeof(array)-2'.
- QualType DstArgTy = DstArg->getType();
-
- // Only handle constant-sized or VLAs, but not flexible members.
- if (const ConstantArrayType *CAT = Context.getAsConstantArrayType(DstArgTy)) {
- // Only issue the FIXIT for arrays of size > 1.
- if (CAT->getSize().getSExtValue() <= 1)
- return;
- } else if (!DstArgTy->isVariableArrayType()) {
- return;
- }
-
SmallString<128> sizeString;
llvm::raw_svector_ostream OS(sizeString);
OS << "sizeof(";
diff --git a/test/Sema/warn-strncat-size.c b/test/Sema/warn-strncat-size.c
index 7157edf..dcc3367 100644
--- a/test/Sema/warn-strncat-size.c
+++ b/test/Sema/warn-strncat-size.c
@@ -59,7 +59,7 @@
char z[1];
char str[] = "hi";
- strncat(z, str, sizeof(z)); // expected-warning{{the value of the size argument in 'strncat' is too large, might lead to a buffer overflow}}
+ strncat(z, str, sizeof(z)); // expected-warning{{the value of the size argument to 'strncat' is wrong}}
}
// Support VLAs.
@@ -69,3 +69,8 @@
strncat(z, str, sizeof(str)); // expected-warning {{size argument in 'strncat' call appears to be size of the source}} expected-note {{change the argument to be the free space in the destination buffer minus the terminating null byte}}
}
+
+// Non-array type gets a different error message.
+void f(char* s, char* d) {
+ strncat(d, s, sizeof(d)); // expected-warning {{the value of the size argument to 'strncat' is wrong}}
+}
