Skip to content

Commit

Permalink
[Sema] Clean up some __builtin_*_chk diagnostics
Browse files Browse the repository at this point in the history 
Namely, print the likely macro name when it's used, and include the actual
computed sizes in the diagnostic message, which are sometimes not obvious.

rdar://43909200

Differential revision: https://reviews.llvm.org/D51697

llvm-svn: 341566
  • Loading branch information
@epilk
epilk committed Sep 6, 2018
1 parent 1b34b01 commit f85e391
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 15 deletions.
3 changes: 2 additions & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,8 @@ def warn_assume_side_effects : Warning<
InGroup<DiagGroup<"assume">>;

def warn_memcpy_chk_overflow : Warning<
"%0 will always overflow destination buffer">,
"'%0' will always overflow; destination buffer has size %1,"
" but size argument is %2">,
InGroup<DiagGroup<"builtin-memcpy-chk-size">>;

/// main()
Expand Down
42 changes: 34 additions & 8 deletions clang/lib/Sema/SemaChecking.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ static bool SemaBuiltinOverflow(Sema &S, CallExpr *TheCall) {

static void SemaBuiltinMemChkCall(Sema &S, FunctionDecl *FDecl,
CallExpr *TheCall, unsigned SizeIdx,
unsigned DstSizeIdx) {
unsigned DstSizeIdx,
StringRef LikelyMacroName) {
if (TheCall->getNumArgs() <= SizeIdx ||
TheCall->getNumArgs() <= DstSizeIdx)
return;
Expand All @@ -256,12 +257,21 @@ static void SemaBuiltinMemChkCall(Sema &S, FunctionDecl *FDecl,
if (Size.ule(DstSize))
return;

// confirmed overflow so generate the diagnostic.
IdentifierInfo *FnName = FDecl->getIdentifier();
// Confirmed overflow, so generate the diagnostic.
StringRef FunctionName = FDecl->getName();
SourceLocation SL = TheCall->getBeginLoc();
SourceRange SR = TheCall->getSourceRange();
SourceManager &SM = S.getSourceManager();
// If we're in an expansion of a macro whose name corresponds to this builtin,
// use the simple macro name and location.
if (SL.isMacroID() && Lexer::getImmediateMacroName(SL, SM, S.getLangOpts()) ==
LikelyMacroName) {
FunctionName = LikelyMacroName;
SL = SM.getImmediateMacroCallerLoc(SL);
}

S.Diag(SL, diag::warn_memcpy_chk_overflow) << SR << FnName;
S.Diag(SL, diag::warn_memcpy_chk_overflow)
<< FunctionName << DstSize.toString(/*Radix=*/10)
<< Size.toString(/*Radix=*/10);
}

static bool SemaBuiltinCallWithStaticChain(Sema &S, CallExpr *BuiltinCall) {
Expand Down Expand Up @@ -1219,21 +1229,37 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
// check secure string manipulation functions where overflows
// are detectable at compile time
case Builtin::BI__builtin___memcpy_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "memcpy");
break;
case Builtin::BI__builtin___memmove_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "memmove");
break;
case Builtin::BI__builtin___memset_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "memset");
break;
case Builtin::BI__builtin___strlcat_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "strlcat");
break;
case Builtin::BI__builtin___strlcpy_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "strlcpy");
break;
case Builtin::BI__builtin___strncat_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "strncat");
break;
case Builtin::BI__builtin___strncpy_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "strncpy");
break;
case Builtin::BI__builtin___stpncpy_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3);
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3, "stpncpy");
break;
case Builtin::BI__builtin___memccpy_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 3, 4);
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 3, 4, "memccpy");
break;
case Builtin::BI__builtin___snprintf_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 1, 3, "snprintf");
break;
case Builtin::BI__builtin___vsnprintf_chk:
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 1, 3);
SemaBuiltinMemChkCall(*this, FDecl, TheCall, 1, 3, "vsnprintf");
break;
case Builtin::BI__builtin_call_with_static_chain:
if (SemaBuiltinCallWithStaticChain(*this, TheCall))
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Sema/builtin-object-size.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ int f3() {
// rdar://6252231 - cannot call vsnprintf with va_list on x86_64
void f4(const char *fmt, ...) {
__builtin_va_list args;
__builtin___vsnprintf_chk (0, 42, 0, 11, fmt, args); // expected-warning {{'__builtin___vsnprintf_chk' will always overflow destination buffer}}
__builtin___vsnprintf_chk (0, 42, 0, 11, fmt, args); // expected-warning {{'__builtin___vsnprintf_chk' will always overflow; destination buffer has size 11, but size argument is 42}}
}

// rdar://18334276
Expand All @@ -50,7 +50,7 @@ void f6(void)
char b[5];
char buf[10];
__builtin___memccpy_chk (buf, b, '\0', sizeof(b), __builtin_object_size (buf, 0));
__builtin___memccpy_chk (b, buf, '\0', sizeof(buf), __builtin_object_size (b, 0)); // expected-warning {{'__builtin___memccpy_chk' will always overflow destination buffer}}
__builtin___memccpy_chk (b, buf, '\0', sizeof(buf), __builtin_object_size (b, 0)); // expected-warning {{'__builtin___memccpy_chk' will always overflow; destination buffer has size 5, but size argument is 10}}
}

