Bug 87137 - [8/9 Regression] Non-virtual member function increases struct size
Summary: [8/9 Regression] Non-virtual member function increases struct size
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 8.2.0
: P3 normal
Target Milestone: 8.3
Assignee: Nathan Sidwell
URL:
Keywords: ABI, wrong-code
Depends on:
Blocks:
 
Reported: 2018-08-29 10:50 UTC by Sandro Mani
Modified: 2018-11-08 21:02 UTC (History)
3 users (show)

See Also:
Host:
Target: i686-w64-mingw32, x86_64-w64-mingw32
Build:
Known to work: 7.3.0
Known to fail: 9.0
Last reconfirmed: 2018-08-29 00:00:00


Attachments
patch (1.02 KB, text/plain)
2018-08-29 14:34 UTC, Nathan Sidwell
Details
this is the testcase (403 bytes, text/x-csrc)
2018-08-29 14:42 UTC, Nathan Sidwell
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sandro Mani 2018-08-29 10:50:49 UTC
With gcc-8.0.0 onwards, specifically commit ab87ee8f509c0b600102195704105d4d98ec59d9, the following test case fails to compile using either i686-w64-mingw32-g++ or x86_64-w64-mingw32-g++ (but compiles normally using the native x86_64-linux-g++):

--- test.cpp ---

template <typename ToCheck, unsigned ExpectedSize, unsigned RealSize = sizeof(ToCheck)>
void check_size() {
  static_assert(ExpectedSize == RealSize, "Size is off!");
}

struct Foo {
    unsigned u1 : 5;
    unsigned u2 : 5;
    unsigned u3 : 3;
    unsigned u4 : 3;
    unsigned u5 : 4;
    unsigned u6 : 2;
    unsigned u7 : 3;
    unsigned u8 : 2;
    unsigned u9 : 1;

    unsigned u10 : 3;
    // 31 bits
    unsigned u11 : 2;
    unsigned u12 : 2;
    unsigned u13 : 2;

    unsigned u14 : 6;
    unsigned u15 : 7;
    unsigned u16 : 1;
    unsigned u17 : 1;
    unsigned u18 : 1;
    unsigned u19 : 1;
    unsigned u20 : 1;

    bool bar() const { return false; }
private:
    unsigned u21 : 1;
    unsigned u22 : 1;
    unsigned u23 : 1;
    unsigned u24 : 1;
    // 59 bits
};

int main(int argc, char* argv[]) {
    check_size<Foo, 8>();
    return 0;
}

-----

$ x86_64-w64-mingw32-g++ test.cpp -o test.exe
test.cpp: In instantiation of 'void check_size() [with ToCheck = Foo; unsigned int ExpectedSize = 8; unsigned int RealSize = 12]':
test.cpp:41:24:   required from here
test.cpp:3:30: error: static assertion failed: Size is off!
   static_assert(ExpectedSize == RealSize, "Size is off!");


Notice the presence of the bar member function. Without the member function, sizeof(Foo) = 8 as expected, but with the member function, sizeof(Foo) = 12.
Comment 1 Sandro Mani 2018-08-29 10:51:41 UTC
CC nathan@acm.org as author of ab87ee8f509c0b600102195704105d4d98ec59d9
Comment 2 Nathan Sidwell 2018-08-29 11:30:55 UTC
What changelog does that has refer to? (I don;t use the git repo)
Comment 4 Jonathan Wakely 2018-08-29 11:59:31 UTC
That's r250413

Confirmed
Comment 5 Nathan Sidwell 2018-08-29 14:04:02 UTC
what should the layout of the following be?
struct Foo {
  unsigned one : 24;
  static int var;
  unsigned two : 4;
};

is this size 8 or 4?
Comment 6 Nathan Sidwell 2018-08-29 14:34:33 UTC
Created attachment 44623 [details]
patch

This patch appears to fix the problem  It'd be good to (a) confirms it also passes on MS's compiler
Comment 7 Nathan Sidwell 2018-08-29 14:42:35 UTC
Created attachment 44624 [details]
this is the testcase
Comment 8 Jonathan Wakely 2018-08-29 14:47:49 UTC
It compiles successfully with MSVC https://godbolt.org/z/Cw-yiW
Comment 9 Liu Hao 2018-08-29 15:29:33 UTC
(In reply to Nathan Sidwell from comment #6)
> Created attachment 44623 [details]
> patch
> 
> This patch appears to fix the problem  It'd be good to (a) confirms it also
> passes on MS's compiler

With this patch applied, GCC 8.2.1 accepts the testcase, as well as Microsoft CL 19.15.26726 .
Comment 10 Nathan Sidwell 2018-08-29 17:37:24 UTC
gcc-8 patch posted https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01900.html
Comment 11 Nathan Sidwell 2018-09-05 10:05:29 UTC
Author: nathan
Date: Wed Sep  5 10:04:58 2018
New Revision: 264119

URL: https://gcc.gnu.org/viewcvs?rev=264119&root=gcc&view=rev
Log:
PR c++/87137] GCC-8 Fix

https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01966.html
	PR c++/87137
	* stor-layout.c (place_field): Scan forwards to check last
	bitfield when ms_bitfield_placement is in effect.
	gcc/testsuite/
	* g++.dg/abi/pr87137.C: New.

Added:
    trunk/gcc/testsuite/g++.dg/abi/pr87137.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/stor-layout.c
    trunk/gcc/testsuite/ChangeLog
Comment 12 Nathan Sidwell 2018-09-05 10:18:31 UTC
Fixed trunk r264119
Comment 13 Nathan Sidwell 2018-09-05 10:43:29 UTC
Fixed gcc-8 r264123.

changes.html remains to do
Comment 14 Nathan Sidwell 2018-09-05 10:43:31 UTC
Author: nathan
Date: Wed Sep  5 10:42:59 2018
New Revision: 264123

URL: https://gcc.gnu.org/viewcvs?rev=264123&root=gcc&view=rev
Log:
[PR c++/87137] GCC-8 Fix

https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01966.html
	PR c++/87137
	* stor-layout.c (place_field): Scan forwards to check last
	bitfield when ms_bitfield_placement is in effect.
	gcc/testsuite/
	* g++.dg/abi/pr87137.C: New.

Added:
    branches/gcc-8-branch/gcc/testsuite/g++.dg/abi/pr87137.C
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/stor-layout.c
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
Comment 15 Nathan Sidwell 2018-11-08 21:02:05 UTC
I forgot to mark this fixed.