Author: Dávid Bolvanský
Date: 2021-10-03T11:06:40+02:00
New Revision: f62d18ff140f67a8776a7a3c62a75645d8d540b5

URL: 
https://github.com/llvm/llvm-project/commit/f62d18ff140f67a8776a7a3c62a75645d8d540b5
DIFF: 
https://github.com/llvm/llvm-project/commit/f62d18ff140f67a8776a7a3c62a75645d8d540b5.diff

LOG: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with 
side effects

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

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

Casting one operand to int is enough to silence this warning: for example 
(int)boolA & boolB or boolA| (int)boolB

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

Differential Revision: https://reviews.llvm.org/D108003

Added: 
    clang/test/Sema/warn-bitwise-and-bool.c
    clang/test/Sema/warn-bitwise-or-bool.c

Modified: 
    clang/include/clang/Basic/DiagnosticGroups.td
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Sema/SemaChecking.cpp
    clang/test/Misc/warning-wall.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 761b323d06166..d9db3482dbda7 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -64,7 +64,8 @@ def StringConversion : DiagGroup<"string-conversion">;
 def SignConversion : DiagGroup<"sign-conversion">;
 def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;
 def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;
-def BoolOperation : DiagGroup<"bool-operation">;
+def BitwiseInsteadOfLogical : DiagGroup<"bitwise-instead-of-logical">;
+def BoolOperation : DiagGroup<"bool-operation", [BitwiseInsteadOfLogical]>;
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
                                                    UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1d4ea92c65205..c71a00b184328 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -66,6 +66,7 @@ def warn_infinite_recursive_function : Warning<
 def warn_comma_operator : Warning<"possible misuse of comma operator here">,
   InGroup<DiagGroup<"comma">>, DefaultIgnore;
 def note_cast_to_void : Note<"cast expression to void to silence warning">;
+def note_cast_operand_to_int : Note<"cast one or both operands to int to 
silence this warning">;
 
 // Constant expressions
 def err_expr_not_ice : Error<
@@ -7423,6 +7424,9 @@ def note_member_declared_here : Note<
   "member %0 declared here">;
 def note_member_first_declared_here : Note<
   "member %0 first declared here">;
+def warn_bitwise_instead_of_logical : Warning<
+  "use of bitwise '%0' with boolean operands">,
+  InGroup<BitwiseInsteadOfLogical>, DefaultIgnore;
 def warn_bitwise_negation_bool : Warning<
   "bitwise negation of a boolean expression%select{;| always evaluates to 
