[clang] e327b52 - [C2x] reject type definitions in offsetof
Yingchi Long via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 12 23:34:15 PST 2023
Author: Yingchi Long
Date: 2023-01-13T15:33:26+08:00
New Revision: e327b52766ed497e4779f4e652b9ad237dfda8e6
URL: https://github.com/llvm/llvm-project/commit/e327b52766ed497e4779f4e652b9ad237dfda8e6
DIFF: https://github.com/llvm/llvm-project/commit/e327b52766ed497e4779f4e652b9ad237dfda8e6.diff
LOG: [C2x] reject type definitions in offsetof
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm made very
clear that it is an UB having type definitions with in offsetof. After
this patch clang will reject any type definitions in __builtin_offsetof.
Fixes https://github.com/llvm/llvm-project/issues/57065
```
local/offsetof.c:10:38: error: 'struct S' cannot be defined in '__builtin_offsetof'
return __builtin_offsetof(struct S{ int a, b;}, a);
^
```
Reviewed By: aaron.ballman, #clang-language-wg
Differential Revision: https://reviews.llvm.org/D133574
Added:
clang/test/C/C2x/n2350.c
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Parse/RAIIObjectsForParser.h
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseExpr.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/C/drs/dr4xx.c
clang/test/Parser/declarators.c
clang/test/SemaCXX/offsetof.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 65a9d65715ddd..c5626ac8fe272 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -668,6 +668,9 @@ C2x Feature Support
va_start(list); // Invalid in C17 and earlier, valid in C2x and later.
va_end(list);
}
+
+- Reject type definitions in the ``type`` argument of ``__builtin_offsetof``
+ according to `WG14 N2350 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm>`_.
C++ Language Changes in Clang
-----------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 02afb098b2395..862ac845bda45 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1650,6 +1650,8 @@ def err_type_defined_in_condition : Error<
"%0 cannot be defined in a condition">;
def err_type_defined_in_enum : Error<
"%0 cannot be defined in an enumeration">;
+def err_type_defined_in_offsetof : Error<
+ "%0 cannot be defined in '%select{__builtin_offsetof|offsetof}1'">;
def note_pure_virtual_function : Note<
"unimplemented pure virtual method %0 in %1">;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 67e163ca512d1..8d934d59a580b 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -61,6 +61,7 @@ class Parser : public CodeCompletionHandler {
friend class ColonProtectionRAIIObject;
friend class ParsingOpenMPDirectiveRAII;
friend class InMessageExpressionRAIIObject;
+ friend class OffsetOfStateRAIIObject;
friend class PoisonSEHIdentifiersRAIIObject;
friend class ObjCDeclContextSwitch;
friend class ParenBraceBracketBalancer;
@@ -247,6 +248,8 @@ class Parser : public CodeCompletionHandler {
/// function call.
bool CalledSignatureHelp = false;
+ Sema::OffsetOfKind OffsetOfState = Sema::OffsetOfKind::OOK_Outside;
+
/// The "depth" of the template parameters currently being parsed.
unsigned TemplateParameterDepth;
diff --git a/clang/include/clang/Parse/RAIIObjectsForParser.h b/clang/include/clang/Parse/RAIIObjectsForParser.h
index 5ae609e600734..cb525c9d0edd6 100644
--- a/clang/include/clang/Parse/RAIIObjectsForParser.h
+++ b/clang/include/clang/Parse/RAIIObjectsForParser.h
@@ -341,6 +341,19 @@ namespace clang {
}
};
+ class OffsetOfStateRAIIObject {
+ Sema::OffsetOfKind &OffsetOfState;
+ Sema::OffsetOfKind OldValue;
+
+ public:
+ OffsetOfStateRAIIObject(Parser &P, Sema::OffsetOfKind Value)
+ : OffsetOfState(P.OffsetOfState), OldValue(P.OffsetOfState) {
+ OffsetOfState = Value;
+ }
+
+ ~OffsetOfStateRAIIObject() { OffsetOfState = OldValue; }
+ };
+
/// RAII object that makes sure paren/bracket/brace count is correct
/// after declaration/statement parsing, even when there's a parsing error.
class ParenBraceBracketBalancer {
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fd60a1f9ac43e..be5265b7968b8 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3304,6 +3304,16 @@ class Sema final {
TUK_Friend // Friend declaration: 'friend struct foo;'
};
+ enum OffsetOfKind {
+ // Not parsing a type within __builtin_offsetof.
+ OOK_Outside,
+ // Parsing a type within __builtin_offsetof.
+ OOK_Builtin,
+ // Parsing a type within macro "offsetof", defined in __buitin_offsetof
+ // To improve our diagnostic message.
+ OOK_Macro,
+ };
+
Decl *ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
SourceLocation KWLoc, CXXScopeSpec &SS, IdentifierInfo *Name,
SourceLocation NameLoc, const ParsedAttributesView &Attr,
@@ -3312,7 +3322,7 @@ class Sema final {
bool &IsDependent, SourceLocation ScopedEnumKWLoc,
bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody = nullptr);
+ OffsetOfKind OOK, SkipBodyInfo *SkipBody = nullptr);
Decl *ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
unsigned TagSpec, SourceLocation TagLoc,
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 74893dee2123c..241a46193d5fe 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4972,7 +4972,7 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
DSC == DeclSpecContext::DSC_type_specifier,
DSC == DeclSpecContext::DSC_template_param ||
DSC == DeclSpecContext::DSC_template_type_arg,
- &SkipBody);
+ OffsetOfState, &SkipBody);
if (SkipBody.ShouldSkip) {
assert(TUK == Sema::TUK_Definition && "can only skip a definition");
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index ebc86c00870e7..6fbf804360116 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2073,7 +2073,7 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
DSC == DeclSpecContext::DSC_type_specifier,
DSC == DeclSpecContext::DSC_template_param ||
DSC == DeclSpecContext::DSC_template_type_arg,
- &SkipBody);
+ OffsetOfState, &SkipBody);
// If ActOnTag said the type was dependent, try again with the
// less common call.
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 95d8b649f19bf..34fead3478004 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2591,10 +2591,21 @@ ExprResult Parser::ParseBuiltinPrimaryExpression() {
}
case tok::kw___builtin_offsetof: {
SourceLocation TypeLoc = Tok.getLocation();
- TypeResult Ty = ParseTypeName();
- if (Ty.isInvalid()) {
- SkipUntil(tok::r_paren, StopAtSemi);
- return ExprError();
+ auto K = Sema::OffsetOfKind::OOK_Builtin;
+ if (Tok.getLocation().isMacroID()) {
+ StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
+ Tok.getLocation(), PP.getSourceManager(), getLangOpts());
+ if (MacroName == "offsetof")
+ K = Sema::OffsetOfKind::OOK_Macro;
+ }
+ TypeResult Ty;
+ {
+ OffsetOfStateRAIIObject InOffsetof(*this, K);
+ Ty = ParseTypeName();
+ if (Ty.isInvalid()) {
+ SkipUntil(tok::r_paren, StopAtSemi);
+ return ExprError();
+ }
}
if (ExpectAndConsume(tok::comma)) {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 3dbd61c2c711c..9ab53f56d29cb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16591,7 +16591,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
SourceLocation ScopedEnumKWLoc,
bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody) {
+ OffsetOfKind OOK, SkipBodyInfo *SkipBody) {
// If this is not a definition, it must have a name.
IdentifierInfo *OrigName = Name;
assert((Name != nullptr || TUK == TUK_Definition) &&
@@ -17364,10 +17364,16 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
cast_or_null<RecordDecl>(PrevDecl));
}
+ if (OOK != OOK_Outside && TUK == TUK_Definition) {
+ Diag(New->getLocation(), diag::err_type_defined_in_offsetof)
+ << Context.getTagDeclType(New) << static_cast<int>(OOK == OOK_Macro);
+ Invalid = true;
+ }
+
// C++11 [dcl.type]p3:
// A type-specifier-seq shall not define a class or enumeration [...].
- if (getLangOpts().CPlusPlus && (IsTypeSpecifier || IsTemplateParamOrArg) &&
- TUK == TUK_Definition) {
+ if (!Invalid && getLangOpts().CPlusPlus &&
+ (IsTypeSpecifier || IsTemplateParamOrArg) && TUK == TUK_Definition) {
Diag(New->getLocation(), diag::err_type_defined_in_type_specifier)
<< Context.getTagDeclType(New);
Invalid = true;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 2ac3cdc04fddb..96d57f4506814 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -16961,15 +16961,15 @@ Decl *Sema::ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
if (SS.isEmpty()) {
bool Owned = false;
bool IsDependent = false;
- return ActOnTag(S, TagSpec, TUK_Friend, TagLoc, SS, Name, NameLoc,
- Attr, AS_public,
+ return ActOnTag(S, TagSpec, TUK_Friend, TagLoc, SS, Name, NameLoc, Attr,
+ AS_public,
/*ModulePrivateLoc=*/SourceLocation(),
MultiTemplateParamsArg(), Owned, IsDependent,
/*ScopedEnumKWLoc=*/SourceLocation(),
/*ScopedEnumUsesClassTag=*/false,
/*UnderlyingType=*/TypeResult(),
/*IsTypeSpecifier=*/false,
- /*IsTemplateParamOrArg=*/false);
+ /*IsTemplateParamOrArg=*/false, /*OOK=*/OOK_Outside);
}
NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 40335c8b507c5..cfa28e4336874 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -10180,13 +10180,12 @@ Sema::ActOnExplicitInstantiation(Scope *S, SourceLocation ExternLoc,
bool Owned = false;
bool IsDependent = false;
- Decl *TagD = ActOnTag(S, TagSpec, Sema::TUK_Reference,
- KWLoc, SS, Name, NameLoc, Attr, AS_none,
- /*ModulePrivateLoc=*/SourceLocation(),
- MultiTemplateParamsArg(), Owned, IsDependent,
- SourceLocation(), false, TypeResult(),
- /*IsTypeSpecifier*/false,
- /*IsTemplateParamOrArg*/false);
+ Decl *TagD = ActOnTag(
+ S, TagSpec, Sema::TUK_Reference, KWLoc, SS, Name, NameLoc, Attr, AS_none,
+ /*ModulePrivateLoc=*/SourceLocation(), MultiTemplateParamsArg(), Owned,
+ IsDependent, SourceLocation(), false, TypeResult(),
+ /*IsTypeSpecifier*/ false,
+ /*IsTemplateParamOrArg=*/false, /*OOK=*/OOK_Outside);
assert(!IsDependent && "explicit instantiation of dependent name not yet handled");
if (!TagD)
diff --git a/clang/test/C/C2x/n2350.c b/clang/test/C/C2x/n2350.c
new file mode 100644
index 0000000000000..3b4bdec26bf77
--- /dev/null
+++ b/clang/test/C/C2x/n2350.c
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c89 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c17 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -verify %s
+
+// Reject definitions in __builtin_offsetof
+// https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
+int simple(void) {
+ return __builtin_offsetof(struct A // expected-error{{'struct A' cannot be defined in '__builtin_offsetof'}}
+ {
+ int a;
+ struct B // expected-error{{'struct B' cannot be defined in '__builtin_offsetof'}}
+ {
+ int c;
+ int d;
+ } x;
+ }, a);
+}
+
+int anonymous_struct() {
+ return __builtin_offsetof(struct // expected-error-re{{'struct (unnamed at {{.*}})' cannot be defined in '__builtin_offsetof'}}
+ {
+ int a;
+ int b;
+ }, a);
+}
+
+int struct_in_second_param() {
+ struct A {
+ int a, b;
+ int x[20];
+ };
+ return __builtin_offsetof(struct A, x[sizeof(struct B{int a;})]); // no-error
+}
+
+
+#define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
+
+
+int macro(void) {
+ return offsetof(struct A // expected-error{{'struct A' cannot be defined in 'offsetof'}}
+ // expected-error at -1{{'struct B' cannot be defined in 'offsetof'}}
+ {
+ int a;
+ struct B // verifier seems to think the error is emitted by the macro
+ // In fact the location of the error is "B" on the line above
+ {
+ int c;
+ int d;
+ } x;
+ }, a);
+}
+
+#undef offsetof
+
+#define offsetof(TYPE, MEMBER) (&((TYPE *)0)->MEMBER)
+
+// no warning for traditional offsetof as a function-like macro
+int * macro_func(void) {
+ return offsetof(struct A // no-warning
+ {
+ int a;
+ int b;
+ }, a);
+}
diff --git a/clang/test/C/drs/dr4xx.c b/clang/test/C/drs/dr4xx.c
index 768897cd4f2bb..f5ad1b82bec4a 100644
--- a/clang/test/C/drs/dr4xx.c
+++ b/clang/test/C/drs/dr4xx.c
@@ -352,11 +352,10 @@ void dr496(void) {
*/
/* The DR asked a question about whether defining a new type within offsetof
- * is allowed. C2x N2350 made this explicitly undefined behavior, but Clang
- * has always supported defining a type in this location, and GCC also
- * supports it.
+ * is allowed. C2x N2350 made this explicitly undefined behavior, but GCC
+ * supports it, Clang diagnoses this a UB and rejects it.
*/
- (void)__builtin_offsetof(struct S { int a; }, a);
+ (void)__builtin_offsetof(struct S { int a; }, a); /* expected-error{{'struct S' cannot be defined in '__builtin_offsetof'}} */
}
/* WG14 DR499: yes
diff --git a/clang/test/Parser/declarators.c b/clang/test/Parser/declarators.c
index 464fafeaa0d27..3af09817e6b63 100644
--- a/clang/test/Parser/declarators.c
+++ b/clang/test/Parser/declarators.c
@@ -80,10 +80,6 @@ struct test9 {
struct test10 { int a; } static test10x;
struct test11 { int a; } const test11x;
-// PR6216
-void test12(void) {
- (void)__builtin_offsetof(struct { char c; int i; }, i);
-}
// rdar://7608537
struct test13 { int a; } (test13x);
diff --git a/clang/test/SemaCXX/offsetof.cpp b/clang/test/SemaCXX/offsetof.cpp
index c4b288aa05d43..3eee6fb41d339 100644
--- a/clang/test/SemaCXX/offsetof.cpp
+++ b/clang/test/SemaCXX/offsetof.cpp
@@ -83,3 +83,20 @@ struct Derived : virtual Base {
expected-error {{invalid application of 'offsetof' to a field of a virtual base}}
};
}
+
+// Reject definitions in __builtin_offsetof
+// https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
+int test_definition(void) {
+ return __builtin_offsetof(struct A // expected-error{{'A' cannot be defined in '__builtin_offsetof'}}
+ {
+ int a;
+ struct B // FIXME: error diagnostic message for nested definitions
+ // https://reviews.llvm.org/D133574
+ // fixme-error{{'A' cannot be defined in '__builtin_offsetof'}}
+ {
+ int c;
+ int d;
+ };
+ B x;
+ }, a);
+}
More information about the cfe-commits
mailing list