Skip to content

[analyzer][NFC] Fix clang-tidy warning in Malloc and UnixApi checkers #145719

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vbvictor
Copy link
Contributor

Mostly else-after-return and else-after-continue warnings

@vbvictor vbvictor requested review from steakhal and NagyDonat June 25, 2025 15:15
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Jun 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2025

@llvm/pr-subscribers-clang

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

Author: Baranov Victor (vbvictor)

Changes

Mostly else-after-return and else-after-continue warnings


Full diff: https://github.com/llvm/llvm-project/pull/145719.diff

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+16-19)
  • (modified) clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp (+8-10)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index fa6e8e4146804..dd5000ea7d734 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -34,7 +34,7 @@
 //       Depends on NewDeleteChecker.
 //
 //   * MismatchedDeallocatorChecker
-//       Enables checking whether memory is deallocated with the correspending
+//       Enables checking whether memory is deallocated with the corresponding
 //       allocation function in MallocChecker, such as malloc() allocated
 //       regions are only freed by free(), new by delete, new[] by delete[].
 //
@@ -1372,8 +1372,8 @@ void MallocChecker::checkIfFreeNameIndex(ProgramStateRef State,
   C.addTransition(State);
 }
 
-const Expr *getPlacementNewBufferArg(const CallExpr *CE,
-                                     const FunctionDecl *FD) {
+static const Expr *getPlacementNewBufferArg(const CallExpr *CE,
+                                            const FunctionDecl *FD) {
   // Checking for signature:
   // void* operator new  ( std::size_t count, void* ptr );
   // void* operator new[]( std::size_t count, void* ptr );
@@ -1682,17 +1682,15 @@ ProgramStateRef MallocChecker::ProcessZeroAllocCheck(
     const RefState *RS = State->get<RegionState>(Sym);
     if (RS) {
       if (RS->isAllocated())
-        return TrueState->set<RegionState>(Sym,
-                                          RefState::getAllocatedOfSizeZero(RS));
-      else
-        return State;
-    } else {
-      // Case of zero-size realloc. Historically 'realloc(ptr, 0)' is treated as
-      // 'free(ptr)' and the returned value from 'realloc(ptr, 0)' is not
-      // tracked. Add zero-reallocated Sym to the state to catch references
-      // to zero-allocated memory.
-      return TrueState->add<ReallocSizeZeroSymbols>(Sym);
+        return TrueState->set<RegionState>(
+            Sym, RefState::getAllocatedOfSizeZero(RS));
+      return State;
     }
+    // Case of zero-size realloc. Historically 'realloc(ptr, 0)' is treated as
+    // 'free(ptr)' and the returned value from 'realloc(ptr, 0)' is not
+    // tracked. Add zero-reallocated Sym to the state to catch references
+    // to zero-allocated memory.
+    return TrueState->add<ReallocSizeZeroSymbols>(Sym);
   }
 
   // Assume the value is non-zero going forward.
@@ -1890,7 +1888,7 @@ void MallocChecker::reportTaintBug(StringRef Msg, ProgramStateRef State,
                                         "Tainted Memory Allocation",
                                         categories::TaintedData));
     auto R = std::make_unique<PathSensitiveBugReport>(*BT_TaintedAlloc, Msg, N);
-    for (auto TaintedSym : TaintedSyms) {
+    for (const auto *TaintedSym : TaintedSyms) {
       R->markInteresting(TaintedSym);
     }
     C.emitReport(std::move(R));
@@ -2280,8 +2278,9 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
 
     // If the pointer is allocated or escaped, but we are now trying to free it,
     // check that the call to free is proper.
-    } else if (RsBase->isAllocated() || RsBase->isAllocatedOfSizeZero() ||
-               RsBase->isEscaped()) {
+    }
+    if (RsBase->isAllocated() || RsBase->isAllocatedOfSizeZero() ||
+        RsBase->isEscaped()) {
 
       // Check if an expected deallocation function matches the real one.
       bool DeallocMatchesAlloc = RsBase->getAllocationFamily() == Family;
@@ -2857,9 +2856,7 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const CallEvent &Call,
 
   const CallExpr *CE = cast<CallExpr>(Call.getOriginExpr());
 
-  if (SuffixWithN && CE->getNumArgs() < 3)
-    return nullptr;
-  else if (CE->getNumArgs() < 2)
+  if ((SuffixWithN && CE->getNumArgs() < 3) || CE->getNumArgs() < 2)
     return nullptr;
 
   const Expr *arg0Expr = CE->getArg(0);
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index ce5887d56b181..e12261603757f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -223,6 +223,10 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
   // All calls should at least provide arguments up to the 'flags' parameter.
   unsigned int MinArgCount = FlagsArgIndex + 1;
 
+  // The frontend should issue a warning for this case. Just return.
+  if (Call.getNumArgs() < MinArgCount)
+    return;
+
   // If the flags has O_CREAT set then open/openat() require an additional
   // argument specifying the file mode (permission bits) for the created file.
   unsigned int CreateModeArgIndex = FlagsArgIndex + 1;
@@ -231,11 +235,7 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
   unsigned int MaxArgCount = CreateModeArgIndex + 1;
 
   ProgramStateRef state = C.getState();
-
-  if (Call.getNumArgs() < MinArgCount) {
-    // The frontend should issue a warning for this case. Just return.
-    return;
-  } else if (Call.getNumArgs() == MaxArgCount) {
+  if (Call.getNumArgs() == MaxArgCount) {
     const Expr *Arg = Call.getArgExpr(CreateModeArgIndex);
     QualType QT = Arg->getType();
     if (!QT->isIntegerType()) {
@@ -541,17 +541,15 @@ void UnixAPIPortabilityChecker::CheckCallocZero(CheckerContext &C,
     if (argVal.isUnknownOrUndef()) {
       if (i == 0)
         continue;
-      else
-        return;
+      return;
     }
 
     if (IsZeroByteAllocation(state, argVal, &trueState, &falseState)) {
       if (ReportZeroByteAllocation(C, falseState, arg, "calloc"))
         return;
-      else if (i == 0)
+      if (i == 0)
         continue;
-      else
-        return;
+      return;
     }
   }
 

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.

3 participants