[analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API

[steakhal]

Summary

CStringChecker is a huge beast.

My effort in improving the analyzer regarding taint analysis is humbled by multiple factors.
I wanted to extend the diagnostics of the CStringChecker with taintedness.

In the long run, the diagnostic emitting parts of the GenericTaintChecker would be migrated to multiple checkers, leaving it's responsibility only to *model* taint propagation.
Eg. the GenericTaintChecker::checkTaintedBufferSize functionality will be mostly part of the CStringChecker.

This plan requires the CStringChecker to be refactored to support a more flexible reporting mechanism.

This patch does only refactorings, such:

• eliminates always false parameters (like WarnAboutSize)
• reduces the number of parameters
• makes strong types differentiating *source* and *destination* buffers (same with size expressions)
• binds the argument expression and the index, making diagnostics accurate and easy to emit
• removes a bunch of default parameters to make it more readable
• remove random const char * warning message parameters, making clear where and what is going to be emitted

Note that:

• CheckBufferAccess now checks *only* one buffer, this removed about 100 LOC code duplication
• not every function was refactored to use the /new/ strongly typed API, since the CString related functions are really closely coupled monolithic beasts, I will refactor them separately
• all tests are preserved and passing; only *the message changed at some places*. In my opinion, these messages are holding the same information.

I would also highlight that this refactoring caught a bug in clang/test/Analysis/string.c:454 where the diagnostic did not reflect reality. This catch backs my effort on simplifying this monolithic CStringChecker.

[steakhal]

Upload the right diff.

[NoQ]

This is fantastic, i love it!

all tests are preserved and passing; only *the message changed at some places*. In my opinion, these messages are holding the same information.

You make it sound like an accident; i encourage you to have a closer look to make sure that there are no other effects than the ones observed on tests.

clang/test/Analysis/string.c

454

Nice catch indeed!~

[steakhal]

In D74806#1882300, @NoQ wrote:

This is fantastic, i love it!

all tests are preserved and passing; only *the message changed at some places*. In my opinion, these messages are holding the same information.

You make it sound like an accident; i encourage you to have a closer look to make sure that there are no other effects than the ones observed on tests.

How should I look for differences/regressions without extending and validating systematically the test cases we have for this checker? That seems to be a huge work since bstring.c has 500+ and the string.c has 1600+ lines.

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

37

I'm not marking it const in this patch since the 496th line swaps two instances of this class, so it cannot be done without refactoring that function.

494–498

It is really interesting that it swaps the arguments in some sense.

I'm pretty sure that now if a diagnostic is emitted after this swap, the argument indexes used to construct the diagnostic message would mismatch. I will dig into this later, probably refactor the whole function to make the intentions clear. What do you think? @NoQ

1580–1587

This part is really interesting.

Using strong types highlighted this particular code. Since SizeArgExpr is conceptually different than a SourceArgExpr this code did not compile without additional effort.

Previously it passed the size argument if it were given, but if the function were unbounded than it passed the source expression where size was expected, which does not make any sense.

Since I plan to refactor the whole cstring related functions, not just the memset, memcpy etc. I tried to leave it as it were, to preserve the current (broken) semantics of the function.

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
+struct AnyArgExpr {
+  // FIXME: Remove constructor in C++17 to turn it into an aggregate.
+  AnyArgExpr(const Expr *Expression, unsigned ArgumentIndex)
+      : Expression{Expression}, ArgumentIndex{ArgumentIndex} {}
+  const Expr *Expression;
+  unsigned ArgumentIndex;
+};
+
+struct SourceArgExpr : AnyArgExpr {
+  using AnyArgExpr::AnyArgExpr; // FIXME: Remove using in C++17.
+};
+
+struct DestinationArgExpr : AnyArgExpr {
+  using AnyArgExpr::AnyArgExpr; // FIXME: Same.
+};
+
+struct SizeArgExpr : AnyArgExpr {
+  using AnyArgExpr::AnyArgExpr; // FIXME: Same.
+};
+
+using ErrorMessage = SmallString<128>;
+enum class AccessKind { write, read };
+
+static ErrorMessage createOutOfBoundErrorMsg(StringRef FunctionDescription,
+                                             AccessKind Access) {
+  ErrorMessage Message;
+  llvm::raw_svector_ostream Os(Message);
+
+  // Function classification like: Memory copy function
+  Os << toUppercase(FunctionDescription.front())
+     << &FunctionDescription.data()[1];
+
+  if (Access == AccessKind::write) {
+    Os << " overflows the destination buffer";
+  } else { // read access
+    Os << " accesses out-of-bound array element";
+  }
+
+  return Message;
+}
+
 enum class ConcatFnKind { none = 0, strcat = 1, strlcat = 2 };
 class CStringChecker : public Checker< eval::Call,
                                          check::PreStmt<DeclStmt>,
                                          check::LiveSymbols,
                                          check::DeadSymbols,
                                          check::RegionChanges
                                          > {
   mutable std::unique_ptr<BugType> BT_Null, BT_Bounds, BT_Overlap,
CallDescription StdCopy{{"std", "copy"}, 3},
       StdCopyBackward{{"std", "copy_backward"}, 3};
 
   FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const;
   void evalMemcpy(CheckerContext &C, const CallExpr *CE) const;
   void evalMempcpy(CheckerContext &C, const CallExpr *CE) const;
   void evalMemmove(CheckerContext &C, const CallExpr *CE) const;
   void evalBcopy(CheckerContext &C, const CallExpr *CE) const;
   void evalCopyCommon(CheckerContext &C, const CallExpr *CE,
-                      ProgramStateRef state,
-                      const Expr *Size,
-                      const Expr *Source,
-                      const Expr *Dest,
-                      bool Restricted = false,
-                      bool IsMempcpy = false) const;
+                      ProgramStateRef state, SizeArgExpr Size,
+                      DestinationArgExpr Dest, SourceArgExpr Source,
+                      bool Restricted, bool IsMempcpy) const;
 
   void evalMemcmp(CheckerContext &C, const CallExpr *CE) const;
 
   void evalstrLength(CheckerContext &C, const CallExpr *CE) const;
   void evalstrnLength(CheckerContext &C, const CallExpr *CE) const;
   void evalstrLengthCommon(CheckerContext &C,
                            const CallExpr *CE,
                            bool IsStrnlen = false) const;
public:
   static bool SummarizeRegion(raw_ostream &os, ASTContext &Ctx,
                               const MemRegion *MR);
 
   static bool memsetAux(const Expr *DstBuffer, SVal CharE,
                         const Expr *Size, CheckerContext &C,
                         ProgramStateRef &State);
 
   // Re-usable checks
-  ProgramStateRef checkNonNull(CheckerContext &C,
-                                   ProgramStateRef state,
-                                   const Expr *S,
-                                   SVal l,
-                                   unsigned IdxOfArg) const;
-  ProgramStateRef CheckLocation(CheckerContext &C,
-                                    ProgramStateRef state,
-                                    const Expr *S,
-                                    SVal l,
-                                    const char *message = nullptr) const;
-  ProgramStateRef CheckBufferAccess(CheckerContext &C,
-                                        ProgramStateRef state,
-                                        const Expr *Size,
-                                        const Expr *FirstBuf,
-                                        const Expr *SecondBuf,
-                                        const char *firstMessage = nullptr,
-                                        const char *secondMessage = nullptr,
-                                        bool WarnAboutSize = false) const;
-
-  ProgramStateRef CheckBufferAccess(CheckerContext &C,
-                                        ProgramStateRef state,
-                                        const Expr *Size,
-                                        const Expr *Buf,
-                                        const char *message = nullptr,
-                                        bool WarnAboutSize = false) const {
-    // This is a convenience overload.
-    return CheckBufferAccess(C, state, Size, Buf, nullptr, message, nullptr,
-                             WarnAboutSize);
-  }
-  ProgramStateRef CheckOverlap(CheckerContext &C,
-                                   ProgramStateRef state,
-                                   const Expr *Size,
-                                   const Expr *First,
-                                   const Expr *Second) const;
+  ProgramStateRef checkNonNull(CheckerContext &C, ProgramStateRef State,
+                               AnyArgExpr Arg, SVal l) const;
+  ProgramStateRef CheckLocation(CheckerContext &C, ProgramStateRef state,
+                                AnyArgExpr Buffer, SVal Element,
+                                AccessKind Access) const;
+  ProgramStateRef CheckBufferAccess(CheckerContext &C, ProgramStateRef State,
+                                    AnyArgExpr Buffer, SizeArgExpr Size,
+                                    AccessKind Access) const;
+  ProgramStateRef CheckOverlap(CheckerContext &C, ProgramStateRef state,
+                               SizeArgExpr Size, AnyArgExpr First,
+                               AnyArgExpr Second) const;
   void emitOverlapBug(CheckerContext &C,
                       ProgramStateRef state,
                       const Stmt *First,
                       const Stmt *Second) const;
 
   void emitNullArgBug(CheckerContext &C, ProgramStateRef State, const Stmt *S,
                       StringRef WarningMsg) const;
   void emitOutOfBoundsBug(CheckerContext &C, ProgramStateRef State,
if (!val)
     return std::pair<ProgramStateRef , ProgramStateRef >(state, state);
 
   SValBuilder &svalBuilder = C.getSValBuilder();
   DefinedOrUnknownSVal zero = svalBuilder.makeZeroVal(Ty);
   return state->assume(svalBuilder.evalEQ(state, *val, zero));
 }
 
 ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
-                                            ProgramStateRef state,
-                                            const Expr *S, SVal l,
-                                            unsigned IdxOfArg) const {
+                                             ProgramStateRef State,
+                                             AnyArgExpr Arg, SVal l) const {
   // If a previous check has failed, propagate the failure.
-  if (!state)
+  if (!State)
     return nullptr;
 
   ProgramStateRef stateNull, stateNonNull;
-  std::tie(stateNull, stateNonNull) = assumeZero(C, state, l, S->getType());
+  std::tie(stateNull, stateNonNull) =
+      assumeZero(C, State, l, Arg.Expression->getType());
 
   if (stateNull && !stateNonNull) {
     if (Filter.CheckCStringNullArg) {
       SmallString<80> buf;
       llvm::raw_svector_ostream OS(buf);
       assert(CurrentFunctionDescription);
-      OS << "Null pointer passed as " << IdxOfArg
-         << llvm::getOrdinalSuffix(IdxOfArg) << " argument to "
+      OS << "Null pointer passed as " << (Arg.ArgumentIndex + 1)
+         << llvm::getOrdinalSuffix(Arg.ArgumentIndex + 1) << " argument to "
          << CurrentFunctionDescription;
 
-      emitNullArgBug(C, stateNull, S, OS.str());
+      emitNullArgBug(C, stateNull, Arg.Expression, OS.str());
     }
     return nullptr;
   }
 
   // From here on, assume that the value is non-null.
   assert(stateNonNull);
   return stateNonNull;
 }
 
 // FIXME: This was originally copied from ArrayBoundChecker.cpp. Refactor?
 ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
                                               ProgramStateRef state,
-                                             const Expr *S, SVal l,
-                                             const char *warningMsg) const {
+                                              AnyArgExpr Buffer, SVal Element,
+                                              AccessKind Access) const {
+
   // If a previous check has failed, propagate the failure.
   if (!state)
     return nullptr;
 
   // Check for out of bound array element access.
-  const MemRegion *R = l.getAsRegion();
+  const MemRegion *R = Element.getAsRegion();
   if (!R)
     return state;
 
-  const ElementRegion *ER = dyn_cast<ElementRegion>(R);
+  const auto *ER = dyn_cast<ElementRegion>(R);
   if (!ER)
     return state;
 
   if (ER->getValueType() != C.getASTContext().CharTy)
     return state;
 
   // Get the size of the array.
-  const SubRegion *superReg = cast<SubRegion>(ER->getSuperRegion());
+  const auto *superReg = cast<SubRegion>(ER->getSuperRegion());
   DefinedOrUnknownSVal Size =
       getDynamicSize(state, superReg, C.getSValBuilder());
 
   // Get the index of the accessed element.
   DefinedOrUnknownSVal Idx = ER->getIndex().castAs<DefinedOrUnknownSVal>();
 
   ProgramStateRef StInBound = state->assumeInBound(Idx, Size, true);
   ProgramStateRef StOutBound = state->assumeInBound(Idx, Size, false);
   if (StOutBound && !StInBound) {
     // These checks are either enabled by the CString out-of-bounds checker
     // explicitly or implicitly by the Malloc checker.
     // In the latter case we only do modeling but do not emit warning.
     if (!Filter.CheckCStringOutOfBounds)
       return nullptr;
-    // Emit a bug report.
-    if (warningMsg) {
-      emitOutOfBoundsBug(C, StOutBound, S, warningMsg);
-    } else {
-      assert(CurrentFunctionDescription);
-      assert(CurrentFunctionDescription[0] != '\0');
 
-      SmallString<80> buf;
-      llvm::raw_svector_ostream os(buf);
-      os << toUppercase(CurrentFunctionDescription[0])
-         << &CurrentFunctionDescription[1]
-         << " accesses out-of-bound array element";
-      emitOutOfBoundsBug(C, StOutBound, S, os.str());
-    }
+    // Emit a bug report.
+    ErrorMessage Message =
+        createOutOfBoundErrorMsg(CurrentFunctionDescription, Access);
+    emitOutOfBoundsBug(C, StOutBound, Buffer.Expression, Message);
     return nullptr;
   }
 
   // Array bound check succeeded.  From this point forward the array bound
   // should always succeed.
   return StInBound;
 }
 
 ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C,
-                                                 ProgramStateRef state,
-                                                 const Expr *Size,
-                                                 const Expr *FirstBuf,
-                                                 const Expr *SecondBuf,
-                                                 const char *firstMessage,
-                                                 const char *secondMessage,
-                                                 bool WarnAboutSize) const {
+                                                  ProgramStateRef State,
+                                                  AnyArgExpr Buffer,
+                                                  SizeArgExpr Size,
+                                                  AccessKind Access) const {
   // If a previous check has failed, propagate the failure.
-  if (!state)
+  if (!State)
     return nullptr;
 
   SValBuilder &svalBuilder = C.getSValBuilder();
   ASTContext &Ctx = svalBuilder.getContext();
-  const LocationContext *LCtx = C.getLocationContext();
 
-  QualType sizeTy = Size->getType();
+  QualType SizeTy = Size.Expression->getType();
   QualType PtrTy = Ctx.getPointerType(Ctx.CharTy);
 
   // Check that the first buffer is non-null.
-  SVal BufVal = C.getSVal(FirstBuf);
-  state = checkNonNull(C, state, FirstBuf, BufVal, 1);
-  if (!state)
+  SVal BufVal = C.getSVal(Buffer.Expression);
+  State = checkNonNull(C, State, Buffer, BufVal);
+  if (!State)
     return nullptr;
 
   // If out-of-bounds checking is turned off, skip the rest.
   if (!Filter.CheckCStringOutOfBounds)
-    return state;
+    return State;
 
   // Get the access length and make sure it is known.
   // FIXME: This assumes the caller has already checked that the access length
   // is positive. And that it's unsigned.
-  SVal LengthVal = C.getSVal(Size);
+  SVal LengthVal = C.getSVal(Size.Expression);
   Optional<NonLoc> Length = LengthVal.getAs<NonLoc>();
   if (!Length)
-    return state;
+    return State;
 
   // Compute the offset of the last element to be accessed: size-1.
-  NonLoc One = svalBuilder.makeIntVal(1, sizeTy).castAs<NonLoc>();
-  SVal Offset = svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy);
+  NonLoc One = svalBuilder.makeIntVal(1, SizeTy).castAs<NonLoc>();
+  SVal Offset = svalBuilder.evalBinOpNN(State, BO_Sub, *Length, One, SizeTy);
   if (Offset.isUnknown())
     return nullptr;
   NonLoc LastOffset = Offset.castAs<NonLoc>();
 
   // Check that the first buffer is sufficiently long.
-  SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType());
+  SVal BufStart =
+      svalBuilder.evalCast(BufVal, PtrTy, Buffer.Expression->getType());
   if (Optional<Loc> BufLoc = BufStart.getAs<Loc>()) {
-    const Expr *warningExpr = (WarnAboutSize ? Size : FirstBuf);
 
-    SVal BufEnd = svalBuilder.evalBinOpLN(state, BO_Add, *BufLoc,
-                                          LastOffset, PtrTy);
-    state = CheckLocation(C, state, warningExpr, BufEnd, firstMessage);
+    SVal BufEnd =
+        svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
 
-    // If the buffer isn't large enough, abort.
-    if (!state)
-      return nullptr;
-  }
+    State = CheckLocation(C, State, Buffer, BufEnd, Access);
 
-  // If there's a second buffer, check it as well.
-  if (SecondBuf) {
-    BufVal = state->getSVal(SecondBuf, LCtx);
-    state = checkNonNull(C, state, SecondBuf, BufVal, 2);
-    if (!state)
+    // If the buffer isn't large enough, abort.
+    if (!State)
       return nullptr;
-
-    BufStart = svalBuilder.evalCast(BufVal, PtrTy, SecondBuf->getType());
-    if (Optional<Loc> BufLoc = BufStart.getAs<Loc>()) {
-      const Expr *warningExpr = (WarnAboutSize ? Size : SecondBuf);
-
-      SVal BufEnd = svalBuilder.evalBinOpLN(state, BO_Add, *BufLoc,
-                                            LastOffset, PtrTy);
-      state = CheckLocation(C, state, warningExpr, BufEnd, secondMessage);
-    }
   }
 
   // Large enough or not, return this state!
-  return state;
+  return State;
 }
 
 ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C,
                                              ProgramStateRef state,
