[PATCH] D83362: Fix warning caused by __builtin_expect_with_probability was not handled in places such as constant folding

Zhi Zhuang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 7 17:27:03 PDT 2020


LukeZhuang created this revision.
LukeZhuang added reviewers: lebedev.ri, erichkeane, xbolva00, RKSimon, rsmith.
LukeZhuang added a project: clang.
Herald added subscribers: cfe-commits, martong.

Previously some places that should have handled `__builtin_expect_with_probability` is missing, so in some case it acts differently than `__builtin_expect`.
For example it was not handled in constant folding, thus in the following program, the "if" condition should be constantly true and folded, but previously it was not handled and cause warning "control may reach end of non-void function" (while `__builtin_expect` does not):

  __attribute__((noreturn)) extern void bar();
  int foo(int x, int y) {
    if (y) {
      if (__builtin_expect_with_probability(1, 1, 1))
        bar();
    }
     else
      return 0;
  }

Now it's fixed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83362

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  clang/test/Sema/builtin-expect-with-probability.cpp


Index: clang/test/Sema/builtin-expect-with-probability.cpp
===================================================================
--- clang/test/Sema/builtin-expect-with-probability.cpp
+++ clang/test/Sema/builtin-expect-with-probability.cpp
@@ -1,4 +1,16 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+
+__attribute__((noreturn)) extern void bar();
+
+int test_no_warn(int x) {
+  if (x) {
+    if (__builtin_expect_with_probability(1, 1, 1))
+      bar();
+  } else {
+    return 0;
+  }
+}  // should not emit warn "control may reach end of non-void function" here since expr is constantly true, so the "if(__bui..)" should be constantly true condition and be ignored
+
 extern int global;
 
 struct S {
Index: clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -64,10 +64,12 @@
 
   case Builtin::BI__builtin_unpredictable:
   case Builtin::BI__builtin_expect:
+  case Builtin::BI__builtin_expect_with_probability:
   case Builtin::BI__builtin_assume_aligned:
   case Builtin::BI__builtin_addressof: {
-    // For __builtin_unpredictable, __builtin_expect, and
-    // __builtin_assume_aligned, just return the value of the subexpression.
+    // For __builtin_unpredictable, __builtin_expect,
+    // __builtin_expect_with_probability and __builtin_assume_aligned,
+    // just return the value of the subexpression.
     // __builtin_addressof is going from a reference to a pointer, but those
     // are represented the same way in the analyzer.
     assert (Call.getNumArgs() > 0);
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11200,6 +11200,7 @@
   }
 
   case Builtin::BI__builtin_expect:
+  case Builtin::BI__builtin_expect_with_probability:
     return Visit(E->getArg(0));
 
   case Builtin::BI__builtin_ffs:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83362.276270.patch
Type: text/x-patch
Size: 2068 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200708/18da179f/attachment.bin>


More information about the cfe-commits mailing list