[clang] [analyzer] Model strchr/strrchr/memchr/strstr/strpbrk/strchrnul (PR #207267)

[Donát Nagy via cfe-commits]

================
@@ -2617,6 +2639,54 @@ void CStringChecker::evalStrsep(CheckerContext &C,
   C.addTransition(State);
 }
 
+void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
+                                      StringRef FnName,
+                                      bool CanReturnNull) const {
+  CurrentFunctionDescription = FnName;
+  ProgramStateRef State = C.getState();
+  const StackFrame *SF = C.getStackFrame();
+  SValBuilder &SVB = C.getSValBuilder();
+  ASTContext &Ctx = C.getASTContext();
+  const Expr *CE = Call.getOriginExpr();
+  assert(CE);
+
+  // These functions always return a pointer.
+  if (!CE->getType()->isPointerType())
+    return;
+
+  // The first argument must be non-null for all functions in this family.
+  SourceArgExpr Src = {{Call.getArgExpr(0), 0}};
+  SVal SrcVal = State->getSVal(Src.Expression, SF);
+  State = checkNonNull(C, State, Src, SrcVal);
+  if (!State)
+    return;
+
+  // NULL (no-match) branch.
+  if (CanReturnNull) {
+    ProgramStateRef NullState =
+        State->BindExpr(CE, SF, SVB.makeNullWithType(CE->getType()));
+    C.addTransition(NullState);
+  }
+
+  // Found branch: a pointer within the source; needs a Loc for the arithmetic.
+  std::optional<Loc> SrcLoc = SrcVal.getAs<Loc>();
+  if (!SrcLoc) {
+    SVal Result = SVB.conjureSymbolVal(Call, C.blockCount());
+    State = State->BindExpr(CE, SF, Result);
+    C.addTransition(State);
+    return;
+  }
+
+  // The result is: Src + SymOffset
----------------
NagyDonat wrote:

> The problem is that this `ElementRegion` can't fold into an offset, which basically disables bounds checks even if we had constraints on `SymOffset`.

This is probably true for `assumeInBounds`, but is not relevant for the more accurate bounds checking logic which is used in `security.ArrayBound` and will be spread to other bounds checkers (e.g. the `cstring` bounds checker) in the upcoming months after merging #202372.

> I've just checked and the Extent of `ElementRegion{Src, SymOffset}` would be `Unknown` regardless if `SymOffset` is constrained or not.

`security.ArrayBounds` does not query the extent of an `ElementRegion`, it queries the extent of the base region and can sum (potentially symbolic) offsets from multiple `ElementRegion` layers.

If you can easily add those constraints, I would be grateful if you added them. I suspect that `security.ArrayBound` will be able to use them (at least in some situations) and I will introduce several bounds checking improvements in the future that will improve these chances.


https://github.com/llvm/llvm-project/pull/207267