You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Differently aligned fields may make inheritance not work properly. #380
After digging a bit more I don't think this is due to virtual inheritance anymore (it sort of is, because there was a bug in our handling of virtual inheritance, but it's not the root cause of this one I believe).
classbasic_ios { };
classbasic_ostream: virtual basic_ios { };
classostrstream:basic_ostream {
int _M_buf;
};
classLogStream:ostrstream {
int ctr_;
};
I think this is because how alignment is done by clang and rustc, mainly, ostrstream is 16-byte aligned on 64 bits, but clang still lays out the int crt_ field contiguously (because it's a base class), while rustc lays it out in the next 16-byte slot because we represent it as a field.
I'll grab a debugger and inspect the fields, but I suspect that offsetof(LogStream, crt_) is going to be 8-byte aligned, but not 16-byte aligned (that is, contiguous to _M_buf).
Yup, not related to virtual inheritance at all, more minimized:
classostrstream {
void* ptr;
int _M_buf;
};
classLogStream:ostrstream {
int ctr_;
};
I'm not sure what's the best way to solve this unfortunately... Ideally having some kind of annotation on rustc, but the short-term solutions I'm thinking about aren't pretty (we wouldn't have easy access to base classes as we have now).
I guess the easiest way to solve this is inlining all the base class fields, then add helper methods to transmute as base classes, instead of the way we represent inheritance now.
It's a hard-ish patch though, and I'm not sure I'll have the time to do it before exams are over :)
Actually, this is going to need some rust compiler magic I believe, otherwise examples like this (that currently work) would stop working if we just inline the fields:
classWordAligned {
void* ptr;
int foo;
};
classWordAlignedButWithDifferentlyAlignedFirstField {
int foo;
void* ptr;
};
classTest : publicWordAligned,
public WordAlignedButWithDifferentlyAlignedFirstField
{};
It seems that the logic is a bit more complicated, and clang only packs bits on the last base class. That is, this test case still fails:
classWordAligned {
void* ptr;
int foo;
};
classWordAlignedButWithDifferentlyAlignedFirstField {
int foo;
void* ptr;
int bar;
};
classTest : publicWordAligned,
public WordAlignedButWithDifferentlyAlignedFirstField
{
int baz;
};
Hope to bump this, I'm on Debian 12 and I'm experience this using the Steamworks SDK. These classes all have bad alignment: ISteamParties, ISteamUGC, ISteamTimeline, ISteamVideo, and ISteamInventory.
My current solution is marking them as opaque_type in the Bindgen::Builder, but I'd love to see this properly fixed.
Activity
emilio commentedon Jan 6, 2017
Gah, virtual inheritance... We don't have any code for handling it and we're missing the base class pointer.
Thanks for reporting, will investigate how to extract this info from clang.
emilio commentedon Jan 6, 2017
After digging a bit more I don't think this is due to virtual inheritance anymore (it sort of is, because there was a bug in our handling of virtual inheritance, but it's not the root cause of this one I believe).
emilio commentedon Jan 6, 2017
More minimized:
I think this is because how alignment is done by clang and rustc, mainly,
ostrstreamis 16-byte aligned on 64 bits, but clang still lays out theint crt_field contiguously (because it's a base class), while rustc lays it out in the next 16-byte slot because we represent it as a field.emilio commentedon Jan 6, 2017
I'll grab a debugger and inspect the fields, but I suspect that
offsetof(LogStream, crt_)is going to be 8-byte aligned, but not 16-byte aligned (that is, contiguous to_M_buf).emilio commentedon Jan 6, 2017
Yup, not related to virtual inheritance at all, more minimized:
I'm not sure what's the best way to solve this unfortunately... Ideally having some kind of annotation on rustc, but the short-term solutions I'm thinking about aren't pretty (we wouldn't have easy access to base classes as we have now).
emilio commentedon Jan 6, 2017
I guess the easiest way to solve this is inlining all the base class fields, then add helper methods to transmute as base classes, instead of the way we represent inheritance now.
It's a hard-ish patch though, and I'm not sure I'll have the time to do it before exams are over :)
emilio commentedon Jan 6, 2017
cc @fitzgen, can you think of any better option?
[-]Unit test failure from size mismatch[/-][+]Differently aligned fields may make inheritance not work properly.[/+]emilio commentedon Jan 6, 2017
Ugh, making that approach work with template parameters will need implementing template parameter resolution logic I really don't want to write :(
emilio commentedon Jan 6, 2017
I guess it's the only sensible option though, more ideas appreciated.
emilio commentedon Jan 6, 2017
Actually, this is going to need some rust compiler magic I believe, otherwise examples like this (that currently work) would stop working if we just inline the fields:
emilio commentedon Jan 6, 2017
It seems that the logic is a bit more complicated, and clang only packs bits on the last base class. That is, this test case still fails:
emilio commentedon Jan 6, 2017
TL;DR: I think the sensible way to do this is inlining the last, and only the last base class members inside a given struct.
That's going to be slightly annoying to implement, but might be doable.
Auto merge of #383 - emilio:virtual-base, r=fitzgen
12 remaining items
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
addiment commentedon Feb 12, 2025
Hope to bump this, I'm on Debian 12 and I'm experience this using the Steamworks SDK. These classes all have bad alignment:
ISteamParties,ISteamUGC,ISteamTimeline,ISteamVideo, andISteamInventory.My current solution is marking them as
opaque_typein the Bindgen::Builder, but I'd love to see this properly fixed.Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Bug 1800882 - Keep includes in AnimationFrameProvider.h down. r=aosmond
Represent base classes by the bytes the target gave them
Give a class the layout the C++ compiler gives it (#93)