[cfe-commits] r137980 - in /cfe/trunk: include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp test/Sema/warn-strlcpycat-size.c
Geoff Keating
geoffk at apple.com
Thu Aug 18 14:57:43 PDT 2011
On 18/08/2011, at 2:39 PM, Ted Kremenek wrote:
>
> On Aug 18, 2011, at 2:33 PM, Eli Friedman wrote:
>
>>> +// 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,
>>> + IdentifierInfo *FnName) {
>>> +
>>> + // Don't crash if the user has the wrong number of arguments
>>> + if (Call->getNumArgs() != 3)
>>> + return;
>>> +
>>> + const Expr *SrcArg = ignoreLiteralAdditions(Call->getArg(1), Context);
>>> + const Expr *SizeArg = ignoreLiteralAdditions(Call->getArg(2), Context);
>>> + const Expr *CompareWithSrc = NULL;
>>> +
>>> + // Look for 'strlcpy(dst, x, sizeof(x))'
>>> + if (const Expr *Ex = getSizeOfExprArg(SizeArg))
>>> + CompareWithSrc = Ex;
>>> + else {
>>> + // Look for 'strlcpy(dst, x, strlen(x))'
>>> + if (const CallExpr *SizeCall = dyn_cast<CallExpr>(SizeArg)) {
>>> + if (SizeCall->isBuiltinCall(Context) == Builtin::BIstrlen
>>> + && SizeCall->getNumArgs() == 1)
>>> + CompareWithSrc = ignoreLiteralAdditions(SizeCall->getArg(0), Context);
>>> + }
>>> + }
>>
>> strlcpy(dst, x, strlen(foo)) is clearly wrong no matter what foo is;
>> should we warn unconditionally?
>
> Seems reasonable to me.
I was thinking of
strlcpy (dst, x, strlen(dst)+1)
which might be OK if you happen to know that 'dst' is already a string, allocated with malloc, and you're trying to overwrite it with something you hope is shorter.
More information about the cfe-commits
mailing list