Address code review comments for Wstrncat-size warning (r161440).

llvm-svn: 161527
This commit is contained in:
Anna Zaks 2012-08-08 21:42:23 +00:00
parent 5d4d8b746b
commit 13b0857ad0
3 changed files with 36 additions and 27 deletions

View File

@ -373,9 +373,11 @@ def note_strlcpycat_wrong_size : Note<
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">;

View File

@ -3101,6 +3101,19 @@ static const Expr *ignoreLiteralAdditions(const Expr *Ex, ASTContext &Ctx) {
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 @@ void Sema::CheckStrlcpycatArguments(const CallExpr *Call,
// 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 @@ void Sema::CheckStrncatArguments(const CallExpr *CE,
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(";

View File

@ -59,7 +59,7 @@ void size_1() {
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 @@ void vlas(int size) {
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}}
}