Skip to content

Commit 3f60d22

Browse files
[-Wunsafe-buffer-usage] Fold the expression "cond ? E1 : E2" when checking safe patterns, if "cond" is a constant (#167989)
In `-Wunsafe-buffer-usage`, many safe pattern checks can benefit from constant folding. This commit improves null-terminated pointer checks by folding conditional expressions. rdar://159374822 --------- Co-authored-by: Balázs Benics <[email protected]>
1 parent 92c8c87 commit 3f60d22

File tree

2 files changed

+52
-5
lines changed

2 files changed

+52
-5
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -781,9 +781,25 @@ struct LibcFunNamePrefixSuffixParser {
781781
}
782782
};
783783

784+
// Constant fold a conditional expression 'cond ? A : B' to
785+
// - 'A', if 'cond' has constant true value;
786+
// - 'B', if 'cond' has constant false value.
787+
static const Expr *tryConstantFoldConditionalExpr(const Expr *E,
788+
const ASTContext &Ctx) {
789+
// FIXME: more places can use this function
790+
if (const auto *CE = dyn_cast<ConditionalOperator>(E)) {
791+
bool CondEval;
792+
793+
if (CE->getCond()->EvaluateAsBooleanCondition(CondEval, Ctx))
794+
return CondEval ? CE->getLHS() : CE->getRHS();
795+
}
796+
return E;
797+
}
798+
784799
// A pointer type expression is known to be null-terminated, if it has the
785800
// form: E.c_str(), for any expression E of `std::string` type.
786-
static bool isNullTermPointer(const Expr *Ptr) {
801+
static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) {
802+
Ptr = tryConstantFoldConditionalExpr(Ptr, Ctx);
787803
if (isa<clang::StringLiteral>(Ptr->IgnoreParenImpCasts()))
788804
return true;
789805
if (isa<PredefinedExpr>(Ptr->IgnoreParenImpCasts()))
@@ -874,7 +890,7 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg,
874890

875891
const Expr *Arg = Call->getArg(ArgIdx);
876892

877-
if (isNullTermPointer(Arg))
893+
if (isNullTermPointer(Arg, Ctx))
878894
// If Arg is a null-terminated pointer, it is safe anyway.
879895
return true; // continue parsing
880896

@@ -922,8 +938,8 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg,
922938
// (including the format argument) is unsafe pointer.
923939
return llvm::any_of(
924940
llvm::make_range(Call->arg_begin() + FmtArgIdx, Call->arg_end()),
925-
[&UnsafeArg](const Expr *Arg) -> bool {
926-
if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg)) {
941+
[&UnsafeArg, &Ctx](const Expr *Arg) -> bool {
942+
if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg, Ctx)) {
927943
UnsafeArg = Arg;
928944
return true;
929945
}
@@ -1175,7 +1191,7 @@ static bool hasUnsafePrintfStringArg(const CallExpr &Node, ASTContext &Ctx,
11751191
// We don't really recognize this "normal" printf, the only thing we
11761192
// can do is to require all pointers to be null-terminated:
11771193
for (const auto *Arg : Node.arguments())
1178-
if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg)) {
1194+
if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg, Ctx)) {
11791195
Result.addNode(Tag, DynTypedNode::create(*Arg));
11801196
return true;
11811197
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %clang_cc1 -fsyntax-only -Wno-all -Wunsafe-buffer-usage -verify %s -std=c++20
2+
// RUN: %clang_cc1 -fsyntax-only -Wno-all -Wunsafe-buffer-usage -verify %s -x c
3+
// expected-no-diagnostics
4+
5+
typedef struct {} FILE;
6+
int fprintf( FILE* stream, const char* format, ... );
7+
FILE * stderr;
8+
9+
#define DEBUG_ASSERT_MESSAGE(name, assertion, label, message, file, line, value) \
10+
fprintf(stderr, "AssertMacros: %s, %s file: %s, line: %d, value: %lld\n", \
11+
assertion, (message!=0) ? message : "", file, line, (long long) (value));
12+
13+
14+
#define Require(assertion, exceptionLabel) \
15+
do \
16+
{ \
17+
if ( __builtin_expect(!(assertion), 0) ) { \
18+
DEBUG_ASSERT_MESSAGE( \
19+
"DEBUG_ASSERT_COMPONENT_NAME_STRING", \
20+
#assertion, #exceptionLabel, 0, __FILE__, __LINE__, 0); \
21+
goto exceptionLabel; \
22+
} \
23+
} while ( 0 )
24+
25+
26+
void f(int x, int y) {
27+
Require(x == y, L1);
28+
L1:
29+
return;
30+
}
31+

0 commit comments

Comments
 (0)