From 25e592e52236669f72bfb54986c1197cc6b640cf Mon Sep 17 00:00:00 2001 From: Kristof Umann Date: Wed, 17 Apr 2019 19:56:40 +0000 Subject: [PATCH] [analyzer] PR41185: Fix regression where __builtin_* functions weren't recognized For the following code snippet: void builtin_function_call_crash_fixes(char *c) { __builtin_strncpy(c, "", 6); __builtin_memset(c, '\0', (0)); __builtin_memcpy(c, c, 0); } security.insecureAPI.DeprecatedOrUnsafeBufferHandling caused a regression, as it didn't recognize functions starting with __builtin_. Fixed exactly that. I wanted to modify an existing test file, but the two I found didn't seem like perfect candidates. While I was there, I prettified their RUN: lines. Differential Revision: https://reviews.llvm.org/D59812 llvm-svn: 358609 --- .../Checkers/CheckSecuritySyntaxOnly.cpp | 4 ++ .../Analysis/security-syntax-checks-no-emit.c | 5 ++- clang/test/Analysis/security-syntax-checks.c | 8 ++++ clang/test/Analysis/security-syntax-checks.m | 45 +++++++++++++++---- 4 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 clang/test/Analysis/security-syntax-checks.c diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 05e25c41597b..3f1c213a5647 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -740,7 +740,11 @@ void WalkAST::checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE, // Issue a warning. ArgIndex == -1: Deprecated but not unsafe (has size // restrictions). enum { DEPR_ONLY = -1, UNKNOWN_CALL = -2 }; + StringRef Name = FD->getIdentifier()->getName(); + if (Name.startswith("__builtin_")) + Name = Name.substr(10); + int ArgIndex = llvm::StringSwitch(Name) .Cases("scanf", "wscanf", "vscanf", "vwscanf", 0) diff --git a/clang/test/Analysis/security-syntax-checks-no-emit.c b/clang/test/Analysis/security-syntax-checks-no-emit.c index 29dd2017745e..746b7595ef1f 100644 --- a/clang/test/Analysis/security-syntax-checks-no-emit.c +++ b/clang/test/Analysis/security-syntax-checks-no-emit.c @@ -1,4 +1,7 @@ -// RUN: %clang_analyze_cc1 -triple i686-pc-linux-gnu -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify +// RUN: %clang_analyze_cc1 -triple i686-pc-linux-gnu %s -verify \ +// RUN: -analyzer-checker=security.insecureAPI \ +// RUN: -analyzer-checker=security.FloatLoopCounter + // expected-no-diagnostics // This file complements 'security-syntax-checks.m', but tests that we omit diff --git a/clang/test/Analysis/security-syntax-checks.c b/clang/test/Analysis/security-syntax-checks.c new file mode 100644 index 000000000000..25ce8b1b8823 --- /dev/null +++ b/clang/test/Analysis/security-syntax-checks.c @@ -0,0 +1,8 @@ +// RUN: %clang_analyze_cc1 %s -verify \ +// RUN: -analyzer-checker=security.insecureAPI + +void builtin_function_call_crash_fixes(char *c) { + __builtin_strncpy(c, "", 6); // expected-warning{{Call to function 'strncpy' is insecure as it does not provide security checks introduced in the C11 standard.}} + __builtin_memset(c, '\0', (0)); // expected-warning{{Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard.}} + __builtin_memcpy(c, c, 0); // expected-warning{{Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard.}} +} diff --git a/clang/test/Analysis/security-syntax-checks.m b/clang/test/Analysis/security-syntax-checks.m index a3efd53d4b67..5c63f0686eec 100644 --- a/clang/test/Analysis/security-syntax-checks.m +++ b/clang/test/Analysis/security-syntax-checks.m @@ -1,11 +1,40 @@ -// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify -// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -DUSE_BUILTINS -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify -// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -DVARIANT -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify -// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -DUSE_BUILTINS -DVARIANT -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify -// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify -// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi -DUSE_BUILTINS -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify -// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi -DVARIANT -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify -// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi -DUSE_BUILTINS -DVARIANT -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify +// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify \ +// RUN: -analyzer-checker=security.insecureAPI \ +// RUN: -analyzer-checker=security.FloatLoopCounter + +// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify \ +// RUN: -DUSE_BUILTINS \ +// RUN: -analyzer-checker=security.insecureAPI \ +// RUN: -analyzer-checker=security.FloatLoopCounter + +// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify \ +// RUN: -DVARIANT \ +// RUN: -analyzer-checker=security.insecureAPI \ +// RUN: -analyzer-checker=security.FloatLoopCounter + +// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify \ +// RUN: -DUSE_BUILTINS -DVARIANT \ +// RUN: -analyzer-checker=security.insecureAPI \ +// RUN: -analyzer-checker=security.FloatLoopCounter + +// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify \ +// RUN: -analyzer-checker=security.insecureAPI \ +// RUN: -analyzer-checker=security.FloatLoopCounter + +// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify \ +// RUN: -DUSE_BUILTINS \ +// RUN: -analyzer-checker=security.insecureAPI \ +// RUN: -analyzer-checker=security.FloatLoopCounter + +// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify \ +// RUN: -DVARIANT \ +// RUN: -analyzer-checker=security.insecureAPI \ +// RUN: -analyzer-checker=security.FloatLoopCounter + +// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify \ +// RUN: -DUSE_BUILTINS -DVARIANT \ +// RUN: -analyzer-checker=security.insecureAPI \ +// RUN: -analyzer-checker=security.FloatLoopCounter #ifdef USE_BUILTINS # define BUILTIN(f) __builtin_ ## f