Skip to content

Differently aligned fields may make inheritance not work properly. #380

Open

Description

@jsgf

This input:

namespace
{
  template < typename _CharT, typename = _CharT > class basic_ostream;
  template < typename, typename > class basic_ios
  {
  };
template < typename _CharT, typename _Traits > class basic_ostream:virtual basic_ios < _CharT,
    _Traits
    >
  {
  };
  class strstreambuf
  {
  };
  class ostrstream:basic_ostream < char >
  {
    strstreambuf _M_buf;
  };
  class LogMessage
  {
    class LogStream:ostrstream
    {
      int ctr_;
    };
  };
}

Command:

bindgen --output logging.rs --no-unstable-rust --whitelist-type '.*::LogMessage' --whitelist-type '.*::LogSeverity' --whitelist-type '.*::int64' --blacklist-type std::allocator --blacklist-type std::allocator_traits --blacklist-type std::__alloc_traits --opaque-type std::string --opaque-type std::basic_fbstring --opaque-type std::vector --opaque-type std::basic_ios --opaque-type std::basic_ostream --opaque-type std::basic_streambuf --generate functions,methods,types test.i -- -std=gnu++14 -x c++

Generates rust code which fails a unit test when built with --test:

thread 'bindgen_test_layout__bindgen_mod_id_1_LogMessage_LogStream' panicked at 'assertion failed: `(left == right)` (left: `24`, right: `16`)', test.rs:66

This is the generated output is here: https://gist.github.com/jsgf/ddcc0db8b239b781d0590199cbe2ec80

Activity

  1. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    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.

  2. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    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).

  3. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    More minimized:

    class basic_ios { };
    
    class basic_ostream: virtual basic_ios { };
    
    class ostrstream:basic_ostream {
      int _M_buf;
    };
    
    class LogStream: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.

  4. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    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).

  5. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    Yup, not related to virtual inheritance at all, more minimized:

    class ostrstream {
      void* ptr;
      int _M_buf;
    };
    
    class LogStream: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).

  6. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    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 :)

  7. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    cc @fitzgen, can you think of any better option?

  8. changed the title [-]Unit test failure from size mismatch[/-] [+]Differently aligned fields may make inheritance not work properly.[/+] on Jan 6, 2017
  9. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    Ugh, making that approach work with template parameters will need implementing template parameter resolution logic I really don't want to write :(

  10. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    I guess it's the only sensible option though, more ideas appreciated.

  11. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    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:

    class WordAligned {
      void* ptr;
      int foo;
    };
    
    class WordAlignedButWithDifferentlyAlignedFirstField {
      int foo;
      void* ptr;
    };
    
    class Test : public WordAligned,
                 public WordAlignedButWithDifferentlyAlignedFirstField
    {};
  12. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    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:

    class WordAligned {
      void* ptr;
      int foo;
    };
    
    class WordAlignedButWithDifferentlyAlignedFirstField {
      int foo;
      void* ptr;
      int bar;
    };
    
    class Test : public WordAligned,
                 public WordAlignedButWithDifferentlyAlignedFirstField
    {
      int baz;
    };
  13. emilio commented on Jan 6, 2017

    @emilio
    Contributor

    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.

  14. added a commit that references this issue on Jan 11, 2017
  15. 12 remaining items

  16. addiment commented on Feb 12, 2025

    @addiment

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions