Currently as-base classes lack DECL_BIT_FIELD_REPRESENTATIVEs which
means RTL expansion doesn't honor the C++ memory model for bitfields
in them thus for the following testcase

struct B {
    B() {}
    int x;
    int a : 6;
    int b : 6;
    int c : 6;
};

struct C : B {
    char d;
};

C c;

int main()
{
  c.c = 1;
  c.d = 2;
}

on x86 we happily store to c.c in a way creating a store data race with
c.d:

main:
.LFB6:
        .cfi_startproc
        movl    c+4(%rip), %eax
        andl    $-258049, %eax
        orb     $16, %ah
        movl    %eax, c+4(%rip)
        movb    $2, c+7(%rip)
        xorl    %eax, %eax
        ret

Fixing the lack of DECL_BIT_FIELD_REPRESENTATIVEs in as-base
classes doesn't help though as the C++ FE builds access trees
for c.c using the non-as-base class FIELD_DECLs which is because
of layout_class_type doing

  /* Now that we're done with layout, give the base fields the real types.  */
  for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
    if (DECL_ARTIFICIAL (field) && IS_FAKE_BASE_TYPE (TREE_TYPE (field)))
      TREE_TYPE (field) = TYPE_CONTEXT (TREE_TYPE (field));

this would basically require us to always treat tail-padding in a
struct conservatively in finish_bitfield_representative (according
to the doubt by the ??? comment I patch out below).

Simply commenting out the above makes fixing build_simple_base_path
necessary but even after that it then complains in the verifier
later ("type mismatch in component reference" - as-base to class
assignment).

But it still somehow ends up using the wrong FIELD_DECL in the end.

Now I think we need to fix the wrong-code issue somehow and
doing so in stor-layout.c by conservatively treating tail-padding
is a possibility.  But that will pessimize PODs and other languages
unless we have a way to know whether a RECORD_TYPE possibly can
have its tail-padding re-used (I'd hate to put a lang_hooks.name
check there and even that would pessimize C++ PODs).

Any guidance here?

I opened PR71694 for this.

Richard.

2016-06-29  Richard Biener  <rguent...@suse.de>

        * stor-layout.c (finish_bitfield_representative): Clarify comment.

        cp/
        * class.c (layout_class_type): Call finish_bitfield_layout on
        the as-base variant.

Index: gcc/stor-layout.c
===================================================================
*** gcc/stor-layout.c   (revision 237840)
--- gcc/stor-layout.c   (working copy)
*************** finish_bitfield_representative (tree rep
*** 1866,1878 ****
      }
    else
      {
!       /* ???  If you consider that tail-padding of this struct might be
!          re-used when deriving from it we cannot really do the following
!        and thus need to set maxsize to bitsize?  Also we cannot
!        generally rely on maxsize to fold to an integer constant, so
!        use bitsize as fallback for this case.  */
        tree maxsize = size_diffop (TYPE_SIZE_UNIT (DECL_CONTEXT (field)),
                                  DECL_FIELD_OFFSET (repr));
        if (tree_fits_uhwi_p (maxsize))
        maxbitsize = (tree_to_uhwi (maxsize) * BITS_PER_UNIT
                      - tree_to_uhwi (DECL_FIELD_BIT_OFFSET (repr)));
--- 1866,1878 ----
      }
    else
      {
!       /* Note that if the C++ FE sets up tail-padding to be re-used it
!          creates a as-base variant of the type with TYPE_SIZE adjusted
!        accordingly.  So it is safe to include tail-padding here.  */
        tree maxsize = size_diffop (TYPE_SIZE_UNIT (DECL_CONTEXT (field)),
                                  DECL_FIELD_OFFSET (repr));
+       /* We cannot generally rely on maxsize to fold to an integer constant,
+        so use bitsize as fallback for this case.  */
        if (tree_fits_uhwi_p (maxsize))
        maxbitsize = (tree_to_uhwi (maxsize) * BITS_PER_UNIT
                      - tree_to_uhwi (DECL_FIELD_BIT_OFFSET (repr)));
Index: gcc/cp/class.c
===================================================================
*** gcc/cp/class.c      (revision 237840)
--- gcc/cp/class.c      (working copy)
*************** layout_class_type (tree t, tree *virtual
*** 6556,6561 ****
--- 6556,6564 ----
          }
        *next_field = NULL_TREE;
  
+       /* Finish bitfield layout of the type.  */
+       finish_bitfield_layout (base_t);
+ 
        /* Record the base version of the type.  */
        CLASSTYPE_AS_BASE (t) = base_t;
        TYPE_CONTEXT (base_t) = t;

Reply via email to