[clang][analyzer] Handle special value AT_FDCWD in affected standard functions

[balazske]

Summary

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.

[balazske]

Fixed formatting problems.

[steakhal]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1530–1536

2137–2138

I think I would recommend hoisting this common subexpression. This would save repetition on subsequent occurrences.

const auto ValidDescriptorOrAtCWDRange = Range({AT_FDCWDv, AT_FDCWDv}, {0, IntMax});

You can explore if hoisting the whole ArgumentCondition would make it even more readable.

[balazske]

added special argument condition function to remove code repetition

[steakhal]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1530–1536

What is your response @balazske?

1982

Should we define a specific constraint description?
If not, then we should not specify it explicitly - given that it's already defined as an empty string by default.

[balazske]

using short way for getting macro value
added constraint description

[balazske]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

1530–1536

This is much better.

1982

Now a description is added. This looks better, but the user should know what is a "valid file descriptor" (is 0 allowed or not), it is not shown now.

[steakhal]

LGTM

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

 void StdLibraryFunctionsChecker::initFunctionSummaries(
     CheckerContext &C) const {
   if (SummariesInitialized)
     return;
 
   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 {
     const ASTContext &ACtx;
 
   public:
     LookupType(const ASTContext &ACtx) : ACtx(ACtx) {}
 
void StdLibraryFunctionsChecker::initFunctionSummaries(
   // Set UCharRangeMax to min of int or uchar maximum value.
   // The C standard states that the arguments of functions like isalpha must
   // be representable as an unsigned char. Their type is 'int', so the max
   // value of the argument should be min(UCharMax, IntMax). This just happen
   // to be true for commonly used and well tested instruction set
   // architectures, but not for others.
   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 {
     const ASTContext &ACtx;
     FunctionSummaryMapType &Map;
     bool DisplayLoadedSummaries;
     AddToFunctionSummaryMap(const ASTContext &ACtx, FunctionSummaryMapType &FSM,
                             bool DisplayLoadedSummaries)
         : ACtx(ACtx), Map(FSM), DisplayLoadedSummaries(DisplayLoadedSummaries) {
if (ModelPOSIX) {
     const auto ReturnsNonnegative =
         ConstraintSet{ReturnValueCondition(WithinRange, Range(0, IntMax))};
     const auto ReturnsNonZero =
         ConstraintSet{ReturnValueCondition(OutOfRange, SingleValue(0))};
     const auto ReturnsFileDescriptor =
         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",
         Signature(ArgTypes{ConstCharPtrRestrictTy, ConstCharPtrRestrictTy},
                   RetType{FilePtrTy}),
         Summary(NoEvalCall)
             .Case({NotNull(Ret)}, ErrnoMustNotBeChecked)
             .Case({IsNull(Ret)}, ErrnoNEZeroIrrelevant)
if (ModelPOSIX) {
     // int faccessat(int dirfd, const char *pathname, int mode, int flags);
     addToFunctionSummaryMap(
         "faccessat",
         Signature(ArgTypes{IntTy, ConstCharPtrTy, IntTy, IntTy},
                   RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsZero, ErrnoMustNotBeChecked)
             .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+            .ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
             .ArgConstraint(NotNull(ArgNo(1))));
 
     // int dup(int fildes);
     addToFunctionSummaryMap(
         "dup", Signature(ArgTypes{IntTy}, RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
             .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
             .ArgConstraint(
if (ModelPOSIX) {
     addToFunctionSummaryMap(
         "symlinkat",
         Signature(ArgTypes{ConstCharPtrTy, IntTy, ConstCharPtrTy},
                   RetType{IntTy}),
         Summary(NoEvalCall)
             .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);
     addToFunctionSummaryMap(
         "lockf", Signature(ArgTypes{IntTy, IntTy, Off_tTy}, RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsZero, ErrnoMustNotBeChecked)
             .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
if (ModelPOSIX) {
 
     // int mkdirat(int dirfd, const char *pathname, mode_t mode);
     addToFunctionSummaryMap(
         "mkdirat",
         Signature(ArgTypes{IntTy, ConstCharPtrTy, Mode_tTy}, RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsZero, ErrnoMustNotBeChecked)
             .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+            .ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
             .ArgConstraint(NotNull(ArgNo(1))));
 
     std::optional<QualType> Dev_tTy = lookupTy("dev_t");
 
     // int mknod(const char *pathname, mode_t mode, dev_t dev);
     addToFunctionSummaryMap(
         "mknod",
         Signature(ArgTypes{ConstCharPtrTy, Mode_tTy, Dev_tTy}, RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsZero, ErrnoMustNotBeChecked)
             .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
             .ArgConstraint(NotNull(ArgNo(0))));
 
     // int mknodat(int dirfd, const char *pathname, mode_t mode, dev_t dev);
     addToFunctionSummaryMap(
         "mknodat",
         Signature(ArgTypes{IntTy, ConstCharPtrTy, Mode_tTy, Dev_tTy},
                   RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsZero, ErrnoMustNotBeChecked)
             .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+            .ArgConstraint(ValidFileDescriptorOrAtFdcwd(ArgNo(0)))
             .ArgConstraint(NotNull(ArgNo(1))));
 
     // int chmod(const char *path, mode_t mode);
     addToFunctionSummaryMap(
         "chmod", Signature(ArgTypes{ConstCharPtrTy, Mode_tTy}, RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsZero, ErrnoMustNotBeChecked)
             .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
             .ArgConstraint(NotNull(ArgNo(0))));
 
     // int fchmodat(int dirfd, const char *pathname, mode_t mode, int flags);
     addToFunctionSummaryMap(
         "fchmodat",
         Signature(ArgTypes{IntTy, ConstCharPtrTy, Mode_tTy, IntTy},
                   RetType{IntTy}),
         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);
     addToFunctionSummaryMap(
         "fchmod", Signature(ArgTypes{IntTy, Mode_tTy}, RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsZero, ErrnoMustNotBeChecked)
             .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
             .ArgConstraint(
                 ArgumentCondition(0, WithinRange, Range(0, IntMax))));
 
     std::optional<QualType> Uid_tTy = lookupTy("uid_t");
     std::optional<QualType> Gid_tTy = lookupTy("gid_t");
 
     // int fchownat(int dirfd, const char *pathname, uid_t owner, gid_t group,
     //              int flags);
     addToFunctionSummaryMap(
         "fchownat",
         Signature(ArgTypes{IntTy, ConstCharPtrTy, Uid_tTy, Gid_tTy, IntTy},
                   RetType{IntTy}),
         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);
     addToFunctionSummaryMap(
         "chown",
         Signature(ArgTypes{ConstCharPtrTy, Uid_tTy, Gid_tTy}, RetType{IntTy}),
         Summary(NoEvalCall)
             .Case(ReturnsZero, ErrnoMustNotBeChecked)
if (ModelPOSIX) {
     //            int flag);
     addToFunctionSummaryMap(
         "linkat",
         Signature(ArgTypes{IntTy, ConstCharPtrTy, IntTy, ConstCharPtrTy, IntTy},
                   RetType{IntTy}),
         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);
     addToFunctionSummaryMap("unlink",
                             Signature(ArgTypes{ConstCharPtrTy}, RetType{IntTy}),
                             Summary(NoEvalCall)
                                 .Case(ReturnsZero, ErrnoMustNotBeChecked)
                                 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
                                 .ArgConstraint(NotNull(ArgNo(0))));
 
     // int unlinkat(int fd, const char *path, int flag);
     addToFunctionSummaryMap(
         "unlinkat",
         Signature(ArgTypes{IntTy, ConstCharPtrTy, IntTy}, RetType{IntTy}),
         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");
     std::optional<QualType> StructStatPtrTy = getPointerTy(StructStatTy);
     std::optional<QualType> StructStatPtrRestrictTy =
         getRestrictTy(StructStatPtrTy);
 
     // int fstat(int fd, struct stat *statbuf);
if (ModelPOSIX) {
     addToFunctionSummaryMap(
         "fstatat",
         Signature(ArgTypes{IntTy, ConstCharPtrRestrictTy,
                            StructStatPtrRestrictTy, IntTy},
                   RetType{IntTy}),
         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))));
 
     // DIR *opendir(const char *name);
     // FIXME: Improve for errno modeling.
     addToFunctionSummaryMap(
         "opendir", Signature(ArgTypes{ConstCharPtrTy}, RetType{DirPtrTy}),
         Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
addToFunctionSummaryMap(
         Signature(
             ArgTypes{IntTy, ConstCharPtrRestrictTy, CharPtrRestrictTy, SizeTy},
             RetType{Ssize_tTy}),
         Summary(NoEvalCall)
             .Case({ReturnValueCondition(LessThanOrEq, ArgNo(3)),
                    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),
                                       /*BufSize=*/ArgNo(3)))
             .ArgConstraint(
                 ArgumentCondition(3, WithinRange, Range(0, SizeMax))));
 
     // int renameat(int olddirfd, const char *oldpath, int newdirfd, const char
     // *newpath);
     addToFunctionSummaryMap(
         "renameat",
         Signature(ArgTypes{IntTy, ConstCharPtrTy, IntTy, ConstCharPtrTy},
                   RetType{IntTy}),
         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,
     //                char *restrict resolved_name);
     // FIXME: Improve for errno modeling.
     addToFunctionSummaryMap(
         "realpath",
         Signature(ArgTypes{ConstCharPtrRestrictTy, CharPtrRestrictTy},

clang/test/Analysis/std-c-library-functions-arg-constraints.c

 // Check the basic reporting/warning and the application of constraints.
 // RUN: %clang_analyze_cc1 %s \
 // 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 \
 // RUN:   -verify=report
 
 // Check the bugpath related to the reports.
 // RUN: %clang_analyze_cc1 %s \
 // 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 \
 // RUN:   -analyzer-output=text \
 // RUN:   -verify=bugpath
 
 #include "Inputs/std-c-library-functions-POSIX.h"
 

 int __buf_size_arg_constraint_concrete(const void *);
 void test_min_buf_size(void) {
   char buf[9];// bugpath-note{{'buf' initialized here}}
   __buf_size_arg_constraint_concrete(buf); // \
   // report-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-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");
+}