[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

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 65a9d65..c5626ac 100644 (file)
--- 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 02afb09..862ac84 100644 (file)
--- 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 67e163c..8d934d5 100644 (file)
--- 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 5ae609e..cb525c9 100644 (file)
--- 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 fd60a1f..be5265b 100644 (file)
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3304,6 +3304,16 @@ public:
     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 @@ public:
                  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 74893de..241a461 100644 (file)
--- 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 ebc86c0..6fbf804 100644 (file)
--- 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 95d8b64..34fead3 100644 (file)
--- 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 3dbd61c..9ab53f5 100644 (file)
--- 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 @@ CreateNewDecl:
                                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 2ac3cdc..96d57f4 100644 (file)
--- 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 40335c8..cfa28e4 100644 (file)
--- 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 (file)
index 0000000..3b4bdec
--- /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@-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 768897c..f5ad1b8 100644 (file)
--- 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 464fafe..3af0981 100644 (file)
--- 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 c4b288a..3eee6fb 100644 (file)
--- 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);
+}