'true';}0 "
   "did you mean logical negation?">,

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 8b53e3504e13a..10a3f30704172 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13249,6 +13249,20 @@ static void AnalyzeImplicitConversions(
           << OrigE->getSourceRange() << T->isBooleanType()
           << FixItHint::CreateReplacement(UO->getBeginLoc(), "!");
 
+  if (const auto *BO = dyn_cast<BinaryOperator>(SourceExpr))
+    if ((BO->getOpcode() == BO_And || BO->getOpcode() == BO_Or) &&
+        BO->getLHS()->isKnownToHaveBooleanValue() &&
+        BO->getRHS()->isKnownToHaveBooleanValue() &&
+        BO->getLHS()->HasSideEffects(S.Context) &&
+        BO->getRHS()->HasSideEffects(S.Context)) {
+      S.Diag(BO->getBeginLoc(), diag::warn_bitwise_instead_of_logical)
+          << (BO->getOpcode() == BO_And ? "&" : "|") << OrigE->getSourceRange()
+          << FixItHint::CreateReplacement(
+                 BO->getOperatorLoc(),
+                 (BO->getOpcode() == BO_And ? "&&" : "||"));
+      S.Diag(BO->getBeginLoc(), diag::note_cast_operand_to_int);
+    }
+
   // For conditional operators, we analyze the arguments as if they
   // were being fed directly into the output.
   if (auto *CO = dyn_cast<AbstractConditionalOperator>(SourceExpr)) {

diff  --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c
index a3686fb96a4ce..a4a79bec934af 100644
--- a/clang/test/Misc/warning-wall.c
+++ b/clang/test/Misc/warning-wall.c
@@ -4,6 +4,7 @@ RUN: FileCheck --input-file=%t %s
      CHECK:-Wall
 CHECK-NEXT:  -Wmost
 CHECK-NEXT:    -Wbool-operation
+CHECK-NEXT:    -Wbitwise-instead-of-logical
 CHECK-NEXT:    -Wchar-subscripts
 CHECK-NEXT:    -Wcomment
 CHECK-NEXT:    -Wdelete-non-virtual-dtor

diff  --git a/clang/test/Sema/warn-bitwise-and-bool.c 
b/clang/test/Sema/warn-bitwise-and-bool.c
new file mode 100644
index 0000000000000..6bec1be1abdef
--- /dev/null
+++ b/clang/test/Sema/warn-bitwise-and-bool.c
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wbool-operation %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c -fsyntax-only -Wbitwise-instead-of-logical 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -fsyntax-only -Wbool-operation 
-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 -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wbitwise-instead-of-logical 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wbool-operation 
-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));
+void sink(boolean);
+
+#define FOO foo()
+
+void test(boolean a, boolean b, int *p, volatile int *q, int i) {
+  b = a & b;
+  b = foo() & a;
+  b = (p != 0) & (*p == 42);   // FIXME: also warn for a non-volatile pointer 
dereference
+  b = foo() & (*q == 42);      // expected-warning {{use of bitwise '&' with 
boolean operands}}
+                               // expected-note@-1 {{cast one or both operands 
to int to silence this warning}}
+  b = foo() & (int)(*q == 42); // OK, no warning expected
+  b = a & foo();
+  b = (int)a & foo();     // OK, no warning expected
+  b = foo() & bar();      // expected-warning {{use of bitwise '&' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+                          // CHECK: 
fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:14}:"&&"
+  b = foo() & (int)bar(); // OK, no warning expected
+  b = foo() & !bar();     // expected-warning {{use of bitwise '&' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+                          // CHECK: 
fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:14}:"&&"
+  b = a & baz();
+  b = bar() & FOO;        // expected-warning {{use of bitwise '&' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+                          // CHECK: 
fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:14}:"&&"
+  b = foo() & (int)FOO;   // OK, no warning expected
+  b = b & foo();
+  b = bar() & (i > 4);
+  b = (i == 7) & foo();
+#ifdef __cplusplus
+  b = foo() bitand bar(); // expected-warning {{use of bitwise '&' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+#endif
+
+  if (foo() & bar())      // expected-warning {{use of bitwise '&' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+    ;
+
+  sink(a & b);
+  sink(a & foo());
+  sink(foo() & bar());    // expected-warning {{use of bitwise '&' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+
+  int n = i + 10;
+  b = (n & (n - 1));
+}

diff  --git a/clang/test/Sema/warn-bitwise-or-bool.c 
b/clang/test/Sema/warn-bitwise-or-bool.c
new file mode 100644
index 0000000000000..ae86790901aac
--- /dev/null
+++ b/clang/test/Sema/warn-bitwise-or-bool.c
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wbool-operation %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c -fsyntax-only -Wbitwise-instead-of-logical 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -fsyntax-only -Wbool-operation 
-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 -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wbitwise-instead-of-logical 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wbool-operation 
-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));
+void sink(boolean);
+
+#define FOO foo()
+
+void test(boolean a, boolean b, int *p, volatile int *q, int i) {
+  b = a | b;
+  b = foo() | a;
+  b = (p != 0) | (*p == 42);   // FIXME: also warn for a non-volatile pointer 
dereference
+  b = foo() | (*q == 42);      // expected-warning {{use of bitwise '|' with 
boolean operands}}
+                               // expected-note@-1 {{cast one or both operands 
to int to silence this warning}}
+  b = foo() | (int)(*q == 42); // OK, no warning expected
+  b = a | foo();
+  b = (int)a | foo();     // OK, no warning expected
+  b = foo() | bar();      // expected-warning {{use of bitwise '|' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+                          // CHECK: 
fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:14}:"||"
+  b = foo() | !bar();     // expected-warning {{use of bitwise '|' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+                          // CHECK: 
fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:14}:"||"
+  b = foo() | (int)bar(); // OK, no warning expected
+  b = a | baz();
+  b = bar() | FOO;        // expected-warning {{use of bitwise '|' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+                          // CHECK: 
fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:14}:"||"
+  b = foo() | (int)FOO;   // OK, no warning expected
+  b = b | foo();
+  b = bar() | (i > 4);
+  b = (i == 7) | foo();
+#ifdef __cplusplus
+  b = foo() bitor bar();  // expected-warning {{use of bitwise '|' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+#endif
+
+  if (foo() | bar())      // expected-warning {{use of bitwise '|' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+    ;
+
+  sink(a | b);
+  sink(a | foo());
+  sink(foo() | bar());    // expected-warning {{use of bitwise '|' with 
boolean operands}}
+                          // expected-note@-1 {{cast one or both operands to 
int to silence this warning}}
+
+  int n = i + 10;
+  b = (n | (n - 1));
+}


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to