Skip to content

Commit

Permalink
[clang][analyzer] Handle special value AT_FDCWD in affected standard …
Browse files Browse the repository at this point in the history 
…functions

Some file and directory related functions have an integer file descriptor argument
that can be a valid file descriptor or a special value AT_FDCWD. This value is
relatively often used in open source projects and is usually defined as a negative
number, and the checker reports false warnings (a valid file descriptor is not
negative) if this fix is not included.

Reviewed By: steakhal

Differential Revision: https://reviews.llvm.org/D149160
  • Loading branch information
@balazske
balazske committed May 16, 2023
1 parent 6c06a07 commit 258c9be
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 16 deletions.
41 changes: 25 additions & 16 deletions clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1392,6 +1392,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
SValBuilder &SVB = C.getSValBuilder();
BasicValueFactory &BVF = SVB.getBasicValueFactory();
const ASTContext &ACtx = BVF.getContext();
Preprocessor &PP = C.getPreprocessor();

// Helper class to lookup a type by its name.
class LookupType {
Expand Down Expand Up @@ -1527,14 +1528,11 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
const RangeInt UCharRangeMax =
std::min(BVF.getMaxValue(ACtx.UnsignedCharTy).getLimitedValue(), IntMax);

// The platform dependent value of EOF.
// Try our best to parse this from the Preprocessor, otherwise fallback to -1.
const auto EOFv = [&C]() -> RangeInt {
if (const std::optional<int> OptInt =
tryExpandAsInteger("EOF", C.getPreprocessor()))
return *OptInt;
return -1;
}();
// Get platform dependent values of some macros.
// Try our best to parse this from the Preprocessor, otherwise fallback to a
// default value (what is found in a library header).
const auto EOFv = tryExpandAsInteger("EOF", PP).value_or(-1);
const auto AT_FDCWDv = tryExpandAsInteger("AT_FDCWD", PP).value_or(-100);

// Auxiliary class to aid adding summaries to the summary map.
struct AddToFunctionSummaryMap {
Expand Down Expand Up @@ -1979,6 +1977,12 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, IntMax))};
const auto &ReturnsValidFileDescriptor = ReturnsNonnegative;

auto ValidFileDescriptorOrAtFdcwd = [&](ArgNo ArgN) {
return std::make_shared<RangeConstraint>(
ArgN, WithinRange, Range({AT_FDCWDv, AT_FDCWDv}, {0, IntMax}),
"a valid file descriptor or AT_FDCWD");
};

// FILE *fopen(const char *restrict pathname, const char *restrict mode);
addToFunctionSummaryMap(
"fopen",
Expand Down Expand Up @@ -2130,6 +2134,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(NoEvalCall)
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1))));

// int dup(int fildes);
Expand Down Expand Up @@ -2207,7 +2212,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(NotNull(ArgNo(0)))
.ArgConstraint(ArgumentCondition(1, WithinRange, Range(0, IntMax)))
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(1)))
.ArgConstraint(NotNull(ArgNo(2))));

// int lockf(int fd, int cmd, off_t len);
Expand Down Expand Up @@ -2318,6 +2323,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(NoEvalCall)
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1))));

std::optional<QualType> Dev_tTy = lookupTy("dev_t");
Expand All @@ -2339,6 +2345,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(NoEvalCall)
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1))));

// int chmod(const char *path, mode_t mode);
Expand All @@ -2357,7 +2364,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(NoEvalCall)
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1))));

// int fchmod(int fildes, mode_t mode);
Expand All @@ -2381,7 +2388,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(NoEvalCall)
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1))));

// int chown(const char *path, uid_t owner, gid_t group);
Expand Down Expand Up @@ -2446,9 +2453,9 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(NoEvalCall)
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1)))
.ArgConstraint(ArgumentCondition(2, WithinRange, Range(0, IntMax)))
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(2)))
.ArgConstraint(NotNull(ArgNo(3))));

// int unlink(const char *pathname);
Expand All @@ -2466,7 +2473,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(NoEvalCall)
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1))));

std::optional<QualType> StructStatTy = lookupTy("stat");
Expand Down Expand Up @@ -2515,7 +2522,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(NoEvalCall)
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1)))
.ArgConstraint(NotNull(ArgNo(2))));

Expand Down Expand Up @@ -2690,7 +2697,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
ReturnValueCondition(WithinRange, Range(0, Ssize_tMax))},
ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1)))
.ArgConstraint(NotNull(ArgNo(2)))
.ArgConstraint(BufferSize(/*Buffer=*/ArgNo(2),
Expand All @@ -2707,7 +2714,9 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
Summary(NoEvalCall)
.Case(ReturnsZero, ErrnoMustNotBeChecked)
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
.ArgConstraint(NotNull(ArgNo(1)))
.ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(2)))
.ArgConstraint(NotNull(ArgNo(3))));

// char *realpath(const char *restrict file_name,
Expand Down
25 changes: 25 additions & 0 deletions clang/test/Analysis/std-c-library-functions-arg-constraints.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
// RUN: -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
// RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -triple x86_64-unknown-linux-gnu \
Expand All @@ -13,6 +14,7 @@
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
// RUN: -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
// RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -triple x86_64-unknown-linux-gnu \
Expand Down Expand Up @@ -309,3 +311,26 @@ void test_min_buf_size(void) {
// bugpath-warning{{The 1st argument to '__buf_size_arg_constraint_concrete' is out of the accepted range; It should be a buffer with size equal to or greater than 10}} \
// bugpath-note{{The 1st argument to '__buf_size_arg_constraint_concrete' is out of the accepted range; It should be a buffer with size equal to or greater than 10}}
}

void test_file_fd_at_functions() {
(void)linkat(-22, "from", AT_FDCWD, "to", 0); // \
// report-warning{{The 1st argument to 'linkat' is -22 but should be a valid file descriptor or AT_FDCWD}} \
// bugpath-warning{{The 1st argument to 'linkat' is -22 but should be a valid file descriptor or AT_FDCWD}} \
// bugpath-note{{The 1st argument to 'linkat' is -22 but should be a valid file descriptor or AT_FDCWD}}

// no warning for these functions if the AT_FDCWD value is used
(void)linkat(AT_FDCWD, "from", AT_FDCWD, "to", 0);
(void)faccessat(AT_FDCWD, "path", 0, 0);
(void)symlinkat("oldpath", AT_FDCWD, "newpath");
(void)mkdirat(AT_FDCWD, "path", 0);
(void)mknodat(AT_FDCWD, "path", 0, 0);
(void)fchmodat(AT_FDCWD, "path", 0, 0);
(void)fchownat(AT_FDCWD, "path", 0, 0, 0);
(void)linkat(AT_FDCWD, "oldpath", AT_FDCWD, "newpath", 0);
(void)unlinkat(AT_FDCWD, "newpath", 0);
struct stat St;
(void)fstatat(AT_FDCWD, "newpath", &St, 0);
char Buf[10];
(void)readlinkat(AT_FDCWD, "newpath", Buf, 10);
(void)renameat(AT_FDCWD, "oldpath", AT_FDCWD, "newpath");
}

0 comments on commit 258c9be

Please sign in to comment.