[clang] 9905dae - Revert "[Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type"

Vitaly Buka via cfe-commits cfe-commits at lists.llvm.org
Sat Sep 3 13:13:28 PDT 2022


Author: Vitaly Buka
Date: 2022-09-03T13:12:49-07:00
New Revision: 9905dae5e18cd55ee6bb8678c95fed940ded1ef9

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

LOG: Revert "[Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type"

Breakes windows bot.

This reverts commit 3ad2fe913ae08ca062105731ad2da2eae825c731.

Added: 
    

Modified: 
    clang/include/clang/Basic/Builtins.def
    clang/lib/CodeGen/CGBuiltin.cpp
    clang/lib/CodeGen/CodeGenFunction.cpp
    clang/lib/Sema/SemaChecking.cpp
    clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
    clang/test/Sema/builtin-assume-aligned.c

Removed: 
    clang/test/CodeGen/catch-alignment-assumption-array.c


################################################################################
diff  --git a/clang/include/clang/Basic/Builtins.def b/clang/include/clang/Basic/Builtins.def
index dc7671ed16ce..249a3913dc0b 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -546,7 +546,7 @@ BUILTIN(__builtin_va_start, "vA.", "nt")
 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.", "nct")
+BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
 BUILTIN(__builtin_bzero, "vv*z", "nF")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 5af3811988e1..d5b504949447 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2781,8 +2781,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
   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;
 

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 0b75f034739d..81c687e4530f 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2443,6 +2443,8 @@ void CodeGenFunction::emitAlignmentAssumption(llvm::Value *PtrValue,
                                               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();
 

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index de132b8300a1..897ab701493c 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -128,28 +128,6 @@ static bool checkArgCountAtLeast(Sema &S, CallExpr *Call,
          << Call->getSourceRange();
 }
 
-/// Checks that a call expression's argument count is at most the desired
-/// number. This is useful when doing custom type-checking on a variadic
-/// function. Returns true on error.
-static bool checkArgCountAtMost(Sema &S, CallExpr *Call, unsigned MaxArgCount) {
-  unsigned ArgCount = Call->getNumArgs();
-  if (ArgCount <= MaxArgCount)
-    return false;
-  return S.Diag(Call->getEndLoc(),
-                diag::err_typecheck_call_too_many_args_at_most)
-         << 0 /*function call*/ << MaxArgCount << ArgCount
-         << Call->getSourceRange();
-}
-
-/// Checks that a call expression's argument count is in the desired range. This
-/// is useful when doing custom type-checking on a variadic function. Returns
-/// true on error.
-static bool checkArgCountRange(Sema &S, CallExpr *Call, unsigned MinArgCount,
-                               unsigned MaxArgCount) {
-  return checkArgCountAtLeast(S, Call, MinArgCount) ||
-         checkArgCountAtMost(S, Call, MaxArgCount);
-}
-
 /// Checks that a call expression's argument count is the desired number.
 /// This is useful when doing custom type-checking.  Returns true on error.
 static bool checkArgCount(Sema &S, CallExpr *Call, unsigned DesiredArgCount) {
@@ -170,20 +148,6 @@ static bool checkArgCount(Sema &S, CallExpr *Call, unsigned DesiredArgCount) {
          << Call->getArg(1)->getSourceRange();
 }
 
-static bool convertArgumentToType(Sema &S, Expr *&Value, QualType Ty) {
-  if (Value->isTypeDependent())
-    return false;
-
-  InitializedEntity Entity =
-      InitializedEntity::InitializeParameter(S.Context, Ty, false);
-  ExprResult Result =
-      S.PerformCopyInitialization(Entity, SourceLocation(), Value);
-  if (Result.isInvalid())
-    return true;
-  Value = Result.get();
-  return false;
-}
-
 /// Check that the first argument to __builtin_annotation is an integer
 /// and the second argument is a non-wide string literal.
 static bool SemaBuiltinAnnotation(Sema &S, CallExpr *TheCall) {
@@ -7680,45 +7644,38 @@ bool Sema::SemaBuiltinAllocaWithAlign(CallExpr *TheCall) {
 /// Handle __builtin_assume_aligned. This is declared
 /// as (const void*, size_t, ...) and can take one optional constant int arg.
 bool Sema::SemaBuiltinAssumeAligned(CallExpr *TheCall) {
-  if (checkArgCountRange(*this, TheCall, 2, 3))
-    return true;
-
   unsigned NumArgs = TheCall->getNumArgs();
-  Expr *FirstArg = TheCall->getArg(0);
 
-  {
-    ExprResult FirstArgResult = DefaultFunctionArrayLvalueConversion(FirstArg);
-    if (FirstArgResult.isInvalid())
-      return true;
-    TheCall->setArg(0, FirstArgResult.get());
-  }
+  if (NumArgs > 3)
+    return Diag(TheCall->getEndLoc(),
+                diag::err_typecheck_call_too_many_args_at_most)
+           << 0 /*function call*/ << 3 << NumArgs << TheCall->getSourceRange();
 
   // The alignment must be a constant integer.
-  Expr *SecondArg = TheCall->getArg(1);
-  if (convertArgumentToType(*this, SecondArg, Context.getSizeType()))
-    return true;
-  TheCall->setArg(1, SecondArg);
+  Expr *Arg = TheCall->getArg(1);
 
   // We can't check the value of a dependent argument.
-  if (!SecondArg->isValueDependent()) {
+  if (!Arg->isTypeDependent() && !Arg->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)
-             << SecondArg->getSourceRange();
+             << Arg->getSourceRange();
 
     if (Result > Sema::MaximumAlignment)
       Diag(TheCall->getBeginLoc(), diag::warn_assume_aligned_too_great)
-          << SecondArg->getSourceRange() << Sema::MaximumAlignment;
+          << Arg->getSourceRange() << Sema::MaximumAlignment;
   }
 
   if (NumArgs > 2) {
-    Expr *ThirdArg = TheCall->getArg(2);
-    if (convertArgumentToType(*this, ThirdArg, Context.getSizeType()))
-      return true;
-    TheCall->setArg(2, ThirdArg);
+    ExprResult Arg(TheCall->getArg(2));
+    InitializedEntity Entity = InitializedEntity::InitializeParameter(Context,
+      Context.getSizeType(), false);
+    Arg = PerformCopyInitialization(Entity, SourceLocation(), Arg);
+    if (Arg.isInvalid()) return true;
+    TheCall->setArg(2, Arg.get());
   }
 
   return false;

diff  --git a/clang/test/CodeGen/catch-alignment-assumption-array.c b/clang/test/CodeGen/catch-alignment-assumption-array.c
deleted file mode 100644
index fbc978410ded..000000000000
--- a/clang/test/CodeGen/catch-alignment-assumption-array.c
+++ /dev/null
@@ -1,32 +0,0 @@
-// 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);
-}

diff  --git a/clang/test/CodeGen/catch-alignment-assumption-ignorelist.c b/clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
index 9087c26e98f3..2f47db344670 100644
--- a/clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
+++ b/clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
@@ -26,9 +26,3 @@ void *dont_ignore_volatile_ptrs(void * volatile x) {
 void *ignore_volatiles(volatile void * x) {
   return __builtin_assume_aligned(x, 1);
 }
-
-// CHECK-LABEL: ignore_array_volatiles
-void *ignore_array_volatiles() {
-  volatile int arr[] = {1};
-  return __builtin_assume_aligned(arr, 4);
-}

diff  --git a/clang/test/Sema/builtin-assume-aligned.c b/clang/test/Sema/builtin-assume-aligned.c
index a5dd7e387181..96f1dca4c139 100644
--- a/clang/test/Sema/builtin-assume-aligned.c
+++ b/clang/test/Sema/builtin-assume-aligned.c
@@ -66,11 +66,6 @@ int test12(int *a) {
 }
 #endif
 
-int test13(int *a) {
-  a = (int *)__builtin_assume_aligned(a, 2 * 2.0); // expected-error {{argument to '__builtin_assume_aligned' must be a constant integer}}
-  return a[0];
-}
-
 void test_void_assume_aligned(void) __attribute__((assume_aligned(32))); // expected-warning {{'assume_aligned' attribute only applies to return values that are pointers}}
 int test_int_assume_aligned(void) __attribute__((assume_aligned(16))); // expected-warning {{'assume_aligned' attribute only applies to return values that are pointers}}
 void *test_ptr_assume_aligned(void) __attribute__((assume_aligned(64))); // no-warning


        


More information about the cfe-commits mailing list