PiotrZSL updated this revision to Diff 547587.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.
Formating + fix tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157188/new/
https://reviews.llvm.org/D157188
Files:
clang-tools-extra/clang-tidy/bugprone/AllocationBoolConversionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/AllocationBoolConversionCheck.h
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/allocation-bool-conversion.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/allocation-bool-conversion.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/allocation-bool-conversion.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/allocation-bool-conversion.cpp
@@ -0,0 +1,99 @@
+// RUN: %check_clang_tidy %s bugprone-allocation-bool-conversion %t -- -config="{CheckOptions: { bugprone-allocation-bool-conversion.portability-restrict-system-includes.AllocationFunctions: 'malloc;allocate;custom' }}"
+
+typedef __SIZE_TYPE__ size_t;
+
+void takeBool(bool);
+void* operator new(size_t count);
+void *malloc(size_t size);
+
+template<typename T>
+struct Allocator {
+ typedef T* pointer;
+ pointer allocate(size_t n, const void* hint = 0);
+};
+
+void* custom();
+void* negative();
+
+static Allocator<int> allocator;
+
+void testImplicit() {
+ takeBool(negative());
+
+ takeBool(new int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(new bool);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(operator new(10));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'operator new' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(malloc(10));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'malloc' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(allocator.allocate(1U));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'allocate' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+
+ takeBool(custom());
+
+ bool value;
+
+ value = negative();
+
+ value = new int;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = new bool;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = operator new(10);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: result of the 'operator new' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = malloc(10);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: result of the 'malloc' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = allocator.allocate(1U);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: result of the 'allocate' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = custom();
+}
+
+void testExplicit() {
+ takeBool(static_cast<bool>(negative()));
+
+ takeBool(static_cast<bool>(new int));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(static_cast<bool>(new bool));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(static_cast<bool>(operator new(10)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: result of the 'operator new' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(static_cast<bool>(malloc(10)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: result of the 'malloc' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(static_cast<bool>(allocator.allocate(1U)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: result of the 'allocate' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(static_cast<bool>(custom()));
+}
+
+void testNegation() {
+ takeBool(!negative());
+
+ takeBool(!new bool);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(!new int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(!operator new(10));
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: result of the 'operator new' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(!malloc(10));
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: result of the 'malloc' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(!allocator.allocate(1U));
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: result of the 'allocate' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ takeBool(!custom());
+
+ bool value;
+
+ value = !negative();
+
+ value = !new int;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = !new bool;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = !operator new(10);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'operator new' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = !malloc(10);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'malloc' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = !allocator.allocate(1U);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'allocate' call is being used as a boolean value, which may lead to unintended behavior or resource leaks [bugprone-allocation-bool-conversion]
+ value = !custom();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -76,6 +76,7 @@
`android-cloexec-socket <android/cloexec-socket.html>`_, "Yes"
`android-comparison-in-temp-failure-retry <android/comparison-in-temp-failure-retry.html>`_,
`boost-use-to-string <boost/use-to-string.html>`_, "Yes"
+ `bugprone-allocation-bool-conversion <bugprone/allocation-bool-conversion.html>`_,
`bugprone-argument-comment <bugprone/argument-comment.html>`_, "Yes"
`bugprone-assert-side-effect <bugprone/assert-side-effect.html>`_,
`bugprone-assignment-in-if-condition <bugprone/assignment-in-if-condition.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/allocation-bool-conversion.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/allocation-bool-conversion.rst
@@ -0,0 +1,56 @@
+.. title:: clang-tidy - bugprone-allocation-bool-conversion
+
+bugprone-allocation-bool-conversion
+===================================
+
+Detects cases where the result of a resource allocation is used as a
+``bool``.
+
+Functions like ``new``, ``malloc``, ``fopen``, etc., dynamically allocate memory
+or resources and return pointers to the newly created objects. However, using
+these pointers directly as boolean values, either through implicit or explicit
+conversion, a hidden problem emerges. The crux of the issue lies in the fact
+that any non-null pointer implicitly converts to ``true``, masking potential
+errors and making the code difficult to comprehend. Worse yet, it may trigger
+memory leaks, as dynamically allocated resources remain inaccessible, never to
+be released by the program.
+
+Example:
+
+.. code-block:: c++
+
+ #include <iostream>
+
+ bool processResource(bool resourceFlag) {
+ if (resourceFlag) {
+ std::cout << "Resource processing successful!" << std::endl;
+ return true;
+ } else {
+ std::cout << "Resource processing failed!" << std::endl;
+ return false;
+ }
+ }
+
+ int main() {
+ // Implicit conversion of int* to bool
+ processResource(new int());
+ return 0;
+ }
+
+In this example, we pass the result of the ``new int()`` expression directly to
+the ``processResource`` function as its argument. Since the pointer returned by
+``new`` is non-null, it is implicitly converted to ``true``, leading to
+incorrect behavior in the ``processResource`` function.
+
+Check does not offer any auto-fixes.
+
+Options
+-------
+
+.. option:: AllocationFunctions
+
+ Custom functions considered as allocators can be specified using a
+ semicolon-separated list of (fully qualified) function names or regular
+ expressions matched against the called function names. Default value:
+ `malloc;calloc;realloc;strdup;fopen;fdopen;freopen;opendir;fdopendir;popen;
+ mmap;allocate`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -116,6 +116,11 @@
New checks
^^^^^^^^^^
+- New :doc:`bugprone-allocation-bool-conversion
+ <clang-tidy/checks/bugprone/allocation-bool-conversion>` check.
+
+ Detects cases where the result of a resource allocation is used as a ``bool``.
+
- New :doc:`bugprone-inc-dec-in-conditions
<clang-tidy/checks/bugprone/inc-dec-in-conditions>` check.
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -4,6 +4,7 @@
)
add_clang_library(clangTidyBugproneModule
+ AllocationBoolConversionCheck.cpp
ArgumentCommentCheck.cpp
AssertSideEffectCheck.cpp
AssignmentInIfConditionCheck.cpp
@@ -22,7 +23,6 @@
ForwardingReferenceOverloadCheck.cpp
ImplicitWideningOfMultiplicationResultCheck.cpp
InaccurateEraseCheck.cpp
- SwitchMissingDefaultCaseCheck.cpp
IncDecInConditionsCheck.cpp
IncorrectRoundingsCheck.cpp
InfiniteLoopCheck.cpp
@@ -66,6 +66,7 @@
SuspiciousSemicolonCheck.cpp
SuspiciousStringCompareCheck.cpp
SwappedArgumentsCheck.cpp
+ SwitchMissingDefaultCaseCheck.cpp
TerminatingContinueCheck.cpp
ThrowKeywordMissingCheck.cpp
TooSmallLoopVariableCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "../cppcoreguidelines/NarrowingConversionsCheck.h"
+#include "AllocationBoolConversionCheck.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
#include "AssignmentInIfConditionCheck.h"
@@ -91,6 +92,8 @@
class BugproneModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<AllocationBoolConversionCheck>(
+ "bugprone-allocation-bool-conversion");
CheckFactories.registerCheck<ArgumentCommentCheck>(
"bugprone-argument-comment");
CheckFactories.registerCheck<AssertSideEffectCheck>(
Index: clang-tools-extra/clang-tidy/bugprone/AllocationBoolConversionCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/AllocationBoolConversionCheck.h
@@ -0,0 +1,35 @@
+//===--- AllocationBoolConversionCheck.h - clang-tidy -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ALLOCATIONBOOLCONVERSIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ALLOCATIONBOOLCONVERSIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include <vector>
+
+namespace clang::tidy::bugprone {
+
+/// Detects cases where the result of a resource allocation is used as a
+/// ``bool``.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/allocation-bool-conversion.html
+class AllocationBoolConversionCheck : public ClangTidyCheck {
+public:
+ AllocationBoolConversionCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ std::vector<llvm::StringRef> AllocationFunctions;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ALLOCATIONBOOLCONVERSIONCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/AllocationBoolConversionCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/AllocationBoolConversionCheck.cpp
@@ -0,0 +1,64 @@
+//===--- AllocationBoolConversionCheck.cpp - clang-tidy -------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AllocationBoolConversionCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+AllocationBoolConversionCheck::AllocationBoolConversionCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ AllocationFunctions(utils::options::parseStringList(
+ Options.get("AllocationFunctions",
+ "malloc;calloc;realloc;strdup;fopen;fdopen;freopen;"
+ "opendir;fdopendir;popen;mmap;allocate"))) {}
+
+void AllocationBoolConversionCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "AllocationFunctions",
+ utils::options::serializeStringList(AllocationFunctions));
+}
+
+void AllocationBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ castExpr(
+ hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ignoringImplicit(anyOf(
+ cxxNewExpr(),
+ callExpr(callee(functionDecl(anyOf(hasAnyOverloadedOperatorName(
+ "new", "new[]"),
+ matchers::matchesAnyListedName(
+ AllocationFunctions)))
+ .bind("func")))))))
+ .bind("cast"),
+ this);
+}
+
+void AllocationBoolConversionCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MatchedExpr = Result.Nodes.getNodeAs<CastExpr>("cast");
+
+ if (const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("func")) {
+ diag(MatchedExpr->getExprLoc(),
+ "result of the %0 call is being used as a boolean value, which "
+ "may lead to unintended behavior or resource leaks")
+ << Function;
+ } else {
+ diag(MatchedExpr->getExprLoc(),
+ "result of the 'new' expression is being used as a boolean value, "
+ "which may lead to unintended behavior or resource leaks");
+ }
+}
+
+} // namespace clang::tidy::bugprone
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
