[PATCH] D45601: Warn on bool* to bool conversion

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya created this revision.
hiraditya added reviewers: Eugene.Zelenko, rsmith.

We found conversion from bool* to bool to be a common source of bug, so we have implemented this warning. Sample use case:

  int bar(bool b) {
    return b;
  }
  
  int baz() {
    bool *b; 
    bar(b);
    return 0;
  }

Typically, there would be a function which takes a bool, which gets a pointer to boolean at the call site. The compiler currently does not warn which results
in a difficult to debug runtime failure.


https://reviews.llvm.org/D45601

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
  test/Sema/static-init.c
  test/SemaCXX/warn-bool-ptr-to-bool.cpp


Index: test/SemaCXX/warn-bool-ptr-to-bool.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/warn-bool-ptr-to-bool.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int foo(bool *b) {
+  if (b)
+    // expected-warning at -1 {{comparing 'bool *' as a boolean}}
+    return 10;
+  return 0;
+}
+
+
+int bar(bool b) {
+  return b;
+}
+
+int baz() {
+  bool *b;
+  bar(b);
+  // expected-warning at -1 {{comparing 'bool *' as a boolean}}
+  return 0;
+}
+
+
+
Index: test/Sema/static-init.c
===================================================================
--- test/Sema/static-init.c
+++ test/Sema/static-init.c
@@ -7,7 +7,7 @@
 
 float r  = (float) (intptr_t) &r; // expected-error {{initializer element is not a compile-time constant}}
 intptr_t s = (intptr_t) &s;
-_Bool t = &t;
+_Bool t = &t; // expected-warning {{comparing '_Bool *' as a boolean}}
 
 
 union bar {
Index: test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
===================================================================
--- test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
+++ test/CXX/expr/expr.unary/expr.unary.op/p6.cpp
@@ -16,6 +16,7 @@
 
 // --  pointer, 
 bool b6 = !&b4; // expected-warning{{address of 'b4' will always evaluate to 'true'}}
+// expected-warning at -1 {{comparing 'bool *' as a boolean}}
 void f();
 bool b61 = !&f;
 
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -9610,6 +9610,12 @@
       S.DiagnoseAlwaysNonNullPointer(E, Expr::NPCK_NotNull, /*IsEqual*/ false,
                                      SourceRange(CC));
     }
+    if (Source->isPointerType() || Source->canDecayToPointerType()) {
+      QualType PointeeType = Source->getPointeeType();
+      // Warn on bool* to bool conversion.
+      if (!PointeeType.isNull() && PointeeType->isBooleanType())
+        return DiagnoseImpCast(S, E, T, CC, diag::warn_pointer_to_bool);
+    }
   }
 
   // Check implicit casts from Objective-C collection literals to specialized
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7801,6 +7801,9 @@
   "format specifies type %0 but the argument has "
   "%select{type|underlying type}2 %1">,
   InGroup<FormatPedantic>;
+def warn_pointer_to_bool : Warning<
+  "comparing %0 as a boolean">,
+  InGroup<Extra>;
 def warn_format_argument_needs_cast : Warning<
   "%select{values of type|enum values with underlying type}2 '%0' should not "
   "be used as format arguments; add an explicit cast to %1 instead">,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45601.142295.patch
Type: text/x-patch
Size: 2746 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180412/1592c265/attachment-0001.bin>