Skip to content

Commit 1f54190

Browse files
authored
Merge branch 'main' into strong-siv-overflow
2 parents ce3c2d1 + bfc322d commit 1f54190

29 files changed

+651
-414
lines changed

clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,19 @@ static auto valueOperatorCall() {
137137
isStatusOrOperatorCallWithName("->")));
138138
}
139139

140+
static clang::ast_matchers::TypeMatcher statusType() {
141+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
142+
return hasCanonicalType(qualType(hasDeclaration(statusClass())));
143+
}
144+
145+
static auto isComparisonOperatorCall(llvm::StringRef operator_name) {
146+
using namespace ::clang::ast_matchers; // NOLINT: Too many names
147+
return cxxOperatorCallExpr(
148+
hasOverloadedOperatorName(operator_name), argumentCountIs(2),
149+
hasArgument(0, anyOf(hasType(statusType()), hasType(statusOrType()))),
150+
hasArgument(1, anyOf(hasType(statusType()), hasType(statusOrType()))));
151+
}
152+
140153
static auto
141154
buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) {
142155
return CFGMatchSwitchBuilder<const Environment,
@@ -312,6 +325,101 @@ static void transferStatusUpdateCall(const CXXMemberCallExpr *Expr,
312325
State.Env.setValue(locForOk(*ThisLoc), NewVal);
313326
}
314327

328+
static BoolValue *evaluateStatusEquality(RecordStorageLocation &LhsStatusLoc,
329+
RecordStorageLocation &RhsStatusLoc,
330+
Environment &Env) {
331+
auto &A = Env.arena();
332+
// Logically, a Status object is composed of an error code that could take one
333+
// of multiple possible values, including the "ok" value. We track whether a
334+
// Status object has an "ok" value and represent this as an `ok` bit. Equality
335+
// of Status objects compares their error codes. Therefore, merely comparing
336+
// the `ok` bits isn't sufficient: when two Status objects are assigned non-ok
337+
// error codes the equality of their respective error codes matters. Since we
338+
// only track the `ok` bits, we can't make any conclusions about equality when
339+
// we know that two Status objects have non-ok values.
340+
341+
auto &LhsOkVal = valForOk(LhsStatusLoc, Env);
342+
auto &RhsOkVal = valForOk(RhsStatusLoc, Env);
343+
344+
auto &Res = Env.makeAtomicBoolValue();
345+
346+
// lhs && rhs => res (a.k.a. !res => !lhs || !rhs)
347+
Env.assume(A.makeImplies(A.makeAnd(LhsOkVal.formula(), RhsOkVal.formula()),
348+
Res.formula()));
349+
// res => (lhs == rhs)
350+
Env.assume(A.makeImplies(
351+
Res.formula(), A.makeEquals(LhsOkVal.formula(), RhsOkVal.formula())));
352+
353+
return &Res;
354+
}
355+
356+
static BoolValue *
357+
evaluateStatusOrEquality(RecordStorageLocation &LhsStatusOrLoc,
358+
RecordStorageLocation &RhsStatusOrLoc,
359+
Environment &Env) {
360+
auto &A = Env.arena();
361+
// Logically, a StatusOr<T> object is composed of two values - a Status and a
362+
// value of type T. Equality of StatusOr objects compares both values.
363+
// Therefore, merely comparing the `ok` bits of the Status values isn't
364+
// sufficient. When two StatusOr objects are engaged, the equality of their
365+
// respective values of type T matters. Similarly, when two StatusOr objects
366+
// have Status values that have non-ok error codes, the equality of the error
367+
// codes matters. Since we only track the `ok` bits of the Status values, we
368+
// can't make any conclusions about equality when we know that two StatusOr
369+
// objects are engaged or when their Status values contain non-ok error codes.
370+
auto &LhsOkVal = valForOk(locForStatus(LhsStatusOrLoc), Env);
371+
auto &RhsOkVal = valForOk(locForStatus(RhsStatusOrLoc), Env);
372+
auto &res = Env.makeAtomicBoolValue();
373+
374+
// res => (lhs == rhs)
375+
Env.assume(A.makeImplies(
376+
res.formula(), A.makeEquals(LhsOkVal.formula(), RhsOkVal.formula())));
377+
return &res;
378+
}
379+
380+
static BoolValue *evaluateEquality(const Expr *LhsExpr, const Expr *RhsExpr,
381+
Environment &Env) {
382+
// Check the type of both sides in case an operator== is added that admits
383+
// different types.
384+
if (isStatusOrType(LhsExpr->getType()) &&
385+
isStatusOrType(RhsExpr->getType())) {
386+
auto *LhsStatusOrLoc = Env.get<RecordStorageLocation>(*LhsExpr);
387+
if (LhsStatusOrLoc == nullptr)
388+
return nullptr;
389+
auto *RhsStatusOrLoc = Env.get<RecordStorageLocation>(*RhsExpr);
390+
if (RhsStatusOrLoc == nullptr)
391+
return nullptr;
392+
393+
return evaluateStatusOrEquality(*LhsStatusOrLoc, *RhsStatusOrLoc, Env);
394+
}
395+
if (isStatusType(LhsExpr->getType()) && isStatusType(RhsExpr->getType())) {
396+
auto *LhsStatusLoc = Env.get<RecordStorageLocation>(*LhsExpr);
397+
if (LhsStatusLoc == nullptr)
398+
return nullptr;
399+
400+
auto *RhsStatusLoc = Env.get<RecordStorageLocation>(*RhsExpr);
401+
if (RhsStatusLoc == nullptr)
402+
return nullptr;
403+
404+
return evaluateStatusEquality(*LhsStatusLoc, *RhsStatusLoc, Env);
405+
}
406+
return nullptr;
407+
}
408+
409+
static void transferComparisonOperator(const CXXOperatorCallExpr *Expr,
410+
LatticeTransferState &State,
411+
bool IsNegative) {
412+
auto *LhsAndRhsVal =
413+
evaluateEquality(Expr->getArg(0), Expr->getArg(1), State.Env);
414+
if (LhsAndRhsVal == nullptr)
415+
return;
416+
417+
if (IsNegative)
418+
State.Env.setValue(*Expr, State.Env.makeNot(*LhsAndRhsVal));
419+
else
420+
State.Env.setValue(*Expr, *LhsAndRhsVal);
421+
}
422+
315423
CFGMatchSwitch<LatticeTransferState>
316424
buildTransferMatchSwitch(ASTContext &Ctx,
317425
CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
@@ -325,6 +433,20 @@ buildTransferMatchSwitch(ASTContext &Ctx,
325433
transferStatusOkCall)
326434
.CaseOfCFGStmt<CXXMemberCallExpr>(isStatusMemberCallWithName("Update"),
327435
transferStatusUpdateCall)
436+
.CaseOfCFGStmt<CXXOperatorCallExpr>(
437+
isComparisonOperatorCall("=="),
438+
[](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &,
439+
LatticeTransferState &State) {
440+
transferComparisonOperator(Expr, State,
441+
/*IsNegative=*/false);
442+
})
443+
.CaseOfCFGStmt<CXXOperatorCallExpr>(
444+
isComparisonOperatorCall("!="),
445+
[](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &,
446+
LatticeTransferState &State) {
447+
transferComparisonOperator(Expr, State,
448+
/*IsNegative=*/true);
449+
})
328450
.Build();
329451
}
330452

clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,8 @@ DignosticsEngineWithDiagOpts::DignosticsEngineWithDiagOpts(
382382

383383
std::pair<std::unique_ptr<driver::Driver>, std::unique_ptr<driver::Compilation>>
384384
buildCompilation(ArrayRef<std::string> ArgStrs, DiagnosticsEngine &Diags,
385-
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) {
385+
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
386+
llvm::BumpPtrAllocator &Alloc) {
386387
SmallVector<const char *, 256> Argv;
387388
Argv.reserve(ArgStrs.size());
388389
for (const std::string &Arg : ArgStrs)
@@ -393,7 +394,6 @@ buildCompilation(ArrayRef<std::string> ArgStrs, DiagnosticsEngine &Diags,
393394
"clang LLVM compiler", FS);
394395
Driver->setTitle("clang_based_tool");
395396

396-
llvm::BumpPtrAllocator Alloc;
397397
bool CLMode = driver::IsClangCL(
398398
driver::getDriverMode(Argv[0], ArrayRef(Argv).slice(1)));
399399

clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ struct TextDiagnosticsPrinterWithOutput {
105105

106106
std::pair<std::unique_ptr<driver::Driver>, std::unique_ptr<driver::Compilation>>
107107
buildCompilation(ArrayRef<std::string> ArgStrs, DiagnosticsEngine &Diags,
108-
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
108+
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
109+
llvm::BumpPtrAllocator &Alloc);
109110

110111
std::unique_ptr<CompilerInvocation>
111112
createCompilerInvocation(ArrayRef<std::string> CommandLine,

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,10 @@ static bool forEachDriverJob(
7878
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
7979
llvm::function_ref<bool(const driver::Command &Cmd)> Callback) {
8080
// Compilation holds a non-owning a reference to the Driver, hence we need to
81-
// keep the Driver alive when we use Compilation.
82-
auto [Driver, Compilation] = buildCompilation(ArgStrs, Diags, FS);
81+
// keep the Driver alive when we use Compilation. Arguments to commands may be
82+
// owned by Alloc when expanded from response files.
83+
llvm::BumpPtrAllocator Alloc;
84+
auto [Driver, Compilation] = buildCompilation(ArgStrs, Diags, FS, Alloc);
8385
if (!Compilation)
8486
return false;
8587
for (const driver::Command &Job : Compilation->getJobs()) {

clang/test/ClangScanDeps/response-file.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
// Check that the scanner can handle a response file input.
1+
// Check that the scanner can handle a response file input. Uses -verbatim-args
2+
// to ensure response files are expanded by the scanner library and not the
3+
// argumeent adjuster in clang-scan-deps.
24

35
// RUN: rm -rf %t
46
// RUN: split-file %s %t
57
// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
68

7-
// RUN: clang-scan-deps -format experimental-full -compilation-database %t/cdb.json > %t/deps.json
9+
// RUN: clang-scan-deps -verbatim-args -format experimental-full -compilation-database %t/cdb.json > %t/deps.json
810

911
// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
1012

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 6
2+
3+
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -O1 -o - %s \
4+
// RUN: | FileCheck %s --check-prefixes=CLEAN-O1
5+
6+
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -O1 -o - %s \
7+
// RUN: -fsanitize=signed-integer-overflow \
8+
// RUN: -fsanitize-skip-hot-cutoff=signed-integer-overflow=1.0 \
9+
// RUN: -fallow-runtime-check-skip-hot-cutoff=1.0 \
10+
// RUN: | FileCheck %s --check-prefixes=UBSAN-O1
11+
12+
// This test shows that -fsanitize-skip-hot-cutoff=...=1.0 plus
13+
// -fallow-runtime-check-skip-hot-cutoff=1.0 does not perfectly undo the
14+
// effects of -fsanitize.
15+
16+
// CLEAN-O1-LABEL: define dso_local i32 @overflow(
17+
// CLEAN-O1-SAME: i32 noundef [[X:%.*]], i32 noundef [[Y:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
18+
// CLEAN-O1-NEXT: [[ENTRY:.*:]]
19+
// CLEAN-O1-NEXT: [[ADD:%.*]] = add nsw i32 [[Y]], [[X]]
20+
// CLEAN-O1-NEXT: ret i32 [[ADD]]
21+
//
22+
// UBSAN-O1-LABEL: define dso_local noundef i32 @overflow(
23+
// UBSAN-O1-SAME: i32 noundef [[X:%.*]], i32 noundef [[Y:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
24+
// UBSAN-O1-NEXT: [[ENTRY:.*:]]
25+
// UBSAN-O1-NEXT: [[TMP0:%.*]] = add i32 [[X]], [[Y]]
26+
// UBSAN-O1-NEXT: ret i32 [[TMP0]]
27+
//
28+
int overflow(int x, int y) {
29+
return x + y;
30+
}

clang/tools/clang-scan-deps/ClangScanDeps.cpp

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ static constexpr bool DoRoundTripDefault = false;
106106
#endif
107107

108108
static bool RoundTripArgs = DoRoundTripDefault;
109+
static bool VerbatimArgs = false;
109110

110111
static void ParseArgs(int argc, char **argv) {
111112
ScanDepsOptTable Tbl;
@@ -239,6 +240,8 @@ static void ParseArgs(int argc, char **argv) {
239240

240241
RoundTripArgs = Args.hasArg(OPT_round_trip_args);
241242

243+
VerbatimArgs = Args.hasArg(OPT_verbatim_args);
244+
242245
if (const llvm::opt::Arg *A = Args.getLastArgNoClaim(OPT_DASH_DASH))
243246
CommandLine.assign(A->getValues().begin(), A->getValues().end());
244247
}
@@ -883,22 +886,24 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
883886

884887
llvm::cl::PrintOptionValues();
885888

886-
// Expand response files in advance, so that we can "see" all the arguments
887-
// when adjusting below.
888-
Compilations = expandResponseFiles(std::move(Compilations),
889-
llvm::vfs::getRealFileSystem());
889+
if (!VerbatimArgs) {
890+
// Expand response files in advance, so that we can "see" all the arguments
891+
// when adjusting below.
892+
Compilations = expandResponseFiles(std::move(Compilations),
893+
llvm::vfs::getRealFileSystem());
890894

891-
Compilations = inferTargetAndDriverMode(std::move(Compilations));
895+
Compilations = inferTargetAndDriverMode(std::move(Compilations));
892896

893-
Compilations = inferToolLocation(std::move(Compilations));
897+
Compilations = inferToolLocation(std::move(Compilations));
898+
}
894899

895900
// The command options are rewritten to run Clang in preprocessor only mode.
896901
auto AdjustingCompilations =
897902
std::make_unique<tooling::ArgumentsAdjustingCompilations>(
898903
std::move(Compilations));
899904
ResourceDirectoryCache ResourceDirCache;
900905

901-
AdjustingCompilations->appendArgumentsAdjuster(
906+
auto ArgsAdjuster =
902907
[&ResourceDirCache](const tooling::CommandLineArguments &Args,
903908
StringRef FileName) {
904909
std::string LastO;
@@ -960,7 +965,10 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
960965
}
961966
AdjustedArgs.insert(AdjustedArgs.end(), FlagsEnd, Args.end());
962967
return AdjustedArgs;
963-
});
968+
};
969+
970+
if (!VerbatimArgs)
971+
AdjustingCompilations->appendArgumentsAdjuster(ArgsAdjuster);
964972

965973
SharedStream Errs(llvm::errs());
966974

clang/tools/clang-scan-deps/Opts.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,6 @@ def verbose : F<"v", "Use verbose output">;
4444

4545
def round_trip_args : F<"round-trip-args", "verify that command-line arguments are canonical by parsing and re-serializing">;
4646

47+
def verbatim_args : F<"verbatim-args", "Pass commands to the scanner verbatim without adjustments">;
48+
4749
def DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>;

0 commit comments

Comments
 (0)