[PATCH] Fix assertion failure when a member is given an address space.

[Richard Smith]

On Wed, Feb 20, 2013 at 8:23 PM, Matt Arsenault
<Matthew.Arsenault at amd.com>wrote:

> Hi rsmith,
>
> Previously adding an address space to a struct member was incorrectly
> accepted, and would later hit an assertion when accessed. This patch causes
> it to emit an error rejecting this.
>
> struct Foo
> {
>     __attribute__((address_space(2))) // Should not be accepted
>     int m_x;
> };
>
> void foo_user()
> {
>     struct Foo bar;
>     bar.m_x = 4; // Assertion failed
> }
>
> http://llvm-reviews.chandlerc.com/D444
>
> Files:
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/Sema/SemaDecl.cpp
>   lib/Sema/SemaExprMember.cpp
>   test/Sema/address_spaces.c
>
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -1721,6 +1721,8 @@
>    "automatic variable qualified with an address space">;
>  def err_arg_with_address_space : Error<
>    "parameter may not be qualified with an address space">;
> +def err_member_with_address_space : Error<
> +  "member may not be qualified with an address space">;
>

"member" is the wrong word to use here (it includes static data members and
member functions too). For diagnostics shared between C and C++, we
generally use "field" (some C++-specific diagnostics use "non-static data
member" instead).


>  def err_attr_objc_ownership_redundant : Error<
>    "the type %0 is already explicitly ownership-qualified">;
>  def err_attribute_not_string : Error<
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -9992,19 +9992,24 @@
>      }
>    }
>
> +  // TR 18037 does not allow fields to be declared with address spaces.
> +  if (T.getQualifiers().hasAddressSpace()) {
> +    Diag(Loc, diag::err_member_with_address_space);
> +    D.setInvalidType();
> +  }
> +
>    // OpenCL 1.2 spec, s6.9 r:
>    // The event type cannot be used to declare a structure or union field.
>    if (LangOpts.OpenCL && T->isEventT()) {
>      Diag(Loc, diag::err_event_t_struct_field);
>      D.setInvalidType();
>    }
>
> -
>    DiagnoseFunctionSpecifiers(D);
>
>    if (D.getDeclSpec().isThreadSpecified())
>      Diag(D.getDeclSpec().getThreadSpecLoc(), diag::err_invalid_thread);
> -
> +
>    // Check to see if this name was declared as a member previously
>    NamedDecl *PrevDecl = 0;
>    LookupResult Previous(*this, II, Loc, LookupMemberName,
> ForRedeclaration);
> Index: lib/Sema/SemaExprMember.cpp
> ===================================================================
> --- lib/Sema/SemaExprMember.cpp
> +++ lib/Sema/SemaExprMember.cpp
> @@ -1598,27 +1598,27 @@
>    } else {
>      QualType BaseType = BaseExpr->getType();
>      if (IsArrow) BaseType =
> BaseType->getAs<PointerType>()->getPointeeType();
> -
> +
>      Qualifiers BaseQuals = BaseType.getQualifiers();
> -
> +
>      // GC attributes are never picked up by members.
>      BaseQuals.removeObjCGCAttr();
> -
> +
>      // CVR attributes from the base are picked up by members,
>      // except that 'mutable' members don't pick up 'const'.
>      if (Field->isMutable()) BaseQuals.removeConst();
> -
> +
>      Qualifiers MemberQuals
>      = S.Context.getCanonicalType(MemberType).getQualifiers();
> -
> -    // TR 18037 does not allow fields to be declared with address spaces.
> +
>      assert(!MemberQuals.hasAddressSpace());
> -
> +
> +
>      Qualifiers Combined = BaseQuals + MemberQuals;
>      if (Combined != MemberQuals)
>        MemberType = S.Context.getQualifiedType(MemberType, Combined);
>    }
> -
> +
>    S.UnusedPrivateFields.remove(Field);
>
>    ExprResult Base =
> Index: test/Sema/address_spaces.c
> ===================================================================
> --- test/Sema/address_spaces.c
> +++ test/Sema/address_spaces.c
> @@ -6,7 +6,7 @@
>
>  void bar(_AS2 int a); // expected-error {{parameter may not be qualified
> with an address space}}
>
> -void foo(_AS3 float *a,
> +void foo(_AS3 float *a,
>           _AS1 float b) // expected-error {{parameter may not be qualified
> with an address space}}
>  {
>    _AS2 *x;// expected-warning {{type specifier missing, defaults to
> 'int'}}
> @@ -48,3 +48,19 @@
>  typedef void ft(void);
>  _AS1 ft qf; // expected-error {{function type may not be qualified with
> an address space}}
>  typedef _AS1 ft qft; // expected-error {{function type may not be
> qualified with an address space}}
> +
> +
> +typedef _AS2 int AS2Int;
> +
> +struct HasASMembers
> +{
> +  _AS2 int as_member; // expected-error {{member may not be qualified
> with an address space}}
> +   AS2Int typedef_as_member; // expected-error {{member may not be
> qualified with an address space}}
> +};
> +
> +// Assertion failure was when the member was accessed
> +void access_as_member()
> +{
> +    struct HasASMembers x;
> +    (void) bar.as_member;
> +}
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130220/62676087/attachment.html>