-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[CodeGen][CFI] Generalize transparent union parameters #158193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CodeGen][CFI] Generalize transparent union parameters #158193
Conversation
Created using spr 1.3.6
Created using spr 1.3.6 [skip ci]
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Vitaly Buka (vitalybuka) ChangesAccording GCC documentation transparent union C++ ignores attribute. Full diff: https://github.com/llvm/llvm-project/pull/158193.diff 5 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index c647003ff389d..46dbd85665e5d 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2339,13 +2339,29 @@ llvm::ConstantInt *CodeGenModule::CreateCrossDsoCfiTypeId(llvm::Metadata *MD) {
return llvm::ConstantInt::get(Int64Ty, llvm::MD5Hash(MDS->getString()));
}
+static QualType GeneralizeTransparentUnion(QualType Ty) {
+ const RecordType *UT = Ty->getAsUnionType();
+ if (!UT)
+ return Ty;
+ const RecordDecl *UD = UT->getOriginalDecl()->getDefinitionOrSelf();
+ if (!UD->hasAttr<TransparentUnionAttr>())
+ return Ty;
+ for (const auto *it : UD->fields()) {
+ return it->getType();
+ }
+ return Ty;
+}
+
+static QualType GeneralizeTransparentUnion(QualType Ty) {
+}
+
// Generalize pointer types to a void pointer with the qualifiers of the
// originally pointed-to type, e.g. 'const char *' and 'char * const *'
// generalize to 'const void *' while 'char *' and 'const char **' generalize to
// 'void *'.
static QualType GeneralizeType(ASTContext &Ctx, QualType Ty,
bool GeneralizePointers) {
- // TODO: Add other generalizations.
+ Ty = GeneralizeTransparentUnion(Ty);
if (!GeneralizePointers || !Ty->isPointerType())
return Ty;
diff --git a/clang/test/CodeGen/cfi-icall-generalize.c b/clang/test/CodeGen/cfi-icall-generalize.c
index 116a99e4e2859..5359134805198 100644
--- a/clang/test/CodeGen/cfi-icall-generalize.c
+++ b/clang/test/CodeGen/cfi-icall-generalize.c
@@ -22,13 +22,13 @@ union Union {
// CHECK: define{{.*}} void @uni({{.*}} !type [[TYPE2:![0-9]+]] !type [[TYPE2_GENERALIZED:![0-9]+]]
void uni(void (*fn)(union Union), union Union arg1) {
- // UNGENERALIZED: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFv5UnionE")
- // GENERALIZED: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFv5UnionE.generalized")
+ // UNGENERALIZED: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFvPcE")
+ // GENERALIZED: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFvPvE.generalized")
fn(arg1);
}
// CHECK: [[TYPE]] = !{i64 0, !"_ZTSFPPiPKcPS2_E"}
// CHECK: [[TYPE_GENERALIZED]] = !{i64 0, !"_ZTSFPvPKvS_E.generalized"}
-// CHECK: [[TYPE2]] = !{i64 0, !"_ZTSFvPFv5UnionES_E"}
-// CHECK: [[TYPE2_GENERALIZED]] = !{i64 0, !"_ZTSFvPv5UnionE.generalized"}
+// CHECK: [[TYPE2]] = !{i64 0, !"_ZTSFvPFv5UnionEPcE"}
+// CHECK: [[TYPE2_GENERALIZED]] = !{i64 0, !"_ZTSFvPvS_E.generalized"}
diff --git a/clang/test/CodeGen/cfi-icall-normalize2.c b/clang/test/CodeGen/cfi-icall-normalize2.c
index c88ecc9f0c3f7..b9d9af7c8a47b 100644
--- a/clang/test/CodeGen/cfi-icall-normalize2.c
+++ b/clang/test/CodeGen/cfi-icall-normalize2.c
@@ -32,11 +32,11 @@ union Union {
void uni(void (*fn)(union Union), union Union arg1) {
// CHECK-LABEL: define{{.*}}uni
// CHECK-SAME: {{.*}}!type ![[TYPE4:[0-9]+]] !type !{{[0-9]+}}
- // CHECK: call i1 @llvm.type.test({{i8\*|ptr}} {{%f|%0}}, metadata !"_ZTSFv5UnionE.normalized")
+ // CHECK: call i1 @llvm.type.test({{i8\*|ptr}} {{%f|%0}}, metadata !"_ZTSFvPu2i8E.normalized")
fn(arg1);
}
// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvPFvu3i32ES_E.normalized"}
// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvPFvu3i32S_ES_S_E.normalized"}
// CHECK: ![[TYPE3]] = !{i64 0, !"_ZTSFvPFvu3i32S_S_ES_S_S_E.normalized"}
-// CHECK: ![[TYPE4]] = !{i64 0, !"_ZTSFvPFv5UnionES_E.normalized"}
+// CHECK: ![[TYPE4]] = !{i64 0, !"_ZTSFvPFv5UnionEPu2i8E.normalized"}
diff --git a/clang/test/CodeGen/kcfi-generalize.c b/clang/test/CodeGen/kcfi-generalize.c
index 89b298f3e2faa..24e054549d527 100644
--- a/clang/test/CodeGen/kcfi-generalize.c
+++ b/clang/test/CodeGen/kcfi-generalize.c
@@ -33,8 +33,8 @@ union Union {
// CHECK: define{{.*}} void @uni({{.*}} !kcfi_type [[TYPE2:![0-9]+]]
void uni(void (*fn)(union Union), union Union arg1) {
- // UNGENERALIZED: call {{.*}} [ "kcfi"(i32 -1037059548) ]
- // GENERALIZED: call {{.*}} [ "kcfi"(i32 422130955) ]
+ // UNGENERALIZED: call {{.*}} [ "kcfi"(i32 -587217045) ]
+ // GENERALIZED: call {{.*}} [ "kcfi"(i32 2139530422) ]
fn(arg1);
}
@@ -44,5 +44,5 @@ void uni(void (*fn)(union Union), union Union arg1) {
// UNGENERALIZED: [[TYPE3]] = !{i32 874141567}
// GENERALIZED: [[TYPE3]] = !{i32 954385378}
-// UNGENERALIZED: [[TYPE2]] = !{i32 981319178}
-// GENERALIZED: [[TYPE2]] = !{i32 -1599950473}
\ No newline at end of file
+// UNGENERALIZED: [[TYPE2]] = !{i32 -1619636625}
+// GENERALIZED: [[TYPE2]] = !{i32 -125078496}
\ No newline at end of file
diff --git a/clang/test/CodeGen/kcfi-normalize.c b/clang/test/CodeGen/kcfi-normalize.c
index cde784962d11a..08f5249b6d6c3 100644
--- a/clang/test/CodeGen/kcfi-normalize.c
+++ b/clang/test/CodeGen/kcfi-normalize.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers -x c++ -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers -o - %s | FileCheck %s --check-prefixes=CHECK,C
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers -x c++ -o - %s | FileCheck %s --check-prefixes=CHECK,CPP
#if !__has_feature(kcfi)
#error Missing kcfi?
#endif
@@ -36,7 +36,8 @@ union Union {
void uni(void (*fn)(union Union), union Union arg1) {
// CHECK-LABEL: define{{.*}}uni
// CHECK-SAME: {{.*}}!kcfi_type ![[TYPE4:[0-9]+]]
- // CHECK: call void %0(ptr %1) [ "kcfi"(i32 -1430221633) ]
+ // C: call void %0(ptr %1) [ "kcfi"(i32 1819770848) ]
+ // CPP: call void %0(ptr %1) [ "kcfi"(i32 -1430221633) ]
fn(arg1);
}
@@ -44,4 +45,5 @@ void uni(void (*fn)(union Union), union Union arg1) {
// CHECK: ![[TYPE1]] = !{i32 -1143117868}
// CHECK: ![[TYPE2]] = !{i32 -460921415}
// CHECK: ![[TYPE3]] = !{i32 -333839615}
-// CHECK: ![[TYPE4]] = !{i32 1766237188}
\ No newline at end of file
+// C: ![[TYPE4]] = !{i32 -650530463}
+// CPP: ![[TYPE4]] = !{i32 1766237188}
\ No newline at end of file
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Created using spr 1.3.6
Created using spr 1.3.6 [skip ci]
Created using spr 1.3.6 [skip ci]
Created using spr 1.3.6 [skip ci]
Created using spr 1.3.6 [skip ci]
…ctionType (#158191) For #158193 --------- Co-authored-by: Alex Langford <[email protected]>
Created using spr 1.3.6 [skip ci]
| const RecordDecl *UD = UT->getOriginalDecl()->getDefinitionOrSelf(); | ||
| if (!UD->hasAttr<TransparentUnionAttr>()) | ||
| return Ty; | ||
| for (const auto *it : UD->fields()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was it intentional to use a range based for loop here only to unconditionally return in the body?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The meaning is as intended, I think. Maybe something like if (!UD->fields().empty()) return UD->fields()[0]; would be better style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, an if statement would signal intent better.
According GCC documentation transparent union
calling convention is the same as the type of the
first member of the union.
C++ ignores attribute.
Note, it does not generalize args of function pointer args.
It's unnecessary with pointer generalization.
It will be fixed in followup patch.