Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[analyzer] Fix "sprintf" parameter modeling in CStringChecker #74345

Closed

Conversation

steakhal
Copy link
Contributor

@steakhal steakhal commented Dec 4, 2023

Review the commits one by one. I plan to merge them manually by pushing both of these at once.
This PR intends to fix #74269.

This change only uplifts existing APIs, without any semantic changes.
This is the continuation of 4482063.

Benefits of using CallEvents over CallExprs:
The callee decl is traced through function pointers if possible.
This will be important to fix llvm#74269 in a follow-up patch.
`CE->getCalleeDecl()` returns `VarDecl` if the callee is actually a
function pointer variable. Consequently, calling `getAsFunction()` will
return null.

To workaround the case, we should use the `CallEvent::parameters()`,
which will internally recover the function being called and do the right
thing.

Fixes llvm#74269
Depends on "[analyzer][NFC] Prefer CallEvent over CallExpr in APIs"
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Dec 4, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 4, 2023

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Balazs Benics (steakhal)

Changes

Review the commits one by one. I plan to merge them manually by pushing both of these at once.
This PR intends to fix #74269.


Patch is 40.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/74345.diff

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (+203-179)
  • (modified) clang/test/Analysis/string.cpp (+21-3)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 31f5b03dcdeba..b7b64c3da4f6c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -121,7 +121,7 @@ class CStringChecker : public Checker< eval::Call,
                        const CallEvent *Call) const;
 
   using FnCheck = std::function<void(const CStringChecker *, CheckerContext &,
-                                     const CallExpr *)>;
+                                     const CallEvent &)>;
 
   CallDescriptionMap<FnCheck> Callbacks = {
       {{CDF_MaybeBuiltin, {"memcpy"}, 3},
@@ -173,56 +173,53 @@ class CStringChecker : public Checker< eval::Call,
       StdCopyBackward{{"std", "copy_backward"}, 3};
 
   FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const;
-  void evalMemcpy(CheckerContext &C, const CallExpr *CE, CharKind CK) const;
-  void evalMempcpy(CheckerContext &C, const CallExpr *CE, CharKind CK) const;
-  void evalMemmove(CheckerContext &C, const CallExpr *CE, CharKind CK) const;
-  void evalBcopy(CheckerContext &C, const CallExpr *CE) const;
-  void evalCopyCommon(CheckerContext &C, const CallExpr *CE,
+  void evalMemcpy(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
+  void evalMempcpy(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
+  void evalMemmove(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
+  void evalBcopy(CheckerContext &C, const CallEvent &Call) const;
+  void evalCopyCommon(CheckerContext &C, const CallEvent &Call,
                       ProgramStateRef state, SizeArgExpr Size,
                       DestinationArgExpr Dest, SourceArgExpr Source,
                       bool Restricted, bool IsMempcpy, CharKind CK) const;
 
-  void evalMemcmp(CheckerContext &C, const CallExpr *CE, CharKind CK) const;
+  void evalMemcmp(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
 
-  void evalstrLength(CheckerContext &C, const CallExpr *CE) const;
-  void evalstrnLength(CheckerContext &C, const CallExpr *CE) const;
-  void evalstrLengthCommon(CheckerContext &C,
-                           const CallExpr *CE,
+  void evalstrLength(CheckerContext &C, const CallEvent &Call) const;
+  void evalstrnLength(CheckerContext &C, const CallEvent &Call) const;
+  void evalstrLengthCommon(CheckerContext &C, const CallEvent &Call,
                            bool IsStrnlen = false) const;
 
-  void evalStrcpy(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrncpy(CheckerContext &C, const CallExpr *CE) const;
-  void evalStpcpy(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrlcpy(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, bool ReturnEnd,
-                        bool IsBounded, ConcatFnKind appendK,
+  void evalStrcpy(CheckerContext &C, const CallEvent &Call) const;
+  void evalStrncpy(CheckerContext &C, const CallEvent &Call) const;
+  void evalStpcpy(CheckerContext &C, const CallEvent &Call) const;
+  void evalStrlcpy(CheckerContext &C, const CallEvent &Call) const;
+  void evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
+                        bool ReturnEnd, bool IsBounded, ConcatFnKind appendK,
                         bool returnPtr = true) const;
 
-  void evalStrcat(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrncat(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrlcat(CheckerContext &C, const CallExpr *CE) const;
+  void evalStrcat(CheckerContext &C, const CallEvent &Call) const;
+  void evalStrncat(CheckerContext &C, const CallEvent &Call) const;
+  void evalStrlcat(CheckerContext &C, const CallEvent &Call) const;
 
-  void evalStrcmp(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrncmp(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrcasecmp(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrncasecmp(CheckerContext &C, const CallExpr *CE) const;
-  void evalStrcmpCommon(CheckerContext &C,
-                        const CallExpr *CE,
-                        bool IsBounded = false,
-                        bool IgnoreCase = false) const;
+  void evalStrcmp(CheckerContext &C, const CallEvent &Call) const;
+  void evalStrncmp(CheckerContext &C, const CallEvent &Call) const;
+  void evalStrcasecmp(CheckerContext &C, const CallEvent &Call) const;
+  void evalStrncasecmp(CheckerContext &C, const CallEvent &Call) const;
+  void evalStrcmpCommon(CheckerContext &C, const CallEvent &Call,
+                        bool IsBounded = false, bool IgnoreCase = false) const;
 
-  void evalStrsep(CheckerContext &C, const CallExpr *CE) const;
+  void evalStrsep(CheckerContext &C, const CallEvent &Call) const;
 
-  void evalStdCopy(CheckerContext &C, const CallExpr *CE) const;
-  void evalStdCopyBackward(CheckerContext &C, const CallExpr *CE) const;
-  void evalStdCopyCommon(CheckerContext &C, const CallExpr *CE) const;
-  void evalMemset(CheckerContext &C, const CallExpr *CE) const;
-  void evalBzero(CheckerContext &C, const CallExpr *CE) const;
+  void evalStdCopy(CheckerContext &C, const CallEvent &Call) const;
+  void evalStdCopyBackward(CheckerContext &C, const CallEvent &Call) const;
+  void evalStdCopyCommon(CheckerContext &C, const CallEvent &Call) const;
+  void evalMemset(CheckerContext &C, const CallEvent &Call) const;
+  void evalBzero(CheckerContext &C, const CallEvent &Call) const;
 
-  void evalSprintf(CheckerContext &C, const CallExpr *CE) const;
-  void evalSnprintf(CheckerContext &C, const CallExpr *CE) const;
-  void evalSprintfCommon(CheckerContext &C, const CallExpr *CE, bool IsBounded,
-                         bool IsBuiltin) const;
+  void evalSprintf(CheckerContext &C, const CallEvent &Call) const;
+  void evalSnprintf(CheckerContext &C, const CallEvent &Call) const;
+  void evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
+                         bool IsBounded, bool IsBuiltin) const;
 
   // Utility methods
   std::pair<ProgramStateRef , ProgramStateRef >
@@ -1291,7 +1288,7 @@ bool CStringChecker::memsetAux(const Expr *DstBuffer, SVal CharVal,
 // evaluation of individual function calls.
 //===----------------------------------------------------------------------===//
 
-void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalCopyCommon(CheckerContext &C, const CallEvent &Call,
                                     ProgramStateRef state, SizeArgExpr Size,
                                     DestinationArgExpr Dest,
                                     SourceArgExpr Source, bool Restricted,
@@ -1313,7 +1310,8 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
   // 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);
+    stateZeroSize =
+        stateZeroSize->BindExpr(Call.getOriginExpr(), LCtx, destVal);
     C.addTransition(stateZeroSize);
     return;
   }
@@ -1361,15 +1359,15 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
       // 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());
+        lastElement = C.getSValBuilder().conjureSymbolVal(
+            nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
 
       // The byte after the last byte copied is the return value.
-      state = state->BindExpr(CE, LCtx, lastElement);
+      state = state->BindExpr(Call.getOriginExpr(), 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);
+      state = state->BindExpr(Call.getOriginExpr(), LCtx, destVal);
     }
 
     // Invalidate the destination (regular invalidation without pointer-escaping
@@ -1391,69 +1389,69 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
   }
 }
 
-void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalMemcpy(CheckerContext &C, const CallEvent &Call,
                                 CharKind CK) const {
   // void *memcpy(void *restrict dst, const void *restrict src, size_t n);
   // The return value is the address of the destination buffer.
-  DestinationArgExpr Dest = {{CE->getArg(0), 0}};
-  SourceArgExpr Src = {{CE->getArg(1), 1}};
-  SizeArgExpr Size = {{CE->getArg(2), 2}};
+  DestinationArgExpr Dest = {{Call.getArgExpr(0), 0}};
+  SourceArgExpr Src = {{Call.getArgExpr(1), 1}};
+  SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
 
   ProgramStateRef State = C.getState();
 
   constexpr bool IsRestricted = true;
   constexpr bool IsMempcpy = false;
-  evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy, CK);
+  evalCopyCommon(C, Call, State, Size, Dest, Src, IsRestricted, IsMempcpy, CK);
 }
 
-void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalMempcpy(CheckerContext &C, const CallEvent &Call,
                                  CharKind CK) 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.
-  DestinationArgExpr Dest = {{CE->getArg(0), 0}};
-  SourceArgExpr Src = {{CE->getArg(1), 1}};
-  SizeArgExpr Size = {{CE->getArg(2), 2}};
+  DestinationArgExpr Dest = {{Call.getArgExpr(0), 0}};
+  SourceArgExpr Src = {{Call.getArgExpr(1), 1}};
+  SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
 
   constexpr bool IsRestricted = true;
   constexpr bool IsMempcpy = true;
-  evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
-                 CK);
+  evalCopyCommon(C, Call, C.getState(), Size, Dest, Src, IsRestricted,
+                 IsMempcpy, CK);
 }
 
-void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalMemmove(CheckerContext &C, const CallEvent &Call,
                                  CharKind CK) const {
   // void *memmove(void *dst, const void *src, size_t n);
   // The return value is the address of the destination buffer.
-  DestinationArgExpr Dest = {{CE->getArg(0), 0}};
-  SourceArgExpr Src = {{CE->getArg(1), 1}};
-  SizeArgExpr Size = {{CE->getArg(2), 2}};
+  DestinationArgExpr Dest = {{Call.getArgExpr(0), 0}};
+  SourceArgExpr Src = {{Call.getArgExpr(1), 1}};
+  SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
 
   constexpr bool IsRestricted = false;
   constexpr bool IsMempcpy = false;
-  evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
-                 CK);
+  evalCopyCommon(C, Call, C.getState(), Size, Dest, Src, IsRestricted,
+                 IsMempcpy, CK);
 }
 
-void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalBcopy(CheckerContext &C, const CallEvent &Call) const {
   // void bcopy(const void *src, void *dst, size_t n);
-  SourceArgExpr Src{{CE->getArg(0), 0}};
-  DestinationArgExpr Dest = {{CE->getArg(1), 1}};
-  SizeArgExpr Size = {{CE->getArg(2), 2}};
+  SourceArgExpr Src{{Call.getArgExpr(0), 0}};
+  DestinationArgExpr Dest = {{Call.getArgExpr(1), 1}};
+  SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
 
   constexpr bool IsRestricted = false;
   constexpr bool IsMempcpy = false;
-  evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
-                 CharKind::Regular);
+  evalCopyCommon(C, Call, C.getState(), Size, Dest, Src, IsRestricted,
+                 IsMempcpy, CharKind::Regular);
 }
 
-void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalMemcmp(CheckerContext &C, const CallEvent &Call,
                                 CharKind CK) const {
   // int memcmp(const void *s1, const void *s2, size_t n);
   CurrentFunctionDescription = "memory comparison function";
 
-  AnyArgExpr Left = {CE->getArg(0), 0};
-  AnyArgExpr Right = {CE->getArg(1), 1};
-  SizeArgExpr Size = {{CE->getArg(2), 2}};
+  AnyArgExpr Left = {Call.getArgExpr(0), 0};
+  AnyArgExpr Right = {Call.getArgExpr(1), 1};
+  SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
 
   ProgramStateRef State = C.getState();
   SValBuilder &Builder = C.getSValBuilder();
@@ -1471,7 +1469,8 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE,
   // have to check either of the buffers.
   if (stateZeroSize) {
     State = stateZeroSize;
-    State = State->BindExpr(CE, LCtx, Builder.makeZeroVal(CE->getType()));
+    State = State->BindExpr(Call.getOriginExpr(), LCtx,
+                            Builder.makeZeroVal(Call.getResultType()));
     C.addTransition(State);
   }
 
@@ -1497,8 +1496,8 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE,
       State = SameBuffer;
       State = CheckBufferAccess(C, State, Left, Size, AccessKind::read);
       if (State) {
-        State =
-            SameBuffer->BindExpr(CE, LCtx, Builder.makeZeroVal(CE->getType()));
+        State = SameBuffer->BindExpr(Call.getOriginExpr(), LCtx,
+                                     Builder.makeZeroVal(Call.getResultType()));
         C.addTransition(State);
       }
       return;
@@ -1511,33 +1510,35 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE,
     State = CheckBufferAccess(C, State, Left, Size, AccessKind::read, CK);
     if (State) {
       // The return value is the comparison result, which we don't know.
-      SVal CmpV = Builder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
-      State = State->BindExpr(CE, LCtx, CmpV);
+      SVal CmpV = Builder.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx,
+                                           C.blockCount());
+      State = State->BindExpr(Call.getOriginExpr(), LCtx, CmpV);
       C.addTransition(State);
     }
   }
 }
 
 void CStringChecker::evalstrLength(CheckerContext &C,
-                                   const CallExpr *CE) const {
+                                   const CallEvent &Call) const {
   // size_t strlen(const char *s);
-  evalstrLengthCommon(C, CE, /* IsStrnlen = */ false);
+  evalstrLengthCommon(C, Call, /* IsStrnlen = */ false);
 }
 
 void CStringChecker::evalstrnLength(CheckerContext &C,
-                                    const CallExpr *CE) const {
+                                    const CallEvent &Call) const {
   // size_t strnlen(const char *s, size_t maxlen);
-  evalstrLengthCommon(C, CE, /* IsStrnlen = */ true);
+  evalstrLengthCommon(C, Call, /* IsStrnlen = */ true);
 }
 
-void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalstrLengthCommon(CheckerContext &C,
+                                         const CallEvent &Call,
                                          bool IsStrnlen) const {
   CurrentFunctionDescription = "string length function";
   ProgramStateRef state = C.getState();
   const LocationContext *LCtx = C.getLocationContext();
 
   if (IsStrnlen) {
-    const Expr *maxlenExpr = CE->getArg(1);
+    const Expr *maxlenExpr = Call.getArgExpr(1);
     SVal maxlenVal = state->getSVal(maxlenExpr, LCtx);
 
     ProgramStateRef stateZeroSize, stateNonZeroSize;
@@ -1547,8 +1548,8 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
     // If the size can be zero, the result will be 0 in that case, and we don't
     // have to check the string itself.
     if (stateZeroSize) {
-      SVal zero = C.getSValBuilder().makeZeroVal(CE->getType());
-      stateZeroSize = stateZeroSize->BindExpr(CE, LCtx, zero);
+      SVal zero = C.getSValBuilder().makeZeroVal(Call.getResultType());
+      stateZeroSize = stateZeroSize->BindExpr(Call.getOriginExpr(), LCtx, zero);
       C.addTransition(stateZeroSize);
     }
 
@@ -1561,7 +1562,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
   }
 
   // Check that the string argument is non-null.
-  AnyArgExpr Arg = {CE->getArg(0), 0};
+  AnyArgExpr Arg = {Call.getArgExpr(0), 0};
   SVal ArgVal = state->getSVal(Arg.Expression, LCtx);
   state = checkNonNull(C, state, Arg, ArgVal);
 
@@ -1584,7 +1585,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
 
     // It's a little unfortunate to be getting this again,
     // but it's not that expensive...
-    const Expr *maxlenExpr = CE->getArg(1);
+    const Expr *maxlenExpr = Call.getArgExpr(1);
     SVal maxlenVal = state->getSVal(maxlenExpr, LCtx);
 
     std::optional<NonLoc> strLengthNL = strLength.getAs<NonLoc>();
@@ -1613,8 +1614,8 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
       // no guarantee the full string length will actually be returned.
       // All we know is the return value is the min of the string length
       // and the limit. This is better than nothing.
-      result = C.getSValBuilder().conjureSymbolVal(nullptr, CE, LCtx,
-                                                   C.blockCount());
+      result = C.getSValBuilder().conjureSymbolVal(
+          nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
       NonLoc resultNL = result.castAs<NonLoc>();
 
       if (strLengthNL) {
@@ -1637,78 +1638,85 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
     // If we don't know the length of the string, conjure a return
     // value, so it can be used in constraints, at least.
     if (result.isUnknown()) {
-      result = C.getSValBuilder().conjureSymbolVal(nullptr, CE, LCtx,
-                                                   C.blockCount());
+      result = C.getSValBuilder().conjureSymbolVal(
+          nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
     }
   }
 
   // Bind the return value.
   assert(!result.isUnknown() && "Should have conjured a value by now");
-  state = state->BindExpr(CE, LCtx, result);
+  state = state->BindExpr(Call.getOriginExpr(), LCtx, result);
   C.addTransition(state);
 }
 
-void CStringChecker::evalStrcpy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStrcpy(CheckerContext &C,
+                                const CallEvent &Call) const {
   // char *strcpy(char *restrict dst, const char *restrict src);
-  evalStrcpyCommon(C, CE,
+  evalStrcpyCommon(C, Call,
                    /* ReturnEnd = */ false,
                    /* IsBounded = */ false,
                    /* appendK = */ ConcatFnKind::none);
 }
 
-void CStringChecker::evalStrncpy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStrncpy(CheckerContext &C,
+                                 const CallEvent &Call) const {
   // char *strncpy(char *restrict dst, const char *restrict src, size_t n);
-  evalStrcpyCommon(C, CE,
+  evalStrcpyCommon(C, Call,
                    /* ReturnEnd = */ false,
                    /* IsBounded = */ true,
                    /* appendK = */ ConcatFnKind::none);
 }
 
-void CStringChecker::evalStpcpy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStpcpy(CheckerContext &C,
+                                const CallEvent &Call) const {
   // char *stpcpy(char *restrict dst, const char *restrict src);
-  evalStrcpyCommon(C, CE,
+  evalStrcpyCommon(C, Call,
                    /* ReturnEnd = */ true,
                    /* IsBounded = */ false,
                    /* appendK = */ ConcatFnKind::none);
 }
 
-void CStringChecker::evalStrlcpy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStrlcpy(CheckerContext &C,
+                                 const CallEvent &Call) const {
   // size_t strlcpy(char *dest, const char *src, size_t size);
-  evalStrcpyCommon(C, CE,
+  evalStrcpyCommon(C, Call,
                    /* ReturnEnd = */ true,
                    /* IsBounded = */ true,
                    /* appendK = */ ConcatFnKind::none,
                    /* returnPtr = */ false);
 }
 
-void CStringChecker::evalStrcat(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStr...
[truncated]

Copy link
Contributor

@NagyDonat NagyDonat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonable semi-automatic refactoring + a small change to fix a nasty crash. It's good to see that you separated the NFC and the bugfix into two commits, it'll make the git history nicer.

It's a bit unfortunate that there are many locations where CE is replaced by the more verbose Call.getOriginExpr(), but I guess that's the price of this improvement.


// Test functions that are called "memcpy" but aren't the memcpy
// we're looking for. Unfortunately, this test cannot be put into
// a namespace. The out-of-class weird memcpy needs to be recognized
// as a normal C function for the test to make sense.
typedef __typeof(sizeof(int)) size_t;
void *memcpy(void *, const void *, size_t);
int sprintf(char *str, const char *format, ...);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bikeshedding: add a blank line above this to emphasize that they're independent from the comment block that's above them.

Personally I slightly prefer the style when the mock library declarations appear directly above the testcase that's using them when there's just one testcase that's using them (and the test file is not brutally large); but this "declare everything at the top" style is also OK and has its own advantages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the newline.

@steakhal
Copy link
Contributor Author

Manually merged as:

@steakhal steakhal closed this Dec 28, 2023
@steakhal steakhal deleted the fix-gh-74269-callee-params-of-vardecl branch December 28, 2023 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-tidy crash in clang::FunctionDecl::getNumParams()
3 participants