-                                            const Expr *Size,
-                                            const Expr *First,
-                                            const Expr *Second) const {
+                                             SizeArgExpr Size, AnyArgExpr First,
+                                             AnyArgExpr Second) const {
   if (!Filter.CheckCStringBufferOverlap)
     return state;
 
   // Do a simple check for overlap: if the two arguments are from the same
   // buffer, see if the end of the first is greater than the start of the second
   // or vice versa.
 
   // If a previous check has failed, propagate the failure.
   if (!state)
     return nullptr;
 
   ProgramStateRef stateTrue, stateFalse;
 
   // Get the buffer values and make sure they're known locations.
   const LocationContext *LCtx = C.getLocationContext();
-  SVal firstVal = state->getSVal(First, LCtx);
-  SVal secondVal = state->getSVal(Second, LCtx);
+  SVal firstVal = state->getSVal(First.Expression, LCtx);
+  SVal secondVal = state->getSVal(Second.Expression, LCtx);
 
   Optional<Loc> firstLoc = firstVal.getAs<Loc>();
   if (!firstLoc)
     return state;
 
   Optional<Loc> secondLoc = secondVal.getAs<Loc>();
   if (!secondLoc)
     return state;
 
   // Are the two values the same?
   SValBuilder &svalBuilder = C.getSValBuilder();
   std::tie(stateTrue, stateFalse) =
       state->assume(svalBuilder.evalEQ(state, *firstLoc, *secondLoc));
 
   if (stateTrue && !stateFalse) {
     // If the values are known to be equal, that's automatically an overlap.
-    emitOverlapBug(C, stateTrue, First, Second);
+    emitOverlapBug(C, stateTrue, First.Expression, Second.Expression);
     return nullptr;
   }
 
   // assume the two expressions are not equal.
   assert(stateFalse);
   state = stateFalse;
 
   // Which value comes first?
   QualType cmpTy = svalBuilder.getConditionType();
-  SVal reverse = svalBuilder.evalBinOpLL(state, BO_GT,
-                                         *firstLoc, *secondLoc, cmpTy);
+  SVal reverse =
+      svalBuilder.evalBinOpLL(state, BO_GT, *firstLoc, *secondLoc, cmpTy);
   Optional<DefinedOrUnknownSVal> reverseTest =
       reverse.getAs<DefinedOrUnknownSVal>();
   if (!reverseTest)
     return state;
 
   std::tie(stateTrue, stateFalse) = state->assume(*reverseTest);
   if (stateTrue) {
     if (stateFalse) {
       // If we don't know which one comes first, we can't perform this test.
       return state;
     } else {
       // Switch the values so that firstVal is before secondVal.
       std::swap(firstLoc, secondLoc);
 
       // Switch the Exprs as well, so that they still correspond.
       std::swap(First, Second);
     }
   }
 
   // Get the length, and make sure it too is known.
-  SVal LengthVal = state->getSVal(Size, LCtx);
+  SVal LengthVal = state->getSVal(Size.Expression, LCtx);
   Optional<NonLoc> Length = LengthVal.getAs<NonLoc>();
   if (!Length)
     return state;
 
   // Convert the first buffer's start address to char*.
   // Bail out if the cast fails.
   ASTContext &Ctx = svalBuilder.getContext();
   QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy);
-  SVal FirstStart = svalBuilder.evalCast(*firstLoc, CharPtrTy,
-                                         First->getType());
+  SVal FirstStart =
+      svalBuilder.evalCast(*firstLoc, CharPtrTy, First.Expression->getType());
   Optional<Loc> FirstStartLoc = FirstStart.getAs<Loc>();
   if (!FirstStartLoc)
     return state;
 
   // Compute the end of the first buffer. Bail out if THAT fails.
-  SVal FirstEnd = svalBuilder.evalBinOpLN(state, BO_Add,
-                                 *FirstStartLoc, *Length, CharPtrTy);
+  SVal FirstEnd = svalBuilder.evalBinOpLN(state, BO_Add, *FirstStartLoc,
+                                          *Length, CharPtrTy);
   Optional<Loc> FirstEndLoc = FirstEnd.getAs<Loc>();
   if (!FirstEndLoc)
     return state;
 
   // Is the end of the first buffer past the start of the second buffer?
-  SVal Overlap = svalBuilder.evalBinOpLL(state, BO_GT,
-                                *FirstEndLoc, *secondLoc, cmpTy);
+  SVal Overlap =
+      svalBuilder.evalBinOpLL(state, BO_GT, *FirstEndLoc, *secondLoc, cmpTy);
   Optional<DefinedOrUnknownSVal> OverlapTest =
       Overlap.getAs<DefinedOrUnknownSVal>();
   if (!OverlapTest)
     return state;
 
   std::tie(stateTrue, stateFalse) = state->assume(*OverlapTest);
 
   if (stateTrue && !stateFalse) {
     // Overlap!
-    emitOverlapBug(C, stateTrue, First, Second);
+    emitOverlapBug(C, stateTrue, First.Expression, Second.Expression);
     return nullptr;
   }
 
   // assume the two expressions don't overlap.
   assert(stateFalse);
   return stateFalse;
 }
 
bool CStringChecker::memsetAux(const Expr *DstBuffer, SVal CharVal,
   }
   return true;
 }
 
 //===----------------------------------------------------------------------===//
 // evaluation of individual function calls.
 //===----------------------------------------------------------------------===//
 
-void CStringChecker::evalCopyCommon(CheckerContext &C,
-                                    const CallExpr *CE,
-                                    ProgramStateRef state,
-                                    const Expr *Size, const Expr *Dest,
-                                    const Expr *Source, bool Restricted,
+void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
+                                    ProgramStateRef state, SizeArgExpr Size,
+                                    DestinationArgExpr Dest,
+                                    SourceArgExpr Source, bool Restricted,
                                     bool IsMempcpy) const {
   CurrentFunctionDescription = "memory copy function";
 
   // See if the size argument is zero.
   const LocationContext *LCtx = C.getLocationContext();
-  SVal sizeVal = state->getSVal(Size, LCtx);
-  QualType sizeTy = Size->getType();
+  SVal sizeVal = state->getSVal(Size.Expression, LCtx);
+  QualType sizeTy = Size.Expression->getType();
 
   ProgramStateRef stateZeroSize, stateNonZeroSize;
   std::tie(stateZeroSize, stateNonZeroSize) =
       assumeZero(C, state, sizeVal, sizeTy);
 
   // Get the value of the Dest.
-  SVal destVal = state->getSVal(Dest, LCtx);
+  SVal destVal = state->getSVal(Dest.Expression, LCtx);
 
   // If the size is zero, there won't be any actual memory access, so
   // just bind the return value to the destination buffer and return.
   if (stateZeroSize && !stateNonZeroSize) {
     stateZeroSize = stateZeroSize->BindExpr(CE, LCtx, destVal);
     C.addTransition(stateZeroSize);
     return;
   }
 
   // If the size can be nonzero, we have to check the other arguments.
   if (stateNonZeroSize) {
     state = stateNonZeroSize;
 
     // Ensure the destination is not null. If it is NULL there will be a
     // NULL pointer dereference.
-    state = checkNonNull(C, state, Dest, destVal, 1);
+    state = checkNonNull(C, state, Dest, destVal);
     if (!state)
       return;
 
     // Get the value of the Src.
-    SVal srcVal = state->getSVal(Source, LCtx);
+    SVal srcVal = state->getSVal(Source.Expression, LCtx);
 
     // Ensure the source is not null. If it is NULL there will be a
     // NULL pointer dereference.
-    state = checkNonNull(C, state, Source, srcVal, 2);
+    state = checkNonNull(C, state, Source, srcVal);
     if (!state)
       return;
 
     // Ensure the accesses are valid and that the buffers do not overlap.
-    const char * const writeWarning =
-      "Memory copy function overflows destination buffer";
-    state = CheckBufferAccess(C, state, Size, Dest, Source,
-                              writeWarning, /* sourceWarning = */ nullptr);
+    state = CheckBufferAccess(C, state, Dest, Size, AccessKind::write);
+    state = CheckBufferAccess(C, state, Source, Size, AccessKind::read);
+
     if (Restricted)
       state = CheckOverlap(C, state, Size, Dest, Source);
 
     if (!state)
       return;
 
     // If this is mempcpy, get the byte after the last byte copied and
     // bind the expr.
     if (IsMempcpy) {
       // Get the byte after the last byte copied.
       SValBuilder &SvalBuilder = C.getSValBuilder();
       ASTContext &Ctx = SvalBuilder.getContext();
       QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy);
       SVal DestRegCharVal =
-          SvalBuilder.evalCast(destVal, CharPtrTy, Dest->getType());
+          SvalBuilder.evalCast(destVal, CharPtrTy, Dest.Expression->getType());
       SVal lastElement = C.getSValBuilder().evalBinOp(
-          state, BO_Add, DestRegCharVal, sizeVal, Dest->getType());
+          state, BO_Add, DestRegCharVal, sizeVal, Dest.Expression->getType());
       // If we don't know how much we copied, we can at least
       // conjure a return value for later.
       if (lastElement.isUnknown())
         lastElement = C.getSValBuilder().conjureSymbolVal(nullptr, CE, LCtx,
                                                           C.blockCount());
 
       // The byte after the last byte copied is the return value.
       state = state->BindExpr(CE, LCtx, lastElement);
     } else {
       // All other copies return the destination buffer.
       // (Well, bcopy() has a void return type, but this won't hurt.)
       state = state->BindExpr(CE, LCtx, destVal);
     }
 
     // Invalidate the destination (regular invalidation without pointer-escaping
     // the address of the top-level region).
     // FIXME: Even if we can't perfectly model the copy, we should see if we
     // can use LazyCompoundVals to copy the source values into the destination.
     // This would probably remove any existing bindings past the end of the
     // copied region, but that's still an improvement over blank invalidation.
