[analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

[martong]

Summary

Both EOF and the max value of unsigned char is platform dependent. In this
patch we try our best to deduce the value of EOF from the Preprocessor,
if we can't we fall back to -1.

[balazske]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

540

Debug message (to be removed)?

542

It would be good to have this function available generally to other checkers, the same functionality is needed in https://reviews.llvm.org/D72705 too.
It could work with any (specified) macro name, there are other special values in API calls. But there can be more difficult cases if the EOF (or other) is not a simple value but another macro or constructed from values some way. (The ULONG_MAX and similar can be get in the same way.)

[martong]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

540

Thanks, good catch!

542

Ok, I am gonna put a generic version of this under CheckerHelpers.h, so all checkers can use it.

[martong]

• Remove debug dump
• Add TryExpandAsInteger to CheckerHelpers.h

[martong]

• Remove PP declaration

[martong]

• EOFMacroIt -> MacroIt

[martong]

Harbormaster failed remote builds in B46321: Diff 244141!

This is actually true, I have a test that crashes! Finally, a case where remote builds are proved to be useful!

[martong]

• Fix crash in TryExpandAsInteger

[balazske]

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h

17

We do not need to include Preprocessor.h here (fwd declaration is enough).

70

Name should begin with lowercase letter?

[Szelethus]

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h

67–71

While I agree that we shouldn't have to reinvent the Preprocessor every time we need something macro related, I doubt that this will catch anything. I checker my system's standard library, and this is what I found:

#ifndef EOF
# define EOF (-1)
#endif

Lets go just one step further and cover the probably majority of the cases where the definition is in parentheses.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

518–523

Hah, would've never thought of doing this with a lambda. Me likey.

577

Huh, what's happening here exactly? Doesn't Range take 2 RangeInts as ctor arguments? What does {EOFv, EOFv}, {0, UCharMax} mean in this context?

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

115–118

This seems a bit clunky even for the Preprocessor -- how about

const auto *MacroII = PP.getIdentifierInfo(Macro);
if (!MacroII)
  return;
const MacroInfo *MI = PP.getMacroInfo(MacroII);
assert(MI);

clang/test/Analysis/std-c-library-functions-eof.c

25

So the test is about checking whether the analyzer correctly assigned -2 to y, right? Then let's check that too.

clang_analyzer_eval(y == 2);

[martong]

• Move include of Preprocessor.h to CheckerHelpers.cpp
• Try -> try
• Use PP.getIdentifierInfo
• Handle parens in tryExpandAsInteger

[martong]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

577

Range is a convenience function that creates a IntRangeVector from the two params. This way we can easily create a list of ranges from two ints. Range(-1, 255) results a list of ranges with one list: {{-1, 255}}.
Now, because EOF's value can be less than the minimum value of the next range we have to explicitly enlist the ranges.
E.g. if EOF is -2 then we will have a list of {-2,-2} and {0, 255}.

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

115–118

Ok, but we cannot assert on MI, because there may be cases when the macro is not defined in a TU. In that case we should just return with None.

[Szelethus]

LGTM! Very nice! I think you can commit as you please, granted you add core checkers to the test RUN: lines.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

319–327

I appreciated you sitting down with me with a piece of paper and a pen to explain what happens here, but for the less fortunate this code snippet should be decorated with some ASCII art.

I don't insist on you doing that within the scope of this patch, but if you did anyways, that would be great :)

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

138

TIL Despite the analyzer having a half-a-decade old checker option that was defaulted to a hexadecimal value, nobody wrote tests for it, and it took 4-5 years to unearth that due to the incorrect parsing of integers (it was set to decimal instead of auto detect) it never ever worked.

clang/test/Analysis/std-c-library-functions-eof.c

2–6

Always enable core checkers for pathsensitive analyses.

[Szelethus]

clang/test/Analysis/std-c-library-functions-eof.c

12

Maybe you could throw in a test where the definition isn't surrounded by parentheses?

[martong]

• Enable core checkers explicitly in test
• Add ASCII art to depict a range list example

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

319–327

Ok, I have added that below the existing comment of the enclosing if block.

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

138

Well, I hope AutoSenseRadix will just detect the correct base.

[NoQ]

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

115–118

What exactly happens when the macro is #undef-ined and redefined? We get the last redefinition that's valid at the end of the translation unit, right? Can we check whether there are multiple definitions and guard against that?

[Szelethus]

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

115–118

Ugh, now that you say it that is a valid concern. I had to deal with that back in the day: https://reviews.llvm.org/D52794?id=171962#inline-476352

[martong]

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

115–118

Solving this does not seem easy in my opinion. To handle #undefs we should build an infrastructure where summaries can reference callable objects. This is necessary, because in evalCall the value of EOF would depend on the souce location of the call expression of the function with the summary. Not impossible to solve, but certainly introduces complexity. Do you think that the redefinition of EOF is so common? I mean maybe it is too much hassle for a very rare edge case (?).

[martong]

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

115–118

The standard library (libc or libc++) should define EOF consitently in stdio.h.
Now, if the application redefines the value of EOF then the code could be broken, or at least it would not be compatible with libc. Consider the following code that is perfectly legit if we don't redefine EOF, but if we do:

#include <stdio.h>
#include <stdlib.h>
#define EOF -2 // Here be dragons !!!
int main(void)
{
    FILE* fp = fopen("test.txt", "r");
    int c;
    while ((c = fgetc(fp)) != EOF) { // BOOM: Infinite loop !!!
       putchar(c);
    }
    fclose(fp);
}

So, redefinition of EOF (or any standard macro) results in broken code.
And this is also a warning:

) clang eof-inf.c
eof-inf.c:3:9: warning: 'EOF' macro redefined [-Wmacro-redefined]
#define EOF -2 // Here be dragons !!!
        ^
/usr/include/x86_64-linux-gnu/bits/libio.h:66:10: note: previous definition is here
# define EOF (-1)
         ^
1 warning generated.

[balazske]

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

115–118

It should be possible to get the list of all macro definitions (of a macro). We can select the first item, or one that is inside a system header (by the SourceLocation).

[martong]

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

115–118

Actually, redefinition of a macro in a standard library is undefined behavior:

C++11:

17.6.4.3.1
[macro.names]
1) A translation unit that includes a standard library header shall not #define or #undef names declared in any standard library header.

and

[reserved.names]
1) The C ++ standard library reserves the following kinds of names:
— macros
— global names
— names with external linkage
2) If a program declares or defines a name in a context where it is reserved, other than as explicitly allowed by this Clause, its behavior is undefined.

Also, this is stated in C99 (7.1).
Plus[[ https://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html | from libc manual ]]:

The names of all library types, macros, variables and functions that come from the ISO C standard are reserved unconditionally; your program may not redefine these names. All other library names are reserved if your program explicitly includes the header file that defines or declares them.

Thus, I don't think we should come up with solutions to handle already broken code (i.e. programs with undefined behavior).

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h

 //== CheckerHelpers.h - Helper functions for checkers ------------*- C++ -*--=//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines CheckerVisitor.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
 #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
 
 #include "clang/AST/Stmt.h"
+#include "llvm/ADT/Optional.h"
 #include <tuple>
 
 namespace clang {
 
 class Expr;
 class VarDecl;
 class QualType;
 class AttributedType;
+class Preprocessor;
 
 namespace ento {
 
 bool containsMacro(const Stmt *S);
 bool containsEnum(const Stmt *S);
 bool containsStaticLocal(const Stmt *S);
 bool containsBuiltinOffsetOf(const Stmt *S);
 template <class T> bool containsStmt(const Stmt *S) {
enum class Nullability : char {
   Nullable,
   Unspecified,
   Nonnull
 };
 
 /// Get nullability annotation for a given type.
 Nullability getNullabilityAnnotation(QualType Type);
 
-} // end GR namespace
+/// Try to parse the value of a defined preprocessor macro. We can only parse
+/// simple expressions that consist of an optional minus sign token and then a
+/// token for an integer. If we cannot parse the value then None is returned.
+llvm::Optional<int> tryExpandAsInteger(StringRef Macro, const Preprocessor &PP);
 
-} // end clang namespace
+} // namespace ento
+
+} // namespace clang
 
 #endif

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 
 using namespace clang;
 using namespace clang::ento;
 
 namespace {
 class StdLibraryFunctionsChecker : public Checker<check::PostCall, eval::Call> {
   /// Below is a series of typedefs necessary to define function specs.
   /// We avoid nesting types here because each additional qualifier
public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
 
 private:
   Optional<Summary> findFunctionSummary(const FunctionDecl *FD,
                                         const CallExpr *CE,
                                         CheckerContext &C) const;
 
-  void initFunctionSummaries(BasicValueFactory &BVF) const;
+  void initFunctionSummaries(CheckerContext &C) const;
 };
 } // end of anonymous namespace
 
 ProgramStateRef StdLibraryFunctionsChecker::ValueRange::applyAsOutOfRange(
     ProgramStateRef State, const CallEvent &Call,
     const Summary &Summary) const {
 
   ProgramStateManager &Mgr = State->getStateManager();
ProgramStateRef StdLibraryFunctionsChecker::ValueRange::applyAsWithinRange(
   BasicValueFactory &BVF = SVB.getBasicValueFactory();
   ConstraintManager &CM = Mgr.getConstraintManager();
   QualType T = getArgType(Summary, getArgNo());
   SVal V = getArgSVal(Call, getArgNo());
 
   // "WithinRange R" is treated as "outside [T_MIN, T_MAX] \ R".
   // We cut off [T_MIN, min(R) - 1] and [max(R) + 1, T_MAX] if necessary,
   // and then cut away all holes in R one by one.
+  //
+  // E.g. consider a range list R as [A, B] and [C, D]
+  // -------+--------+------------------+------------+----------->
+  //        A        B                  C            D
+  // Then we assume that the value is not in [-inf, A - 1],
+  // then not in [D + 1, +inf], then not in [B + 1, C - 1]
   if (auto N = V.getAs<NonLoc>()) {
     const IntRangeVector &R = getRanges();
     size_t E = R.size();
 
     const llvm::APSInt &MinusInf = BVF.getMinValue(T);
     const llvm::APSInt &PlusInf = BVF.getMaxValue(T);
 
     const llvm::APSInt &Left = BVF.getValue(R[0].first - 1ULL, T);
     if (Left != PlusInf) {
       assert(MinusInf <= Left);
       State = CM.assumeInclusiveRange(State, *N, MinusInf, Left, false);
       if (!State)
         return nullptr;
     }
 
     const llvm::APSInt &Right = BVF.getValue(R[E - 1].second + 1ULL, T);
     if (Right != MinusInf) {
       assert(Right <= PlusInf);
       State = CM.assumeInclusiveRange(State, *N, Right, PlusInf, false);
       if (!State)
         return nullptr;
     }
 
     for (size_t I = 1; I != E; ++I) {
       const llvm::APSInt &Min = BVF.getValue(R[I - 1].second + 1ULL, T);
       const llvm::APSInt &Max = BVF.getValue(R[I].first - 1ULL, T);
-      assert(Min <= Max);
+      if (Min <= Max) {
         State = CM.assumeInclusiveRange(State, *N, Min, Max, false);
         if (!State)
           return nullptr;
       }
     }
+  }
 
   return State;
 }
 
 ProgramStateRef
 StdLibraryFunctionsChecker::ValueRange::applyAsComparesToArgument(
     ProgramStateRef State, const CallEvent &Call,
     const Summary &Summary) const {
StdLibraryFunctionsChecker::findFunctionSummary(const FunctionDecl *FD,
                                                 CheckerContext &C) const {
   // Note: we cannot always obtain FD from CE
   // (eg. virtual call, or call by pointer).
   assert(CE);
 
   if (!FD)
     return None;
 
-  SValBuilder &SVB = C.getSValBuilder();
-  BasicValueFactory &BVF = SVB.getBasicValueFactory();
-  initFunctionSummaries(BVF);
+  initFunctionSummaries(C);
 
   IdentifierInfo *II = FD->getIdentifier();
   if (!II)
     return None;
   StringRef Name = II->getName();
   if (Name.empty() || !C.isCLibraryFunction(FD, Name))
     return None;
 
StdLibraryFunctionsChecker::findFunctionSummary(const FunctionDecl *FD,
   for (const Summary &Spec : SpecVariants)
     if (Spec.matchesCall(CE))
       return Spec;
 
   return None;
 }
 
 void StdLibraryFunctionsChecker::initFunctionSummaries(
-    BasicValueFactory &BVF) const {
+    CheckerContext &C) const {
   if (!FunctionSummaryMap.empty())
     return;
 
-  ASTContext &ACtx = BVF.getContext();
+  SValBuilder &SVB = C.getSValBuilder();
+  BasicValueFactory &BVF = SVB.getBasicValueFactory();
+  const ASTContext &ACtx = BVF.getContext();
 
   // These types are useful for writing specifications quickly,
   // New specifications should probably introduce more types.
   // Some types are hard to obtain from the AST, eg. "ssize_t".
   // In such cases it should be possible to provide multiple variants
   // of function summary for common cases (eg. ssize_t could be int or long
   // or long long, so three summary variants would be enough).
   // Of course, function variants are also useful for C++ overloads.
-  QualType Irrelevant; // A placeholder, whenever we do not care about the type.
-  QualType IntTy = ACtx.IntTy;
-  QualType LongTy = ACtx.LongTy;
-  QualType LongLongTy = ACtx.LongLongTy;
-  QualType SizeTy = ACtx.getSizeType();
-
-  RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
-  RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
-  RangeInt LongLongMax = BVF.getMaxValue(LongLongTy).getLimitedValue();
+  const QualType
+      Irrelevant; // A placeholder, whenever we do not care about the type.
+  const QualType IntTy = ACtx.IntTy;
+  const QualType LongTy = ACtx.LongTy;
+  const QualType LongLongTy = ACtx.LongLongTy;
+  const QualType SizeTy = ACtx.getSizeType();
+
+  const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
+  const RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
+  const RangeInt LongLongMax = BVF.getMaxValue(LongLongTy).getLimitedValue();
+
+  const RangeInt UCharMax =
+      BVF.getMaxValue(ACtx.UnsignedCharTy).getLimitedValue();
+
+  // 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 llvm::Optional<int> OptInt =
+            tryExpandAsInteger("EOF", C.getPreprocessor()))
+      return *OptInt;
+    return -1;
+  }();
 
   // We are finally ready to define specifications for all supported functions.
   //
   // The signature needs to have the correct number of arguments.
   // However, we insert `Irrelevant' when the type is insignificant.
   //
   // Argument ranges should always cover all variants. If return value
   // is completely unknown, omit it from the respective range set.
   //
   // All types in the spec need to be canonical.
   //
   // Every item in the list of range sets represents a particular
   // execution path the analyzer would need to explore once
   // the call is modeled - a new program state is constructed
   // for every range set, and each range line in the range set
   // corresponds to a specific constraint within this state.
   //
   // Upon comparing to another argument, the other argument is casted
   // to the current argument's type. This avoids proper promotion but
   // seems useful. For example, read() receives size_t argument,
   // and its return value, which is of type ssize_t, cannot be greater
   // than this argument. If we made a promotion, and the size argument
   // is equal to, say, 10, then we'd impose a range of [0, 10] on the
   // return value, however the correct range is [-1, 10].
   //
   // Please update the list of functions in the header after editing!
   //
void StdLibraryFunctionsChecker::initFunctionSummaries(
   };
   auto IsLessThan = [](ArgNo ArgN) { return IntRangeVector{{BO_LE, ArgN}}; };
 
   using RetType = QualType;
 
   // Templates for summaries that are reused by many functions.
   auto Getc = [&]() {
     return Summary(ArgTypes{Irrelevant}, RetType{IntTy}, NoEvalCall)
-        .Case({ReturnValueCondition(WithinRange, Range(-1, 255))});
+        .Case(
+            {ReturnValueCondition(WithinRange, {{EOFv, EOFv}, {0, UCharMax}})});
   };
   auto Read = [&](RetType R, RangeInt Max) {
     return Summary(ArgTypes{Irrelevant, Irrelevant, SizeTy}, RetType{R},
                    NoEvalCall)
         .Case({ReturnValueCondition(ComparesToArgument, IsLessThan(2)),
                ReturnValueCondition(WithinRange, Range(-1, Max))});
   };
   auto Fread = [&]() {
FunctionSummaryMap = {
                   // Boils down to isupper() or islower() or isdigit().
                   .Case(
                       {ArgumentCondition(0U, WithinRange,
                                          {{'0', '9'}, {'A', 'Z'}, {'a', 'z'}}),
                        ReturnValueCondition(OutOfRange, SingleValue(0))})
                   // The locale-specific range.
                   // No post-condition. We are completely unaware of
                   // locale-specific return values.
-                  .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
-                  .Case({ArgumentCondition(
-                             0U, OutOfRange,
-                             {{'0', '9'}, {'A', 'Z'}, {'a', 'z'}, {128, 255}}),
+                  .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
+                  .Case({ArgumentCondition(0U, OutOfRange,
+                                           {{'0', '9'},
+                                            {'A', 'Z'},
+                                            {'a', 'z'},
+                                            {128, UCharMax}}),
                          ReturnValueCondition(WithinRange, SingleValue(0))})},
       },
       {
           "isalpha",
           Summaries{
               Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
                   .Case({ArgumentCondition(0U, WithinRange,
                                            {{'A', 'Z'}, {'a', 'z'}}),
                          ReturnValueCondition(OutOfRange, SingleValue(0))})
                   // The locale-specific range.
-                  .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
-                  .Case(
-                      {ArgumentCondition(0U, OutOfRange,
-                                         {{'A', 'Z'}, {'a', 'z'}, {128, 255}}),
+                  .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
+                  .Case({ArgumentCondition(
+                             0U, OutOfRange,
+                             {{'A', 'Z'}, {'a', 'z'}, {128, UCharMax}}),
                          ReturnValueCondition(WithinRange, SingleValue(0))})},
       },
       {
           "isascii",
           Summaries{
               Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
                   .Case({ArgumentCondition(0U, WithinRange, Range(0, 127)),
                          ReturnValueCondition(OutOfRange, SingleValue(0))})
                   .Case({ArgumentCondition(0U, OutOfRange, Range(0, 127)),
FunctionSummaryMap = {
                   // Is certainly lowercase.
                   .Case({ArgumentCondition(0U, WithinRange, Range('a', 'z')),
                          ReturnValueCondition(OutOfRange, SingleValue(0))})
                   // Is ascii but not lowercase.
                   .Case({ArgumentCondition(0U, WithinRange, Range(0, 127)),
                          ArgumentCondition(0U, OutOfRange, Range('a', 'z')),
                          ReturnValueCondition(WithinRange, SingleValue(0))})
                   // The locale-specific range.
-                  .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
+                  .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
                   // Is not an unsigned char.
-                  .Case({ArgumentCondition(0U, OutOfRange, Range(0, 255)),
+                  .Case({ArgumentCondition(0U, OutOfRange, Range(0, UCharMax)),
                          ReturnValueCondition(WithinRange, SingleValue(0))})},
       },
       {
           "isprint",
           Summaries{
               Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
                   .Case({ArgumentCondition(0U, WithinRange, Range(32, 126)),
                          ReturnValueCondition(OutOfRange, SingleValue(0))})
FunctionSummaryMap = {
           "isspace",
           Summaries{
               Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
                   // Space, '\f', '\n', '\r', '\t', '\v'.
                   .Case({ArgumentCondition(0U, WithinRange,
                                            {{9, 13}, {' ', ' '}}),
                          ReturnValueCondition(OutOfRange, SingleValue(0))})
                   // The locale-specific range.
-                  .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
-                  .Case({ArgumentCondition(0U, OutOfRange,
-                                           {{9, 13}, {' ', ' '}, {128, 255}}),
+                  .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
+                  .Case({ArgumentCondition(
+                             0U, OutOfRange,
+                             {{9, 13}, {' ', ' '}, {128, UCharMax}}),
                          ReturnValueCondition(WithinRange, SingleValue(0))})},
       },
       {
           "isupper",
           Summaries{
               Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
                   // Is certainly uppercase.
                   .Case({ArgumentCondition(0U, WithinRange, Range('A', 'Z')),
                          ReturnValueCondition(OutOfRange, SingleValue(0))})
                   // The locale-specific range.
-                  .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
+                  .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
                   // Other.
                   .Case({ArgumentCondition(0U, OutOfRange,
-                                           {{'A', 'Z'}, {128, 255}}),
+                                           {{'A', 'Z'}, {128, UCharMax}}),
                          ReturnValueCondition(WithinRange, SingleValue(0))})},
       },
       {
           "isxdigit",
           Summaries{
               Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure)
                   .Case(
                       {ArgumentCondition(0U, WithinRange,
                                          {{'0', '9'}, {'A', 'F'}, {'a', 'f'}}),
                        ReturnValueCondition(OutOfRange, SingleValue(0))})
                   .Case(
                       {ArgumentCondition(0U, OutOfRange,
                                          {{'0', '9'}, {'A', 'F'}, {'a', 'f'}}),
                        ReturnValueCondition(WithinRange, SingleValue(0))})},
       },
 
       // The getc() family of functions that returns either a char or an EOF.
       {"getc", Summaries{Getc()}},
       {"fgetc", Summaries{Getc()}},
-      {"getchar", Summaries{Summary(ArgTypes{}, RetType{IntTy}, NoEvalCall)
-                                .Case({ReturnValueCondition(WithinRange,
-                                                            Range(-1, 255))})}},
+      {"getchar",
+       Summaries{Summary(ArgTypes{}, RetType{IntTy}, NoEvalCall)
+                     .Case({ReturnValueCondition(
+                         WithinRange, {{EOFv, EOFv}, {0, UCharMax}})})}},
 
       // read()-like functions that never return more than buffer size.
       // We are not sure how ssize_t is defined on every platform, so we
       // provide three variants that should cover common cases.
       {"read", Summaries{Read(IntTy, IntMax), Read(LongTy, LongMax),
                          Read(LongLongTy, LongLongMax)}},
       {"write", Summaries{Read(IntTy, IntMax), Read(LongTy, LongMax),
                           Read(LongLongTy, LongLongMax)}},

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

 //===---- CheckerHelpers.cpp - Helper functions for checkers ----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines several static functions for use in checkers.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
+#include "clang/Lex/Preprocessor.h"
 
 namespace clang {
 
 namespace ento {
 
 // Recursively find any substatements containing macros
 bool containsMacro(const Stmt *S) {
   if (S->getBeginLoc().isMacroID())
if (!AttrType)
     return Nullability::Unspecified;
   if (AttrType->getAttrKind() == attr::TypeNullable)
     return Nullability::Nullable;
   else if (AttrType->getAttrKind() == attr::TypeNonNull)
     return Nullability::Nonnull;
   return Nullability::Unspecified;
 }
 
+llvm::Optional<int> tryExpandAsInteger(StringRef Macro,
+                                       const Preprocessor &PP) {
+  const auto *MacroII = PP.getIdentifierInfo(Macro);
+  if (!MacroII)
+    return llvm::None;
+  const MacroInfo *MI = PP.getMacroInfo(MacroII);
+  if (!MI)
+    return llvm::None;
+
+  // Filter out parens.
+  std::vector<Token> FilteredTokens;
+  FilteredTokens.reserve(MI->tokens().size());
+  for (auto &T : MI->tokens())
+    if (!T.isOneOf(tok::l_paren, tok::r_paren))
+      FilteredTokens.push_back(T);
+
+  if (FilteredTokens.size() > 2)
+    return llvm::None;
+
+  // Parse an integer at the end of the macro definition.
+  const Token &T = FilteredTokens.back();
+  if (!T.isLiteral())
+    return llvm::None;
+  StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
+  llvm::APInt IntValue;
+  constexpr unsigned AutoSenseRadix = 0;
+  if (ValueStr.getAsInteger(AutoSenseRadix, IntValue))
+    return llvm::None;
+
+  // Parse an optional minus sign.
+  if (FilteredTokens.size() == 2) {
+    if (FilteredTokens.front().is(tok::minus))
+      IntValue = -IntValue;
+    else
+      return llvm::None;
+  }
+
+  return IntValue.getSExtValue();
+}
 
-} // end namespace ento
-} // end namespace clang
+} // namespace ento
+} // namespace clang

clang/test/Analysis/std-c-library-functions-eof.c

+// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=core,apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=core,apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=core,apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple thumbv7-a15-linux -analyzer-checker=core,apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+
+void clang_analyzer_eval(int);
+
+typedef struct FILE FILE;
+// Unorthodox EOF value.
+#define EOF (-2)
+
+int getc(FILE *);
+void test_getc(FILE *fp) {
+
+  int x;
+  while ((x = getc(fp)) != EOF) {
+    clang_analyzer_eval(x > 255); // expected-warning{{FALSE}}
+    clang_analyzer_eval(x >= 0); // expected-warning{{TRUE}}
+  }
+
+  int y = getc(fp);
+  if (y < 0) {
+    clang_analyzer_eval(y == -2); // expected-warning{{TRUE}}
+  }
+}