[clang] [clang][test] Add function type discrimination tests to static destructor tests (PR #99604)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/99604
>From 640779f0327c3b3773ff055923c59a82ae6c7f30 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Thu, 18 Jul 2024 21:08:12 -0700
Subject: [PATCH 1/2] [clang][test] Add function type discrimination tests to
ptrauth-static-destructors
I accidentally did not include tests for the setting up runtime calls
when compiling with -fptrauth-function-pointer-type-discrimination
---
clang/test/CodeGenCXX/ptrauth-static-destructors.cpp | 12
1 file changed, 12 insertions(+)
diff --git a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
index cad43dc0746df..92daf6bbea8b7 100644
--- a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
+++ b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
@@ -5,6 +5,13 @@
// RUN:-fno-use-cxa-atexit \
// RUN: | FileCheck %s --check-prefix=ATEXIT
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s \
+// RUN: -fptrauth-function-pointer-type-discrimination -o - | FileCheck %s
--check-prefix=CXAATEXIT_DISC
+
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN: -fptrauth-function-pointer-type-discrimination -fno-use-cxa-atexit \
+// RUN: | FileCheck %s --check-prefix=ATEXIT_DISC
+
class Foo {
public:
~Foo() {
@@ -16,9 +23,14 @@ Foo global;
// CXAATEXIT: define internal void @__cxx_global_var_init()
// CXAATEXIT: call i32 @__cxa_atexit(ptr ptrauth (ptr @_ZN3FooD1Ev, i32 0),
ptr @global, ptr @__dso_handle)
+// CXAATEXIT_DISC: define internal void @__cxx_global_var_init()
+// CXAATEXIT_DISC: call i32 @__cxa_atexit(ptr ptrauth (ptr @_ZN3FooD1Ev, i32
0, i64 10942), ptr @global, ptr @__dso_handle)
// ATEXIT: define internal void @__cxx_global_var_init()
// ATEXIT: %{{.*}} = call i32 @atexit(ptr ptrauth (ptr @__dtor_global, i32
0))
// ATEXIT: define internal void @__dtor_global() {{.*}} section
"__TEXT,__StaticInit,regular,pure_instructions" {
// ATEXIT: %{{.*}} = call ptr @_ZN3FooD1Ev(ptr @global)
+
+// ATEXIT_DISC: define internal void @__cxx_global_var_init()
+// ATEXIT_DISC: %{{.*}} = call i32 @atexit(ptr ptrauth (ptr @__dtor_global,
i32 0, i64 10942))
>From 890a3183ed32fd5f5629368d9249b108d97adfee Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Mon, 22 Jul 2024 22:58:58 -0700
Subject: [PATCH 2/2] Update for trunk, include elf tests
---
.../CodeGenCXX/ptrauth-static-destructors.cpp | 30 ---
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
index 4b5eef55d38b1..634450bf62ea9 100644
--- a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
+++ b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
@@ -2,20 +2,27 @@
// RUN: | FileCheck %s --check-prefix=CXAATEXIT
// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
-// RUN:-fno-use-cxa-atexit | FileCheck %s --check-prefixes=ATEXIT,DARWIN
+// RUN:-fno-use-cxa-atexit | FileCheck %s
--check-prefixes=ATEXIT,ATEXIT_DARWIN
// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
// RUN: | FileCheck %s --check-prefix=CXAATEXIT
// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
-// RUN:-fno-use-cxa-atexit | FileCheck %s --check-prefixes=ATEXIT,ELF
+// RUN:-fno-use-cxa-atexit | FileCheck %s
--check-prefixes=ATEXIT,ATEXIT_ELF
// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s \
// RUN: -fptrauth-function-pointer-type-discrimination -o - | FileCheck %s
--check-prefix=CXAATEXIT_DISC
// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
// RUN: -fptrauth-function-pointer-type-discrimination -fno-use-cxa-atexit \
-// RUN: | FileCheck %s --check-prefix=ATEXIT_DISC
+// RUN: | FileCheck %s --check-prefixes=ATEXIT_DISC,ATEXIT_DISC_DARWIN
+
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -emit-llvm
-std=c++11 %s \
+// RUN: -fptrauth-function-pointer-type-discrimination -o - | FileCheck %s
--check-prefix=CXAATEXIT_DISC
+
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN: -fptrauth-function-pointer-type-discrimination -fno-use-cxa-atexit \
+// RUN: | FileCheck %s --check-prefixes=ATEXIT_DISC,ATEXIT_DISC_ELF
class Foo {
public:
@@ -34,13 +41,16 @@ Foo global;
// ATEXIT: define internal void @__cxx_global_var_init()
// ATEXIT: %{{.*}} = call i32 @atexit(ptr ptrauth (ptr @__dtor_global, i32
0))
-// DARWIN: define internal void @__dtor_global() {{.*}} section
"__TEXT,__StaticInit,regular,pure_instructions" {
-// ELF:define internal void @__dtor_global() {{.*}} section
".text.startup" {
-// DARWIN: %{{.*}} = call
[clang] [PAC][clang][test] Implement missing tests for some PAuth features (PR #100206)
ojhunt wrote: Looks good to me (spent a bit of time verifying correct auth logic, but for the actual elf specific codegen I can't comment) https://github.com/llvm/llvm-project/pull/100206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Update argument checking tablegen code to use a 'full' name (PR #99993)
ojhunt wrote: @AaronBallman hmmm, looks like we may have lost some diff at some point, I'll investigate to work out what is going on https://github.com/llvm/llvm-project/pull/3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Update argument checking tablegen code to use a 'full' name (PR #99993)
ojhunt wrote: @AaronBallman ah with more context, I see we didn't actually land "// Do something if not an identifier" (implying partial commit history at some point". I'm looking at exactly what happens, or if we're missing some test cases -- it's possible that when written there was the intent to either add more similar parameters, do better error checks, or possibly just over engineering. Will need to investigate further to verify, will try to determine the expected behaviour today or tomorrow (I agree if it is not necessary it should be removed - ie. if it was just added "for the future" or similar and it's had quite a few years for that future to happen and not be needed) https://github.com/llvm/llvm-project/pull/3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Ensure pointers passed to runtime support functions are correctly signed (PR #98276)
ojhunt wrote: I notice the r+, but I haven't fully addressed this feedback - are you suggesting corrections in a follow up patch, or just conceptually approving with this PR? :D https://github.com/llvm/llvm-project/pull/98276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Ensure pointers passed to runtime support functions are correctly signed (PR #98276)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/98276
>From 4b92c4af87a1a381dad09b243db4d3ec71d64738 Mon Sep 17 00:00:00 2001
From: John McCall
Date: Wed, 18 Sep 2019 02:21:37 -0400
Subject: [PATCH 1/5] Sign function pointers passed to atexit and __cxa_atexit.
Patch by Akira Hatanaka.
---
clang/lib/CodeGen/CGDeclCXX.cpp | 9 +--
clang/lib/CodeGen/CodeGenFunction.h | 2 +-
clang/lib/CodeGen/ItaniumCXXABI.cpp | 10 ++-
.../CodeGenCXX/ptrauth-static-destructors.cpp | 26 +++
4 files changed, 43 insertions(+), 4 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 05dd7ddb86fa6..36248a9c75188 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -232,7 +232,7 @@ void CodeGenFunction::EmitCXXGlobalVarDeclInit(const
VarDecl &D,
/// Create a stub function, suitable for being passed to atexit,
/// which passes the given address to the given destructor function.
-llvm::Function *CodeGenFunction::createAtExitStub(const VarDecl &VD,
+llvm::Constant *CodeGenFunction::createAtExitStub(const VarDecl &VD,
llvm::FunctionCallee dtor,
llvm::Constant *addr) {
// Get the destructor function type, void(*)(void).
@@ -264,7 +264,12 @@ llvm::Function *CodeGenFunction::createAtExitStub(const
VarDecl &VD,
CGF.FinishFunction();
- return fn;
+ // Get a proper function pointer.
+ FunctionProtoType::ExtProtoInfo EPI(getContext().getDefaultCallingConvention(
+ /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+ QualType fnType = getContext().getFunctionType(getContext().VoidTy,
+ {getContext().VoidPtrTy},
EPI);
+ return CGM.getFunctionPointer(fn, fnType);
}
/// Create a stub function, suitable for being passed to __pt_atexit_np,
diff --git a/clang/lib/CodeGen/CodeGenFunction.h
b/clang/lib/CodeGen/CodeGenFunction.h
index 13f12b5d878a6..e33267c4787fd 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4862,7 +4862,7 @@ class CodeGenFunction : public CodeGenTypeCache {
void EmitCXXGlobalVarDeclInit(const VarDecl &D, llvm::GlobalVariable *GV,
bool PerformInit);
- llvm::Function *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee
Dtor,
+ llvm::Constant *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee
Dtor,
llvm::Constant *Addr);
llvm::Function *createTLSAtExitStub(const VarDecl &VD,
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index e1d056765a866..1924c07c1529d 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2699,6 +2699,14 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction
&CGF,
if (llvm::Function *fn = dyn_cast(atexit.getCallee()))
fn->setDoesNotThrow();
+ auto &Context = CGF.CGM.getContext();
+ FunctionProtoType::ExtProtoInfo EPI(Context.getDefaultCallingConvention(
+ /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+ QualType fnType =
+ Context.getFunctionType(Context.VoidTy, {Context.VoidPtrTy}, EPI);
+ llvm::Constant *dtorCallee = cast(dtor.getCallee());
+ dtorCallee = CGF.CGM.getFunctionPointer(dtorCallee, fnType);
+
if (!addr)
// addr is null when we are trying to register a dtor annotated with
// __attribute__((destructor)) in a constructor function. Using null here
is
@@ -2706,7 +2714,7 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction
&CGF,
// function.
addr = llvm::Constant::getNullValue(CGF.Int8PtrTy);
- llvm::Value *args[] = {dtor.getCallee(), addr, handle};
+ llvm::Value *args[] = {dtorCallee, addr, handle};
CGF.EmitNounwindRuntimeCall(atexit, args);
}
diff --git a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
new file mode 100644
index 0..6c8d0c560681a
--- /dev/null
+++ b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN: | FileCheck %s --check-prefix=CXAATEXIT
+
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN:-fno-use-cxa-atexit \
+// RUN: | FileCheck %s --check-prefix=ATEXIT
+
+class Foo {
+ public:
+ ~Foo() {
+ }
+};
+
+Foo global;
+
+// CXAATEXIT: @_ZN3FooD1Ev.ptrauth = private constant { i8*, i32, i64, i64 } {
i8* bitcast (%class.Foo* (%class.Foo*)* @_ZN3FooD1Ev to i8*), i32 0, i64 0, i64
0 }, section "llvm.ptrauth", align 8
+// CXAATEXIT: define internal void @__cxx_global_var_init()
+// CXAATEXIT: call i32 @__cxa_atexit(void (i8*)* bitcast ({ i8*
[clang] [clang] Implement authentication for C++ member function pointers (PR #99576)
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/99576
Introduces type based signing of member function pointers. To support this
discrimination schema we no longer emit member function pointer to virtual
methods and indices into a vtable but migrate to using thunks. This does mean
member function pointers are no longer necessarily directly comparable, however
as such comparisons are UB this is acceptable.
We derive the discriminator from the C++ mangling of the type of the pointer
being authenticated.
Co-Authored-By: Akira Hatanaka ahatan...@apple.com
Co-Authored-By: John McCall rjmcc...@apple.com
>From 571ab7b4f78cd2e4f89afcd9c35da1c07848742b Mon Sep 17 00:00:00 2001
From: Ahmed Bougacha
Date: Fri, 10 May 2024 15:58:57 -0700
Subject: [PATCH] [clang] Implement pointer authentication for C++ member
function pointers.
Introduces type based signing of member function pointers. To support this
discrimination schema we no longer emit member function pointer to virtual
methods and indices into a vtable but migrate to using thunks. This does mean
member function pointers are no longer necessarily directly comparable,
however as such comparisons are UB this is acceptable.
We derive the discriminator from the C++ mangling of the type of the pointer
being authenticated.
Co-Authored-By: Akira Hatanaka ahatan...@apple.com
Co-Authored-By: John McCall rjmcc...@apple.com
---
clang/include/clang/AST/ASTContext.h | 2 +-
.../include/clang/Basic/PointerAuthOptions.h | 3 +
clang/lib/AST/ASTContext.cpp | 12 +-
clang/lib/CodeGen/CGCall.cpp | 18 +-
clang/lib/CodeGen/CGPointerAuth.cpp | 34 ++
clang/lib/CodeGen/CodeGenFunction.h | 3 +-
clang/lib/CodeGen/CodeGenModule.h | 8 +
clang/lib/CodeGen/ItaniumCXXABI.cpp | 255 ++-
clang/lib/Frontend/CompilerInvocation.cpp | 2 +
.../ptrauth-member-function-pointer.cpp | 433 ++
clang/test/Preprocessor/ptrauth_feature.c | 9 +
11 files changed, 757 insertions(+), 22 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ptrauth-member-function-pointer.cpp
diff --git a/clang/include/clang/AST/ASTContext.h
b/clang/include/clang/AST/ASTContext.h
index 608bd90fcc3ff..f2a17a56dc201 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1287,7 +1287,7 @@ class ASTContext : public RefCountedBase {
getPointerAuthVTablePointerDiscriminator(const CXXRecordDecl *RD);
/// Return the "other" type-specific discriminator for the given type.
- uint16_t getPointerAuthTypeDiscriminator(QualType T) const;
+ uint16_t getPointerAuthTypeDiscriminator(QualType T);
/// Apply Objective-C protocol qualifiers to the given type.
/// \param allowOnPointerType specifies if we can apply protocol
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..595322b8271b6 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -175,6 +175,9 @@ struct PointerAuthOptions {
/// The ABI for variadic C++ virtual function pointers.
PointerAuthSchema CXXVirtualVariadicFunctionPointers;
+
+ /// The ABI for C++ member function pointers.
+ PointerAuthSchema CXXMemberFunctionPointers;
};
} // end namespace clang
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a8e599f7ebe04..58bb259a5395a 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3404,7 +3404,7 @@ static void encodeTypeForFunctionPointerAuth(const
ASTContext &Ctx,
}
}
-uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
+uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) {
assert(!T->isDependentType() &&
"cannot compute type discriminator of a dependent type");
@@ -3414,11 +3414,13 @@ uint16_t
ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
if (T->isFunctionPointerType() || T->isFunctionReferenceType())
T = T->getPointeeType();
- if (T->isFunctionType())
+ if (T->isFunctionType()) {
encodeTypeForFunctionPointerAuth(*this, Out, T);
- else
-llvm_unreachable(
-"type discrimination of non-function type not implemented yet");
+ } else {
+T = T.getUnqualifiedType();
+std::unique_ptr MC(createMangleContext());
+MC->mangleCanonicalTypeName(T, Out);
+ }
return llvm::getPointerAuthStableSipHash(Str);
}
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index d582aba679ddc..f5b669460aa33 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5034,7 +5034,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
ReturnValueSlot ReturnValue,
const CallArgList &CallArgs,
[clang] [clang] Implement authentication for C++ member function pointers (PR #99576)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/99576
>From c2bc964341fb4d0738e94d4afa6abc84eed0113c Mon Sep 17 00:00:00 2001
From: Ahmed Bougacha
Date: Fri, 10 May 2024 15:58:57 -0700
Subject: [PATCH] [clang] Implement pointer authentication for C++ member
function pointers.
Introduces type based signing of member function pointers. To support this
discrimination schema we no longer emit member function pointer to virtual
methods and indices into a vtable but migrate to using thunks. This does mean
member function pointers are no longer necessarily directly comparable,
however as such comparisons are UB this is acceptable.
We derive the discriminator from the C++ mangling of the type of the pointer
being authenticated.
Co-Authored-By: Akira Hatanaka ahatan...@apple.com
Co-Authored-By: John McCall rjmcc...@apple.com
---
clang/include/clang/AST/ASTContext.h | 2 +-
.../include/clang/Basic/PointerAuthOptions.h | 3 +
clang/lib/AST/ASTContext.cpp | 12 +-
clang/lib/CodeGen/CGCall.cpp | 212 +
clang/lib/CodeGen/CGPointerAuth.cpp | 34 ++
clang/lib/CodeGen/CodeGenFunction.h | 3 +-
clang/lib/CodeGen/CodeGenModule.h | 8 +
clang/lib/CodeGen/ItaniumCXXABI.cpp | 254 +-
clang/lib/Frontend/CompilerInvocation.cpp | 2 +
.../ptrauth-member-function-pointer.cpp | 433 ++
clang/test/Preprocessor/ptrauth_feature.c | 9 +
11 files changed, 855 insertions(+), 117 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ptrauth-member-function-pointer.cpp
diff --git a/clang/include/clang/AST/ASTContext.h
b/clang/include/clang/AST/ASTContext.h
index 608bd90fcc3ff..f2a17a56dc201 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1287,7 +1287,7 @@ class ASTContext : public RefCountedBase {
getPointerAuthVTablePointerDiscriminator(const CXXRecordDecl *RD);
/// Return the "other" type-specific discriminator for the given type.
- uint16_t getPointerAuthTypeDiscriminator(QualType T) const;
+ uint16_t getPointerAuthTypeDiscriminator(QualType T);
/// Apply Objective-C protocol qualifiers to the given type.
/// \param allowOnPointerType specifies if we can apply protocol
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..595322b8271b6 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -175,6 +175,9 @@ struct PointerAuthOptions {
/// The ABI for variadic C++ virtual function pointers.
PointerAuthSchema CXXVirtualVariadicFunctionPointers;
+
+ /// The ABI for C++ member function pointers.
+ PointerAuthSchema CXXMemberFunctionPointers;
};
} // end namespace clang
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a8e599f7ebe04..58bb259a5395a 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3404,7 +3404,7 @@ static void encodeTypeForFunctionPointerAuth(const
ASTContext &Ctx,
}
}
-uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
+uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) {
assert(!T->isDependentType() &&
"cannot compute type discriminator of a dependent type");
@@ -3414,11 +3414,13 @@ uint16_t
ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
if (T->isFunctionPointerType() || T->isFunctionReferenceType())
T = T->getPointeeType();
- if (T->isFunctionType())
+ if (T->isFunctionType()) {
encodeTypeForFunctionPointerAuth(*this, Out, T);
- else
-llvm_unreachable(
-"type discrimination of non-function type not implemented yet");
+ } else {
+T = T.getUnqualifiedType();
+std::unique_ptr MC(createMangleContext());
+MC->mangleCanonicalTypeName(T, Out);
+ }
return llvm::getPointerAuthStableSipHash(Str);
}
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index d582aba679ddc..52e61cf37189d 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5034,7 +5034,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
ReturnValueSlot ReturnValue,
const CallArgList &CallArgs,
llvm::CallBase **callOrInvoke, bool
IsMustTail,
- SourceLocation Loc) {
+ SourceLocation Loc,
+ bool IsVirtualFunctionPointerThunk) {
// FIXME: We no longer need the types from CallArgs; lift up and simplify.
assert(Callee.isOrdinary() || Callee.isVirtual());
@@ -5098,7 +5099,11 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
RawAddress SRetAlloca = RawAddress::invalid();
llvm::Value *UnusedReturnSizePtr =
[clang] [clang] Implement authentication for C++ member function pointers (PR #99576)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/99576
>From 547ce3e9c7fe7e046b20f51f2f0d370a683659d1 Mon Sep 17 00:00:00 2001
From: Ahmed Bougacha
Date: Fri, 10 May 2024 15:58:57 -0700
Subject: [PATCH] [PAC] Implement pointer authentication for C++ member
function pointers.
Introduces type based signing of member function pointers. To support this
discrimination schema we no longer emit member function pointer to virtual
methods and indices into a vtable but migrate to using thunks. This does mean
member function pointers are no longer necessarily directly comparable,
however as such comparisons are UB this is acceptable.
We derive the discriminator from the C++ mangling of the type of the pointer
being authenticated.
Co-Authored-By: Akira Hatanaka ahatan...@apple.com
Co-Authored-By: John McCall rjmcc...@apple.com
---
clang/include/clang/AST/ASTContext.h | 2 +-
.../include/clang/Basic/PointerAuthOptions.h | 3 +
clang/lib/AST/ASTContext.cpp | 12 +-
clang/lib/CodeGen/CGCall.cpp | 212 +
clang/lib/CodeGen/CGPointerAuth.cpp | 34 ++
clang/lib/CodeGen/CodeGenFunction.h | 3 +-
clang/lib/CodeGen/CodeGenModule.h | 8 +
clang/lib/CodeGen/ItaniumCXXABI.cpp | 254 +-
clang/lib/Frontend/CompilerInvocation.cpp | 2 +
.../ptrauth-member-function-pointer.cpp | 433 ++
clang/test/Preprocessor/ptrauth_feature.c | 9 +
11 files changed, 855 insertions(+), 117 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ptrauth-member-function-pointer.cpp
diff --git a/clang/include/clang/AST/ASTContext.h
b/clang/include/clang/AST/ASTContext.h
index 608bd90fcc3ff..f2a17a56dc201 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1287,7 +1287,7 @@ class ASTContext : public RefCountedBase {
getPointerAuthVTablePointerDiscriminator(const CXXRecordDecl *RD);
/// Return the "other" type-specific discriminator for the given type.
- uint16_t getPointerAuthTypeDiscriminator(QualType T) const;
+ uint16_t getPointerAuthTypeDiscriminator(QualType T);
/// Apply Objective-C protocol qualifiers to the given type.
/// \param allowOnPointerType specifies if we can apply protocol
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..595322b8271b6 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -175,6 +175,9 @@ struct PointerAuthOptions {
/// The ABI for variadic C++ virtual function pointers.
PointerAuthSchema CXXVirtualVariadicFunctionPointers;
+
+ /// The ABI for C++ member function pointers.
+ PointerAuthSchema CXXMemberFunctionPointers;
};
} // end namespace clang
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a8e599f7ebe04..58bb259a5395a 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3404,7 +3404,7 @@ static void encodeTypeForFunctionPointerAuth(const
ASTContext &Ctx,
}
}
-uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
+uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) {
assert(!T->isDependentType() &&
"cannot compute type discriminator of a dependent type");
@@ -3414,11 +3414,13 @@ uint16_t
ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
if (T->isFunctionPointerType() || T->isFunctionReferenceType())
T = T->getPointeeType();
- if (T->isFunctionType())
+ if (T->isFunctionType()) {
encodeTypeForFunctionPointerAuth(*this, Out, T);
- else
-llvm_unreachable(
-"type discrimination of non-function type not implemented yet");
+ } else {
+T = T.getUnqualifiedType();
+std::unique_ptr MC(createMangleContext());
+MC->mangleCanonicalTypeName(T, Out);
+ }
return llvm::getPointerAuthStableSipHash(Str);
}
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index d582aba679ddc..52e61cf37189d 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5034,7 +5034,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
ReturnValueSlot ReturnValue,
const CallArgList &CallArgs,
llvm::CallBase **callOrInvoke, bool
IsMustTail,
- SourceLocation Loc) {
+ SourceLocation Loc,
+ bool IsVirtualFunctionPointerThunk) {
// FIXME: We no longer need the types from CallArgs; lift up and simplify.
assert(Callee.isOrdinary() || Callee.isVirtual());
@@ -5098,7 +5099,11 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
RawAddress SRetAlloca = RawAddress::invalid();
llvm::Value *UnusedReturnSizePtr = n
[clang] [PAC] Implement authentication for C++ member function pointers (PR #99576)
https://github.com/ojhunt edited https://github.com/llvm/llvm-project/pull/99576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][test] Add function type discrimination tests to static destructor tests (PR #99604)
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/99604
I accidentally did not include tests for the setting up runtime calls when
compiling with -fptrauth-function-pointer-type-discrimination
>From 640779f0327c3b3773ff055923c59a82ae6c7f30 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Thu, 18 Jul 2024 21:08:12 -0700
Subject: [PATCH] [clang][test] Add function type discrimination tests to
ptrauth-static-destructors
I accidentally did not include tests for the setting up runtime calls
when compiling with -fptrauth-function-pointer-type-discrimination
---
clang/test/CodeGenCXX/ptrauth-static-destructors.cpp | 12
1 file changed, 12 insertions(+)
diff --git a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
index cad43dc0746df..92daf6bbea8b7 100644
--- a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
+++ b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
@@ -5,6 +5,13 @@
// RUN:-fno-use-cxa-atexit \
// RUN: | FileCheck %s --check-prefix=ATEXIT
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s \
+// RUN: -fptrauth-function-pointer-type-discrimination -o - | FileCheck %s
--check-prefix=CXAATEXIT_DISC
+
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN: -fptrauth-function-pointer-type-discrimination -fno-use-cxa-atexit \
+// RUN: | FileCheck %s --check-prefix=ATEXIT_DISC
+
class Foo {
public:
~Foo() {
@@ -16,9 +23,14 @@ Foo global;
// CXAATEXIT: define internal void @__cxx_global_var_init()
// CXAATEXIT: call i32 @__cxa_atexit(ptr ptrauth (ptr @_ZN3FooD1Ev, i32 0),
ptr @global, ptr @__dso_handle)
+// CXAATEXIT_DISC: define internal void @__cxx_global_var_init()
+// CXAATEXIT_DISC: call i32 @__cxa_atexit(ptr ptrauth (ptr @_ZN3FooD1Ev, i32
0, i64 10942), ptr @global, ptr @__dso_handle)
// ATEXIT: define internal void @__cxx_global_var_init()
// ATEXIT: %{{.*}} = call i32 @atexit(ptr ptrauth (ptr @__dtor_global, i32
0))
// ATEXIT: define internal void @__dtor_global() {{.*}} section
"__TEXT,__StaticInit,regular,pure_instructions" {
// ATEXIT: %{{.*}} = call ptr @_ZN3FooD1Ev(ptr @global)
+
+// ATEXIT_DISC: define internal void @__cxx_global_var_init()
+// ATEXIT_DISC: %{{.*}} = call i32 @atexit(ptr ptrauth (ptr @__dtor_global,
i32 0, i64 10942))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][test] Add function type discrimination tests to static destructor tests (PR #99604)
https://github.com/ojhunt ready_for_review https://github.com/llvm/llvm-project/pull/99604 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PAC] Implement authentication for C++ member function pointers (PR #99576)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/99576
>From 547ce3e9c7fe7e046b20f51f2f0d370a683659d1 Mon Sep 17 00:00:00 2001
From: Ahmed Bougacha
Date: Fri, 10 May 2024 15:58:57 -0700
Subject: [PATCH] [PAC] Implement pointer authentication for C++ member
function pointers.
Introduces type based signing of member function pointers. To support this
discrimination schema we no longer emit member function pointer to virtual
methods and indices into a vtable but migrate to using thunks. This does mean
member function pointers are no longer necessarily directly comparable,
however as such comparisons are UB this is acceptable.
We derive the discriminator from the C++ mangling of the type of the pointer
being authenticated.
Co-Authored-By: Akira Hatanaka ahatan...@apple.com
Co-Authored-By: John McCall rjmcc...@apple.com
---
clang/include/clang/AST/ASTContext.h | 2 +-
.../include/clang/Basic/PointerAuthOptions.h | 3 +
clang/lib/AST/ASTContext.cpp | 12 +-
clang/lib/CodeGen/CGCall.cpp | 212 +
clang/lib/CodeGen/CGPointerAuth.cpp | 34 ++
clang/lib/CodeGen/CodeGenFunction.h | 3 +-
clang/lib/CodeGen/CodeGenModule.h | 8 +
clang/lib/CodeGen/ItaniumCXXABI.cpp | 254 +-
clang/lib/Frontend/CompilerInvocation.cpp | 2 +
.../ptrauth-member-function-pointer.cpp | 433 ++
clang/test/Preprocessor/ptrauth_feature.c | 9 +
11 files changed, 855 insertions(+), 117 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ptrauth-member-function-pointer.cpp
diff --git a/clang/include/clang/AST/ASTContext.h
b/clang/include/clang/AST/ASTContext.h
index 608bd90fcc3ff..f2a17a56dc201 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1287,7 +1287,7 @@ class ASTContext : public RefCountedBase {
getPointerAuthVTablePointerDiscriminator(const CXXRecordDecl *RD);
/// Return the "other" type-specific discriminator for the given type.
- uint16_t getPointerAuthTypeDiscriminator(QualType T) const;
+ uint16_t getPointerAuthTypeDiscriminator(QualType T);
/// Apply Objective-C protocol qualifiers to the given type.
/// \param allowOnPointerType specifies if we can apply protocol
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..595322b8271b6 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -175,6 +175,9 @@ struct PointerAuthOptions {
/// The ABI for variadic C++ virtual function pointers.
PointerAuthSchema CXXVirtualVariadicFunctionPointers;
+
+ /// The ABI for C++ member function pointers.
+ PointerAuthSchema CXXMemberFunctionPointers;
};
} // end namespace clang
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a8e599f7ebe04..58bb259a5395a 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3404,7 +3404,7 @@ static void encodeTypeForFunctionPointerAuth(const
ASTContext &Ctx,
}
}
-uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
+uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) {
assert(!T->isDependentType() &&
"cannot compute type discriminator of a dependent type");
@@ -3414,11 +3414,13 @@ uint16_t
ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
if (T->isFunctionPointerType() || T->isFunctionReferenceType())
T = T->getPointeeType();
- if (T->isFunctionType())
+ if (T->isFunctionType()) {
encodeTypeForFunctionPointerAuth(*this, Out, T);
- else
-llvm_unreachable(
-"type discrimination of non-function type not implemented yet");
+ } else {
+T = T.getUnqualifiedType();
+std::unique_ptr MC(createMangleContext());
+MC->mangleCanonicalTypeName(T, Out);
+ }
return llvm::getPointerAuthStableSipHash(Str);
}
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index d582aba679ddc..52e61cf37189d 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5034,7 +5034,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
ReturnValueSlot ReturnValue,
const CallArgList &CallArgs,
llvm::CallBase **callOrInvoke, bool
IsMustTail,
- SourceLocation Loc) {
+ SourceLocation Loc,
+ bool IsVirtualFunctionPointerThunk) {
// FIXME: We no longer need the types from CallArgs; lift up and simplify.
assert(Callee.isOrdinary() || Callee.isVirtual());
@@ -5098,7 +5099,11 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
RawAddress SRetAlloca = RawAddress::invalid();
llvm::Value *UnusedReturnSizePtr = n
[clang] [PAC] Implement authentication for C++ member function pointers (PR #99576)
https://github.com/ojhunt ready_for_review https://github.com/llvm/llvm-project/pull/99576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PAC] Implement authentication for C++ member function pointers (PR #99576)
ojhunt wrote: @asl could you check this as well? (I apparently can't add/request reviewers?) https://github.com/llvm/llvm-project/pull/99576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PAC] Incorrect codegen for constant global init with polymorphic MI (PR #99741)
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/99741
Fixes an error where we use the wrong authentication schema for secondary
vtable pointers in constant initialized globals of types with multiple
polymorphic base classes.
>From 5822eb8f5b0d420a89d539f8fd2e23c69983967a Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 19 Jul 2024 22:48:26 -0700
Subject: [PATCH] [PAC] Incorrect codegen for constant global init with
polymorphic MI
Fixes an error where we use the wrong authentication schema for
secondary vtable pointers in constant initialized globals of types
with multiple polymorphic base classes.
---
clang/lib/CodeGen/CGExprConstant.cpp | 2 +-
.../ptrauth-global-constant-initializers.cpp | 234 ++
2 files changed, 235 insertions(+), 1 deletion(-)
create mode 100644
clang/test/CodeGenCXX/ptrauth-global-constant-initializers.cpp
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp
b/clang/lib/CodeGen/CGExprConstant.cpp
index 7c65fccb60855..3c6a522004f3a 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -815,7 +815,7 @@ bool ConstStructBuilder::Build(const APValue &Val, const
RecordDecl *RD,
CGM.getCXXABI().getVTableAddressPoint(BaseSubobject(CD, Offset),
VTableClass);
if (auto Authentication =
- CGM.getVTablePointerAuthentication(VTableClass)) {
+ CGM.getVTablePointerAuthentication(CD)) {
VTableAddressPoint = Emitter.tryEmitConstantSignedPointer(
VTableAddressPoint, *Authentication);
if (!VTableAddressPoint)
diff --git a/clang/test/CodeGenCXX/ptrauth-global-constant-initializers.cpp
b/clang/test/CodeGenCXX/ptrauth-global-constant-initializers.cpp
new file mode 100644
index 0..f0c3ea83d8958
--- /dev/null
+++ b/clang/test/CodeGenCXX/ptrauth-global-constant-initializers.cpp
@@ -0,0 +1,234 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -fno-rtti
-fptrauth-vtable-pointer-type-discrimination
-fptrauth-vtable-pointer-address-discrimination -emit-llvm -o - %s | FileCheck
%s
+
+// CHECK: %struct.Base1 = type { ptr }
+// CHECK: %struct.Base2 = type { ptr }
+// CHECK: %struct.Derived1 = type { %struct.Base1, %struct.Base2 }
+// CHECK: %struct.Derived2 = type { %struct.Base2, %struct.Base1 }
+// CHECK: %struct.Derived3 = type { %struct.Base1, %struct.Base2 }
+
+// CHECK: @_ZTV5Base1 = linkonce_odr unnamed_addr constant { [3 x ptr] } { [3
x ptr] [ptr null, ptr null, ptr ptrauth (ptr @_ZN5Base11aEv, i32 0, i64
[[BASE1_A_DISC:38871]], ptr getelementptr inbounds ({ [3 x ptr] }, ptr
@_ZTV5Base1, i32 0, i32 0, i32 2))] }, align 8
+// CHECK: @g_b1 = global %struct.Base1 { ptr ptrauth (ptr getelementptr
inbounds inrange(-16, 8) ({ [3 x ptr] }, ptr @_ZTV5Base1, i32 0, i32 0, i32 2),
i32 2, i64 [[BASE1_VTABLE_DISC:6511]], ptr @g_b1) }, align 8
+// CHECK: @_ZTV5Base2 = linkonce_odr unnamed_addr constant { [3 x ptr] } { [3
x ptr] [ptr null, ptr null, ptr ptrauth (ptr @_ZN5Base21bEv, i32 0, i64
[[BASE2_B_DISC:27651]], ptr getelementptr inbounds ({ [3 x ptr] }, ptr
@_ZTV5Base2, i32 0, i32 0, i32 2))] }, align 8
+// CHECK: @g_b2 = global %struct.Base2 { ptr ptrauth (ptr getelementptr
inbounds inrange(-16, 8) ({ [3 x ptr] }, ptr @_ZTV5Base2, i32 0, i32 0, i32 2),
i32 2, i64 [[BASE2_VTABLE_DISC:63631]], ptr @g_b2) }, align 8
+// CHECK: @_ZTV8Derived1 = linkonce_odr unnamed_addr constant { [5 x ptr], [3
x ptr] } { [5 x ptr] [ptr null, ptr null, ptr ptrauth (ptr @_ZN5Base11aEv, i32
0, i64 [[BASE1_A_DISC]], ptr getelementptr inbounds ({ [5 x ptr], [3 x ptr] },
ptr @_ZTV8Derived1, i32 0, i32 0, i32 2)), ptr ptrauth (ptr @_ZN8Derived11cEv,
i32 0, i64 [[DERIVED1_C_DISC:54092]], ptr getelementptr inbounds ({ [5 x ptr],
[3 x ptr] }, ptr @_ZTV8Derived1, i32 0, i32 0, i32 3)), ptr ptrauth (ptr
@_ZN8Derived11dEv, i32 0, i64 [[DERIVED1_D_DISC:37391]], ptr getelementptr
inbounds ({ [5 x ptr], [3 x ptr] }, ptr @_ZTV8Derived1, i32 0, i32 0, i32 4))],
[3 x ptr] [ptr inttoptr (i64 -8 to ptr), ptr null, ptr ptrauth (ptr
@_ZN5Base21bEv, i32 0, i64 [[BASE2_B_DISC]], ptr getelementptr inbounds ({ [5 x
ptr], [3 x ptr] }, ptr @_ZTV8Derived1, i32 0, i32 1, i32 2))] }, align 8
+// CHECK: @g_d1 = global { ptr, ptr } { ptr ptrauth (ptr getelementptr
inbounds inrange(-16, 24) ({ [5 x ptr], [3 x ptr] }, ptr @_ZTV8Derived1, i32 0,
i32 0, i32 2), i32 2, i64 [[BASE1_VTABLE_DISC]], ptr @g_d1), ptr ptrauth (ptr
getelementptr inbounds inrange(-16, 8) ({ [5 x ptr], [3 x ptr] }, ptr
@_ZTV8Derived1, i32 0, i32 1, i32 2), i32 2, i64 [[BASE2_VTABLE_DISC]], ptr
getelementptr inbounds ({ ptr, ptr }, ptr @g_d1, i32 0, i32 1)) }, align 8
+// CHECK: @_ZTV8Derived2 = linkonce_odr unnamed_addr constant { [5 x ptr], [3
x ptr] } { [5 x ptr] [ptr null, ptr null, ptr ptrauth (ptr @_ZN5Base21bEv, i32
0, i64 [[BASE2_B_DISC]], ptr getelementptr inbounds ({ [5 x ptr], [3 x ptr] },
ptr @_ZTV8
[clang] [PAC] Incorrect codegen for constant global init with polymorphic MI (PR #99741)
ojhunt wrote: @ahmedbougacha @asl @kovdan01 pinging for review https://github.com/llvm/llvm-project/pull/99741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PAC] Incorrect codegen for constant global init with polymorphic MI (PR #99741)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/99741
>From 5822eb8f5b0d420a89d539f8fd2e23c69983967a Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 19 Jul 2024 22:48:26 -0700
Subject: [PATCH 1/2] [PAC] Incorrect codegen for constant global init with
polymorphic MI
Fixes an error where we use the wrong authentication schema for
secondary vtable pointers in constant initialized globals of types
with multiple polymorphic base classes.
---
clang/lib/CodeGen/CGExprConstant.cpp | 2 +-
.../ptrauth-global-constant-initializers.cpp | 234 ++
2 files changed, 235 insertions(+), 1 deletion(-)
create mode 100644
clang/test/CodeGenCXX/ptrauth-global-constant-initializers.cpp
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp
b/clang/lib/CodeGen/CGExprConstant.cpp
index 7c65fccb60855..3c6a522004f3a 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -815,7 +815,7 @@ bool ConstStructBuilder::Build(const APValue &Val, const
RecordDecl *RD,
CGM.getCXXABI().getVTableAddressPoint(BaseSubobject(CD, Offset),
VTableClass);
if (auto Authentication =
- CGM.getVTablePointerAuthentication(VTableClass)) {
+ CGM.getVTablePointerAuthentication(CD)) {
VTableAddressPoint = Emitter.tryEmitConstantSignedPointer(
VTableAddressPoint, *Authentication);
if (!VTableAddressPoint)
diff --git a/clang/test/CodeGenCXX/ptrauth-global-constant-initializers.cpp
b/clang/test/CodeGenCXX/ptrauth-global-constant-initializers.cpp
new file mode 100644
index 0..f0c3ea83d8958
--- /dev/null
+++ b/clang/test/CodeGenCXX/ptrauth-global-constant-initializers.cpp
@@ -0,0 +1,234 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -fno-rtti
-fptrauth-vtable-pointer-type-discrimination
-fptrauth-vtable-pointer-address-discrimination -emit-llvm -o - %s | FileCheck
%s
+
+// CHECK: %struct.Base1 = type { ptr }
+// CHECK: %struct.Base2 = type { ptr }
+// CHECK: %struct.Derived1 = type { %struct.Base1, %struct.Base2 }
+// CHECK: %struct.Derived2 = type { %struct.Base2, %struct.Base1 }
+// CHECK: %struct.Derived3 = type { %struct.Base1, %struct.Base2 }
+
+// CHECK: @_ZTV5Base1 = linkonce_odr unnamed_addr constant { [3 x ptr] } { [3
x ptr] [ptr null, ptr null, ptr ptrauth (ptr @_ZN5Base11aEv, i32 0, i64
[[BASE1_A_DISC:38871]], ptr getelementptr inbounds ({ [3 x ptr] }, ptr
@_ZTV5Base1, i32 0, i32 0, i32 2))] }, align 8
+// CHECK: @g_b1 = global %struct.Base1 { ptr ptrauth (ptr getelementptr
inbounds inrange(-16, 8) ({ [3 x ptr] }, ptr @_ZTV5Base1, i32 0, i32 0, i32 2),
i32 2, i64 [[BASE1_VTABLE_DISC:6511]], ptr @g_b1) }, align 8
+// CHECK: @_ZTV5Base2 = linkonce_odr unnamed_addr constant { [3 x ptr] } { [3
x ptr] [ptr null, ptr null, ptr ptrauth (ptr @_ZN5Base21bEv, i32 0, i64
[[BASE2_B_DISC:27651]], ptr getelementptr inbounds ({ [3 x ptr] }, ptr
@_ZTV5Base2, i32 0, i32 0, i32 2))] }, align 8
+// CHECK: @g_b2 = global %struct.Base2 { ptr ptrauth (ptr getelementptr
inbounds inrange(-16, 8) ({ [3 x ptr] }, ptr @_ZTV5Base2, i32 0, i32 0, i32 2),
i32 2, i64 [[BASE2_VTABLE_DISC:63631]], ptr @g_b2) }, align 8
+// CHECK: @_ZTV8Derived1 = linkonce_odr unnamed_addr constant { [5 x ptr], [3
x ptr] } { [5 x ptr] [ptr null, ptr null, ptr ptrauth (ptr @_ZN5Base11aEv, i32
0, i64 [[BASE1_A_DISC]], ptr getelementptr inbounds ({ [5 x ptr], [3 x ptr] },
ptr @_ZTV8Derived1, i32 0, i32 0, i32 2)), ptr ptrauth (ptr @_ZN8Derived11cEv,
i32 0, i64 [[DERIVED1_C_DISC:54092]], ptr getelementptr inbounds ({ [5 x ptr],
[3 x ptr] }, ptr @_ZTV8Derived1, i32 0, i32 0, i32 3)), ptr ptrauth (ptr
@_ZN8Derived11dEv, i32 0, i64 [[DERIVED1_D_DISC:37391]], ptr getelementptr
inbounds ({ [5 x ptr], [3 x ptr] }, ptr @_ZTV8Derived1, i32 0, i32 0, i32 4))],
[3 x ptr] [ptr inttoptr (i64 -8 to ptr), ptr null, ptr ptrauth (ptr
@_ZN5Base21bEv, i32 0, i64 [[BASE2_B_DISC]], ptr getelementptr inbounds ({ [5 x
ptr], [3 x ptr] }, ptr @_ZTV8Derived1, i32 0, i32 1, i32 2))] }, align 8
+// CHECK: @g_d1 = global { ptr, ptr } { ptr ptrauth (ptr getelementptr
inbounds inrange(-16, 24) ({ [5 x ptr], [3 x ptr] }, ptr @_ZTV8Derived1, i32 0,
i32 0, i32 2), i32 2, i64 [[BASE1_VTABLE_DISC]], ptr @g_d1), ptr ptrauth (ptr
getelementptr inbounds inrange(-16, 8) ({ [5 x ptr], [3 x ptr] }, ptr
@_ZTV8Derived1, i32 0, i32 1, i32 2), i32 2, i64 [[BASE2_VTABLE_DISC]], ptr
getelementptr inbounds ({ ptr, ptr }, ptr @g_d1, i32 0, i32 1)) }, align 8
+// CHECK: @_ZTV8Derived2 = linkonce_odr unnamed_addr constant { [5 x ptr], [3
x ptr] } { [5 x ptr] [ptr null, ptr null, ptr ptrauth (ptr @_ZN5Base21bEv, i32
0, i64 [[BASE2_B_DISC]], ptr getelementptr inbounds ({ [5 x ptr], [3 x ptr] },
ptr @_ZTV8Derived2, i32 0, i32 0, i32 2)), ptr ptrauth (ptr @_ZN8Derived21cEv,
i32 0, i64 [[DERIVED2_C_DISC:15537]], ptr getelementptr inbounds ({ [5 x ptr],
[3 x ptr] }, ptr @_Z
[clang] [PAC] Implement authentication for C++ member function pointers (PR #99576)
@@ -71,6 +71,15 @@ void has_ptrauth_vtable_pointer_type_discrimination() {}
void no_ptrauth_vtable_pointer_type_discrimination() {}
#endif
+// This is always enabled when ptrauth_calls is enabled, on new enough clangs.
ojhunt wrote:
Weird mismerge I guess?
- [ ] fix the merge
https://github.com/llvm/llvm-project/pull/99576
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PAC] Implement authentication for C++ member function pointers (PR #99576)
@@ -1036,9 +1155,32 @@ llvm::Constant *ItaniumCXXABI::BuildMemberPointer(const CXXMethodDecl *MD, // least significant bit of adj then makes exactly the same // discrimination as the least significant bit of ptr does for // Itanium. - MemPtr[0] = llvm::ConstantInt::get(CGM.PtrDiffTy, VTableOffset); - MemPtr[1] = llvm::ConstantInt::get(CGM.PtrDiffTy, - 2 * ThisAdjustment.getQuantity() + 1); + + // We cannot use the Itanium ABI's representation for virtual member + // function pointers under pointer authentication because it would + // require us to store both the virtual offset and the constant + // discriminator in the pointer, which would be immediately vulnerable + // to attack. Instead we introduce a thunk that does the virtual dispatch + // and store it as if it were a non-virtual member function. This means + // that virtual function pointers may not compare equal anymore, but + // fortunately they aren't required to by the standard, and we do make + // a best-effort attempt to re-use the thunk. + // + // To support interoperation with code in which pointer authentication + // is disabled, derefencing a member function pointer must still handle + // the virtual case, but it can use a discriminator which should never + // be valid. + const auto &Schema = + CGM.getCodeGenOpts().PointerAuth.CXXMemberFunctionPointers; + if (Schema) +MemPtr[0] = llvm::ConstantExpr::getPtrToInt( +getSignedVirtualMemberFunctionPointer(MD), CGM.PtrDiffTy); + else +MemPtr[0] = llvm::ConstantInt::get(CGM.PtrDiffTy, VTableOffset); + // Don't set the LSB of adj to 1 if pointer authentication for member ojhunt wrote: @kovdan01 what do you mean here? the use of a thunk in general? https://github.com/llvm/llvm-project/pull/99576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PAC] Implement authentication for C++ member function pointers (PR #99576)
https://github.com/ojhunt edited https://github.com/llvm/llvm-project/pull/99576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PAC] Implement authentication for C++ member function pointers (PR #99576)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/99576
>From 547ce3e9c7fe7e046b20f51f2f0d370a683659d1 Mon Sep 17 00:00:00 2001
From: Ahmed Bougacha
Date: Fri, 10 May 2024 15:58:57 -0700
Subject: [PATCH 1/2] [PAC] Implement pointer authentication for C++ member
function pointers.
Introduces type based signing of member function pointers. To support this
discrimination schema we no longer emit member function pointer to virtual
methods and indices into a vtable but migrate to using thunks. This does mean
member function pointers are no longer necessarily directly comparable,
however as such comparisons are UB this is acceptable.
We derive the discriminator from the C++ mangling of the type of the pointer
being authenticated.
Co-Authored-By: Akira Hatanaka ahatan...@apple.com
Co-Authored-By: John McCall rjmcc...@apple.com
---
clang/include/clang/AST/ASTContext.h | 2 +-
.../include/clang/Basic/PointerAuthOptions.h | 3 +
clang/lib/AST/ASTContext.cpp | 12 +-
clang/lib/CodeGen/CGCall.cpp | 212 +
clang/lib/CodeGen/CGPointerAuth.cpp | 34 ++
clang/lib/CodeGen/CodeGenFunction.h | 3 +-
clang/lib/CodeGen/CodeGenModule.h | 8 +
clang/lib/CodeGen/ItaniumCXXABI.cpp | 254 +-
clang/lib/Frontend/CompilerInvocation.cpp | 2 +
.../ptrauth-member-function-pointer.cpp | 433 ++
clang/test/Preprocessor/ptrauth_feature.c | 9 +
11 files changed, 855 insertions(+), 117 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ptrauth-member-function-pointer.cpp
diff --git a/clang/include/clang/AST/ASTContext.h
b/clang/include/clang/AST/ASTContext.h
index 608bd90fcc3ff..f2a17a56dc201 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1287,7 +1287,7 @@ class ASTContext : public RefCountedBase {
getPointerAuthVTablePointerDiscriminator(const CXXRecordDecl *RD);
/// Return the "other" type-specific discriminator for the given type.
- uint16_t getPointerAuthTypeDiscriminator(QualType T) const;
+ uint16_t getPointerAuthTypeDiscriminator(QualType T);
/// Apply Objective-C protocol qualifiers to the given type.
/// \param allowOnPointerType specifies if we can apply protocol
diff --git a/clang/include/clang/Basic/PointerAuthOptions.h
b/clang/include/clang/Basic/PointerAuthOptions.h
index 197d63642ca6d..595322b8271b6 100644
--- a/clang/include/clang/Basic/PointerAuthOptions.h
+++ b/clang/include/clang/Basic/PointerAuthOptions.h
@@ -175,6 +175,9 @@ struct PointerAuthOptions {
/// The ABI for variadic C++ virtual function pointers.
PointerAuthSchema CXXVirtualVariadicFunctionPointers;
+
+ /// The ABI for C++ member function pointers.
+ PointerAuthSchema CXXMemberFunctionPointers;
};
} // end namespace clang
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a8e599f7ebe04..58bb259a5395a 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3404,7 +3404,7 @@ static void encodeTypeForFunctionPointerAuth(const
ASTContext &Ctx,
}
}
-uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
+uint16_t ASTContext::getPointerAuthTypeDiscriminator(QualType T) {
assert(!T->isDependentType() &&
"cannot compute type discriminator of a dependent type");
@@ -3414,11 +3414,13 @@ uint16_t
ASTContext::getPointerAuthTypeDiscriminator(QualType T) const {
if (T->isFunctionPointerType() || T->isFunctionReferenceType())
T = T->getPointeeType();
- if (T->isFunctionType())
+ if (T->isFunctionType()) {
encodeTypeForFunctionPointerAuth(*this, Out, T);
- else
-llvm_unreachable(
-"type discrimination of non-function type not implemented yet");
+ } else {
+T = T.getUnqualifiedType();
+std::unique_ptr MC(createMangleContext());
+MC->mangleCanonicalTypeName(T, Out);
+ }
return llvm::getPointerAuthStableSipHash(Str);
}
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index d582aba679ddc..52e61cf37189d 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5034,7 +5034,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
ReturnValueSlot ReturnValue,
const CallArgList &CallArgs,
llvm::CallBase **callOrInvoke, bool
IsMustTail,
- SourceLocation Loc) {
+ SourceLocation Loc,
+ bool IsVirtualFunctionPointerThunk) {
// FIXME: We no longer need the types from CallArgs; lift up and simplify.
assert(Callee.isOrdinary() || Callee.isVirtual());
@@ -5098,7 +5099,11 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
RawAddress SRetAlloca = RawAddress::invalid();
llvm::Value *UnusedReturnSizePtr
[clang] [clang] Ensure pointers passed to runtime support functions are correctly signed (PR #98276)
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/98276
Updates codegen for global destructors and raising exceptions to ensure that
the function pointers being passed are signed using the correct schema.
Notably this requires that CodeGenFunction::createAtExitStub to return an
opaque Constant* rather than a Function* as the value being emitted is no
longer necessarily a raw function pointer depending on the configured ABI.
>From 4b92c4af87a1a381dad09b243db4d3ec71d64738 Mon Sep 17 00:00:00 2001
From: John McCall
Date: Wed, 18 Sep 2019 02:21:37 -0400
Subject: [PATCH 1/3] Sign function pointers passed to atexit and __cxa_atexit.
Patch by Akira Hatanaka.
---
clang/lib/CodeGen/CGDeclCXX.cpp | 9 +--
clang/lib/CodeGen/CodeGenFunction.h | 2 +-
clang/lib/CodeGen/ItaniumCXXABI.cpp | 10 ++-
.../CodeGenCXX/ptrauth-static-destructors.cpp | 26 +++
4 files changed, 43 insertions(+), 4 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 05dd7ddb86fa6..36248a9c75188 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -232,7 +232,7 @@ void CodeGenFunction::EmitCXXGlobalVarDeclInit(const
VarDecl &D,
/// Create a stub function, suitable for being passed to atexit,
/// which passes the given address to the given destructor function.
-llvm::Function *CodeGenFunction::createAtExitStub(const VarDecl &VD,
+llvm::Constant *CodeGenFunction::createAtExitStub(const VarDecl &VD,
llvm::FunctionCallee dtor,
llvm::Constant *addr) {
// Get the destructor function type, void(*)(void).
@@ -264,7 +264,12 @@ llvm::Function *CodeGenFunction::createAtExitStub(const
VarDecl &VD,
CGF.FinishFunction();
- return fn;
+ // Get a proper function pointer.
+ FunctionProtoType::ExtProtoInfo EPI(getContext().getDefaultCallingConvention(
+ /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+ QualType fnType = getContext().getFunctionType(getContext().VoidTy,
+ {getContext().VoidPtrTy},
EPI);
+ return CGM.getFunctionPointer(fn, fnType);
}
/// Create a stub function, suitable for being passed to __pt_atexit_np,
diff --git a/clang/lib/CodeGen/CodeGenFunction.h
b/clang/lib/CodeGen/CodeGenFunction.h
index 13f12b5d878a6..e33267c4787fd 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4862,7 +4862,7 @@ class CodeGenFunction : public CodeGenTypeCache {
void EmitCXXGlobalVarDeclInit(const VarDecl &D, llvm::GlobalVariable *GV,
bool PerformInit);
- llvm::Function *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee
Dtor,
+ llvm::Constant *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee
Dtor,
llvm::Constant *Addr);
llvm::Function *createTLSAtExitStub(const VarDecl &VD,
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index e1d056765a866..1924c07c1529d 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2699,6 +2699,14 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction
&CGF,
if (llvm::Function *fn = dyn_cast(atexit.getCallee()))
fn->setDoesNotThrow();
+ auto &Context = CGF.CGM.getContext();
+ FunctionProtoType::ExtProtoInfo EPI(Context.getDefaultCallingConvention(
+ /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+ QualType fnType =
+ Context.getFunctionType(Context.VoidTy, {Context.VoidPtrTy}, EPI);
+ llvm::Constant *dtorCallee = cast(dtor.getCallee());
+ dtorCallee = CGF.CGM.getFunctionPointer(dtorCallee, fnType);
+
if (!addr)
// addr is null when we are trying to register a dtor annotated with
// __attribute__((destructor)) in a constructor function. Using null here
is
@@ -2706,7 +2714,7 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction
&CGF,
// function.
addr = llvm::Constant::getNullValue(CGF.Int8PtrTy);
- llvm::Value *args[] = {dtor.getCallee(), addr, handle};
+ llvm::Value *args[] = {dtorCallee, addr, handle};
CGF.EmitNounwindRuntimeCall(atexit, args);
}
diff --git a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
new file mode 100644
index 0..6c8d0c560681a
--- /dev/null
+++ b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN: | FileCheck %s --check-prefix=CXAATEXIT
+
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN:-fno-use-cxa-atexit \
+// RUN: | FileCheck %s --check-prefix=ATEXIT
+
+class Foo
[clang] [clang] Ensure pointers passed to runtime support functions are correctly signed (PR #98276)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/98276
>From 4b92c4af87a1a381dad09b243db4d3ec71d64738 Mon Sep 17 00:00:00 2001
From: John McCall
Date: Wed, 18 Sep 2019 02:21:37 -0400
Subject: [PATCH 1/3] Sign function pointers passed to atexit and __cxa_atexit.
Patch by Akira Hatanaka.
---
clang/lib/CodeGen/CGDeclCXX.cpp | 9 +--
clang/lib/CodeGen/CodeGenFunction.h | 2 +-
clang/lib/CodeGen/ItaniumCXXABI.cpp | 10 ++-
.../CodeGenCXX/ptrauth-static-destructors.cpp | 26 +++
4 files changed, 43 insertions(+), 4 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 05dd7ddb86fa6..36248a9c75188 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -232,7 +232,7 @@ void CodeGenFunction::EmitCXXGlobalVarDeclInit(const
VarDecl &D,
/// Create a stub function, suitable for being passed to atexit,
/// which passes the given address to the given destructor function.
-llvm::Function *CodeGenFunction::createAtExitStub(const VarDecl &VD,
+llvm::Constant *CodeGenFunction::createAtExitStub(const VarDecl &VD,
llvm::FunctionCallee dtor,
llvm::Constant *addr) {
// Get the destructor function type, void(*)(void).
@@ -264,7 +264,12 @@ llvm::Function *CodeGenFunction::createAtExitStub(const
VarDecl &VD,
CGF.FinishFunction();
- return fn;
+ // Get a proper function pointer.
+ FunctionProtoType::ExtProtoInfo EPI(getContext().getDefaultCallingConvention(
+ /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+ QualType fnType = getContext().getFunctionType(getContext().VoidTy,
+ {getContext().VoidPtrTy},
EPI);
+ return CGM.getFunctionPointer(fn, fnType);
}
/// Create a stub function, suitable for being passed to __pt_atexit_np,
diff --git a/clang/lib/CodeGen/CodeGenFunction.h
b/clang/lib/CodeGen/CodeGenFunction.h
index 13f12b5d878a6..e33267c4787fd 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4862,7 +4862,7 @@ class CodeGenFunction : public CodeGenTypeCache {
void EmitCXXGlobalVarDeclInit(const VarDecl &D, llvm::GlobalVariable *GV,
bool PerformInit);
- llvm::Function *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee
Dtor,
+ llvm::Constant *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee
Dtor,
llvm::Constant *Addr);
llvm::Function *createTLSAtExitStub(const VarDecl &VD,
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index e1d056765a866..1924c07c1529d 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2699,6 +2699,14 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction
&CGF,
if (llvm::Function *fn = dyn_cast(atexit.getCallee()))
fn->setDoesNotThrow();
+ auto &Context = CGF.CGM.getContext();
+ FunctionProtoType::ExtProtoInfo EPI(Context.getDefaultCallingConvention(
+ /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+ QualType fnType =
+ Context.getFunctionType(Context.VoidTy, {Context.VoidPtrTy}, EPI);
+ llvm::Constant *dtorCallee = cast(dtor.getCallee());
+ dtorCallee = CGF.CGM.getFunctionPointer(dtorCallee, fnType);
+
if (!addr)
// addr is null when we are trying to register a dtor annotated with
// __attribute__((destructor)) in a constructor function. Using null here
is
@@ -2706,7 +2714,7 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction
&CGF,
// function.
addr = llvm::Constant::getNullValue(CGF.Int8PtrTy);
- llvm::Value *args[] = {dtor.getCallee(), addr, handle};
+ llvm::Value *args[] = {dtorCallee, addr, handle};
CGF.EmitNounwindRuntimeCall(atexit, args);
}
diff --git a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
new file mode 100644
index 0..6c8d0c560681a
--- /dev/null
+++ b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN: | FileCheck %s --check-prefix=CXAATEXIT
+
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN:-fno-use-cxa-atexit \
+// RUN: | FileCheck %s --check-prefix=ATEXIT
+
+class Foo {
+ public:
+ ~Foo() {
+ }
+};
+
+Foo global;
+
+// CXAATEXIT: @_ZN3FooD1Ev.ptrauth = private constant { i8*, i32, i64, i64 } {
i8* bitcast (%class.Foo* (%class.Foo*)* @_ZN3FooD1Ev to i8*), i32 0, i64 0, i64
0 }, section "llvm.ptrauth", align 8
+// CXAATEXIT: define internal void @__cxx_global_var_init()
+// CXAATEXIT: call i32 @__cxa_atexit(void (i8*)* bitcast ({ i8*
[clang] [clang] Ensure pointers passed to runtime support functions are correctly signed (PR #98276)
https://github.com/ojhunt edited https://github.com/llvm/llvm-project/pull/98276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Ensure pointers passed to runtime support functions are correctly signed (PR #98276)
https://github.com/ojhunt ready_for_review https://github.com/llvm/llvm-project/pull/98276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Ensure pointers passed to runtime support functions are correctly signed (PR #98276)
@@ -333,7 +338,8 @@ void CodeGenFunction::registerGlobalDtorWithLLVM(const
VarDecl &VD,
llvm::FunctionCallee Dtor,
llvm::Constant *Addr) {
// Create a function which calls the destructor.
- llvm::Function *dtorStub = createAtExitStub(VD, Dtor, Addr);
+ llvm::Function *dtorStub =
ojhunt wrote:
Oh curses, I missed these comments due to earlier insanity this week and/or
GitHub mail not be filtered correctly (which I'll check on)
I _think_ these casts occurred a long time ago as merge resolutions or similar.
I meant to ask about what the best solution would be, we _could_ just convert
everything to Constant* but I looked at that and it spreads a lot.
It also means losing type system guard for is this being the right type. It
feels like the best approach would be to introduce an wrapper type, something
akin to
struct ConstantMaybeSignedFunction {
FunctionType *FT;
Constant *Value;
};
That would allow places that do want to reason about the type to continue to do
so. In principle this could be MaybeSigned
https://github.com/llvm/llvm-project/pull/98276
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Ensure pointers passed to runtime support functions are correctly signed (PR #98276)
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -fcxx-exceptions
-emit-llvm %s -o - | FileCheck %s
+
+class Foo {
+ public:
+ ~Foo() {
+ }
+};
+
+// CHECK-LABEL: define void @_Z1fv()
+// CHECK: call void @__cxa_throw(ptr %{{.*}}, ptr @_ZTI3Foo, ptr ptrauth (ptr
@_ZN3FooD1Ev, i32 0))
+void f() {
+ throw Foo();
+}
+
+// __cxa_throw is defined to take its destructor as "void (*)(void *)" in the
ABI.
+// CHECK-LABEL: define void @__cxa_throw({{.*}})
+// CHECK: call void {{%.*}}(ptr noundef {{%.*}}) [ "ptrauth"(i32 0, i64 0) ]
ojhunt wrote:
Agreed, I had to actually remove that from the test because that PR had not
landed
https://github.com/llvm/llvm-project/pull/98276
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Ensure pointers passed to runtime support functions are correctly signed (PR #98276)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/98276
>From 4b92c4af87a1a381dad09b243db4d3ec71d64738 Mon Sep 17 00:00:00 2001
From: John McCall
Date: Wed, 18 Sep 2019 02:21:37 -0400
Subject: [PATCH 1/4] Sign function pointers passed to atexit and __cxa_atexit.
Patch by Akira Hatanaka.
---
clang/lib/CodeGen/CGDeclCXX.cpp | 9 +--
clang/lib/CodeGen/CodeGenFunction.h | 2 +-
clang/lib/CodeGen/ItaniumCXXABI.cpp | 10 ++-
.../CodeGenCXX/ptrauth-static-destructors.cpp | 26 +++
4 files changed, 43 insertions(+), 4 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 05dd7ddb86fa6..36248a9c75188 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -232,7 +232,7 @@ void CodeGenFunction::EmitCXXGlobalVarDeclInit(const
VarDecl &D,
/// Create a stub function, suitable for being passed to atexit,
/// which passes the given address to the given destructor function.
-llvm::Function *CodeGenFunction::createAtExitStub(const VarDecl &VD,
+llvm::Constant *CodeGenFunction::createAtExitStub(const VarDecl &VD,
llvm::FunctionCallee dtor,
llvm::Constant *addr) {
// Get the destructor function type, void(*)(void).
@@ -264,7 +264,12 @@ llvm::Function *CodeGenFunction::createAtExitStub(const
VarDecl &VD,
CGF.FinishFunction();
- return fn;
+ // Get a proper function pointer.
+ FunctionProtoType::ExtProtoInfo EPI(getContext().getDefaultCallingConvention(
+ /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+ QualType fnType = getContext().getFunctionType(getContext().VoidTy,
+ {getContext().VoidPtrTy},
EPI);
+ return CGM.getFunctionPointer(fn, fnType);
}
/// Create a stub function, suitable for being passed to __pt_atexit_np,
diff --git a/clang/lib/CodeGen/CodeGenFunction.h
b/clang/lib/CodeGen/CodeGenFunction.h
index 13f12b5d878a6..e33267c4787fd 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4862,7 +4862,7 @@ class CodeGenFunction : public CodeGenTypeCache {
void EmitCXXGlobalVarDeclInit(const VarDecl &D, llvm::GlobalVariable *GV,
bool PerformInit);
- llvm::Function *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee
Dtor,
+ llvm::Constant *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee
Dtor,
llvm::Constant *Addr);
llvm::Function *createTLSAtExitStub(const VarDecl &VD,
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index e1d056765a866..1924c07c1529d 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2699,6 +2699,14 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction
&CGF,
if (llvm::Function *fn = dyn_cast(atexit.getCallee()))
fn->setDoesNotThrow();
+ auto &Context = CGF.CGM.getContext();
+ FunctionProtoType::ExtProtoInfo EPI(Context.getDefaultCallingConvention(
+ /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+ QualType fnType =
+ Context.getFunctionType(Context.VoidTy, {Context.VoidPtrTy}, EPI);
+ llvm::Constant *dtorCallee = cast(dtor.getCallee());
+ dtorCallee = CGF.CGM.getFunctionPointer(dtorCallee, fnType);
+
if (!addr)
// addr is null when we are trying to register a dtor annotated with
// __attribute__((destructor)) in a constructor function. Using null here
is
@@ -2706,7 +2714,7 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction
&CGF,
// function.
addr = llvm::Constant::getNullValue(CGF.Int8PtrTy);
- llvm::Value *args[] = {dtor.getCallee(), addr, handle};
+ llvm::Value *args[] = {dtorCallee, addr, handle};
CGF.EmitNounwindRuntimeCall(atexit, args);
}
diff --git a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
new file mode 100644
index 0..6c8d0c560681a
--- /dev/null
+++ b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN: | FileCheck %s --check-prefix=CXAATEXIT
+
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm
-std=c++11 %s -o - \
+// RUN:-fno-use-cxa-atexit \
+// RUN: | FileCheck %s --check-prefix=ATEXIT
+
+class Foo {
+ public:
+ ~Foo() {
+ }
+};
+
+Foo global;
+
+// CXAATEXIT: @_ZN3FooD1Ev.ptrauth = private constant { i8*, i32, i64, i64 } {
i8* bitcast (%class.Foo* (%class.Foo*)* @_ZN3FooD1Ev to i8*), i32 0, i64 0, i64
0 }, section "llvm.ptrauth", align 8
+// CXAATEXIT: define internal void @__cxx_global_var_init()
+// CXAATEXIT: call i32 @__cxa_atexit(void (i8*)* bitcast ({ i8*
[clang] [llvm] [PAC] Fix address discrimination for type info vtable pointers (PR #102199)
@@ -3955,9 +3955,23 @@ void ItaniumRTTIBuilder::BuildVTablePointer(const Type
*Ty) {
VTable, Two);
}
- if (auto &Schema = CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer)
-VTable = CGM.getConstantSignedPointer(VTable, Schema, nullptr,
GlobalDecl(),
- QualType(Ty, 0));
+ if (const auto &Schema =
+ CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer) {
+llvm::PointerType *PtrTy = llvm::PointerType::get(
+CGM.getLLVMContext(),
+CGM.getModule().getDataLayout().getProgramAddressSpace());
+llvm::Constant *StorageAddress =
ojhunt wrote:
I'd prefer this be structured rather than using ?:
llvm::Constant *StorageAddress = nullptr;
if (Schema.isAddressDescriminated()) {
StorageAddress = llvm::ConstantExpr::getIntToPtr(
llvm::ConstantInt::get(
CGM.IntPtrTy,
llvm::ConstantPtrAuth::
AddrDiscriminator_CXXTypeInfoVTablePointer),
PtrTy);
}
This bug does make me wonder if `getConstantSignedPointer(..)` etc should use a
std::optional<&> rather than a pointer as that might have made it more obvious
that the address was not being used in this code (obviously this is not a thing
we should be changing in this PR)
https://github.com/llvm/llvm-project/pull/102199
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PAC] Fix address discrimination for type info vtable pointers (PR #102199)
ojhunt wrote: Ok, it took me a moment to understand what was happening here - I had to talk to Ahmed because I didn't recognize the AddrDiscriminator_* enums and had assumed I'd forgotten them :D This needs to update `ItaniumRTTIBuilder::BuildVTablePointer` to take the storage address, and then use that provided address. Using any kind of signalling tags is too much of a footgun as you can easily end up accidentally (and silently) using a constant discriminator (as demonstrated by the existing code silently using no addr discriminator without being noticed :-O) https://github.com/llvm/llvm-project/pull/102199 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Add wraps attribute (for granular integer overflow handling) (PR #86618)
ojhunt wrote: I’m a little concerned about not allowing the attribute in C++ - the existence of other options in C++ does not mean they are an option (due to various and sundry restrictions of C++ version upgrades different projects have), but also you trivially end up in cases where header code is correct/safe in C, but undefined when included in C++. To me that seems like a significant footgun. https://github.com/llvm/llvm-project/pull/86618 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
@@ -2234,6 +2234,17 @@ enum class CXXNewInitializationStyle {
Braces
};
+struct ImplicitAllocationParameters {
+ bool PassTypeIdentity;
ojhunt wrote:
What is the rationale for bitfields?
I'm generally opposed to flag enums as the language support is so poor, and
unless there are very real performance considerations I'm not convinced they're
worth the code complexity cost.
I did originally try to unify Allocation and Deallocation but the usage of them
in the code base differs a bit (one is much more "try to do this" vs the
other's "do this") and I found that keeping them distinct gave clarity to the
code.
In general the way the new and delete AST construction works is kind of gnarly
in a way I wish we could avoid as I think it creates unnecessary complexity in
some places, and just causes weirdness in others, but I didn't want to combine
"lets add a new feature to new/delete" and at the same time "lets completely
restructure how new/delete work". I adopted these structs largely because there
were already two undifferentiated bool args, and adding a third made things
even more opaque, especially in conjunction with the extensive use of default
parameters.
https://github.com/llvm/llvm-project/pull/113510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
@@ -4749,6 +4753,15 @@ class Sema final : public SemaBase {
CXXRecordDecl *getStdBadAlloc() const;
EnumDecl *getStdAlignValT() const;
+ ClassTemplateDecl *getStdTypeIdentity() const;
+ std::optional InstantiateSpecializedTypeIdentity(QualType Subject);
ojhunt wrote:
What is the correct coding style? Over time I've been told upper case, whereas
other times lower - in this particular case I followed lower case for the `get`
for consistency with the neighboring `get` methods, and upper case for the new
one because it didn't have a "consistency with similar methods" rationale and
the feedback on other patches seemed to imply that functions are _meant_ to
have upper case first letter (I think I actually intentionally capitalized this
function before pushing the PR \o/)
I'd prefer lower case start, but I just want to know which it is actually meant
to be :D
https://github.com/llvm/llvm-project/pull/113510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
@@ -8126,7 +8143,7 @@ class Sema final : public SemaBase {
/// The scope in which to find allocation functions.
enum AllocationFunctionScope {
-/// Only look for allocation functions in the global scope.
+/// Only look for allocation functions in the global scope
ojhunt wrote:
*looks shifty*
Nope, 100% absolutely critical for correctness
https://github.com/llvm/llvm-project/pull/113510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
@@ -3358,6 +3358,12 @@ bool FunctionDecl::isReservedGlobalPlacementOperator()
const {
return false;
const auto *proto = getType()->castAs();
+ if (proto->getNumParams() < 2)
+return false;
+ bool IsTypeAwareAllocator =
ojhunt wrote:
Yeah, I think this may date back to before a bunch of the helpers like
isTypeIdentitySpecialization() so the logic was probably somewhat longer
https://github.com/llvm/llvm-project/pull/113510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
https://github.com/ojhunt converted_to_draft https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
@@ -307,6 +307,10 @@ EXTENSION(datasizeof, LangOpts.CPlusPlus) FEATURE(cxx_abi_relative_vtable, LangOpts.CPlusPlus && LangOpts.RelativeCXXABIVTables) +// Type aware allocators +FEATURE(cxx_type_aware_allocators, LangOpts.TypeAwareAllocators) ojhunt wrote: Oh, I don't believe I've ever made a feature test macro (everything I've worked on has been via direct `__has_feature()` checks, I'll look into existing macros to see how they're implemented/exposed) https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
ojhunt wrote: also sorry for cycling to and from "review" the GH interface is still not obvious to me https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
ojhunt wrote: Ok, so I've gone through all my GitHub settings, and am hoping for a comment or something, to see if GH will actually ping me this time :D https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
https://github.com/ojhunt edited https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Initial implementation of P2719 (PR #113510)
ojhunt wrote: @cor3ntin oh yeah, I know, I'm also there :D https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PAC] Re-sign a pointer to a noexcept member function when it is converted to a pointer to a member function without noexcept (PR #109056)
ojhunt wrote: LGTM https://github.com/llvm/llvm-project/pull/109056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116784)
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/116784
Very simply extends the bitfield sema checks for assignment to fields with a
preferred type specified to consider the preferred type if the decl storage
type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different storage
requirements we may not warn in all possible cases, but that's a scenario for
which the warnings are much more complex and confusing.
>From d800f5371739670a415b7bfc6f7d1643cd8750a1 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 29 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 552 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..3d5d6f3285b12d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,21 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ assert(PreferredTypeDiagIndex == 1);
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116784)
https://github.com/ojhunt closed https://github.com/llvm/llvm-project/pull/116784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/116785
Very simply extends the bitfield sema checks for assignment to fields with a
preferred type specified to consider the preferred type if the decl storage
type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different storage
requirements we may not warn in all possible cases, but that's a scenario for
which the warnings are much more complex and confusing.
>From d800f5371739670a415b7bfc6f7d1643cd8750a1 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 29 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 552 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..3d5d6f3285b12d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,21 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ assert(PreferredTypeDiagIndex == 1);
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Preferre
[clang] [RFC] Initial implementation of P2719 (PR #113510)
ojhunt wrote: Could anyone with a windows machine see if you can work out what is happening with the windows test failure? I don't understand why the tests are failing on the windows bot as it seems like it should simply fail everything (e.g. windows driver is going wrong) or it should work https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/3] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
@@ -0,0 +1,108 @@
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -fsyntax-only -verify
-std=c11 -Wno-unused-value -Wno-unused-but-set-variable -Wbitfield-width
-Wbitfield-enum-conversion
+
+enum A {
+ A_a,
+ A_b,
+ A_c,
+ A_d
+};
+
+struct S {
+ enum A a1 : 1; // #1
ojhunt wrote:
Yeah, I just discovered while working on the C++ version of the test that
bookmarks aren't required to be numbers :D
https://github.com/llvm/llvm-project/pull/116785
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
ojhunt wrote: There's a side note that I didn't want to change in this PR: gcc warns on the declaration of the fields, not just the use, but that would be a completely new warning so I didn't want to conflate the issues. https://github.com/llvm/llvm-project/pull/116785 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
ojhunt wrote:
I think I did this intentionally as I sometimes feel like explicit true/false
returns are clearer, I'm happy with either Aaron or @Sirraide's suggestion but
I want to confirm that others don't have my clarity concerns :D
https://github.com/llvm/llvm-project/pull/117428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
ojhunt wrote:
Thanks, I'm terrible at wording (I prodded discord specifically to get
diagnostic wording)
https://github.com/llvm/llvm-project/pull/117428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/2] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/3] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/4] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/4] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] [Darwin][Driver][clang] Prioritise `-isysroot` over `--sysroot` consistently (PR #115993)
ojhunt wrote: Independently of the rationale for the current behaviour, I would be extremely concerned about potential breakage from this behavior change (more due to hyrum's law that intentional choices) https://github.com/llvm/llvm-project/pull/115993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/5] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
ojhunt wrote: > So I take it we decided not to enable it by default in `-fms-compatibility` > mode then? I don't believe it is appropriate to do so. The intent of this warning is to indicate MSVC compatibility issues when building in non-ms-compatibility modes. The existing padding warnings already trigger for these layouts in the relevant ms compatibility modes https://github.com/llvm/llvm-project/pull/117428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
ojhunt wrote: @erichkeane @Fznamznon what should the release note be - just something akin to "added warning flag -W... to warn when bitfield packing differs from MSVC"? https://github.com/llvm/llvm-project/pull/116785 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
ojhunt wrote:
Must I :D ?
I don't like unbraced multiline blocks even if it's a single statement and so
the braces are "unnecessary".
https://github.com/llvm/llvm-project/pull/116785
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH] Add an off-by-default warning to complain about MSVC bitfield
padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
+enu
[clang] [RFC] Initial implementation of P2719 (PR #113510)
https://github.com/ojhunt ready_for_review https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/3] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/4] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/6] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] Add support for referencable labels for attribute documentation (PR #118428)
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/118428
The existing mechanism being used is to manually add a reference in the
documentation. These references link to the beginning of the text rather than
the heading for the attribute which is what this PR allows.
>From 35fb507b3bedf61538a7a42ea3836c9ce9074358 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Mon, 2 Dec 2024 21:25:54 -0800
Subject: [PATCH] Add support for referencable labels for attribute
documentation
The existing mechanism being used is to manually add a reference in the
documentation. These references link to the beginning of the text rather
than the heading for the attribute which is what this PR allows.
---
clang/include/clang/Basic/Attr.td | 3 +++
clang/include/clang/Basic/AttrDocs.td | 12
clang/utils/TableGen/ClangAttrEmitter.cpp | 2 ++
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 618252f3e75247..522821a79063ac 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -55,6 +55,9 @@ class Documentation {
// When set, specifies that the attribute is deprecated and can optionally
// specify a replacement attribute.
DocDeprecated Deprecated;
+
+ // When set, specifies a label that can be used to reference the
documentation
+ string Label = "";
}
// Specifies that the attribute is explicitly omitted from the documentation,
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 5de39be4805600..7a82b8fa320590 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3360,9 +3360,8 @@ def NoSanitizeAddressDocs : Documentation {
// This function has multiple distinct spellings, and so it requires a custom
// heading to be specified. The most common spelling is sufficient.
let Heading = "no_sanitize_address, no_address_safety_analysis";
+ let Label = "langext-address_sanitizer";
let Content = [{
-.. _langext-address_sanitizer:
-
Use ``__attribute__((no_sanitize_address))`` on a function or a global
variable declaration to specify that address safety instrumentation
(e.g. AddressSanitizer) should not be applied.
@@ -3372,9 +3371,8 @@ variable declaration to specify that address safety
instrumentation
def NoSanitizeThreadDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_thread";
+ let Label = "langext-thread_sanitizer";
let Content = [{
-.. _langext-thread_sanitizer:
-
Use ``__attribute__((no_sanitize_thread))`` on a function declaration to
specify that checks for data races on plain (non-atomic) memory accesses should
not be inserted by ThreadSanitizer. The function is still instrumented by the
@@ -3385,9 +3383,8 @@ tool to avoid false positives and provide meaningful
stack traces.
def NoSanitizeMemoryDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_memory";
+ let Label = "langext-memory_sanitizer";
let Content = [{
-.. _langext-memory_sanitizer:
-
Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
specify that checks for uninitialized memory should not be inserted
(e.g. by MemorySanitizer). The function may still be instrumented by the tool
@@ -3398,9 +3395,8 @@ to avoid false positives in other places.
def CFICanonicalJumpTableDocs : Documentation {
let Category = DocCatFunction;
let Heading = "cfi_canonical_jump_table";
+ let Label = "langext-cfi_canonical_jump_table";
let Content = [{
-.. _langext-cfi_canonical_jump_table:
-
Use ``__attribute__((cfi_canonical_jump_table))`` on a function declaration to
make the function's CFI jump table canonical. See :ref:`the CFI documentation
` for more details.
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 534bf2d01d7957..a66868ba2cc044 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -5178,6 +5178,8 @@ GetAttributeHeadingAndSpellings(const Record
&Documentation,
static void WriteDocumentation(const RecordKeeper &Records,
const DocumentationData &Doc, raw_ostream &OS) {
+ if (const StringRef label = Doc.Documentation->getValueAsString("Label");
!label.empty())
+OS << ".. _" << label << ":\n\n";
OS << Doc.Heading << "\n" << std::string(Doc.Heading.length(), '-') << "\n";
// List what spelling syntaxes the attribute supports.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add support for referencable labels for attribute documentation (PR #118428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/118428
>From aea2b4001aa8e9239909875153152083b56a6a59 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Mon, 2 Dec 2024 21:25:54 -0800
Subject: [PATCH] Add support for referencable labels for attribute
documentation
The existing mechanism being used is to manually add a reference in the
documentation. These references link to the beginning of the text rather
than the heading for the attribute which is what this PR allows.
---
clang/include/clang/Basic/Attr.td | 3 +++
clang/include/clang/Basic/AttrDocs.td | 12
clang/utils/TableGen/ClangAttrEmitter.cpp | 3 +++
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 618252f3e75247..522821a79063ac 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -55,6 +55,9 @@ class Documentation {
// When set, specifies that the attribute is deprecated and can optionally
// specify a replacement attribute.
DocDeprecated Deprecated;
+
+ // When set, specifies a label that can be used to reference the
documentation
+ string Label = "";
}
// Specifies that the attribute is explicitly omitted from the documentation,
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 5de39be4805600..7a82b8fa320590 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3360,9 +3360,8 @@ def NoSanitizeAddressDocs : Documentation {
// This function has multiple distinct spellings, and so it requires a custom
// heading to be specified. The most common spelling is sufficient.
let Heading = "no_sanitize_address, no_address_safety_analysis";
+ let Label = "langext-address_sanitizer";
let Content = [{
-.. _langext-address_sanitizer:
-
Use ``__attribute__((no_sanitize_address))`` on a function or a global
variable declaration to specify that address safety instrumentation
(e.g. AddressSanitizer) should not be applied.
@@ -3372,9 +3371,8 @@ variable declaration to specify that address safety
instrumentation
def NoSanitizeThreadDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_thread";
+ let Label = "langext-thread_sanitizer";
let Content = [{
-.. _langext-thread_sanitizer:
-
Use ``__attribute__((no_sanitize_thread))`` on a function declaration to
specify that checks for data races on plain (non-atomic) memory accesses should
not be inserted by ThreadSanitizer. The function is still instrumented by the
@@ -3385,9 +3383,8 @@ tool to avoid false positives and provide meaningful
stack traces.
def NoSanitizeMemoryDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_memory";
+ let Label = "langext-memory_sanitizer";
let Content = [{
-.. _langext-memory_sanitizer:
-
Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
specify that checks for uninitialized memory should not be inserted
(e.g. by MemorySanitizer). The function may still be instrumented by the tool
@@ -3398,9 +3395,8 @@ to avoid false positives in other places.
def CFICanonicalJumpTableDocs : Documentation {
let Category = DocCatFunction;
let Heading = "cfi_canonical_jump_table";
+ let Label = "langext-cfi_canonical_jump_table";
let Content = [{
-.. _langext-cfi_canonical_jump_table:
-
Use ``__attribute__((cfi_canonical_jump_table))`` on a function declaration to
make the function's CFI jump table canonical. See :ref:`the CFI documentation
` for more details.
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 534bf2d01d7957..60cb4ae452326d 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -5178,6 +5178,9 @@ GetAttributeHeadingAndSpellings(const Record
&Documentation,
static void WriteDocumentation(const RecordKeeper &Records,
const DocumentationData &Doc, raw_ostream &OS) {
+ if (const StringRef label = Doc.Documentation->getValueAsString("Label");
+ !label.empty())
+OS << ".. _" << label << ":\n\n";
OS << Doc.Heading << "\n" << std::string(Doc.Heading.length(), '-') << "\n";
// List what spelling syntaxes the attribute supports.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add support for referencable labels for attribute documentation (PR #118428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/118428
>From aea2b4001aa8e9239909875153152083b56a6a59 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Mon, 2 Dec 2024 21:25:54 -0800
Subject: [PATCH] Add support for referencable labels for attribute
documentation
The existing mechanism being used is to manually add a reference in the
documentation. These references link to the beginning of the text rather
than the heading for the attribute which is what this PR allows.
---
clang/include/clang/Basic/Attr.td | 3 +++
clang/include/clang/Basic/AttrDocs.td | 12
clang/utils/TableGen/ClangAttrEmitter.cpp | 3 +++
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 618252f3e75247..522821a79063ac 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -55,6 +55,9 @@ class Documentation {
// When set, specifies that the attribute is deprecated and can optionally
// specify a replacement attribute.
DocDeprecated Deprecated;
+
+ // When set, specifies a label that can be used to reference the
documentation
+ string Label = "";
}
// Specifies that the attribute is explicitly omitted from the documentation,
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 5de39be4805600..7a82b8fa320590 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3360,9 +3360,8 @@ def NoSanitizeAddressDocs : Documentation {
// This function has multiple distinct spellings, and so it requires a custom
// heading to be specified. The most common spelling is sufficient.
let Heading = "no_sanitize_address, no_address_safety_analysis";
+ let Label = "langext-address_sanitizer";
let Content = [{
-.. _langext-address_sanitizer:
-
Use ``__attribute__((no_sanitize_address))`` on a function or a global
variable declaration to specify that address safety instrumentation
(e.g. AddressSanitizer) should not be applied.
@@ -3372,9 +3371,8 @@ variable declaration to specify that address safety
instrumentation
def NoSanitizeThreadDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_thread";
+ let Label = "langext-thread_sanitizer";
let Content = [{
-.. _langext-thread_sanitizer:
-
Use ``__attribute__((no_sanitize_thread))`` on a function declaration to
specify that checks for data races on plain (non-atomic) memory accesses should
not be inserted by ThreadSanitizer. The function is still instrumented by the
@@ -3385,9 +3383,8 @@ tool to avoid false positives and provide meaningful
stack traces.
def NoSanitizeMemoryDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_memory";
+ let Label = "langext-memory_sanitizer";
let Content = [{
-.. _langext-memory_sanitizer:
-
Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
specify that checks for uninitialized memory should not be inserted
(e.g. by MemorySanitizer). The function may still be instrumented by the tool
@@ -3398,9 +3395,8 @@ to avoid false positives in other places.
def CFICanonicalJumpTableDocs : Documentation {
let Category = DocCatFunction;
let Heading = "cfi_canonical_jump_table";
+ let Label = "langext-cfi_canonical_jump_table";
let Content = [{
-.. _langext-cfi_canonical_jump_table:
-
Use ``__attribute__((cfi_canonical_jump_table))`` on a function declaration to
make the function's CFI jump table canonical. See :ref:`the CFI documentation
` for more details.
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 534bf2d01d7957..60cb4ae452326d 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -5178,6 +5178,9 @@ GetAttributeHeadingAndSpellings(const Record
&Documentation,
static void WriteDocumentation(const RecordKeeper &Records,
const DocumentationData &Doc, raw_ostream &OS) {
+ if (const StringRef label = Doc.Documentation->getValueAsString("Label");
+ !label.empty())
+OS << ".. _" << label << ":\n\n";
OS << Doc.Heading << "\n" << std::string(Doc.Heading.length(), '-') << "\n";
// List what spelling syntaxes the attribute supports.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
ojhunt wrote: > > > So I take it we decided not to enable it by default in > > > `-fms-compatibility` mode then? > > > > > > I don't believe it is appropriate to do so. The intent of this warning is > > to indicate MSVC compatibility issues when building in non-ms-compatibility > > modes. The existing padding warnings already trigger for these layouts in > > the relevant ms compatibility modes > > We don't typically add new off-by-default warnings though; users don't enable > them often enough to be worth the costs. My thought process is: if we enable > this diagnostic by default in `-fms-compatibility` mode as telling the user > their bit-fields aren't packing, then it's on by default for some > configurations so it meets our bar for inclusion, and it helps us directly > because we have Windows precommit (and post-commit) CI coverage for building > Clang and LLVM. Users on other platforms can opt-in to the diagnostic if they > want the diagnostics for compatibility reasons. My concerns with attaching it to `-fms-compatibility` is that projects using that and that care about padding already have the padding/packing warnings enabled. For every other user we will be warning them that the padding matches the abi they're targeting. The problem I'm wanting to address is not "I am targeting the ms abi" it is "I am not targeting ms abi but want to be told about packing/padding that would be different on ms platforms". The intention would be to then enable the warning (maybe with a `-Werror=...` once the build is clean :D ) for all llvm projects, not just the windows targets - maybe with some evangelism so larger projects that have similar issues can be made aware of the flag. Then we can start migrating away from unending unsigned bit-fields and adopt enum bit-fields > > If we want to leave it off by default, then I wonder if we want to roll the > functionality into `-Wpadded-bitfield` which already exists and is off by > default. The problem with this is that that people who do not care about windows/ms abi will then be getting what to them are spurious warnings. https://github.com/llvm/llvm-project/pull/117428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add support for referencable labels for attribute documentation (PR #118428)
ojhunt wrote: > LGTM after fixing one more thing. > > I personally think this is a reasonable addition now, but please wait for > Aaron or Erich to approve this too before merging. Righto, I've committed the change, is `const StringRef` a style violation (I recognize StringRef is a constant type)? (Pondering adding support for style rule enforcement for things like "this is a constant type so doesn't need to be const") https://github.com/llvm/llvm-project/pull/118428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add support for referencable labels for attribute documentation (PR #118428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/118428
>From aea2b4001aa8e9239909875153152083b56a6a59 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Mon, 2 Dec 2024 21:25:54 -0800
Subject: [PATCH 1/3] Add support for referencable labels for attribute
documentation
The existing mechanism being used is to manually add a reference in the
documentation. These references link to the beginning of the text rather
than the heading for the attribute which is what this PR allows.
---
clang/include/clang/Basic/Attr.td | 3 +++
clang/include/clang/Basic/AttrDocs.td | 12
clang/utils/TableGen/ClangAttrEmitter.cpp | 3 +++
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 618252f3e75247..522821a79063ac 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -55,6 +55,9 @@ class Documentation {
// When set, specifies that the attribute is deprecated and can optionally
// specify a replacement attribute.
DocDeprecated Deprecated;
+
+ // When set, specifies a label that can be used to reference the
documentation
+ string Label = "";
}
// Specifies that the attribute is explicitly omitted from the documentation,
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 5de39be4805600..7a82b8fa320590 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3360,9 +3360,8 @@ def NoSanitizeAddressDocs : Documentation {
// This function has multiple distinct spellings, and so it requires a custom
// heading to be specified. The most common spelling is sufficient.
let Heading = "no_sanitize_address, no_address_safety_analysis";
+ let Label = "langext-address_sanitizer";
let Content = [{
-.. _langext-address_sanitizer:
-
Use ``__attribute__((no_sanitize_address))`` on a function or a global
variable declaration to specify that address safety instrumentation
(e.g. AddressSanitizer) should not be applied.
@@ -3372,9 +3371,8 @@ variable declaration to specify that address safety
instrumentation
def NoSanitizeThreadDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_thread";
+ let Label = "langext-thread_sanitizer";
let Content = [{
-.. _langext-thread_sanitizer:
-
Use ``__attribute__((no_sanitize_thread))`` on a function declaration to
specify that checks for data races on plain (non-atomic) memory accesses should
not be inserted by ThreadSanitizer. The function is still instrumented by the
@@ -3385,9 +3383,8 @@ tool to avoid false positives and provide meaningful
stack traces.
def NoSanitizeMemoryDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_memory";
+ let Label = "langext-memory_sanitizer";
let Content = [{
-.. _langext-memory_sanitizer:
-
Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
specify that checks for uninitialized memory should not be inserted
(e.g. by MemorySanitizer). The function may still be instrumented by the tool
@@ -3398,9 +3395,8 @@ to avoid false positives in other places.
def CFICanonicalJumpTableDocs : Documentation {
let Category = DocCatFunction;
let Heading = "cfi_canonical_jump_table";
+ let Label = "langext-cfi_canonical_jump_table";
let Content = [{
-.. _langext-cfi_canonical_jump_table:
-
Use ``__attribute__((cfi_canonical_jump_table))`` on a function declaration to
make the function's CFI jump table canonical. See :ref:`the CFI documentation
` for more details.
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 534bf2d01d7957..60cb4ae452326d 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -5178,6 +5178,9 @@ GetAttributeHeadingAndSpellings(const Record
&Documentation,
static void WriteDocumentation(const RecordKeeper &Records,
const DocumentationData &Doc, raw_ostream &OS) {
+ if (const StringRef label = Doc.Documentation->getValueAsString("Label");
+ !label.empty())
+OS << ".. _" << label << ":\n\n";
OS << Doc.Heading << "\n" << std::string(Doc.Heading.length(), '-') << "\n";
// List what spelling syntaxes the attribute supports.
>From 3f05ad04019139789aff09dced2360bfa536afe8 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 3 Dec 2024 21:24:22 -0800
Subject: [PATCH 2/3] Adjust style
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/utils/TableGen/ClangAttrEmitter.cpp | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 522821a79063ac..17fc36fbe2ac8c 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -56,7 +56,7 @@ class Documentation {
// specify a repl
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/6] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] Add support for referencable labels for attribute documentation (PR #118428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/118428
>From aea2b4001aa8e9239909875153152083b56a6a59 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Mon, 2 Dec 2024 21:25:54 -0800
Subject: [PATCH 1/3] Add support for referencable labels for attribute
documentation
The existing mechanism being used is to manually add a reference in the
documentation. These references link to the beginning of the text rather
than the heading for the attribute which is what this PR allows.
---
clang/include/clang/Basic/Attr.td | 3 +++
clang/include/clang/Basic/AttrDocs.td | 12
clang/utils/TableGen/ClangAttrEmitter.cpp | 3 +++
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 618252f3e75247..522821a79063ac 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -55,6 +55,9 @@ class Documentation {
// When set, specifies that the attribute is deprecated and can optionally
// specify a replacement attribute.
DocDeprecated Deprecated;
+
+ // When set, specifies a label that can be used to reference the
documentation
+ string Label = "";
}
// Specifies that the attribute is explicitly omitted from the documentation,
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 5de39be4805600..7a82b8fa320590 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3360,9 +3360,8 @@ def NoSanitizeAddressDocs : Documentation {
// This function has multiple distinct spellings, and so it requires a custom
// heading to be specified. The most common spelling is sufficient.
let Heading = "no_sanitize_address, no_address_safety_analysis";
+ let Label = "langext-address_sanitizer";
let Content = [{
-.. _langext-address_sanitizer:
-
Use ``__attribute__((no_sanitize_address))`` on a function or a global
variable declaration to specify that address safety instrumentation
(e.g. AddressSanitizer) should not be applied.
@@ -3372,9 +3371,8 @@ variable declaration to specify that address safety
instrumentation
def NoSanitizeThreadDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_thread";
+ let Label = "langext-thread_sanitizer";
let Content = [{
-.. _langext-thread_sanitizer:
-
Use ``__attribute__((no_sanitize_thread))`` on a function declaration to
specify that checks for data races on plain (non-atomic) memory accesses should
not be inserted by ThreadSanitizer. The function is still instrumented by the
@@ -3385,9 +3383,8 @@ tool to avoid false positives and provide meaningful
stack traces.
def NoSanitizeMemoryDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_memory";
+ let Label = "langext-memory_sanitizer";
let Content = [{
-.. _langext-memory_sanitizer:
-
Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
specify that checks for uninitialized memory should not be inserted
(e.g. by MemorySanitizer). The function may still be instrumented by the tool
@@ -3398,9 +3395,8 @@ to avoid false positives in other places.
def CFICanonicalJumpTableDocs : Documentation {
let Category = DocCatFunction;
let Heading = "cfi_canonical_jump_table";
+ let Label = "langext-cfi_canonical_jump_table";
let Content = [{
-.. _langext-cfi_canonical_jump_table:
-
Use ``__attribute__((cfi_canonical_jump_table))`` on a function declaration to
make the function's CFI jump table canonical. See :ref:`the CFI documentation
` for more details.
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 534bf2d01d7957..60cb4ae452326d 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -5178,6 +5178,9 @@ GetAttributeHeadingAndSpellings(const Record
&Documentation,
static void WriteDocumentation(const RecordKeeper &Records,
const DocumentationData &Doc, raw_ostream &OS) {
+ if (const StringRef label = Doc.Documentation->getValueAsString("Label");
+ !label.empty())
+OS << ".. _" << label << ":\n\n";
OS << Doc.Heading << "\n" << std::string(Doc.Heading.length(), '-') << "\n";
// List what spelling syntaxes the attribute supports.
>From 3f05ad04019139789aff09dced2360bfa536afe8 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 3 Dec 2024 21:24:22 -0800
Subject: [PATCH 2/3] Adjust style
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/utils/TableGen/ClangAttrEmitter.cpp | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 522821a79063ac..17fc36fbe2ac8c 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -56,7 +56,7 @@ class Documentation {
// specify a repl
[clang] Add support for referencable labels for attribute documentation (PR #118428)
ojhunt wrote: > Also, if/when this is merged, it would be nice if someone could go through > the rest of the docs and apply this wherever possible (assuming there are any > other places where we still need to do that). yeah, I wanted to isolate this for now to just the attribute docs as these are the only cases I could find, and I found them while working on the docs for #117428. I'm kind of surprised I couldn't find other labels anywhere, but it could just be because other docs don't reference each other due to the inability to easily do so? https://github.com/llvm/llvm-project/pull/118428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/7] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/5] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/4] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] [RFC] Initial implementation of P2719 (PR #113510)
ojhunt wrote: This has been tentatively approved C++26 - CWG didn't have time to look at it this week but EWG and SG23 have approved it. I did spend some time discussing semantics with a couple of folk in EWG and CWG and the feedback was fairly strongly on the side of "the operator delete used for cleanup when a constructor throws must have the same typed awaredness" (e.g. the existing exact placement new parameters match requirement). But this implementation currently allows fallback to a non-type aware delete, so I'm changing that and adding test cases to ensure correctness according to those semantics. https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
ojhunt wrote: Oh, my goal for this warning is to have us turn it on in the llvm+clang builds, so we can then move away from endless use of unsigned bitfields and just use enums as nature intended :D (cc @AaronBallman ) https://github.com/llvm/llvm-project/pull/117428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RFC] Initial implementation of P2719 (PR #113510)
ojhunt wrote: > > Could anyone with a windows machine see if you can work out what is > > happening with the windows test failure? I don't understand why the tests > > are failing on the windows bot as it seems like it should simply fail > > everything (e.g. windows driver is going wrong) or it should work > > It looks like the latest commit did pass on Windows, so this may be redundant > but it also passes locally on my Windows machine. Yeah, it took some time to work out the issue and it was basically just the standard "I wrote tests without a sufficiently explicit target triple" :D https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/117428
This just adds a warning for bitfields placed next to other bitfields where the
underlying type has different storage. Under the MS struct bitfield packing ABI
such bitfields are not packed.
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH] Add an off-by-default warning to complain about MSVC bitfield
padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsi
[clang] Add support for referencable labels for attribute documentation (PR #118428)
ojhunt wrote:
> I have no idea what is going on here... @AaronBallman probably needs to take
> a better look. Else, could the author please give me an ELI5 sorta thing here
> or show me what the change looks like on the generated stuff?
Sure! This example will cover the docs used in the [above
screenshot](#issuecomment-2516159888)
So currently the documentation for the attribute is:
```markdown
def NoSanitizeMemoryDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_memory";
let Content = [{
.. _langext-memory_sanitizer:
Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
}];
}
```
this results in generated docs like this:
```rst
no_sanitize_memory
--
.. csv-table:: Supported Syntaxes
:header: "GNU", "C++11", "C23", "``__declspec``", "Keyword", "``#pragma``",
"HLSL Annotation", "``#pragma clang attribute``"
"``no_address_safety_analysis`` |br| ``no_sanitize_address`` |br|
``no_sanitize_thread`` |br|
``no_sanitize_memory``","``gnu::no_address_safety_analysis`` |br|
``gnu::no_sanitize_address`` |br| ``gnu::no_sanitize_thread`` |br|
``clang::no_sanitize_memory``","``gnu::no_address_safety_analysis`` |br|
``gnu::no_sanitize_address`` |br| ``gnu::no_sanitize_thread`` |br|
``clang::no_sanitize_memory``","","","","","Yes"
.. _langext-memory_sanitizer:
Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
...
```
The result is that a `:ref:langext-memory_sanitizer` link goes into the middle
of the attribute documentation, after the name and spelling table.
This change simply adds a `Label` field to the root Documentation object and
updates the codgen to use it, and removes the currently manually specified
labels, after which we can remove the manual label from the doc string above
and use the explicit Label field which then results in this generated
documentation:
```rst
.. _langext-memory_sanitizer:
no_sanitize_memory
--
.. csv-table:: Supported Syntaxes
:header: "GNU", "C++11", "C23", "``__declspec``", "Keyword", "``#pragma``",
"HLSL Annotation", "``#pragma clang attribute``"
"``no_address_safety_analysis`` |br| ``no_sanitize_address`` |br|
``no_sanitize_thread`` |br|
``no_sanitize_memory``","``gnu::no_address_safety_analysis`` |br|
``gnu::no_sanitize_address`` |br| ``gnu::no_sanitize_thread`` |br|
``clang::no_sanitize_memory``","``gnu::no_address_safety_analysis`` |br|
``gnu::no_sanitize_address`` |br| ``gnu::no_sanitize_thread`` |br|
``clang::no_sanitize_memory``","","","","","Yes"
Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
...
```
This puts the link in a much better location for cross linking documentation
https://github.com/llvm/llvm-project/pull/118428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add support for referencable labels for attribute documentation (PR #118428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/118428
>From aea2b4001aa8e9239909875153152083b56a6a59 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Mon, 2 Dec 2024 21:25:54 -0800
Subject: [PATCH 1/2] Add support for referencable labels for attribute
documentation
The existing mechanism being used is to manually add a reference in the
documentation. These references link to the beginning of the text rather
than the heading for the attribute which is what this PR allows.
---
clang/include/clang/Basic/Attr.td | 3 +++
clang/include/clang/Basic/AttrDocs.td | 12
clang/utils/TableGen/ClangAttrEmitter.cpp | 3 +++
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 618252f3e75247..522821a79063ac 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -55,6 +55,9 @@ class Documentation {
// When set, specifies that the attribute is deprecated and can optionally
// specify a replacement attribute.
DocDeprecated Deprecated;
+
+ // When set, specifies a label that can be used to reference the
documentation
+ string Label = "";
}
// Specifies that the attribute is explicitly omitted from the documentation,
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 5de39be4805600..7a82b8fa320590 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3360,9 +3360,8 @@ def NoSanitizeAddressDocs : Documentation {
// This function has multiple distinct spellings, and so it requires a custom
// heading to be specified. The most common spelling is sufficient.
let Heading = "no_sanitize_address, no_address_safety_analysis";
+ let Label = "langext-address_sanitizer";
let Content = [{
-.. _langext-address_sanitizer:
-
Use ``__attribute__((no_sanitize_address))`` on a function or a global
variable declaration to specify that address safety instrumentation
(e.g. AddressSanitizer) should not be applied.
@@ -3372,9 +3371,8 @@ variable declaration to specify that address safety
instrumentation
def NoSanitizeThreadDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_thread";
+ let Label = "langext-thread_sanitizer";
let Content = [{
-.. _langext-thread_sanitizer:
-
Use ``__attribute__((no_sanitize_thread))`` on a function declaration to
specify that checks for data races on plain (non-atomic) memory accesses should
not be inserted by ThreadSanitizer. The function is still instrumented by the
@@ -3385,9 +3383,8 @@ tool to avoid false positives and provide meaningful
stack traces.
def NoSanitizeMemoryDocs : Documentation {
let Category = DocCatFunction;
let Heading = "no_sanitize_memory";
+ let Label = "langext-memory_sanitizer";
let Content = [{
-.. _langext-memory_sanitizer:
-
Use ``__attribute__((no_sanitize_memory))`` on a function declaration to
specify that checks for uninitialized memory should not be inserted
(e.g. by MemorySanitizer). The function may still be instrumented by the tool
@@ -3398,9 +3395,8 @@ to avoid false positives in other places.
def CFICanonicalJumpTableDocs : Documentation {
let Category = DocCatFunction;
let Heading = "cfi_canonical_jump_table";
+ let Label = "langext-cfi_canonical_jump_table";
let Content = [{
-.. _langext-cfi_canonical_jump_table:
-
Use ``__attribute__((cfi_canonical_jump_table))`` on a function declaration to
make the function's CFI jump table canonical. See :ref:`the CFI documentation
` for more details.
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 534bf2d01d7957..60cb4ae452326d 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -5178,6 +5178,9 @@ GetAttributeHeadingAndSpellings(const Record
&Documentation,
static void WriteDocumentation(const RecordKeeper &Records,
const DocumentationData &Doc, raw_ostream &OS) {
+ if (const StringRef label = Doc.Documentation->getValueAsString("Label");
+ !label.empty())
+OS << ".. _" << label << ":\n\n";
OS << Doc.Heading << "\n" << std::string(Doc.Heading.length(), '-') << "\n";
// List what spelling syntaxes the attribute supports.
>From 3f05ad04019139789aff09dced2360bfa536afe8 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 3 Dec 2024 21:24:22 -0800
Subject: [PATCH 2/2] Adjust style
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/utils/TableGen/ClangAttrEmitter.cpp | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 522821a79063ac..17fc36fbe2ac8c 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -56,7 +56,7 @@ class Documentation {
// specify a repl
[clang] Add support for referencable labels for attribute documentation (PR #118428)
ojhunt wrote: > Out of curiosity, why does the existing mechanism not suffice? it "works" but creates the tag at the beginning of the text rather than the beginning of the attribute documentation. The result is that the scroll target for linking is the beginning of the text rather than the documentation as well, e.g. the location pointed to here: https://github.com/user-attachments/assets/9a003cd6-3f29-4d4c-85b1-8b37d64467bf";> As a result when you follow the link the actual attribute name, syntax table, etc is not visible and it is necessary to scroll to see the actual documentation being linked. https://github.com/llvm/llvm-project/pull/118428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/6] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
ojhunt wrote: > Looking at diagnostics in the test you add, I wonder why don't we issue them, > and especially the fixit, when we parse bit-field declaration. Why waiting > until assignment happens? Yeah, it's super frustrating to me that we do that - I did _consider_ making them early but I really didn't want to simultaneously change that behavior and add a feature, and I thought making just preferred_type warnings early would be especially insane :D GCC at least does warn on declaration, I'm not sure if there is a deliberate and historical reason that we delay notification, or if it is just happenstance. I'd be onboard with making it a declaration warning in future. https://github.com/llvm/llvm-project/pull/116785 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
ojhunt wrote:
> > and I thought making just preferred_type warnings early would be especially
> > insane :D
>
> I don't necessarily agree. `preferred_type` is a relatively new thing, so we
> can try doing something new here.
>
Yeah, but the implementation is essentially identical, so I would literally
just be implementing it for preferred type and explicitly ignoring *actual*
enums :D
> > [edit: I just realized even that might need to be an option because I can
> > imagine someone, somewhere, depending on -Werror and not hitting this by
> > luck]
>
> I don't believe we need to be concerned about forward compatibility for
> `-Werror` users. They should know they signed themselves up for pain, and
> we're not going to compromise our QoI because of that.
Yeah, I was also trying to think of cases where this might not trigger a
warning/error currently, because there's an explicit warning path for assigning
a constant and (while too lazy to godbolt right now) imagine it being possible
to have
```cpp
enum class Enum {
A, B, C, D
};
struct S {
Enum e: 1; // I'm a rockstar developer and "know" that I'll only ever assign
A or B here
}
...
S s;
if (x)
s.e = Enum::A;
else
s.e = Enum::B;
```
and not trip an error/warning?
> In any case, this PR is step in the right direction, so I don't want to block
> its progress.
Cheers, I can file a follow on "warn on declaration" bug/PR
https://github.com/llvm/llvm-project/pull/116785
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
ojhunt wrote:
> (while too lazy to godbolt right now)
Given I basically wrote the entire example, I thought I would just do the
godbolting, and lo
```cpp
enum class Enum {
A, B, C, D
};
struct S {
Enum e: 1; // I'm a rockstar developer and "know" that I'll only ever assign
A or B here
};
void f(bool x) {
S s;
if (x)
s.e = Enum::A;
else
s.e = Enum::B;
}
```
Compiles cleanly with `-Wbitfield-width -Wbitfield-enum-conversion`:
https://godbolt.org/z/a1fErxv74
https://github.com/llvm/llvm-project/pull/116785
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/5] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/6] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/7] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/6] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
ojhunt wrote:
> > This does mean that if the preferred and explicit types have different
> > storage requirements we may not warn in all possible cases, but that's a
> > scenario for which the warnings are much more complex and confusing
>
> If I understand the patch correctly, we always warn, but about the wrong
> thing.
>
> Should we pick the minimum of both sizes? Should we make it an error for a
> bit-field to be _larger_ than the prefered type?
Hm, we could warn about both, though it might make the code grosser.
However I just realized that the current diagnostics do have a gap, take:
e.g
```cpp
enum class Foo : unsigned {
twoTo10 = 1 << 9
};
enum class Bar : unsigned {
twoTo11 = 1 << 10
};
struct S {
Foo f1: 10; // no warn today
Bar f2: 11; // no warn today
__attribute__((preferred_type(Bar)))
Foo f3: 10;
__attribute__((preferred_type(Bar)))
Foo f4: 9;
};
void f(S* s, Foo f, Bar b) {
s->f1 = f;
s->f2 = b;
s->f3 = f;
// ** REAL ISSUE **
// Because of our deferred warnings I don't think we warn on this scenario
s->f3 = (Foo)b;
s->f4 = f; // Warns about f being too wide
s->f4 = (Foo)b; // Warns about f being too wide, but is oblivious to the b
conversion
}
```
I wrote this code based on the actual usage - unsigned/int field wrapping an
enum, but for enum vs enum, which is something that preferred type permits,
then I think we don't see the `(Foo)b` conversion and truncation. I'm going to
add some tests that, and then maybe accept need to refactor out the diagnostic.
Ugh.
https://github.com/llvm/llvm-project/pull/116785
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
ojhunt wrote: > > This does mean that if the preferred and explicit types have different > > storage requirements we may not warn in all possible cases, but that's a > > scenario for which the warnings are much more complex and confusing > > If I understand the patch correctly, we always warn, but about the wrong > thing. > > Should we pick the minimum of both sizes? Should we make it an error for a > bit-field to be _larger_ than the prefered type? I've done a bunch of messing around, the error handling and semantics become super terrible pretty quickly. I think these semantics work for the general use case - "it's an integer actually containing an enum" - even though it could be an enum type with a different enum type as the preferred type. I think a follow on patch should add gcc like semantics where we just warn at the declaration, and then it becomes much easier to say "the preferred and real type must both fit in the bitfield" at the point of the declaration. https://github.com/llvm/llvm-project/pull/116785 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Consider preferred_type in bitfield warnings (#116760) (PR #116785)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/116785
>From 5f260726253e78a00d2dff02c22837ce02b49075 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Tue, 19 Nov 2024 11:55:11 +0100
Subject: [PATCH 1/6] [Clang] Consider preferred_type in bitfield warnings
(#116760)
Very simply extends the bitfield sema checks for assignment to fields
with a preferred type specified to consider the preferred type if the
decl storage type is not explicitly an enum type.
This does mean that if the preferred and explicit types have different
storage requirements we may not warn in all possible cases, but that's
a scenario for which the warnings are much more complex and confusing.
---
.../clang/Basic/DiagnosticSemaKinds.td| 9 +-
clang/lib/Sema/SemaChecking.cpp | 23 +-
.../Sema/bitfield-preferred-type-sizing.c | 108 +
.../bitfield-preferred-type-sizing.cpp| 413 ++
4 files changed, 546 insertions(+), 7 deletions(-)
create mode 100644 clang/test/Sema/bitfield-preferred-type-sizing.c
create mode 100644 clang/test/SemaCXX/bitfield-preferred-type-sizing.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..226b52f58d88d9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6404,20 +6404,23 @@ def warn_bitfield_width_exceeds_type_width: Warning<
def err_bitfield_too_wide : Error<
"%select{bit-field %1|anonymous bit-field}0 is too wide (%2 bits)">;
def warn_bitfield_too_small_for_enum : Warning<
- "bit-field %0 is not wide enough to store all enumerators of %1">,
+ "bit-field %0 is not wide enough to store all enumerators of
%select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_widen_bitfield : Note<
"widen this field to %0 bits to store all values of %1">;
def warn_unsigned_bitfield_assigned_signed_enum : Warning<
- "assigning value of signed enum type %1 to unsigned bit-field %0; "
+ "assigning value of %select{|preferred }1signed enum type %2 to unsigned
bit-field %0; "
"negative enumerators of enum %1 will be converted to positive values">,
InGroup, DefaultIgnore;
def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
- "enumerators of %1">,
+ "enumerators of %select{|preferred type }1%2">,
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def note_bitfield_preferred_type : Note<
+ "preferred type for bitfield %0 specified here"
+>;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..254284e950c7e5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10488,7 +10488,14 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
// The RHS is not constant. If the RHS has an enum type, make sure the
// bitfield is wide enough to hold all the values of the enum without
// truncation.
-if (const auto *EnumTy = OriginalInit->getType()->getAs()) {
+const auto *EnumTy = OriginalInit->getType()->getAs();
+const PreferredTypeAttr *PTAttr = nullptr;
+if (!EnumTy) {
+ PTAttr = Bitfield->getAttr();
+ if (PTAttr)
+EnumTy = PTAttr->getType()->getAs();
+}
+if (EnumTy) {
EnumDecl *ED = EnumTy->getDecl();
bool SignedBitfield = BitfieldType->isSignedIntegerType();
@@ -10509,14 +10516,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
ED->getNumPositiveBits() == FieldWidth) {
DiagID = diag::warn_signed_bitfield_enum_conversion;
}
-
+ unsigned PreferredTypeDiagIndex = PTAttr != nullptr;
if (DiagID) {
-S.Diag(InitLoc, DiagID) << Bitfield << ED;
+S.Diag(InitLoc, DiagID) << Bitfield << PreferredTypeDiagIndex << ED;
TypeSourceInfo *TSI = Bitfield->getTypeSourceInfo();
SourceRange TypeRange =
TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange();
S.Diag(Bitfield->getTypeSpecStartLoc(),
diag::note_change_bitfield_sign)
<< SignedEnum << TypeRange;
+if (PTAttr) {
+ S.Diag(PTAttr->getLocation(), diag::note_bitfield_preferred_type)
+ << ED;
+}
}
// Compute the required bitwidth. If the enum has negative values, we
need
@@ -10530,9 +10541,13 @@ static bool AnalyzeBitFieldAssignment(Sema &S,
FieldDecl *Bitfield, Expr *Init,
if (BitsNeeded > FieldWidth) {
Expr *WidthExpr = Bitfield->getBitWidth();
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_enum)
-<< Bitfield << ED;
+<< Bitfield << Pref
[clang] Add an off-by-default warning to complain about MSVC bitfield padding (PR #117428)
https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 1/7] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp| 180 ++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d0398..57bdda4b8f8b47 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eb05a6a77978af..aa13e3438d3739 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC ABI">,
+ InGroup, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 74b0e5ad23bd48..18aeca3b34f659 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector RecFields;
-
+ std::optional PreviousField;
for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast(*i), ++i) {
FieldDecl *FD = cast(*i);
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+return false;
+ if (FD->getType()->isDependentType())
+return false;
+ return true;
+};
+
+if (Record && PreviousField && IsNonDependentBitField(FD) &&
+IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+<< FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+Diag((*PreviousField)->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+<< *PreviousField << (*PreviousField)->getType();
+ }
+}
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 00..001086de5bbd10
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields
-verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
[clang] [RFC] Initial implementation of P2719 (PR #113510)
ojhunt wrote: These updates in principle bring the major semantics up to match P2719r4 Primary changes: * CV qualifiers are dropped * size and alignment parameters become mandatory, semantics for placement cleanup are updated to permit this, and to permit cleanup when the cleanup operator delete "matches" a usual deallocation function * the allocation and cleanup delete operators must be either both type aware or neither type aware To do: * A class that defines a type aware new or delete must define a complementary type aware operator https://github.com/llvm/llvm-project/pull/113510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][diagnostics] Fix structured binding shadows template param loc (PR #129116)
@@ -162,3 +162,8 @@ struct A {
};
A<0>::B a;
}
+
+template void shadow9() { // expected-note{{template parameter
is declared here}}
+ using arr = int[1]; // expected-warning@+1 {{decomposition declarations are
a C++17 extension}}
+ auto [T] = arr{}; // expected-error {{declaration of 'T' shadows template
parameter}}
ojhunt wrote:
I think this test case would benefit from the new lines added in the initial
issue report - the reported behavior is due to us choosing to report an error
based on the wrong part of the declaration, and I don't believe that this test
case is sufficient to highlight the problem (or more to the point catch a
regression in the behavior)
https://github.com/llvm/llvm-project/pull/129116
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