int pr28314(void) {
Expand Down
19 changes: 15 additions & 4 deletions clang/test/Sema/builtins.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,30 +221,30 @@ void Test19(void)
// expected-note {{change size argument to be the size of the destination}}
__builtin___strlcpy_chk(buf, b, sizeof(b), __builtin_object_size(buf, 0)); // expected-warning {{size argument in '__builtin___strlcpy_chk' call appears to be size of the source; expected the size of the destination}} \
// expected-note {{change size argument to be the size of the destination}} \
// expected-warning {{'__builtin___strlcpy_chk' will always overflow destination buffer}}
// expected-warning {{'__builtin___strlcpy_chk' will always overflow; destination buffer has size 20, but size argument is 40}}

strlcat(buf, b, sizeof(b)); // expected-warning {{size argument in 'strlcat' call appears to be size of the source; expected the size of the destination}} \
// expected-note {{change size argument to be the size of the destination}}

__builtin___strlcat_chk(buf, b, sizeof(b), __builtin_object_size(buf, 0)); // expected-warning {{size argument in '__builtin___strlcat_chk' call appears to be size of the source; expected the size of the destination}} \
// expected-note {{change size argument to be the size of the destination}} \
// expected-warning {{'__builtin___strlcat_chk' will always overflow destination buffer}}
// expected-warning {{'__builtin___strlcat_chk' will always overflow; destination buffer has size 20, but size argument is 40}}
}

// rdar://11076881
char * Test20(char *p, const char *in, unsigned n)
{
static char buf[10];

__builtin___memcpy_chk (&buf[6], in, 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow destination buffer}}
__builtin___memcpy_chk (&buf[6], in, 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow; destination buffer has size 4, but size argument is 5}}

__builtin___memcpy_chk (p, "abcde", n, __builtin_object_size (p, 0));

__builtin___memcpy_chk (&buf[5], "abcde", 5, __builtin_object_size (&buf[5], 0));

__builtin___memcpy_chk (&buf[5], "abcde", n, __builtin_object_size (&buf[5], 0));

__builtin___memcpy_chk (&buf[6], "abcde", 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow destination buffer}}
__builtin___memcpy_chk (&buf[6], "abcde", 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow; destination buffer has size 4, but size argument is 5}}

return buf;
}
Expand Down Expand Up @@ -276,3 +276,14 @@ void test22(void) {
(void)__builtin_signbitl(1.0f);
(void)__builtin_signbitl(1.0L);
}

// rdar://43909200
#define memcpy(x,y,z) __builtin___memcpy_chk(x,y,z, __builtin_object_size(x,0))
#define my_memcpy(x,y,z) __builtin___memcpy_chk(x,y,z, __builtin_object_size(x,0))

void test23() {
char src[1024];
char buf[10];
memcpy(buf, src, 11); // expected-warning{{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
my_memcpy(buf, src, 11); // expected-warning{{'__builtin___memcpy_chk' will always overflow; destination buffer has size 10, but size argument is 11}}
}

0 comments on commit f85e391

Please sign in to comment.