[PATCH] D108003: [Clang] Extend -Wbool-operation to warn for bitwise and of bools with side effects

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 created this revision.
xbolva00 added reviewers: hans, nickdesaulniers, aaron.ballman, rsmith.
xbolva00 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Motivation: https://arstechnica.com/gadgets/2021/07/google-pushed-a-one-character-typo-to-production-bricking-chrome-os-devices/

Warn for pattern boolA & boolB where boolB has possible side effects.

Fixes https://bugs.llvm.org/show_bug.cgi?id=51216


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108003

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/warn-bitwise-and-bool.c


Index: clang/test/Sema/warn-bitwise-and-bool.c
===================================================================
--- /dev/null
+++ clang/test/Sema/warn-bitwise-and-bool.c
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wbool-operation %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wbool-operation %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#ifdef __cplusplus
+typedef bool boolean;
+#else
+typedef _Bool boolean;
+#endif
+
+boolean foo(void);
+boolean bar(void);
+boolean baz(void) __attribute__((const));
+
+#define FOO foo();
+
+void test(boolean a, boolean b, int i) {
+  b = a & b;
+  b = a & foo();      // expected-warning {{bitwise and of a boolean expression; did you mean logical and?}}
+                      // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:10}:"&&"
+  b = foo() & bar();  // expected-warning {{bitwise and of a boolean expression; did you mean logical and?}}
+                      // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:14}:"&&"
+  b = foo() & !bar(); // expected-warning {{bitwise and of a boolean expression; did you mean logical and?}}
+                      // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:14}:"&&"
+  b = a & baz();
+  b = a & FOO;    // expected-warning {{bitwise and of a boolean expression; did you mean logical and?}}
+                  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:10}:"&&"
+  i = !b & foo(); // expected-warning {{bitwise and of a boolean expression; did you mean logical and?}}
+                  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:11}:"&&"
+  b = bar() & (i > 4);
+  b = (i == 7) & foo(); // expected-warning {{bitwise and of a boolean expression; did you mean logical and?}}
+#ifdef __cplusplus
+  b = a and foo();
+#endif
+}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13073,6 +13073,15 @@
           << OrigE->getSourceRange() << T->isBooleanType()
           << FixItHint::CreateReplacement(UO->getBeginLoc(), "!");
 
+  if (const auto *BO = dyn_cast<BinaryOperator>(SourceExpr))
+    if (BO->getOpcode() == BO_And &&
+        BO->getLHS()->isKnownToHaveBooleanValue() &&
+        BO->getRHS()->isKnownToHaveBooleanValue() &&
+        BO->getRHS()->HasSideEffects(S.Context))
+      S.Diag(BO->getBeginLoc(), diag::warn_bitwise_and_bool)
+          << OrigE->getSourceRange()
+          << FixItHint::CreateReplacement(BO->getOperatorLoc(), "&&");
+
   // For conditional operators, we analyze the arguments as if they
   // were being fed directly into the output.
   if (auto *CO = dyn_cast<AbstractConditionalOperator>(SourceExpr)) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7424,7 +7424,10 @@
 def warn_bitwise_negation_bool : Warning<
   "bitwise negation of a boolean expression%select{;| always evaluates to 'true';}0 "
   "did you mean logical negation?">,
-  InGroup<DiagGroup<"bool-operation">>;
+  InGroup<BoolOperationNegation>;
+def warn_bitwise_and_bool : Warning<
+  "bitwise and of a boolean expression; did you mean logical and?">,
+  InGroup<BoolOperationAnd>;
 def err_decrement_bool : Error<"cannot decrement expression of type bool">;
 def warn_increment_bool : Warning<
   "incrementing expression of type bool is deprecated and "
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -64,6 +64,10 @@
 def SignConversion : DiagGroup<"sign-conversion">;
 def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;
 def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;
+def BoolOperationNegation : DiagGroup<"bool-operation-negation">;
+def BoolOperationAnd : DiagGroup<"bool-operation-and">;
+def BoolOperation : DiagGroup<"bool-operation", [BoolOperationNegation,
+                                                 BoolOperationAnd]>;
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
                                                    UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108003.366120.patch
Type: text/x-patch
Size: 4695 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210812/339f3fe3/attachment-0001.bin>