yihanaa updated this revision to Diff 455918.
yihanaa added a comment.

Thanks John and Erich for your comments.
In this update:

Check args in Sema, and both in C and C++, emit an error if the 1st arg of 
__builtin_assume_aligned is volatile qualified or 1st arg not a pointer type.

Remove getSubExprAsWritten in emitAlignmentAssumption, and Emit a cast if 1st 
arg not void * type.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131979/new/

https://reviews.llvm.org/D131979

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaChecking.cpp
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-first-arg-cannot-volatile.c
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-first-arg-not-pointer.c
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-implicit-cast.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c

Index: clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
===================================================================
--- clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
+++ clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
@@ -21,8 +21,3 @@
   // CHECK: call void @__ubsan_handle_alignment_assumption(
   return __builtin_assume_aligned(x, 1);
 }
-
-// CHECK-LABEL: ignore_volatiles
-void *ignore_volatiles(volatile void * x) {
-  return __builtin_assume_aligned(x, 1);
-}
Index: clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-implicit-cast.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-implicit-cast.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fno-sanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-trap=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// CHECK-SANITIZE-ANYRECOVER: @[[CHAR:.*]] = {{.*}} c"'char *'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[ALIGNMENT_ASSUMPTION:.*]] = {{.*}}, i32 31, i32 35 }, {{.*}}* @[[CHAR]] }
+
+void *caller(void) {
+  char str[] = "";
+  // CHECK:                           define{{.*}}
+  // CHECK-NEXT:                      entry:
+  // CHECK-NEXT:                        %[[STR:.*]] = alloca [1 x i8], align 1
+  // CHECK-NEXT:                        %[[BITCAST:.*]] = bitcast [1 x i8]* %[[STR]] to i8*
+  // CHECK-NEXT:                        call void @llvm.memset.p0i8.i64(i8* align 1 %[[BITCAST]], i8 0, i64 1, i1 false)
+  // CHECK-NEXT:                        %[[ARRAYDECAY:.*]] = getelementptr inbounds [1 x i8], [1 x i8]* %[[STR]], i64 0, i64 0
+  // CHECK-SANITIZE-NEXT:               %[[PTRINT:.*]] = ptrtoint i8* %[[ARRAYDECAY]] to i64
+  // CHECK-SANITIZE-NEXT:               %[[MASKEDPTR:.*]] = and i64 %[[PTRINT]], 0
+  // CHECK-SANITIZE-NEXT:               %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0
+  // CHECK-SANITIZE-NEXT:               %[[PTRINT_DUP:.*]] = ptrtoint i8* %[[ARRAYDECAY]] to i64, !nosanitize
+  // CHECK-SANITIZE-NEXT:               br i1 %[[MASKCOND]], label %[[CONT:.*]], label %[[HANDLER_ALIGNMENT_ASSUMPTION:[^,]+]],{{.*}} !nosanitize
+  // CHECK-SANITIZE:                  [[HANDLER_ALIGNMENT_ASSUMPTION]]:
+  // CHECK-SANITIZE-NORECOVER-NEXT:     call void @__ubsan_handle_alignment_assumption_abort(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1, i64 0){{.*}}, !nosanitize
+  // CHECK-SANITIZE-RECOVER-NEXT:       call void @__ubsan_handle_alignment_assumption(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1, i64 0){{.*}}, !nosanitize
+  // CHECK-SANITIZE-TRAP-NEXT:          call void @llvm.ubsantrap(i8 23){{.*}}, !nosanitize
+  // CHECK-SANITIZE-UNREACHABLE-NEXT:   unreachable, !nosanitize
+  // CHECK-SANITIZE:                  [[CONT]]:
+  // CHECK-NEXT:                        call void @llvm.assume(i1 true) [ "align"(i8* %[[ARRAYDECAY]], i64 1) ] 
+  // CHECK-NEXT:                        ret i8* %[[ARRAYDECAY]]
+  // CHECK-NEXT:                      }
+  return __builtin_assume_aligned(str, 1);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-first-arg-not-pointer.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-first-arg-not-pointer.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void *must_be_pointer(int x) {
+  return __builtin_assume_aligned(x, 1); // expected-error {{first argument to __builtin_assume_aligned must be a pointer ('int' invalid)}}
+}
Index: clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-first-arg-cannot-volatile.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-first-arg-cannot-volatile.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void *cannot_volatile(volatile void * x) {
+  return __builtin_assume_aligned(x, 1); // expected-error {{the pointee of first argument to __builtin_assume_aligned cannot be volatile-qualified ('volatile void *' invalid)}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -7645,39 +7645,73 @@
 /// as (const void*, size_t, ...) and can take one optional constant int arg.
 bool Sema::SemaBuiltinAssumeAligned(CallExpr *TheCall) {
   unsigned NumArgs = TheCall->getNumArgs();
+  Expr *Callee = TheCall->getCallee();
+  DeclRefExpr *DRE = cast<DeclRefExpr>(Callee->IgnoreParenCasts());
 
-  if (NumArgs > 3)
+  // Ensure that we have at least one argument to do type inference from.
+  if (NumArgs < 1) {
+    return Diag(TheCall->getEndLoc(),
+                diag::err_typecheck_call_too_few_args_at_least)
+           << 0 << 1 << TheCall->getNumArgs() << Callee->getSourceRange();
+  }
+
+  if (NumArgs > 3) {
     return Diag(TheCall->getEndLoc(),
                 diag::err_typecheck_call_too_many_args_at_most)
            << 0 /*function call*/ << 3 << NumArgs << TheCall->getSourceRange();
+  }
+
+  // The address argument must be a pointer and pointee cannot be volatile.
+  Expr *FirstArg = TheCall->getArg(0);
+  ExprResult FirstArgResult = DefaultFunctionArrayLvalueConversion(FirstArg);
+  if (FirstArgResult.isInvalid())
+    return true;
+
+  FirstArg = FirstArgResult.get();
+  TheCall->setArg(0, FirstArg);
+
+  const PointerType *PtrTy = FirstArg->getType()->getAs<PointerType>();
+  if (!PtrTy) {
+    return Diag(DRE->getBeginLoc(),
+                diag::err_builtin_assume_aligned_first_arg_must_be_pointer)
+           << FirstArg->getType() << FirstArg->getSourceRange();
+  }
+
+  QualType ValTy = PtrTy->getPointeeType();
+  if (ValTy.isVolatileQualified()) {
+    return Diag(DRE->getBeginLoc(),
+                diag::err_builtin_assume_aligned_first_arg_cannot_be_volatile)
+           << FirstArg->getType() << FirstArg->getSourceRange();
+  }
 
   // The alignment must be a constant integer.
-  Expr *Arg = TheCall->getArg(1);
+  Expr *SecondArg = TheCall->getArg(1);
 
   // We can't check the value of a dependent argument.
-  if (!Arg->isTypeDependent() && !Arg->isValueDependent()) {
+  if (!SecondArg->isTypeDependent() && !SecondArg->isValueDependent()) {
     llvm::APSInt Result;
     if (SemaBuiltinConstantArg(TheCall, 1, Result))
       return true;
 
     if (!Result.isPowerOf2())
       return Diag(TheCall->getBeginLoc(), diag::err_alignment_not_power_of_two)
-             << Arg->getSourceRange();
+             << SecondArg->getSourceRange();
 
     if (Result > Sema::MaximumAlignment)
       Diag(TheCall->getBeginLoc(), diag::warn_assume_aligned_too_great)
-          << Arg->getSourceRange() << Sema::MaximumAlignment;
+          << SecondArg->getSourceRange() << Sema::MaximumAlignment;
   }
 
   if (NumArgs > 2) {
     ExprResult Arg(TheCall->getArg(2));
-    InitializedEntity Entity = InitializedEntity::InitializeParameter(Context,
-      Context.getSizeType(), false);
+    InitializedEntity Entity = InitializedEntity::InitializeParameter(
+        Context, Context.getSizeType(), false);
     Arg = PerformCopyInitialization(Entity, SourceLocation(), Arg);
-    if (Arg.isInvalid()) return true;
+    if (Arg.isInvalid())
+      return true;
     TheCall->setArg(2, Arg.get());
   }
-
+  
   return false;
 }
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2427,8 +2427,6 @@
   llvm::Instruction *Assumption = Builder.CreateAlignmentAssumption(
       CGM.getDataLayout(), PtrValue, Alignment, OffsetValue);
 
-  if (!SanOpts.has(SanitizerKind::Alignment))
-    return;
   emitAlignmentAssumptionCheck(PtrValue, Ty, Loc, AssumptionLoc, Alignment,
                                OffsetValue, TheCheck, Assumption);
 }
@@ -2438,11 +2436,8 @@
                                               SourceLocation AssumptionLoc,
                                               llvm::Value *Alignment,
                                               llvm::Value *OffsetValue) {
-  if (auto *CE = dyn_cast<CastExpr>(E))
-    E = CE->getSubExprAsWritten();
   QualType Ty = E->getType();
   SourceLocation Loc = E->getExprLoc();
-
   emitAlignmentAssumption(PtrValue, Ty, Loc, AssumptionLoc, Alignment,
                           OffsetValue);
 }
@@ -2709,11 +2704,6 @@
   if (!SanOpts.has(SanitizerKind::Alignment))
     return;
 
-  // Don't check pointers to volatile data. The behavior here is implementation-
-  // defined.
-  if (Ty->getPointeeType().isVolatileQualified())
-    return;
-
   // We need to temorairly remove the assumption so we can insert the
   // sanitizer check before it, else the check will be dropped by optimizations.
   Assumption->removeFromParent();
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2786,6 +2786,8 @@
   case Builtin::BI__builtin_assume_aligned: {
     const Expr *Ptr = E->getArg(0);
     Value *PtrValue = EmitScalarExpr(Ptr);
+    if (PtrValue->getType() != VoidPtrTy)
+      PtrValue = EmitCastToVoidPtr(PtrValue);
     Value *OffsetValue =
       (E->getNumArgs() > 2) ? EmitScalarExpr(E->getArg(2)) : nullptr;
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9939,6 +9939,10 @@
   "this builtin is only available on 32-bit targets">;
 def err_builtin_x64_aarch64_only : Error<
   "this builtin is only available on x86-64 and aarch64 targets">;
+def err_builtin_assume_aligned_first_arg_must_be_pointer : Error<
+  "first argument to __builtin_assume_aligned must be a pointer (%0 invalid)">;
+def err_builtin_assume_aligned_first_arg_cannot_be_volatile : Error<
+  "the pointee of first argument to __builtin_assume_aligned cannot be volatile-qualified (%0 invalid)">;
 def err_mips_builtin_requires_dsp : Error<
   "this builtin requires 'dsp' ASE, please use -mdsp">;
 def err_mips_builtin_requires_dspr2 : Error<
Index: clang/include/clang/Basic/Builtins.def
===================================================================
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -546,7 +546,7 @@
 BUILTIN(__builtin_va_end, "vA", "n")
 BUILTIN(__builtin_va_copy, "vAA", "n")
 BUILTIN(__builtin_stdarg_start, "vA.", "nt")
-BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
+BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nt")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
 BUILTIN(__builtin_bzero, "vv*z", "nF")
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to