[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]

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.