Skip to content

Silence spurious -Wnontrivial-memcall warnings in C mode #137429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9724,9 +9724,9 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
// completed later. GCC does not diagnose such code, but we may want to
// consider diagnosing it in the future, perhaps under a different, but
// related, diagnostic group.
bool MayBeTriviallyCopyableCXXRecord =
RT->isIncompleteType() ||
RT->desugar().isTriviallyCopyableType(Context);
bool NonTriviallyCopyableCXXRecord =
getLangOpts().CPlusPlus && !RT->isIncompleteType() &&
!RT->desugar().isTriviallyCopyableType(Context);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to have isTriviallyCopyableType do any required desugaring or canonicalization rather than requiring every call site do this work, it's a constant hazard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to desugar the type before calling isTriviallyCopyableType? It doesn't look like other call sites are doing so.

Why is the call to desugar` needed here?


if ((BId == Builtin::BImemset || BId == Builtin::BIbzero) &&
RT->getDecl()->isNonTrivialToPrimitiveDefaultInitialize()) {
Expand All @@ -9735,7 +9735,7 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
<< ArgIdx << FnName << PointeeTy << 0);
SearchNonTrivialToInitializeField::diag(PointeeTy, Dest, *this);
} else if ((BId == Builtin::BImemset || BId == Builtin::BIbzero) &&
!MayBeTriviallyCopyableCXXRecord && ArgIdx == 0) {
NonTriviallyCopyableCXXRecord && ArgIdx == 0) {
// FIXME: Limiting this warning to dest argument until we decide
// whether it's valid for source argument too.
DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
Expand All @@ -9748,7 +9748,7 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
<< ArgIdx << FnName << PointeeTy << 1);
SearchNonTrivialToCopyField::diag(PointeeTy, Dest, *this);
} else if ((BId == Builtin::BImemcpy || BId == Builtin::BImemmove) &&
!MayBeTriviallyCopyableCXXRecord && ArgIdx == 0) {
NonTriviallyCopyableCXXRecord && ArgIdx == 0) {
// FIXME: Limiting this warning to dest argument until we decide
// whether it's valid for source argument too.
DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
Expand Down
60 changes: 60 additions & 0 deletions clang/test/Sema/warn-nontrivial-struct-memaccess-ptrauth.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -fptrauth-intrinsics -fsyntax-only -verify=c,expected %s
// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -fptrauth-intrinsics -fsyntax-only -verify=c,expected %s
// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -fptrauth-intrinsics -fsyntax-only -x c++ -verify=cxx,expected %s
// RUN: %clang_cc1 -triple aarch64-linux-gnu -fptrauth-calls -fptrauth-intrinsics -fsyntax-only -x c++ -verify=cxx,expected %s

#if defined __cplusplus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahahah, I was literally thinking "add a -D argument to say it's C++" because I'm a muppet and forgot the builtin macro :D

extern "C" {
#endif

void *memset(void *, int, __SIZE_TYPE__);
void bzero(void *, __SIZE_TYPE__);
void *memcpy(void *, const void *, __SIZE_TYPE__);
void *memmove(void *, const void *, __SIZE_TYPE__);

#if defined __cplusplus
}
#endif

#define AQ __ptrauth(1,1,50)
#define IQ __ptrauth(1,0,50)

struct PtrAuthTrivial {
int f0;
int * IQ f1;
};

struct PtrAuthNonTrivial0 {
int f0;
int * AQ f1; // c-note 2 {{non-trivial to copy}}
int f2;
};

struct PtrAuthNonTrivial1 {
int * AQ f0; // c-note 2 {{non-trivial to copy}}
int f1;
struct PtrAuthNonTrivial0 f2;
};

void testPtrAuthTrivial(struct PtrAuthTrivial *d, struct PtrAuthTrivial *s) {
memset(d, 0, sizeof(struct PtrAuthTrivial));
memset(d, 1, sizeof(struct PtrAuthTrivial));
bzero(d, sizeof(struct PtrAuthTrivial));
memcpy(d, s, sizeof(struct PtrAuthTrivial));
memmove(d, s, sizeof(struct PtrAuthTrivial));
}

void testPtrAuthNonTrivial1(struct PtrAuthNonTrivial1 *d,
struct PtrAuthNonTrivial1 *s) {
memset(d, 0, sizeof(struct PtrAuthNonTrivial1)); // cxx-warning {{is a pointer to non-trivially copyable type 'struct PtrAuthNonTrivial1'}} // cxx-note {{explicitly cast the pointer to silence}}
memset(d, 1, sizeof(struct PtrAuthNonTrivial1)); // cxx-warning {{is a pointer to non-trivially copyable type 'struct PtrAuthNonTrivial1'}} // cxx-note {{explicitly cast the pointer to silence}}
bzero(d, sizeof(struct PtrAuthNonTrivial1)); // cxx-warning {{is a pointer to non-trivially copyable type 'struct PtrAuthNonTrivial1'}} // cxx-note {{explicitly cast the pointer to silence}}
memcpy(d, s, sizeof(struct PtrAuthNonTrivial1));
// c-warning@-1 {{that is not trivial to primitive-copy}}
// cxx-warning@-2 {{is a pointer to non-trivially copyable type 'struct PtrAuthNonTrivial1'}}
// expected-note@-3 {{explicitly cast the pointer to silence}}
memmove(d, s, sizeof(struct PtrAuthNonTrivial1));
// c-warning@-1 {{that is not trivial to primitive-copy}}
// cxx-warning@-2 {{is a pointer to non-trivially copyable type 'struct PtrAuthNonTrivial1'}}
// expected-note@-3 {{explicitly cast the pointer to silence}}
}