[PATCH] D138137: [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Thank you so much for your review, @rjmccall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138137: [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Thank you so much for your review, @rjmccall , can you land this patch for me? 
Please use 'yronglin ' to commit the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138137: [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

[Lin Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 476093.
yronglin added a comment.

Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138137

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/arm-vaarg.c


Index: clang/test/CodeGen/arm-vaarg.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-vaarg.c
@@ -0,0 +1,23 @@
+// RUN: %clang -Xclang -no-opaque-pointers -mfloat-abi=soft -target 
arm-linux-gnu -emit-llvm -S -o - %s | FileCheck %s
+
+struct Empty {};
+
+struct Empty emptyvar;
+
+void take_args(int a, ...) {
+// CHECK: [[ALLOCA_VA_LIST:%[a-zA-Z0-9._]+]] = alloca %struct.__va_list, align 
4
+// CHECK: call void @llvm.va_start
+// CHECK-NEXT: [[AP_ADDR:%[a-zA-Z0-9._]+]] = bitcast %struct.__va_list* 
[[ALLOCA_VA_LIST]] to i8**
+// CHECK-NEXT: [[LOAD_AP:%[a-zA-Z0-9._]+]] = load i8*, i8** [[AP_ADDR]], align 
4
+// CHECK-NEXT: [[EMPTY_PTR:%[a-zA-Z0-9._]+]] = bitcast i8* [[LOAD_AP]] to 
%struct.Empty*
+
+  // It's conceivable that EMPTY_PTR may not actually be a valid pointer
+  // (e.g. it's at the very bottom of the stack and the next page is
+  // invalid). This doesn't matter provided it's never loaded (there's no
+  // well-defined way to tell), but it becomes a problem if we do try to use 
it.
+// CHECK-NOT: load %struct.Empty, %struct.Empty* [[EMPTY_PTR]]
+  __builtin_va_list l;
+  __builtin_va_start(l, a);
+  emptyvar = __builtin_va_arg(l, struct Empty);
+  __builtin_va_end(l);
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7050,10 +7050,10 @@
 
   // Empty records are ignored for parameter passing purposes.
   if (isEmptyRecord(getContext(), Ty, true)) {
-Address Addr = Address(CGF.Builder.CreateLoad(VAListAddr),
-   getVAListElementType(CGF), SlotSize);
-Addr = CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
-return Addr;
+VAListAddr = CGF.Builder.CreateElementBitCast(VAListAddr, CGF.Int8PtrTy);
+auto *Load = CGF.Builder.CreateLoad(VAListAddr);
+Address Addr = Address(Load, CGF.Int8Ty, SlotSize);
+return CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
   }
 
   CharUnits TySize = getContext().getTypeSizeInChars(Ty);


Index: clang/test/CodeGen/arm-vaarg.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-vaarg.c
@@ -0,0 +1,23 @@
+// RUN: %clang -Xclang -no-opaque-pointers -mfloat-abi=soft -target arm-linux-gnu -emit-llvm -S -o - %s | FileCheck %s
+
+struct Empty {};
+
+struct Empty emptyvar;
+
+void take_args(int a, ...) {
+// CHECK: [[ALLOCA_VA_LIST:%[a-zA-Z0-9._]+]] = alloca %struct.__va_list, align 4
+// CHECK: call void @llvm.va_start
+// CHECK-NEXT: [[AP_ADDR:%[a-zA-Z0-9._]+]] = bitcast %struct.__va_list* [[ALLOCA_VA_LIST]] to i8**
+// CHECK-NEXT: [[LOAD_AP:%[a-zA-Z0-9._]+]] = load i8*, i8** [[AP_ADDR]], align 4
+// CHECK-NEXT: [[EMPTY_PTR:%[a-zA-Z0-9._]+]] = bitcast i8* [[LOAD_AP]] to %struct.Empty*
+
+  // It's conceivable that EMPTY_PTR may not actually be a valid pointer
+  // (e.g. it's at the very bottom of the stack and the next page is
+  // invalid). This doesn't matter provided it's never loaded (there's no
+  // well-defined way to tell), but it becomes a problem if we do try to use it.
+// CHECK-NOT: load %struct.Empty, %struct.Empty* [[EMPTY_PTR]]
+  __builtin_va_list l;
+  __builtin_va_start(l, a);
+  emptyvar = __builtin_va_arg(l, struct Empty);
+  __builtin_va_end(l);
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7050,10 +7050,10 @@
 
   // Empty records are ignored for parameter passing purposes.
   if (isEmptyRecord(getContext(), Ty, true)) {
-Address Addr = Address(CGF.Builder.CreateLoad(VAListAddr),
-   getVAListElementType(CGF), SlotSize);
-Addr = CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
-return Addr;
+VAListAddr = CGF.Builder.CreateElementBitCast(VAListAddr, CGF.Int8PtrTy);
+auto *Load = CGF.Builder.CreateLoad(VAListAddr);
+Address Addr = Address(Load, CGF.Int8Ty, SlotSize);
+return CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
   }
 
   CharUnits TySize = getContext().getTypeSizeInChars(Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138137: [CodeGen][ARM] Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

[Lin Yurong via Phabricator via cfe-commits]

yronglin created this revision.
yronglin added reviewers: rjmccall, asl.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
yronglin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix ARMABIInfo::EmitVAAarg crash with empty record type variadic arg

Open issue: https://github.com/llvm/llvm-project/issues/58794


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138137

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/arm-vaarg.c


Index: clang/test/CodeGen/arm-vaarg.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-vaarg.c
@@ -0,0 +1,23 @@
+// RUN: %clang -Xclang -no-opaque-pointers -mfloat-abi=soft -target 
arm-linux-gnu -emit-llvm -S -o - %s | FileCheck %s
+
+struct Empty {};
+
+struct Empty emptyvar;
+
+void take_args(int a, ...) {
+// CHECK: [[ALLOCA_VA_LIST:%[0-9]]] = alloca %struct.__va_list, align 4
+// CHECK: call void @llvm.va_start
+// CHECK-NEXT: [[AP_ADDR:%[0-9]+]] = bitcast %struct.__va_list* 
[[ALLOCA_VA_LIST]] to i8**
+// CHECK-NEXT: [[LOAD_AP:%[0-9]+]] = load i8*, i8** [[AP_ADDR]], align 4
+// CHECK-NEXT: [[EMPTY_PTR:%[0-9]+]] = bitcast i8* [[LOAD_AP]] to 
%struct.Empty*
+
+  // It's conceivable that EMPTY_PTR may not actually be a valid pointer
+  // (e.g. it's at the very bottom of the stack and the next page is
+  // invalid). This doesn't matter provided it's never loaded (there's no
+  // well-defined way to tell), but it becomes a problem if we do try to use 
it.
+// CHECK-NOT: load %struct.Empty, %struct.Empty* [[EMPTY_PTR]]
+  __builtin_va_list l;
+  __builtin_va_start(l, a);
+  emptyvar = __builtin_va_arg(l, struct Empty);
+  __builtin_va_end(l);
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7050,10 +7050,10 @@
 
   // Empty records are ignored for parameter passing purposes.
   if (isEmptyRecord(getContext(), Ty, true)) {
-Address Addr = Address(CGF.Builder.CreateLoad(VAListAddr),
-   getVAListElementType(CGF), SlotSize);
-Addr = CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
-return Addr;
+VAListAddr = CGF.Builder.CreateElementBitCast(VAListAddr, CGF.Int8PtrTy);
+auto *Load = CGF.Builder.CreateLoad(VAListAddr);
+Address Addr = Address(Load, CGF.Int8Ty, SlotSize);
+return CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
   }
 
   CharUnits TySize = getContext().getTypeSizeInChars(Ty);


Index: clang/test/CodeGen/arm-vaarg.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-vaarg.c
@@ -0,0 +1,23 @@
+// RUN: %clang -Xclang -no-opaque-pointers -mfloat-abi=soft -target arm-linux-gnu -emit-llvm -S -o - %s | FileCheck %s
+
+struct Empty {};
+
+struct Empty emptyvar;
+
+void take_args(int a, ...) {
+// CHECK: [[ALLOCA_VA_LIST:%[0-9]]] = alloca %struct.__va_list, align 4
+// CHECK: call void @llvm.va_start
+// CHECK-NEXT: [[AP_ADDR:%[0-9]+]] = bitcast %struct.__va_list* [[ALLOCA_VA_LIST]] to i8**
+// CHECK-NEXT: [[LOAD_AP:%[0-9]+]] = load i8*, i8** [[AP_ADDR]], align 4
+// CHECK-NEXT: [[EMPTY_PTR:%[0-9]+]] = bitcast i8* [[LOAD_AP]] to %struct.Empty*
+
+  // It's conceivable that EMPTY_PTR may not actually be a valid pointer
+  // (e.g. it's at the very bottom of the stack and the next page is
+  // invalid). This doesn't matter provided it's never loaded (there's no
+  // well-defined way to tell), but it becomes a problem if we do try to use it.
+// CHECK-NOT: load %struct.Empty, %struct.Empty* [[EMPTY_PTR]]
+  __builtin_va_list l;
+  __builtin_va_start(l, a);
+  emptyvar = __builtin_va_arg(l, struct Empty);
+  __builtin_va_end(l);
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7050,10 +7050,10 @@
 
   // Empty records are ignored for parameter passing purposes.
   if (isEmptyRecord(getContext(), Ty, true)) {
-Address Addr = Address(CGF.Builder.CreateLoad(VAListAddr),
-   getVAListElementType(CGF), SlotSize);
-Addr = CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
-return Addr;
+VAListAddr = CGF.Builder.CreateElementBitCast(VAListAddr, CGF.Int8PtrTy);
+auto *Load = CGF.Builder.CreateLoad(VAListAddr);
+Address Addr = Address(Load, CGF.Int8Ty, SlotSize);
+return CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty));
   }
 
   CharUnits TySize = getContext().getTypeSizeInChars(Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

In D133583#3803002 , @aaron.ballman 
wrote:

> In D133583#3802451 , @yronglin 
> wrote:
>
>> Thanks for take a review @aaron.ballman @rjmccall , can you land this patch 
>> for me? Please use 'yronglin ' to commit the change.
>
> Happy to do so! I've landed on your behalf in 
> https://github.com/llvm/llvm-project/commit/8392f1cc78270c7039970b413dfd836bf4def602

Thanks very much!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Thanks for take a review @aaron.ballman @rjmccall , can you land this patch for 
me? Please use 'yronglin ' to commit the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Thanks for your suggestion @aaron.ballman , also I have added a new test case 
for C++ polymorphism classes




Comment at: clang/test/Sema/builtin-redecl.cpp:5-6
 
+#include 
+
 // Redeclaring library builtins is OK.

aaron.ballman wrote:
> yronglin wrote:
> > aaron.ballman wrote:
> > > We can be tricky instead of including a header file.
> > Use `decltype` not works in C , can we use `typedef unsigned long size_t 
> > `instead? 
> Oh good catch, I had missed the `-x c` on a RUN line and was looking at the 
> file extension alone. :-) We can't use that typedef because there are some 
> platforms where that type is wrong, but we can be tricky in a different way: 
> `typedef __typeof__(sizeof(0)) size_t;`
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 460870.
yronglin added a comment.

Add test case for C++ polymorphism class
Use `typedef __typeof__(sizeof(0)) size_t;` instead of `#include `


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/Basic/Builtins.cpp
  clang/lib/Sema/SemaChecking.cpp
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-polymorphism.cpp
  clang/test/Sema/builtin-redecl.cpp

Index: clang/test/Sema/builtin-redecl.cpp
===
--- clang/test/Sema/builtin-redecl.cpp
+++ clang/test/Sema/builtin-redecl.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
 
+typedef __typeof__(sizeof(0)) size_t;
+
 // Redeclaring library builtins is OK.
 void exit(int);
 
@@ -16,3 +18,9 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
+
+#ifdef __cplusplus
+void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+#else
+void *__builtin_assume_aligned(const void *, size_t, ...);
+#endif
Index: clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-polymorphism.cpp
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-polymorphism.cpp
@@ -0,0 +1,64 @@
+// 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"'B *'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[LINE_100_ALIGNMENT_ASSUMPTION:.*]] = {{.*}}, i32 100, i32 35 }, {{.*}}* @[[CHAR]] }
+
+struct A { int n; };
+struct B { int n; };
+struct C : A, B {};
+
+void *f(C *c) {
+  // CHECK: define {{.*}} i8* @{{.*}}(%struct.C* noundef %[[C:.*]]) {{.*}} {
+  // CHECK-NEXT:[[ENTRY:.*]]:
+  // CHECK-NEXT:  %[[C_ADDR:.*]] = alloca %struct.C*
+  // CHECK-NEXT:  store %struct.C* %[[C]], %struct.C** %[[C_ADDR]]
+  // CHECK-NEXT:  %[[C_RELOAD:.*]] = load %struct.C*, %struct.C** %[[C_ADDR]]
+  // CHECK-NEXT:  %[[IS_NULL:.*]] = icmp eq %struct.C* %[[C_RELOAD]], null
+  // CHECK-NEXT:  br i1 %[[IS_NULL]], label %[[CAST_END:[^,]+]], label %[[CAST_NOT_NULL:[^,]+]]
+  // CHECK: [[CAST_NOT_NULL]]:
+  // CHECK-NOSANITIZE-NEXT:   %[[BITCAST:.*]] = bitcast %struct.C* %[[C_RELOAD]] to i8*
+  // CHECK-NOSANITIZE-NEXT:   %[[ADD_PTR:.*]] = getelementptr inbounds i8, i8* %[[BITCAST]], i64 4
+  // CHECK-NOSANITIZE-NEXT:   %[[BITCAST2:.*]] = bitcast i8* %[[ADD_PTR]] to %struct.B*
+  // CHECK-NOSANITIZE-NEXT:   br label %[[CAST_END]]
+  // CHECK-SANITIZE-NEXT: %[[PTRTOINT:.*]] = ptrtoint %struct.C* %[[C_RELOAD]] to i64, !nosanitize
+  // CHECK-SANITIZE-NEXT: %[[MASKEDPTR:.*]] = and i64 %[[PTRTOINT]], 3, !nosanitize
+  // CHECK-SANITIZE-NEXT: %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0, !nosanitize
+  // CHECK-SANITIZE-NEXT: br i1 %[[MASKCOND]], label %[[CONT:[^,]+]], label %[[HANDLER_TYPE_MISMATCH:[^,]+]]
+  // CHECK-SANITIZE:[[HANDLER_TYPE_MISMATCH]]:
+  // CHECK-SANITIZE-NORECOVER-NEXT:   call void @__ubsan_handle_type_mismatch_v1_abort(
+  // CHECK-SANITIZE-RECOVER-NEXT: call void @__ubsan_handle_type_mismatch_v1(
+  // CHECK-SANITIZE-TRAP-NEXT:call void @llvm.ubsantrap(
+  // CHECK-SANITIZE-UNREACHABLE-NEXT: unreachable, !nosanitize
+  // CHECK-SANITIZE:[[CONT]]:
+  // CHECK-SANITIZE-NEXT: %[[BITCAST:.*]] = bitcast %struct.C* %[[C_RELOAD]] to i8*
+  // CHECK-SANITIZE-NEXT: %[[ADD_PTR:.*]] 

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

In D133583#3792625 , @aaron.ballman 
wrote:

> In D133583#3780460 , @yronglin 
> wrote:
>
>> Hi, follow D133202  , should I both fix 
>> alignment in this patch or in another separate patch? (this seems have 
>> different behavior with gcc https://godbolt.org/z/7dvM8zhnh )
>
> I think that's a separate patch -- this one is fixing a mistake with the 
> type-checking related crash fix and the other is about the behavior of the 
> expression itself when it's valid.
>
> The current patch looks reasonable to me, but I'd love a second opinion.

Thanks for your comments @aaron.ballman




Comment at: clang/test/Sema/builtin-redecl.cpp:5-6
 
+#include 
+
 // Redeclaring library builtins is OK.

aaron.ballman wrote:
> We can be tricky instead of including a header file.
Use `decltype` not works in C , can we use `typedef unsigned long size_t 
`instead? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 459092.
yronglin added a comment.

format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/Basic/Builtins.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtin-redecl.cpp


Index: clang/test/Sema/builtin-redecl.cpp
===
--- clang/test/Sema/builtin-redecl.cpp
+++ clang/test/Sema/builtin-redecl.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
 
+#include 
+
 // Redeclaring library builtins is OK.
 void exit(int);
 
@@ -16,3 +18,9 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
+
+#ifdef __cplusplus
+void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+#else
+void *__builtin_assume_aligned(const void *, size_t, ...);
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -141,6 +141,15 @@
  << 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 , 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 , CallExpr *Call, unsigned DesiredArgCount) {
@@ -7643,17 +7652,15 @@
 /// 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 (checkArgCountAtMost(*this, TheCall, 3))
+  if (checkArgCountRange(*this, TheCall, 2, 3))
 return true;
 
   unsigned NumArgs = TheCall->getNumArgs();
   Expr *FirstArg = TheCall->getArg(0);
-  if (auto *CE = dyn_cast(FirstArg))
-FirstArg = CE->getSubExprAsWritten();
 
   {
 ExprResult FirstArgResult =
-DefaultFunctionArrayLvalueConversion(FirstArg, /*Diagnose=*/false);
+DefaultFunctionArrayLvalueConversion(FirstArg);
 if (FirstArgResult.isInvalid())
   return true;
 TheCall->setArg(0, FirstArgResult.get());
Index: clang/lib/Basic/Builtins.cpp
===
--- clang/lib/Basic/Builtins.cpp
+++ clang/lib/Basic/Builtins.cpp
@@ -209,6 +209,7 @@
 
 bool Builtin::Context::canBeRedeclared(unsigned ID) const {
   return ID == Builtin::NotBuiltin || ID == Builtin::BI__va_start ||
+ ID == Builtin::BI__builtin_assume_aligned ||
  (!hasReferenceArgsOrResult(ID) && !hasCustomTypechecking(ID)) ||
  isInStdNamespace(ID);
 }
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.", "nct")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
 BUILTIN(__builtin_bzero, "vv*z", "nF")


Index: clang/test/Sema/builtin-redecl.cpp
===
--- clang/test/Sema/builtin-redecl.cpp
+++ clang/test/Sema/builtin-redecl.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
 
+#include 
+
 // Redeclaring library builtins is OK.
 void exit(int);
 
@@ -16,3 +18,9 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
+
+#ifdef __cplusplus
+void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+#else
+void *__builtin_assume_aligned(const void *, size_t, ...);
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -141,6 +141,15 @@
  << 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 , CallExpr *Call, unsigned MinArgCount,
+   unsigned 

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

I've a new patch D133583  , please can you 
guys take a look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Hi, should I both fix alignment in this patch or in another separate patch? 
(this seems have different behavior with gcc https://godbolt.org/z/7dvM8zhnh )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Lin Yurong via Phabricator via cfe-commits]

yronglin created this revision.
yronglin added reviewers: rjmccall, rsmith.
Herald added a project: All.
yronglin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix __builtin_assume_aligned incorrect type descriptor

example from @rsmith

  struct A { int n; };
  struct B { int n; };
  struct C : A, B {};
  
  void *f(C *c) {
// Incorrectly returns `c` rather than the address of the B base class.
return __builtin_assume_aligned((B*)c, 8);
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133583

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/Basic/Builtins.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/builtin-redecl.cpp


Index: clang/test/Sema/builtin-redecl.cpp
===
--- clang/test/Sema/builtin-redecl.cpp
+++ clang/test/Sema/builtin-redecl.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
 
+#include 
+
 // Redeclaring library builtins is OK.
 void exit(int);
 
@@ -16,3 +18,9 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
+
+#ifdef __cplusplus
+void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+#else
+void *__builtin_assume_aligned(const void *, size_t, ...);
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -141,6 +141,15 @@
  << 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 , 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 , CallExpr *Call, unsigned DesiredArgCount) {
@@ -7643,17 +7652,15 @@
 /// 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 (checkArgCountAtMost(*this, TheCall, 3))
+  if (checkArgCountRange(*this, TheCall, 2, 3))
 return true;
 
   unsigned NumArgs = TheCall->getNumArgs();
   Expr *FirstArg = TheCall->getArg(0);
-  if (auto *CE = dyn_cast(FirstArg))
-FirstArg = CE->getSubExprAsWritten();
-
+  
   {
 ExprResult FirstArgResult =
-DefaultFunctionArrayLvalueConversion(FirstArg, /*Diagnose=*/false);
+DefaultFunctionArrayLvalueConversion(FirstArg);
 if (FirstArgResult.isInvalid())
   return true;
 TheCall->setArg(0, FirstArgResult.get());
Index: clang/lib/Basic/Builtins.cpp
===
--- clang/lib/Basic/Builtins.cpp
+++ clang/lib/Basic/Builtins.cpp
@@ -209,6 +209,7 @@
 
 bool Builtin::Context::canBeRedeclared(unsigned ID) const {
   return ID == Builtin::NotBuiltin || ID == Builtin::BI__va_start ||
+ ID == Builtin::BI__builtin_assume_aligned ||
  (!hasReferenceArgsOrResult(ID) && !hasCustomTypechecking(ID)) ||
  isInStdNamespace(ID);
 }
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.", "nct")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
 BUILTIN(__builtin_bzero, "vv*z", "nF")


Index: clang/test/Sema/builtin-redecl.cpp
===
--- clang/test/Sema/builtin-redecl.cpp
+++ clang/test/Sema/builtin-redecl.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 // RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
 
+#include 
+
 // Redeclaring library builtins is OK.
 void exit(int);
 
@@ -16,3 +18,9 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
+
+#ifdef __cplusplus
+void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+#else
+void *__builtin_assume_aligned(const void *, size_t, ...);
+#endif
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ 

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Thanks very much for your comments @rjmccall @rsmith , I've take a look at 
D45383 , I believe that user code isn't 
allowed to declare  __builtin_*, but seems `intrin0.inl.h` is a system header 
on windows, should we keep compatibility(like `__va_start` in D45383 
) with it? or breaking it, but I'd like some 
further guidance. can you help me make that decision? I think you guys are 
experts in this area and have more experience than me, I'll see if I can 
accomplish soon! (maybe I should submit a new RFC patch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Seems that a builtin can't be redeclared which has custom type checking
https://github.com/llvm/llvm-project/blob/ec8f2905a33ba970543c8edb4141c47f30d325f7/clang/lib/Basic/Builtins.cpp#L210-L214


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Reproduce windows broken test:

ignore.cpp

  #include 
  
  void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
  void foo() {
int a;
(void) __builtin_assume_aligned(, 4);
  }



  FunctionDecl 0x14a80e480 <./ignore.cpp:3:1, col:7> col:7 
__builtin_assume_aligned 'void *(const void *, size_t, ...) noexcept'
  |-ParmVarDecl 0x14a80df80  col:44 'const void *'
  `-ParmVarDecl 0x14a80e050  col:52 'size_t':'unsigned long'
  FunctionDecl 0x14a80e210 <./ignore.cpp:3:7> col:7 implicit 
__builtin_assume_aligned 'void *(const void *, unsigned long, ...) noexcept' 
extern
  |-ParmVarDecl 0x14a80e308 <>  'const void *'
  |-ParmVarDecl 0x14a80e370 <>  'unsigned long'
  |-BuiltinAttr 0x14a80e2b0 <> Implicit 402
  |-NoThrowAttr 0x14a80e3e8  Implicit
  `-ConstAttr 0x14a80e410  Implicit

  ./ignore.cpp:3:7: error: cannot redeclare builtin function 
'__builtin_assume_aligned'
  void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
^
  ./ignore.cpp:3:7: note: '__builtin_assume_aligned' is a builtin with type 
'void *(const void *, unsigned long, ...) noexcept'
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:7651-7652
+  Expr *FirstArg = TheCall->getArg(0);
+  if (auto *CE = dyn_cast(FirstArg))
+FirstArg = CE->getSubExprAsWritten();
 

rjmccall wrote:
> rsmith wrote:
> > This looks very suspicious to me: this will remove a cast expression that 
> > was written in the source code from the AST. That loses source fidelity, 
> > can give the wrong answer if the cast changed the value (such as a C++ 
> > derived-to-base conversion to a non-primary base class), and in any case 
> > this is only done once where there could be multiple explicit casts written 
> > on an argument to the builtin, so if it's necessary, then it's not being 
> > done fully.
> > 
> > Can this be removed?
> Somehow I missed this in my review.  Yes, this line should be unnecessary, 
> and as you say, it is definitely wrong.
CodeGen need real `user-written-type` to generate 
`__ubsan_handle_alignment_assumption ` 's arg, but not `const void *`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Thanks a lot for your comments @rsmith @rjmccall .

Firstly, as far as I know, turning on the `-fsanitizer=alignment` options when 
calling `__builtin_assume_aligned` in C code, Clang will emit `call void 
@__ubsan_handle_alignment_assumption(...)` in CodeGen,  and CodeGen need 
`user-written-type` to generate correct `TypeDescriptor` (this class in 
compiler-rt/UBSan).

Secondly, before this patch, 
`clang::CodeGen::CodeGenFunction::emitAlignmentAssumption` use 
`CastExpr->getSubExprAsWritten` to get `user-written-type` in CodeGen,  In 
`Diff 457643` , with John's comments, we use custom sema checking. we  just use 
`DefaultFunctionArrayLvalueConversion` to convert 1st arg, but not implicit 
cast 1st arg to `const void *`(We expect pass `user-written-type` to CodeGen).

Unfortunately,  `Diff 457643` broken windows sanitize test, because there have 
a forward declaration `__MACHINE(void * __cdecl __builtin_assume_aligned(const 
void *, size_t, ...) noexcept)`in `intrin0.inl.h`, I think the reason for this 
problem is we use `nct` in `BUILTIN(__builtin_assume_aligned, "v*vC*z.", 
"nct")`, I try to find a solution based on `Diff 457643`, what do you all think 
about?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

In D133202#3774905 , @aaron.ballman 
wrote:

> I've pushed the changes up for you, thanks!
>
> In D133202#3774801 , @yronglin 
> wrote:
>
>> ping~
>
> Something to keep in mind for the future: we typically only "ping" a review 
> after about a week of no activity on it (just because folks end up being 
> busy, taking vacation, want to contemplate the review for a while, etc).

Thanks a lot Aaron, I will remember what you said.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

ping~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 458403.
yronglin retitled this revision from "[Clang][CodeGen] Avoid 
__builtin_assume_aligned crash when the 1st arg is array type" to "[Clang] 
Avoid __builtin_assume_aligned crash when the 1st arg is array type".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/catch-alignment-assumption-array.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
  clang/test/Sema/builtin-assume-aligned.c

Index: clang/test/Sema/builtin-assume-aligned.c
===
--- clang/test/Sema/builtin-assume-aligned.c
+++ clang/test/Sema/builtin-assume-aligned.c
@@ -66,6 +66,11 @@
 }
 #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
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
@@ -26,3 +26,9 @@
 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);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-array.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-array.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 

[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Thanks for your review @aaron.ballman , I've updated ReleaseNotes. I don’t have 
commit access, can you land this patch for me?Please use 'yronglin 
' to commit the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 458315.
yronglin added a comment.

Update ReleaseNotes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/catch-alignment-assumption-array.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
  clang/test/Sema/builtin-assume-aligned.c

Index: clang/test/Sema/builtin-assume-aligned.c
===
--- clang/test/Sema/builtin-assume-aligned.c
+++ clang/test/Sema/builtin-assume-aligned.c
@@ -66,6 +66,11 @@
 }
 #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
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
@@ -26,3 +26,9 @@
 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);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-array.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-array.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
+  // 

[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 458019.
yronglin added a project: Sanitizers.
yronglin added a comment.

Fix windows build bolt broken 
https://lab.llvm.org/buildbot/#/builders/127/builds/35304


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/catch-alignment-assumption-array.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
  clang/test/Sema/builtin-assume-aligned.c

Index: clang/test/Sema/builtin-assume-aligned.c
===
--- clang/test/Sema/builtin-assume-aligned.c
+++ clang/test/Sema/builtin-assume-aligned.c
@@ -66,6 +66,11 @@
 }
 #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
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
@@ -26,3 +26,9 @@
 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);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-array.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-array.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*), 

[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

In D133202#3768770 , @vitalybuka 
wrote:

> Broken by the patch https://lab.llvm.org/buildbot/#/builders/127/builds/35304

Thanks @vitalybuka , seems `intrin0.inl.h` have a forward declaration 
`__MACHINE(void * __cdecl __builtin_assume_aligned(const void *, size_t, ...) 
noexcept)`, should we use `nc` in `BUILTIN(__builtin_assume_aligned, "v*vC*z.", 
"nct")` but not `nct` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ad2fe913ae0: [Clang][CodeGen] Avoid 
__builtin_assume_aligned crash when the 1st arg is array… (authored by 
yronglin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

Files:
  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-array.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
  clang/test/Sema/builtin-assume-aligned.c

Index: clang/test/Sema/builtin-assume-aligned.c
===
--- clang/test/Sema/builtin-assume-aligned.c
+++ clang/test/Sema/builtin-assume-aligned.c
@@ -66,6 +66,11 @@
 }
 #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
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
@@ -26,3 +26,9 @@
 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);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-array.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-array.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 ({ {{{.*}}}, 

[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

In D133202#3767414 , @rjmccall wrote:

> Thanks, LGTM

Thanks for your review John, but I don’t have commit access, can you land this 
patch for me? Please use "yronglin " to commit the 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:7697
   // The alignment must be a constant integer.
-  Expr *Arg = TheCall->getArg(1);
+  Expr *SecondArg = TheCall->getArg(1);
 

rjmccall wrote:
> This should be:
> 
> ```
> Expr *SecondArg = TheCall->getArg(1);
> if (convertArgumentToType(*this, SecondArg, Context.getSizeType()))
>   return true;
> TheCall->setArg(1, SecondArg);
> 
> if (!SecondArg->isValueDependent()) {
>   llvm::APSInt Result;
>   
> }
> ```
> 
> Test case is to pass a floating-point expression or something like that.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang][CodeGen] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[Lin Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 457643.
yronglin added a comment.

Update patch with john's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

Files:
  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-array.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
  clang/test/Sema/builtin-assume-aligned.c

Index: clang/test/Sema/builtin-assume-aligned.c
===
--- clang/test/Sema/builtin-assume-aligned.c
+++ clang/test/Sema/builtin-assume-aligned.c
@@ -66,6 +66,11 @@
 }
 #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
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
@@ -26,3 +26,9 @@
 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);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-array.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-array.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

[PATCH] D133202: [Clang][CodeGen] Fix __builtin_assume_aligned crash

[Lin Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 457580.
yronglin added a comment.

Format code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

Files:
  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-array.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
@@ -26,3 +26,9 @@
 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);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-array.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-array.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/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -128,6 +128,28 @@
  << 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 

[PATCH] D133202: [Clang][CodeGen] Fix __builtin_assume_aligned crash

[Lin Yurong via Phabricator via cfe-commits]

yronglin added a comment.

Hi John, I just have a new account, we continue here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133202: [Clang][CodeGen] Fix __builtin_assume_aligned crash

[Lin Yurong via Phabricator via cfe-commits]

yronglin created this revision.
yronglin added a reviewer: rjmccall.
Herald added a project: All.
yronglin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang will crash when __builtin_assume_aligned's 1st arg is array type(or 
string literal).
Open issue: https://github.com/llvm/llvm-project/issues/57169


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133202

Files:
  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-array.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
@@ -26,3 +26,9 @@
 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);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-array.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-array.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/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -128,6 +128,28 @@
  <<