[UBSan] Diagnose assumption violation #104741

dtcxzyw · 2024-08-19T07:28:47Z

This patch extends D34590 to check assumption violations.

llvmbot · 2024-08-19T07:29:19Z

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-compiler-rt-sanitizer

@llvm/pr-subscribers-clang

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch extends D34590 to check assumption violations.

Full diff: https://github.com/llvm/llvm-project/pull/104741.diff

7 Files Affected:

(modified) clang/lib/CodeGen/CGBuiltin.cpp (+12-6)
(modified) clang/lib/CodeGen/CGStmt.cpp (+2-1)
(modified) clang/lib/CodeGen/CodeGenFunction.h (+1)
(modified) clang/test/CodeGen/ubsan-builtin-checks.c (+17)
(modified) compiler-rt/lib/ubsan/ubsan_handlers.cpp (+5-3)
(modified) compiler-rt/lib/ubsan/ubsan_handlers.h (+1)
(modified) compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp (+14-2)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f424ddaa175400..83da5d7d75be3b 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1996,16 +1996,21 @@ struct CallObjCArcUse final : EHScopeStack::Cleanup {
 
 Value *CodeGenFunction::EmitCheckedArgForBuiltin(const Expr *E,
                                                  BuiltinCheckKind Kind) {
-  assert((Kind == BCK_CLZPassedZero || Kind == BCK_CTZPassedZero)
-          && "Unsupported builtin check kind");
+  assert((Kind == BCK_CLZPassedZero || Kind == BCK_CTZPassedZero ||
+          Kind == BCK_AssumePassedFalse) &&
+         "Unsupported builtin check kind");
 
-  Value *ArgValue = EmitScalarExpr(E);
+  Value *ArgValue =
+      Kind == BCK_AssumePassedFalse ? EvaluateExprAsBool(E) : EmitScalarExpr(E);
   if (!SanOpts.has(SanitizerKind::Builtin))
     return ArgValue;
 
   SanitizerScope SanScope(this);
-  Value *Cond = Builder.CreateICmpNE(
-      ArgValue, llvm::Constant::getNullValue(ArgValue->getType()));
+  Value *Cond =
+      Kind == BCK_AssumePassedFalse
+          ? ArgValue
+          : Builder.CreateICmpNE(
+                ArgValue, llvm::Constant::getNullValue(ArgValue->getType()));
   EmitCheck(std::make_pair(Cond, SanitizerKind::Builtin),
             SanitizerHandler::InvalidBuiltin,
             {EmitCheckSourceLocation(E->getExprLoc()),
@@ -3424,7 +3429,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     if (E->getArg(0)->HasSideEffects(getContext()))
       return RValue::get(nullptr);
 
-    Value *ArgValue = EmitScalarExpr(E->getArg(0));
+    Value *ArgValue =
+        EmitCheckedArgForBuiltin(E->getArg(0), BCK_AssumePassedFalse);
     Function *FnAssume = CGM.getIntrinsic(Intrinsic::assume);
     Builder.CreateCall(FnAssume, ArgValue);
     return RValue::get(nullptr);
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 7158a06e6bc3b3..291639346d8952 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -754,7 +754,8 @@ void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) {
       const Expr *Assumption = cast<CXXAssumeAttr>(A)->getAssumption();
       if (getLangOpts().CXXAssumptions &&
           !Assumption->HasSideEffects(getContext())) {
-        llvm::Value *AssumptionVal = EvaluateExprAsBool(Assumption);
+        llvm::Value *AssumptionVal =
+            EmitCheckedArgForBuiltin(Assumption, BCK_AssumePassedFalse);
         Builder.CreateAssumption(AssumptionVal);
       }
     } break;
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 57e0b7f91e9bf8..99db330f3316e5 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -5070,6 +5070,7 @@ class CodeGenFunction : public CodeGenTypeCache {
   enum BuiltinCheckKind {
     BCK_CTZPassedZero,
     BCK_CLZPassedZero,
+    BCK_AssumePassedFalse,
   };
 
   /// Emits an argument for a call to a builtin. If the builtin sanitizer is
diff --git a/clang/test/CodeGen/ubsan-builtin-checks.c b/clang/test/CodeGen/ubsan-builtin-checks.c
index c7f6078f903bad..8535ec915ac346 100644
--- a/clang/test/CodeGen/ubsan-builtin-checks.c
+++ b/clang/test/CodeGen/ubsan-builtin-checks.c
@@ -51,3 +51,20 @@ void check_clz(int n) {
   // CHECK: call void @__ubsan_handle_invalid_builtin
   __builtin_clzg((unsigned int)n);
 }
+
+// CHECK: define{{.*}} void @check_assume
+void check_assume(int n) {
+  // CHECK: [[TOBOOL:%.*]] = icmp ne i32 [[N:%.*]], 0
+  // CHECK-NEXT: br i1 [[TOBOOL]]
+  //
+  // Handler block:
+  // CHECK: call void @__ubsan_handle_invalid_builtin
+  // CHECK-NEXT: unreachable
+  //
+  // Continuation block:
+  // CHECK: call void @llvm.assume(i1 [[TOBOOL]])
+  __builtin_assume(n);
+
+  // CHECK: call void @__ubsan_handle_invalid_builtin
+  __attribute__((assume(n)));
+}
diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp
index 27d01653f088da..44aec6b8b05cd2 100644
--- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp
+++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp
@@ -633,9 +633,11 @@ static void handleInvalidBuiltin(InvalidBuiltinData *Data, ReportOptions Opts) {
 
   ScopedReport R(Opts, Loc, ET);
 
-  Diag(Loc, DL_Error, ET,
-       "passing zero to %0, which is not a valid argument")
-    << ((Data->Kind == BCK_CTZPassedZero) ? "ctz()" : "clz()");
+  if (Data->Kind == BCK_AssumePassedFalse)
+    Diag(Loc, DL_Error, ET, "assumption is violated during execution");
+  else
+    Diag(Loc, DL_Error, ET, "passing zero to %0, which is not a valid argument")
+        << ((Data->Kind == BCK_CTZPassedZero) ? "ctz()" : "clz()");
 }
 
 void __ubsan::__ubsan_handle_invalid_builtin(InvalidBuiltinData *Data) {
diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.h b/compiler-rt/lib/ubsan/ubsan_handlers.h
index bae661a56833dd..4ffa1439a1323f 100644
--- a/compiler-rt/lib/ubsan/ubsan_handlers.h
+++ b/compiler-rt/lib/ubsan/ubsan_handlers.h
@@ -159,6 +159,7 @@ RECOVERABLE(implicit_conversion, ImplicitConversionData *Data, ValueHandle Src,
 enum BuiltinCheckKind : unsigned char {
   BCK_CTZPassedZero,
   BCK_CLZPassedZero,
+  BCK_AssumePassedFalse,
 };
 
 struct InvalidBuiltinData {
diff --git a/compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp b/compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp
index f8f564cb7baae2..3ebc8094298a0a 100644
--- a/compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp
+++ b/compiler-rt/test/ubsan/TestCases/Misc/builtins.cpp
@@ -1,8 +1,8 @@
 // REQUIRES: target={{x86_64.*}}
 //
-// RUN: %clangxx -fsanitize=builtin -w %s -O3 -o %t
+// RUN: %clangxx -fsanitize=builtin -fno-inline -w %s -O3 -o %t
 // RUN: %run %t 2>&1 | FileCheck %s --check-prefix=RECOVER
-// RUN: %clangxx -fsanitize=builtin -fno-sanitize-recover=builtin -w %s -O3 -o %t.abort
+// RUN: %clangxx -fsanitize=builtin -fno-inline -fno-sanitize-recover=builtin -w %s -O3 -o %t.abort
 // RUN: not %run %t.abort 2>&1 | FileCheck %s --check-prefix=ABORT
 
 void check_ctz(int n) {
@@ -28,8 +28,20 @@ void check_clz(int n) {
   __builtin_clzll(n);
 }
 
+void check_assume(int n) {
+  // RECOVER: builtins.cpp:[[@LINE+1]]:20: runtime error: assumption is violated during execution
+  __builtin_assume(n);
+}
+
+void check_assume_attr(int n) {
+  // RECOVER: builtins.cpp:[[@LINE+1]]:25: runtime error: assumption is violated during execution
+  __attribute__((assume(n)));
+}
+
 int main() {
   check_ctz(0);
   check_clz(0);
+  check_assume(0);
+  check_assume_attr(0);
   return 0;
 }

dtcxzyw · 2024-08-28T12:09:07Z

Ping.

dtcxzyw · 2024-09-08T08:46:48Z

Ping.

fmayer · 2024-09-09T17:48:59Z

LGTM, but would like @vitalybuka to also take a quick look.

clang/lib/CodeGen/CGBuiltin.cpp

compiler-rt/lib/ubsan/ubsan_handlers.cpp

vitalybuka · 2024-09-18T02:50:15Z

Thanks!
LGTM in general.

compiler-rt/lib/ubsan/ubsan_handlers.cpp

vitalybuka · 2024-09-24T23:06:29Z

I'm concerned about the amount of code that might break.

We have internally fsanitize=builtin enabled, as is patch will break and force us to disable the sanitizers.
I will try to run internal tests, and clang itself with a new check.

If it breaks to much, we will need to create a new sanitizers, e.g -fsanitize=assume.

vitalybuka · 2024-09-25T00:13:50Z

clang with UBSAN is fine, i am running other tests.

…109088) This patch improves error message, and fixes a copy-paste mistake in GET_REPORT_OPTIONS argument. Address comment #104741 (comment). --------- Co-authored-by: Vitaly Buka <[email protected]>

vitalybuka · 2024-09-25T04:15:10Z

Other tests are also fine.

github-actions · 2024-09-25T05:08:29Z

✅ With the latest revision this PR passed the C/C++ code formatter.

chapuni · 2024-09-26T01:09:12Z

This has triggered a failure in our private builder (aarch64-ubuntu20.04)

******************** TEST 'libFuzzer-aarch64-default-Linux :: strncmp.test' FAILED ********************
Exit Code: 1
Command Output (stderr):
--
RUN: at line 2: /home/bb/clang-aarch64/build/2/./bin/clang    -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta   --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/bb/clang-aarch64/llvm-project/compiler-rt/lib/fuzzer  -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta  /home/bb/clang-aarch64/llvm-project/compiler-rt/test/fuzzer/StrncmpTest.cpp -o /home/bb/clang-aarch64/build/2/projects/compiler-rt/test/fuzzer/AARCH64DefaultLinuxConfig/Output/strncmp.test.tmp-StrncmpTest
+ /home/bb/clang-aarch64/build/2/./bin/clang -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/bb/clang-aarch64/llvm-project/compiler-rt/lib/fuzzer -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta /home/bb/clang-aarch64/llvm-project/compiler-rt/test/fuzzer/StrncmpTest.cpp -o /home/bb/clang-aarch64/build/2/projects/compiler-rt/test/fuzzer/AARCH64DefaultLinuxConfig/Output/strncmp.test.tmp-StrncmpTest
RUN: at line 3: not  /home/bb/clang-aarch64/build/2/projects/compiler-rt/test/fuzzer/AARCH64DefaultLinuxConfig/Output/strncmp.test.tmp-StrncmpTest              -seed=2 -runs=10000000   2>&1 | FileCheck /home/bb/clang-aarch64/llvm-project/compiler-rt/test/fuzzer/strncmp.test
+ not /home/bb/clang-aarch64/build/2/projects/compiler-rt/test/fuzzer/AARCH64DefaultLinuxConfig/Output/strncmp.test.tmp-StrncmpTest -seed=2 -runs=10000000
+ FileCheck /home/bb/clang-aarch64/llvm-project/compiler-rt/test/fuzzer/strncmp.test
/home/bb/clang-aarch64/llvm-project/compiler-rt/test/fuzzer/strncmp.test:4:8: error: CHECK: expected string not found in input
CHECK: BINGO
       ^
<stdin>:1:1: note: scanning from here
INFO: Running with entropic power schedule (0xFF, 100).
^
Input file: <stdin>
Check file: /home/bb/clang-aarch64/llvm-project/compiler-rt/test/fuzzer/strncmp.test
-dump-input=help explains the following input dump.
Input was:
<<<<<<
         1: INFO: Running with entropic power schedule (0xFF, 100). 
check:4     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
         2: INFO: Seed: 2 
check:4     ~~~~~~~~~~~~~~
         3: INFO: Loaded 1 modules (12 inline 8-bit counters): 12 [0xaaaaccfe3f00, 0xaaaaccfe3f0c),  
check:4     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         4: INFO: Loaded 1 PC tables (12 PCs): 12 [0xaaaaccfe3f10,0xaaaaccfe3fd0),  
check:4     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         5: INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes 
check:4     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         6: INFO: A corpus is not provided, starting from an empty corpus 
check:4     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         .
         .
         .
>>>>>>
--
********************

dtcxzyw · 2024-09-26T02:13:51Z

-fsanitize=address,fuzzer

I think it is not related to this patch. It only works with -fsanitize=builtin or -fsanitize=undefined.

dtcxzyw requested review from MaskRay, vitalybuka and AaronBallman August 19, 2024 07:28

llvmbot added clang Clang issues not falling into any other category compiler-rt clang:codegen IR generation bugs: mangling, exceptions, etc. compiler-rt:ubsan Undefined behavior sanitizer compiler-rt:sanitizer labels Aug 19, 2024

dtcxzyw requested review from fmayer, zmodem, ilovepi and efriedma-quic September 8, 2024 08:47

vitalybuka reviewed Sep 18, 2024

View reviewed changes

clang/lib/CodeGen/CGBuiltin.cpp Show resolved Hide resolved

vitalybuka reviewed Sep 18, 2024

View reviewed changes

compiler-rt/lib/ubsan/ubsan_handlers.cpp Outdated Show resolved Hide resolved

dtcxzyw mentioned this pull request Sep 18, 2024

[Clang][compiler-rt][UBSan] Improve __ubsan_handle_invalid_builtin #109088

Merged

vitalybuka reviewed Sep 24, 2024

View reviewed changes

compiler-rt/lib/ubsan/ubsan_handlers.cpp Show resolved Hide resolved

vitalybuka self-requested a review September 24, 2024 23:47

vitalybuka approved these changes Sep 25, 2024

View reviewed changes

dtcxzyw and others added 3 commits September 25, 2024 12:27

[UBSan] Diagnose assumption violation

fdadb0f

Extract EmitCheckedArgForAssume

6cce506

format

7bcaa3b

dtcxzyw force-pushed the ubsan-check-assume branch from 0a1584d to 7bcaa3b Compare September 25, 2024 05:04

Format

78c2bcc

dtcxzyw merged commit d8f555d into llvm:main Sep 25, 2024
8 checks passed

dtcxzyw deleted the ubsan-check-assume branch September 25, 2024 05:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[UBSan] Diagnose assumption violation #104741

[UBSan] Diagnose assumption violation #104741

Uh oh!

dtcxzyw commented Aug 19, 2024

Uh oh!

llvmbot commented Aug 19, 2024 •

edited

Loading

Uh oh!

dtcxzyw commented Aug 28, 2024

Uh oh!

dtcxzyw commented Sep 8, 2024

Uh oh!

fmayer commented Sep 9, 2024

Uh oh!

Uh oh!

Uh oh!

vitalybuka commented Sep 18, 2024

Uh oh!

Uh oh!

vitalybuka commented Sep 24, 2024 •

edited

Loading

Uh oh!

vitalybuka commented Sep 25, 2024

Uh oh!

vitalybuka commented Sep 25, 2024

Uh oh!

github-actions bot commented Sep 25, 2024 •

edited

Loading

Uh oh!

Uh oh!

chapuni commented Sep 26, 2024

Uh oh!

dtcxzyw commented Sep 26, 2024

Uh oh!

Uh oh!

[UBSan] Diagnose assumption violation #104741

[UBSan] Diagnose assumption violation #104741

Uh oh!

Conversation

dtcxzyw commented Aug 19, 2024

Uh oh!

llvmbot commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtcxzyw commented Aug 28, 2024

Uh oh!

dtcxzyw commented Sep 8, 2024

Uh oh!

fmayer commented Sep 9, 2024

Uh oh!

Uh oh!

Uh oh!

vitalybuka commented Sep 18, 2024

Uh oh!

Uh oh!

vitalybuka commented Sep 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vitalybuka commented Sep 25, 2024

Uh oh!

vitalybuka commented Sep 25, 2024

Uh oh!

github-actions bot commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

chapuni commented Sep 26, 2024

Uh oh!

dtcxzyw commented Sep 26, 2024

Uh oh!

Uh oh!

llvmbot commented Aug 19, 2024 •

edited

Loading

vitalybuka commented Sep 24, 2024 •

edited

Loading

github-actions bot commented Sep 25, 2024 •

edited

Loading