-    state = InvalidateBuffer(C, state, Dest, C.getSVal(Dest),
-                             /*IsSourceBuffer*/false, Size);
+    state =
+        InvalidateBuffer(C, state, Dest.Expression, C.getSVal(Dest.Expression),
+                         /*IsSourceBuffer*/ false, Size.Expression);
 
     // Invalidate the source (const-invalidation without const-pointer-escaping
     // the address of the top-level region).
-    state = InvalidateBuffer(C, state, Source, C.getSVal(Source),
-                             /*IsSourceBuffer*/true, nullptr);
+    state = InvalidateBuffer(C, state, Source.Expression,
+                             C.getSVal(Source.Expression),
+                             /*IsSourceBuffer*/ true, nullptr);
 
     C.addTransition(state);
   }
 }
 
-
 void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE) const {
   // void *memcpy(void *restrict dst, const void *restrict src, size_t n);
   // The return value is the address of the destination buffer.
-  const Expr *Dest = CE->getArg(0);
-  ProgramStateRef state = C.getState();
+  DestinationArgExpr Dest = {CE->getArg(0), 0};
+  SourceArgExpr Src = {CE->getArg(1), 1};
+  SizeArgExpr Size = {CE->getArg(2), 2};
 
-  evalCopyCommon(C, CE, state, CE->getArg(2), Dest, CE->getArg(1), true);
+  ProgramStateRef State = C.getState();
+
+  constexpr bool IsRestricted = true;
+  constexpr bool IsMempcpy = false;
+  evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy);
 }
 
 void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE) const {
   // void *mempcpy(void *restrict dst, const void *restrict src, size_t n);
   // The return value is a pointer to the byte following the last written byte.
-  const Expr *Dest = CE->getArg(0);
-  ProgramStateRef state = C.getState();
-
-  evalCopyCommon(C, CE, state, CE->getArg(2), Dest, CE->getArg(1), true, true);
+  DestinationArgExpr Dest = {CE->getArg(0), 0};
+  SourceArgExpr Src = {CE->getArg(1), 1};
+  SizeArgExpr Size = {CE->getArg(2), 2};
+
+  constexpr bool IsRestricted = true;
+  constexpr bool IsMempcpy = true;
+  evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy);
 }
 
 void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const {
   // void *memmove(void *dst, const void *src, size_t n);
   // The return value is the address of the destination buffer.
-  const Expr *Dest = CE->getArg(0);
-  ProgramStateRef state = C.getState();
-
-  evalCopyCommon(C, CE, state, CE->getArg(2), Dest, CE->getArg(1));
+  DestinationArgExpr Dest = {CE->getArg(0), 0};
+  SourceArgExpr Src = {CE->getArg(1), 1};
+  SizeArgExpr Size = {CE->getArg(2), 2};
+
+  constexpr bool IsRestricted = false;
+  constexpr bool IsMempcpy = false;
+  evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy);
 }
 
 void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const {
   // void bcopy(const void *src, void *dst, size_t n);
-  evalCopyCommon(C, CE, C.getState(),
-                 CE->getArg(2), CE->getArg(1), CE->getArg(0));
+  SourceArgExpr Src(CE->getArg(0), 0);
+  DestinationArgExpr Dest = {CE->getArg(1), 1};
+  SizeArgExpr Size = {CE->getArg(2), 2};
+
+  constexpr bool IsRestricted = false;
+  constexpr bool IsMempcpy = false;
+  evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy);
 }
 
 void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const {
   // int memcmp(const void *s1, const void *s2, size_t n);
   CurrentFunctionDescription = "memory comparison function";
 
-  const Expr *Left = CE->getArg(0);
-  const Expr *Right = CE->getArg(1);
-  const Expr *Size = CE->getArg(2);
+  AnyArgExpr Left = {CE->getArg(0), 0};
+  AnyArgExpr Right = {CE->getArg(1), 1};
+  SizeArgExpr Size = {CE->getArg(2), 2};
 
-  ProgramStateRef state = C.getState();
-  SValBuilder &svalBuilder = C.getSValBuilder();
+  ProgramStateRef State = C.getState();
+  SValBuilder &Builder = C.getSValBuilder();
+  const LocationContext *LCtx = C.getLocationContext();
 
   // See if the size argument is zero.
-  const LocationContext *LCtx = C.getLocationContext();
-  SVal sizeVal = state->getSVal(Size, LCtx);
-  QualType sizeTy = Size->getType();
+  SVal sizeVal = State->getSVal(Size.Expression, LCtx);
+  QualType sizeTy = Size.Expression->getType();
 
   ProgramStateRef stateZeroSize, stateNonZeroSize;
   std::tie(stateZeroSize, stateNonZeroSize) =
-    assumeZero(C, state, sizeVal, sizeTy);
+      assumeZero(C, State, sizeVal, sizeTy);
 
   // If the size can be zero, the result will be 0 in that case, and we don't
   // have to check either of the buffers.
   if (stateZeroSize) {
-    state = stateZeroSize;
-    state = state->BindExpr(CE, LCtx,
-                            svalBuilder.makeZeroVal(CE->getType()));
-    C.addTransition(state);
+    State = stateZeroSize;
+    State = State->BindExpr(CE, LCtx, Builder.makeZeroVal(CE->getType()));
+    C.addTransition(State);
   }
 
   // If the size can be nonzero, we have to check the other arguments.
   if (stateNonZeroSize) {
-    state = stateNonZeroSize;
+    State = stateNonZeroSize;
     // If we know the two buffers are the same, we know the result is 0.
     // First, get the two buffers' addresses. Another checker will have already
     // made sure they're not undefined.
     DefinedOrUnknownSVal LV =
-        state->getSVal(Left, LCtx).castAs<DefinedOrUnknownSVal>();
+        State->getSVal(Left.Expression, LCtx).castAs<DefinedOrUnknownSVal>();
     DefinedOrUnknownSVal RV =
-        state->getSVal(Right, LCtx).castAs<DefinedOrUnknownSVal>();
+        State->getSVal(Right.Expression, LCtx).castAs<DefinedOrUnknownSVal>();
 
     // See if they are the same.
-    DefinedOrUnknownSVal SameBuf = svalBuilder.evalEQ(state, LV, RV);
-    ProgramStateRef StSameBuf, StNotSameBuf;
-    std::tie(StSameBuf, StNotSameBuf) = state->assume(SameBuf);
+    ProgramStateRef SameBuffer, NotSameBuffer;
+    std::tie(SameBuffer, NotSameBuffer) =
+        State->assume(Builder.evalEQ(State, LV, RV));
 
     // If the two arguments are the same buffer, we know the result is 0,
     // and we only need to check one size.
-    if (StSameBuf && !StNotSameBuf) {
-      state = StSameBuf;
-      state = CheckBufferAccess(C, state, Size, Left);
-      if (state) {
-        state = StSameBuf->BindExpr(CE, LCtx,
-                                    svalBuilder.makeZeroVal(CE->getType()));
-        C.addTransition(state);
+    if (SameBuffer && !NotSameBuffer) {
+      State = SameBuffer;
+      State = CheckBufferAccess(C, State, Left, Size, AccessKind::read);
+      if (State) {
+        State =
+            SameBuffer->BindExpr(CE, LCtx, Builder.makeZeroVal(CE->getType()));
+        C.addTransition(State);
       }
       return;
     }
 
     // If the two arguments might be different buffers, we have to check
     // the size of both of them.
-    assert(StNotSameBuf);
-    state = CheckBufferAccess(C, state, Size, Left, Right);
-    if (state) {
+    assert(NotSameBuffer);
+    State = CheckBufferAccess(C, State, Right, Size, AccessKind::read);
+    State = CheckBufferAccess(C, State, Left, Size, AccessKind::read);
+    if (State) {
       // The return value is the comparison result, which we don't know.
-      SVal CmpV =
-          svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
-      state = state->BindExpr(CE, LCtx, CmpV);
-      C.addTransition(state);
+      SVal CmpV = Builder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
+      State = State->BindExpr(CE, LCtx, CmpV);
+      C.addTransition(State);
     }
   }
 }
 
 void CStringChecker::evalstrLength(CheckerContext &C,
                                    const CallExpr *CE) const {
   // size_t strlen(const char *s);
   evalstrLengthCommon(C, CE, /* IsStrnlen = */ false);
if (IsStrnlen) {
     if (!stateNonZeroSize)
       return;
 
     // Otherwise, record the assumption that the size is nonzero.
     state = stateNonZeroSize;
   }
 
   // Check that the string argument is non-null.
-  const Expr *Arg = CE->getArg(0);
-  SVal ArgVal = state->getSVal(Arg, LCtx);
-
-  state = checkNonNull(C, state, Arg, ArgVal, 1);
+  AnyArgExpr Arg = {CE->getArg(0), 0};
+  SVal ArgVal = state->getSVal(Arg.Expression, LCtx);
+  state = checkNonNull(C, state, Arg, ArgVal);
 
   if (!state)
     return;
 
-  SVal strLength = getCStringLength(C, state, Arg, ArgVal);
+  SVal strLength = getCStringLength(C, state, Arg.Expression, ArgVal);
 
   // If the argument isn't a valid C string, there's no valid state to
   // transition to.
   if (strLength.isUndef())
     return;
 
   DefinedOrUnknownSVal result = UnknownVal();
 

 void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
                                       bool ReturnEnd, bool IsBounded,
                                       ConcatFnKind appendK,
                                       bool returnPtr) const {
   if (appendK == ConcatFnKind::none)
     CurrentFunctionDescription = "string copy function";
   else
     CurrentFunctionDescription = "string concatenation function";
+
   ProgramStateRef state = C.getState();
   const LocationContext *LCtx = C.getLocationContext();
 
   // Check that the destination is non-null.
-  const Expr *Dst = CE->getArg(0);
-  SVal DstVal = state->getSVal(Dst, LCtx);
-
-  state = checkNonNull(C, state, Dst, DstVal, 1);
+  DestinationArgExpr Dst = {CE->getArg(0), 0};
+  SVal DstVal = state->getSVal(Dst.Expression, LCtx);
+  state = checkNonNull(C, state, Dst, DstVal);
   if (!state)
     return;
 
   // Check that the source is non-null.
-  const Expr *srcExpr = CE->getArg(1);
-  SVal srcVal = state->getSVal(srcExpr, LCtx);
-  state = checkNonNull(C, state, srcExpr, srcVal, 2);
+  SourceArgExpr srcExpr = {CE->getArg(1), 1};
+  SVal srcVal = state->getSVal(srcExpr.Expression, LCtx);
+  state = checkNonNull(C, state, srcExpr, srcVal);
   if (!state)
     return;
 
   // Get the string length of the source.
-  SVal strLength = getCStringLength(C, state, srcExpr, srcVal);
+  SVal strLength = getCStringLength(C, state, srcExpr.Expression, srcVal);
   Optional<NonLoc> strLengthNL = strLength.getAs<NonLoc>();
 
   // Get the string length of the destination buffer.
-  SVal dstStrLength = getCStringLength(C, state, Dst, DstVal);
+  SVal dstStrLength = getCStringLength(C, state, Dst.Expression, DstVal);
   Optional<NonLoc> dstStrLengthNL = dstStrLength.getAs<NonLoc>();
 
   // If the source isn't a valid C string, give up.
   if (strLength.isUndef())
     return;
 
   SValBuilder &svalBuilder = C.getSValBuilder();
   QualType cmpTy = svalBuilder.getConditionType();
   QualType sizeTy = svalBuilder.getContext().getSizeType();
 
   // These two values allow checking two kinds of errors:
   // - actual overflows caused by a source that doesn't fit in the destination
   // - potential overflows caused by a bound that could exceed the destination
   SVal amountCopied = UnknownVal();
   SVal maxLastElementIndex = UnknownVal();
   const char *boundWarning = nullptr;
 
-  state = CheckOverlap(C, state, IsBounded ? CE->getArg(2) : CE->getArg(1), Dst,
-                       srcExpr);
+  // FIXME: Why do we choose the srcExpr if the access has no size?
+  //  Note that the 3rd argument of the call would be the size parameter.
+  SizeArgExpr SrcExprAsSizeDummy = {srcExpr.Expression, srcExpr.ArgumentIndex};
+  state = CheckOverlap(
+      C, state,
+      (IsBounded ? SizeArgExpr{CE->getArg(2), 2} : SrcExprAsSizeDummy), Dst,
 
   if (!state)
     return;
 
   // If the function is strncpy, strncat, etc... it is bounded.
   if (IsBounded) {
     // Get the max number of characters to copy.
-    const Expr *lenExpr = CE->getArg(2);
-    SVal lenVal = state->getSVal(lenExpr, LCtx);
+    SizeArgExpr lenExpr = {CE->getArg(2), 2};
+    SVal lenVal = state->getSVal(lenExpr.Expression, LCtx);
 
     // Protect against misdeclared strncpy().
-    lenVal = svalBuilder.evalCast(lenVal, sizeTy, lenExpr->getType());
+    lenVal =
+        svalBuilder.evalCast(lenVal, sizeTy, lenExpr.Expression->getType());
 
     Optional<NonLoc> lenValNL = lenVal.getAs<NonLoc>();
 
     // If we know both values, we might be able to figure out how much
     // we're copying.
     if (strLengthNL && lenValNL) {
       switch (appendK) {
       case ConcatFnKind::none:
void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
   }
 
   assert(state);
 
   // If the destination is a MemRegion, try to check for a buffer overflow and
   // record the new string length.
   if (Optional<loc::MemRegionVal> dstRegVal =
       DstVal.getAs<loc::MemRegionVal>()) {
-    QualType ptrTy = Dst->getType();
+    QualType ptrTy = Dst.Expression->getType();
 
     // If we have an exact value on a bounded copy, use that to check for
     // overflows, rather than our estimate about how much is actually copied.
-    if (boundWarning) {
-      if (Optional<NonLoc> maxLastNL = maxLastElementIndex.getAs<NonLoc>()) {
-        SVal maxLastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal,
-            *maxLastNL, ptrTy);
-        state = CheckLocation(C, state, CE->getArg(2), maxLastElement,
-            boundWarning);
-        if (!state)
-          return;
-      }
-    }
+      SVal maxLastElement =
+          svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal, *maxLastNL, ptrTy);
+
+      state = CheckLocation(C, state, Dst, maxLastElement, AccessKind::write);
 
     // Then, if the final length is known...
     if (Optional<NonLoc> knownStrLength = finalStrLength.getAs<NonLoc>()) {
       SVal lastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal,
           *knownStrLength, ptrTy);
 
       // ...and we haven't checked the bound, we'll check the actual copy.
       if (!boundWarning) {
-        const char * const warningMsg =
-          "String copy function overflows destination buffer";
-        state = CheckLocation(C, state, Dst, lastElement, warningMsg);
+        state = CheckLocation(C, state, Dst, lastElement, AccessKind::write);
         if (!state)
           return;
       }
 
       // If this is a stpcpy-style copy, the last element is the return value.
       if (returnPtr && ReturnEnd)
         Result = lastElement;
     }
 
     // Invalidate the destination (regular invalidation without pointer-escaping
     // the address of the top-level region). This must happen before we set the
     // C string length because invalidation will clear the length.
     // FIXME: Even if we can't perfectly model the copy, we should see if we
     // can use LazyCompoundVals to copy the source values into the destination.
     // This would probably remove any existing bindings past the end of the
     // string, but that's still an improvement over blank invalidation.
-    state = InvalidateBuffer(C, state, Dst, *dstRegVal,
-        /*IsSourceBuffer*/false, nullptr);
+    state = InvalidateBuffer(C, state, Dst.Expression, *dstRegVal,
+                             /*IsSourceBuffer*/ false, nullptr);
 
     // Invalidate the source (const-invalidation without const-pointer-escaping
     // the address of the top-level region).
-    state = InvalidateBuffer(C, state, srcExpr, srcVal, /*IsSourceBuffer*/true,
-        nullptr);
+    state = InvalidateBuffer(C, state, srcExpr.Expression, srcVal,
+                             /*IsSourceBuffer*/ true, nullptr);
 
     // Set the C string length of the destination, if we know it.
     if (IsBounded && (appendK == ConcatFnKind::none)) {
       // strncpy is annoying in that it doesn't guarantee to null-terminate
       // the result string. If the original string didn't fit entirely inside
       // the bound (including the null-terminator), we don't know how long the
       // result is.
       if (amountCopied != strLength)

 
 void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE,
     bool IsBounded, bool IgnoreCase) const {
   CurrentFunctionDescription = "string comparison function";
   ProgramStateRef state = C.getState();
   const LocationContext *LCtx = C.getLocationContext();
 
   // Check that the first string is non-null
-  const Expr *s1 = CE->getArg(0);
-  SVal s1Val = state->getSVal(s1, LCtx);
-  state = checkNonNull(C, state, s1, s1Val, 1);
+  AnyArgExpr Left = {CE->getArg(0), 0};
+  SVal LeftVal = state->getSVal(Left.Expression, LCtx);
+  state = checkNonNull(C, state, Left, LeftVal);
   if (!state)
     return;
 
   // Check that the second string is non-null.
-  const Expr *s2 = CE->getArg(1);
-  SVal s2Val = state->getSVal(s2, LCtx);
-  state = checkNonNull(C, state, s2, s2Val, 2);
+  AnyArgExpr Right = {CE->getArg(1), 1};
+  SVal RightVal = state->getSVal(Right.Expression, LCtx);
+  state = checkNonNull(C, state, Right, RightVal);
   if (!state)
     return;
 
   // Get the string length of the first string or give up.
-  SVal s1Length = getCStringLength(C, state, s1, s1Val);
-  if (s1Length.isUndef())
+  SVal LeftLength = getCStringLength(C, state, Left.Expression, LeftVal);
+  if (LeftLength.isUndef())
     return;
 
   // Get the string length of the second string or give up.
-  SVal s2Length = getCStringLength(C, state, s2, s2Val);
-  if (s2Length.isUndef())
+  SVal RightLength = getCStringLength(C, state, Right.Expression, RightVal);
+  if (RightLength.isUndef())
     return;
 
   // If we know the two buffers are the same, we know the result is 0.
   // First, get the two buffers' addresses. Another checker will have already
   // made sure they're not undefined.
-  DefinedOrUnknownSVal LV = s1Val.castAs<DefinedOrUnknownSVal>();
-  DefinedOrUnknownSVal RV = s2Val.castAs<DefinedOrUnknownSVal>();
+  DefinedOrUnknownSVal LV = LeftVal.castAs<DefinedOrUnknownSVal>();
+  DefinedOrUnknownSVal RV = RightVal.castAs<DefinedOrUnknownSVal>();
 
   // See if they are the same.
   SValBuilder &svalBuilder = C.getSValBuilder();
   DefinedOrUnknownSVal SameBuf = svalBuilder.evalEQ(state, LV, RV);
   ProgramStateRef StSameBuf, StNotSameBuf;
   std::tie(StSameBuf, StNotSameBuf) = state->assume(SameBuf);
 
   // If the two arguments might be the same buffer, we know the result is 0,
void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE,
 
   assert(StNotSameBuf);
   state = StNotSameBuf;
 
   // At this point we can go about comparing the two buffers.
   // For now, we only do this if they're both known string literals.
 
   // Attempt to extract string literals from both expressions.
-  const StringLiteral *s1StrLiteral = getCStringLiteral(C, state, s1, s1Val);
-  const StringLiteral *s2StrLiteral = getCStringLiteral(C, state, s2, s2Val);
+  const StringLiteral *LeftStrLiteral =
+      getCStringLiteral(C, state, Left.Expression, LeftVal);
+  const StringLiteral *RightStrLiteral =
+      getCStringLiteral(C, state, Right.Expression, RightVal);
   bool canComputeResult = false;
   SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx,
       C.blockCount());
 
-  if (s1StrLiteral && s2StrLiteral) {
-    StringRef s1StrRef = s1StrLiteral->getString();
-    StringRef s2StrRef = s2StrLiteral->getString();
+  if (LeftStrLiteral && RightStrLiteral) {
+    StringRef LeftStrRef = LeftStrLiteral->getString();
+    StringRef RightStrRef = RightStrLiteral->getString();
 
     if (IsBounded) {
       // Get the max number of characters to compare.
       const Expr *lenExpr = CE->getArg(2);
       SVal lenVal = state->getSVal(lenExpr, LCtx);
 
       // If the length is known, we can get the right substrings.
       if (const llvm::APSInt *len = svalBuilder.getKnownValue(state, lenVal)) {
         // Create substrings of each to compare the prefix.
-        s1StrRef = s1StrRef.substr(0, (size_t)len->getZExtValue());
-        s2StrRef = s2StrRef.substr(0, (size_t)len->getZExtValue());
+        LeftStrRef = LeftStrRef.substr(0, (size_t)len->getZExtValue());
+        RightStrRef = RightStrRef.substr(0, (size_t)len->getZExtValue());
         canComputeResult = true;
       }
     } else {
       // This is a normal, unbounded strcmp.
       canComputeResult = true;
     }
 
     if (canComputeResult) {
       // Real strcmp stops at null characters.
-      size_t s1Term = s1StrRef.find('\0');
+      size_t s1Term = LeftStrRef.find('\0');
       if (s1Term != StringRef::npos)
-        s1StrRef = s1StrRef.substr(0, s1Term);
+        LeftStrRef = LeftStrRef.substr(0, s1Term);
 
-      size_t s2Term = s2StrRef.find('\0');
+      size_t s2Term = RightStrRef.find('\0');
       if (s2Term != StringRef::npos)
-        s2StrRef = s2StrRef.substr(0, s2Term);
+        RightStrRef = RightStrRef.substr(0, s2Term);
 
       // Use StringRef's comparison methods to compute the actual result.
-      int compareRes = IgnoreCase ? s1StrRef.compare_lower(s2StrRef)
-        : s1StrRef.compare(s2StrRef);
+      int compareRes = IgnoreCase ? LeftStrRef.compare_lower(RightStrRef)
+                                  : LeftStrRef.compare(RightStrRef);
 
       // The strcmp function returns an integer greater than, equal to, or less
       // than zero, [c11, p7.24.4.2].
       if (compareRes == 0) {
         resultVal = svalBuilder.makeIntVal(compareRes, CE->getType());
       }
       else {
         DefinedSVal zeroVal = svalBuilder.makeIntVal(0, CE->getType());
void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE,
 
   // Record this as a possible path.
   C.addTransition(state);
 }
 
 void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const {
   //char *strsep(char **stringp, const char *delim);
   // Sanity: does the search string parameter match the return type?
-  const Expr *SearchStrPtr = CE->getArg(0);
-  QualType CharPtrTy = SearchStrPtr->getType()->getPointeeType();
+  SourceArgExpr SearchStrPtr = {CE->getArg(0), 0};
+
+  QualType CharPtrTy = SearchStrPtr.Expression->getType()->getPointeeType();
   if (CharPtrTy.isNull() ||
       CE->getType().getUnqualifiedType() != CharPtrTy.getUnqualifiedType())
     return;
 
   CurrentFunctionDescription = "strsep()";
   ProgramStateRef State = C.getState();
   const LocationContext *LCtx = C.getLocationContext();
 
   // Check that the search string pointer is non-null (though it may point to
   // a null string).
-  SVal SearchStrVal = State->getSVal(SearchStrPtr, LCtx);
-  State = checkNonNull(C, State, SearchStrPtr, SearchStrVal, 1);
+  SVal SearchStrVal = State->getSVal(SearchStrPtr.Expression, LCtx);
+  State = checkNonNull(C, State, SearchStrPtr, SearchStrVal);
   if (!State)
     return;
 
   // Check that the delimiter string is non-null.
-  const Expr *DelimStr = CE->getArg(1);
-  SVal DelimStrVal = State->getSVal(DelimStr, LCtx);
-  State = checkNonNull(C, State, DelimStr, DelimStrVal, 2);
+  AnyArgExpr DelimStr = {CE->getArg(1), 1};
+  SVal DelimStrVal = State->getSVal(DelimStr.Expression, LCtx);
+  State = checkNonNull(C, State, DelimStr, DelimStrVal);
   if (!State)
     return;
 
   SValBuilder &SVB = C.getSValBuilder();
   SVal Result;
   if (Optional<Loc> SearchStrLoc = SearchStrVal.getAs<Loc>()) {
     // Get the current value of the search string pointer, as a char*.
     Result = State->getSVal(*SearchStrLoc, CharPtrTy);
 
     // Invalidate the search string, representing the change of one delimiter
     // character to NUL.
-    State = InvalidateBuffer(C, State, SearchStrPtr, Result,
-        /*IsSourceBuffer*/false, nullptr);
+    State = InvalidateBuffer(C, State, SearchStrPtr.Expression, Result,
+                             /*IsSourceBuffer*/ false, nullptr);
 
     // Overwrite the search string pointer. The new value is either an address
     // further along in the same string, or NULL if there are no more tokens.
     State = State->bindLoc(*SearchStrLoc,
         SVB.conjureSymbolVal(getTag(),
           CE,
           LCtx,
           CharPtrTy,
void CStringChecker::evalStdCopyCommon(CheckerContext &C,
 
   SVal ResultVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
   State = State->BindExpr(CE, LCtx, ResultVal);
 
   C.addTransition(State);
 }
 
 void CStringChecker::evalMemset(CheckerContext &C, const CallExpr *CE) const {
+  // void *memset(void *s, int c, size_t n);
   CurrentFunctionDescription = "memory set function";
 
-  const Expr *Mem = CE->getArg(0);
-  const Expr *CharE = CE->getArg(1);
-  const Expr *Size = CE->getArg(2);
+  DestinationArgExpr Buffer = {CE->getArg(0), 0};
+  AnyArgExpr CharE = {CE->getArg(1), 1};
+  SizeArgExpr Size = {CE->getArg(2), 2};
+
   ProgramStateRef State = C.getState();
 
   // See if the size argument is zero.
   const LocationContext *LCtx = C.getLocationContext();
-  SVal SizeVal = State->getSVal(Size, LCtx);
-  QualType SizeTy = Size->getType();
+  SVal SizeVal = C.getSVal(Size.Expression);
+  QualType SizeTy = Size.Expression->getType();
 
-  ProgramStateRef StateZeroSize, StateNonZeroSize;
-  std::tie(StateZeroSize, StateNonZeroSize) =
-    assumeZero(C, State, SizeVal, SizeTy);
+  ProgramStateRef ZeroSize, NonZeroSize;
+  std::tie(ZeroSize, NonZeroSize) = assumeZero(C, State, SizeVal, SizeTy);
 
   // Get the value of the memory area.
-  SVal MemVal = State->getSVal(Mem, LCtx);
+  SVal BufferPtrVal = C.getSVal(Buffer.Expression);
 
   // If the size is zero, there won't be any actual memory access, so
-  // just bind the return value to the Mem buffer and return.
-  if (StateZeroSize && !StateNonZeroSize) {
-    StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, MemVal);
-    C.addTransition(StateZeroSize);
+  // just bind the return value to the buffer and return.
+  if (ZeroSize && !NonZeroSize) {
+    ZeroSize = ZeroSize->BindExpr(CE, LCtx, BufferPtrVal);
+    C.addTransition(ZeroSize);
     return;
   }
 
   // Ensure the memory area is not null.
   // If it is NULL there will be a NULL pointer dereference.
-  State = checkNonNull(C, StateNonZeroSize, Mem, MemVal, 1);
+  State = checkNonNull(C, NonZeroSize, Buffer, BufferPtrVal);
   if (!State)
     return;
 
-  State = CheckBufferAccess(C, State, Size, Mem);
+  State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write);
   if (!State)
     return;
 
   // According to the values of the arguments, bind the value of the second
   // argument to the destination buffer and set string length, or just
   // invalidate the destination buffer.
-  if (!memsetAux(Mem, C.getSVal(CharE), Size, C, State))
+  if (!memsetAux(Buffer.Expression, C.getSVal(CharE.Expression),
+                 Size.Expression, C, State))
     return;
 
-  State = State->BindExpr(CE, LCtx, MemVal);
+  State = State->BindExpr(CE, LCtx, BufferPtrVal);
   C.addTransition(State);
 }
 
 void CStringChecker::evalBzero(CheckerContext &C, const CallExpr *CE) const {
   CurrentFunctionDescription = "memory clearance function";
 
-  const Expr *Mem = CE->getArg(0);
-  const Expr *Size = CE->getArg(1);
+  DestinationArgExpr Buffer = {CE->getArg(0), 0};
+  SizeArgExpr Size = {CE->getArg(1), 1};
   SVal Zero = C.getSValBuilder().makeZeroVal(C.getASTContext().IntTy);
 
   ProgramStateRef State = C.getState();
 
   // See if the size argument is zero.
-  SVal SizeVal = C.getSVal(Size);
-  QualType SizeTy = Size->getType();
+  SVal SizeVal = C.getSVal(Size.Expression);
+  QualType SizeTy = Size.Expression->getType();
 
   ProgramStateRef StateZeroSize, StateNonZeroSize;
   std::tie(StateZeroSize, StateNonZeroSize) =
     assumeZero(C, State, SizeVal, SizeTy);
 
   // If the size is zero, there won't be any actual memory access,
   // In this case we just return.
   if (StateZeroSize && !StateNonZeroSize) {
     C.addTransition(StateZeroSize);
     return;
   }
 
   // Get the value of the memory area.
-  SVal MemVal = C.getSVal(Mem);
+  SVal MemVal = C.getSVal(Buffer.Expression);
 
   // Ensure the memory area is not null.
   // If it is NULL there will be a NULL pointer dereference.
-  State = checkNonNull(C, StateNonZeroSize, Mem, MemVal, 1);
+  State = checkNonNull(C, StateNonZeroSize, Buffer, MemVal);
   if (!State)
     return;
 
-  State = CheckBufferAccess(C, State, Size, Mem);
+  State = CheckBufferAccess(C, State, Buffer, Size, AccessKind::write);
   if (!State)
     return;
 
-  if (!memsetAux(Mem, Zero, Size, C, State))
+  if (!memsetAux(Buffer.Expression, Zero, Size.Expression, C, State))
     return;
 
   C.addTransition(State);
 }
 
 //===----------------------------------------------------------------------===//
 // The driver method, and other Checker callbacks.
 //===----------------------------------------------------------------------===//

clang/test/Analysis/bsd-string.c

void f2() {
   clang_analyzer_eval(len == 4); // expected-warning{{TRUE}}
   len = strlcat(buf, "efgh", sizeof(buf)); // expected-no-warning
   clang_analyzer_eval(len == 8); // expected-warning{{TRUE}}
 }
 
 void f3() {
   char dst[2];
   const char *src = "abdef";
-  strlcpy(dst, src, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
+  strlcpy(dst, src, 5); // expected-warning{{String copy function overflows the destination buffer}}
 }
 
 void f4() {
   strlcpy(NULL, "abcdef", 6); // expected-warning{{Null pointer passed as 1st argument to string copy function}}
 }
 
 void f5() {
   strlcat(NULL, "abcdef", 6); // expected-warning{{Null pointer passed as 1st argument to string concatenation function}}
void f9(int unknown_size, char* unknown_src, char* unknown_dst){
   clang_analyzer_eval(strlen(buf));// expected-warning{{UNKNOWN}}
 
   //src, dst is unknown
   len = strlcpy(unknown_dst, unknown_src, unknown_size);
   clang_analyzer_eval(len);// expected-warning{{UNKNOWN}}
   clang_analyzer_eval(strlen(unknown_dst));// expected-warning{{UNKNOWN}}
 
   //size is unknown
-  len = strlcat(buf+2,unknown_src+1, sizeof(buf));// expected-warning{{Size argument is greater than the length of the destination buffer}};
+  len = strlcat(buf + 2, unknown_src + 1, sizeof(buf));
+  // expected-warning@-1 {{String concatenation function overflows the destination buffer}}
 }
 
 void f10(){
   char buf[8];
   size_t len;
 
   len = strlcpy(buf,"abba",sizeof(buf));
   clang_analyzer_eval(len==4);// expected-warning{{TRUE}}
-  strlcat(buf, "efghi",9);// expected-warning{{Size argument is greater than the length of the destination buffer}}
+  strlcat(buf, "efghi", 9);
+  // expected-warning@-1 {{String concatenation function overflows the destination buffer}}
 }
 
 void f11() {
   //test for Bug 41729
   char a[256], b[256];
   strlcpy(a, "world", sizeof(a));
   strlcpy(b, "hello ", sizeof(b));
   strlcat(b, a, sizeof(b)); // no-warning
 }
 
 int a, b;
 void unknown_val_crash() {
   // We're unable to evaluate the integer-to-pointer cast.
   strlcat(&b, a, 0); // no-crash
 }

clang/test/Analysis/bstring.c

void memcpy1 () {
 
   memcpy(dst, src, 5); // expected-warning{{Memory copy function accesses out-of-bound array element}}
 }
 
 void memcpy2 () {
   char src[] = {1, 2, 3, 4};
   char dst[1];
 
-  memcpy(dst, src, 4);  // expected-warning{{Memory copy function overflows destination buffer}}
+  memcpy(dst, src, 4); // expected-warning {{Memory copy function overflows the destination buffer}}
 #ifndef VARIANT
-  // expected-warning@-2{{memcpy' will always overflow; destination buffer has size 1, but size argument is 4}}
+  // expected-warning@-2 {{memcpy' will always overflow; destination buffer has size 1, but size argument is 4}}
 #endif
 }
 
 void memcpy3 () {
   char src[] = {1, 2, 3, 4};
   char dst[3];
 
   memcpy(dst+1, src+2, 2); // no-warning
 }
 
 void memcpy4 () {
   char src[] = {1, 2, 3, 4};
   char dst[10];
 
   memcpy(dst+2, src+2, 3); // expected-warning{{Memory copy function accesses out-of-bound array element}}
 }
 
 void memcpy5() {
   char src[] = {1, 2, 3, 4};
   char dst[3];
 
-  memcpy(dst+2, src+2, 2); // expected-warning{{Memory copy function overflows destination buffer}}
+  memcpy(dst + 2, src + 2, 2); // expected-warning{{Memory copy function overflows the destination buffer}}
 #ifndef VARIANT
   // expected-warning@-2{{memcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
 #endif
 }
 
 void memcpy6() {
   int a[4] = {0};
   memcpy(a, a, 8); // expected-warning{{overlapping}}  
void mempcpy1 () {
 
   mempcpy(dst, src, 5); // expected-warning{{Memory copy function accesses out-of-bound array element}}
 }
 
 void mempcpy2 () {
   char src[] = {1, 2, 3, 4};
   char dst[1];
 
-  mempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}}
+  mempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows the destination buffer}}
 #ifndef VARIANT
 // expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 4}}
 #endif
 }
 
 void mempcpy3 () {
   char src[] = {1, 2, 3, 4};
   char dst[3];
 
   mempcpy(dst+1, src+2, 2); // no-warning
 }
 
 void mempcpy4 () {
   char src[] = {1, 2, 3, 4};
   char dst[10];
 
   mempcpy(dst+2, src+2, 3); // expected-warning{{Memory copy function accesses out-of-bound array element}}
 }
 
 void mempcpy5() {
   char src[] = {1, 2, 3, 4};
   char dst[3];
 
-  mempcpy(dst+2, src+2, 2); // expected-warning{{Memory copy function overflows destination buffer}}
+  mempcpy(dst + 2, src + 2, 2); // expected-warning{{Memory copy function overflows the destination buffer}}
 #ifndef VARIANT
 // expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
 #endif
 }
 
 void mempcpy6() {
   int a[4] = {0};
   mempcpy(a, a, 8); // expected-warning{{overlapping}}  
void memmove1 () {
 
   memmove(dst, src, 5); // expected-warning{{out-of-bound}}
 }
 
 void memmove2 () {
   char src[] = {1, 2, 3, 4};
   char dst[1];
 
-  memmove(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}}
+  memmove(dst, src, 4); // expected-warning{{Memory copy function overflows the destination buffer}}
 #ifndef VARIANT
   // expected-warning@-2{{memmove' will always overflow; destination buffer has size 1, but size argument is 4}}
 #endif
 }
 
 //===----------------------------------------------------------------------===
 // memcmp()
 //===----------------------------------------------------------------------===

clang/test/Analysis/null-deref-ps-region.c

 void testHeapSymbol() {
   char *buf = (char *)malloc(13);
   memset(buf, 0, 1); // no-warning
   free(buf);
 }
 
 void testStackArrayOutOfBound() {
   char buf[1];
-  memset(buf, 0, 1024); // expected-warning {{Memory set function accesses out-of-bound array element}} expected-warning {{'memset' will always overflow; destination buffer has size 1, but size argument is 1024}}
+  memset(buf, 0, 1024);
+  // expected-warning@-1 {{Memory set function overflows the destination buffer}}
+  // expected-warning@-2 {{'memset' will always overflow; destination buffer has size 1, but size argument is 1024}}
 }
 
 void testHeapSymbolOutOfBound() {
   char *buf = (char *)malloc(1);
-  memset(buf, 0, 1024); // expected-warning {{Memory set function accesses out-of-bound array element}}
+  memset(buf, 0, 1024);
+  // expected-warning@-1 {{Memory set function overflows the destination buffer}}
   free(buf);
 }
 
 void testStackArraySameSize() {
   char buf[1];
   memset(buf, 0, sizeof(buf)); // no-warning
 }
 
 void testHeapSymbolSameSize() {
   char *buf = (char *)malloc(1);
   memset(buf, 0, 1); // no-warning
   free(buf);
 }

clang/test/Analysis/string.c

void strcpy_effects(char *x, char *y) {
   clang_analyzer_eval(a == x[0]); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}}
 }
 
 #ifndef SUPPRESS_OUT_OF_BOUND
 void strcpy_overflow(char *y) {
   char x[4];
   if (strlen(y) == 4)
-    strcpy(x, y); // expected-warning{{String copy function overflows destination buffer}}
+    strcpy(x, y); // expected-warning{{String copy function overflows the destination buffer}}
 }
 #endif
 
 void strcpy_no_overflow(char *y) {
   char x[4];
   if (strlen(y) == 3)
     strcpy(x, y); // no-warning
 }
void stpcpy_effect(char *x, char *y) {
   clang_analyzer_eval(strlen(x) == strlen(y)); // expected-warning{{TRUE}}
   clang_analyzer_eval(a == x[0]); // expected-warning{{UNKNOWN}}
 }
 
 #ifndef SUPPRESS_OUT_OF_BOUND
 void stpcpy_overflow(char *y) {
   char x[4];
   if (strlen(y) == 4)
-    stpcpy(x, y); // expected-warning{{String copy function overflows destination buffer}}
+    stpcpy(x, y); // expected-warning{{String copy function overflows the destination buffer}}
 }
 #endif
 
 void stpcpy_no_overflow(char *y) {
   char x[4];
   if (strlen(y) == 3)
     stpcpy(x, y); // no-warning
 }
void strcat_effects(char *y) {
   clang_analyzer_eval(strcat(x, y) == x); // expected-warning{{TRUE}}
   clang_analyzer_eval((int)strlen(x) == (orig_len + strlen(y))); // expected-warning{{TRUE}}
 }
 
 #ifndef SUPPRESS_OUT_OF_BOUND
 void strcat_overflow_0(char *y) {
   char x[4] = "12";
   if (strlen(y) == 4)
-    strcat(x, y); // expected-warning{{String copy function overflows destination buffer}}
+    strcat(x, y); // expected-warning{{String concatenation function overflows the destination buffer}}
 }
 
 void strcat_overflow_1(char *y) {
   char x[4] = "12";
   if (strlen(y) == 3)
-    strcat(x, y); // expected-warning{{String copy function overflows destination buffer}}
+    strcat(x, y); // expected-warning{{String concatenation function overflows the destination buffer}}
 }
 
 void strcat_overflow_2(char *y) {
   char x[4] = "12";
   if (strlen(y) == 2)
-    strcat(x, y); // expected-warning{{String copy function overflows destination buffer}}
+    strcat(x, y); // expected-warning{{String concatenation function overflows the destination buffer}}
 }
 #endif
 
 void strcat_no_overflow(char *y) {
   char x[5] = "12";
   if (strlen(y) == 2)
     strcat(x, y); // no-warning
 }
void strncpy_effects(char *x, char *y) {
   clang_analyzer_eval(a == x[0]); // expected-warning{{UNKNOWN}}
 }
 
 #ifndef SUPPRESS_OUT_OF_BOUND
 // Enabling the malloc checker enables some of the buffer-checking portions
 // of the C-string checker.
 void cstringchecker_bounds_nocrash() {
   char *p = malloc(2);
-  strncpy(p, "AAA", sizeof("AAA")); // expected-warning {{Size argument is greater than the length of the destination buffer}}
+  strncpy(p, "AAA", sizeof("AAA"));
+  // expected-warning@-1 {{String copy function overflows the destination buffer}}
   free(p);
 }
 
 void strncpy_overflow(char *y) {
   char x[4];
   if (strlen(y) == 4)
-    strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
+    strncpy(x, y, 5);
+    // expected-warning@-1 {{String copy function overflows the destination buffer}}
 #ifndef VARIANT
-  // expected-warning@-2{{size argument is too large; destination buffer has size 4, but size argument is 5}}
+    // expected-warning@-3 {{size argument is too large; destination buffer has size 4, but size argument is 5}}
 #endif
 }
 
 void strncpy_no_overflow(char *y) {
   char x[4];
   if (strlen(y) == 3)
-    strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
+    strncpy(x, y, 5);
+    // expected-warning@-1 {{String copy function overflows the destination buffer}}
 #ifndef VARIANT
-  // expected-warning@-2{{size argument is too large; destination buffer has size 4, but size argument is 5}}
+    // expected-warning@-3 {{size argument is too large; destination buffer has size 4, but size argument is 5}}
 #endif
 }
 
 void strncpy_no_overflow2(char *y, int n) {
   if (n <= 4)
     return;
 
   char x[4];
   if (strlen(y) == 3)
-    strncpy(x, y, n); // expected-warning{{Size argument is greater than the length of the destination buffer}}
+    strncpy(x, y, n);
+  // expected-warning@-1 {{String copy function overflows the destination buffer}}
 }
 #endif
 
 void strncpy_truncate(char *y) {
   char x[4];
   if (strlen(y) == 4)
     strncpy(x, y, 3); // no-warning
 }
void strncat_effects(char *y) {
   clang_analyzer_eval(strncat(x, y, strlen(y)) == x); // expected-warning{{TRUE}}
   clang_analyzer_eval(strlen(x) == (orig_len + strlen(y))); // expected-warning{{TRUE}}
 }
 
 #ifndef SUPPRESS_OUT_OF_BOUND
 void strncat_overflow_0(char *y) {
   char x[4] = "12";
   if (strlen(y) == 4)
-    strncat(x, y, strlen(y)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+    strncat(x, y, strlen(y));
+  // expected-warning@-1 {{String concatenation function overflows the destination buffer}}
 }
 
 void strncat_overflow_1(char *y) {
   char x[4] = "12";
   if (strlen(y) == 3)
-    strncat(x, y, strlen(y)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+    strncat(x, y, strlen(y));
+  // expected-warning@-1 {{String concatenation function overflows the destination buffer}}
 }
 
 void strncat_overflow_2(char *y) {
   char x[4] = "12";
   if (strlen(y) == 2)
-    strncat(x, y, strlen(y)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+    strncat(x, y, strlen(y));
+  // expected-warning@-1 {{String concatenation function overflows the destination buffer}}
 }
 
 void strncat_overflow_3(char *y) {
   char x[4] = "12";
   if (strlen(y) == 4)
-    strncat(x, y, 2); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+    strncat(x, y, 2);
+  // expected-warning@-1 {{String concatenation function overflows the destination buffer}}
 }
 #endif
 
 void strncat_no_overflow_1(char *y) {
   char x[5] = "12";
   if (strlen(y) == 2)
     strncat(x, y, strlen(y)); // no-warning
 }

 
 #ifndef SUPPRESS_OUT_OF_BOUND
 void strncat_symbolic_src_length(char *src) {
   char dst[8] = "1234";
   strncat(dst, src, 3);
   clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}}
 
   char dst2[8] = "1234";
-  strncat(dst2, src, 4); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+  strncat(dst2, src, 4);
+  // expected-warning@-1 {{String concatenation function overflows the destination buffer}}
 }
 
 void strncat_unknown_src_length(char *src, int offset) {
   char dst[8] = "1234";
   strncat(dst, &src[offset], 3);
   clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}}
 
   char dst2[8] = "1234";
-  strncat(dst2, &src[offset], 4); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+  strncat(dst2, &src[offset], 4);
+  // expected-warning@-1 {{String concatenation function overflows the destination buffer}}
 }
 #endif
 
 // There is no strncat_unknown_dst_length because if we can't get a symbolic
 // length for the "before" strlen, we won't be able to set one for "after".
 
 void strncat_symbolic_limit(unsigned limit) {
   char dst[6] = "1234";

 
 void explicit_bzero3_out_ofbound() {
   char *privkey = (char *)malloc(7);
   const char newprivkey[10] = "mysafekey";
 
   strcpy(privkey, "random");
   explicit_bzero(privkey, sizeof(newprivkey));
 #ifndef SUPPRESS_OUT_OF_BOUND
-  // expected-warning@-2 {{Memory clearance function accesses out-of-bound array element}}
+  // expected-warning@-2 {{Memory clearance function overflows the destination buffer}}
 #endif
   clang_analyzer_eval(privkey[0] == '\0');
 #ifdef SUPPRESS_OUT_OF_BOUND
   // expected-warning@-2 {{UNKNOWN}}
 #endif
   free(privkey);
 }