This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][C++] C++ bitfield memory model for as-base classes


On 06/29/2016 05:54 AM, Richard Biener wrote:

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?
Note that if we change tail-padding re-use properties, then we effectively have an ABI change. Given that, the only path forward is to use smaller memory operations.

Do any other compilers gets this right (LLVM?)

jeff



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]