itanium-cxx-abi / cxx-abi

C++ ABI Summary
508 stars 96 forks source link

More types should be non-trivial for the purposes of calls #138

Open frederick-vs-ja opened 2 years ago

frederick-vs-ja commented 2 years ago

Consider this class:

struct Weird {
    struct NonTrivial {
        NonTrivial() = default;
        NonTrivial(const NonTrivial&) = default;
        constexpr NonTrivial(NonTrivial&&) noexcept {} // non-trivial
        NonTrivial &operator=(const NonTrivial&) = default;
        NonTrivial &operator=(NonTrivial&&) = default;
    };
    NonTrivial member;
    Weird() = default;
    Weird(const Weird&) = default;
    Weird &operator=(const Weird&) = default;
};

Weird has no move constructor, because the implicit declaration is suppressed by the explicitly defaulted copy constructor. As a result, Weird is trivial for the purposes of calls (according to the current specification), although the type of its member is not. However, clang and gcc don't pass or return Weird in register, as if it is non-trivial for the purposes of calls. (Compiler Explorer link)

Perhaps we should additionally require that in order to make a class trivial for the purposes of calls, the types of its non-static data members and its base classes must also be trivial for the purposes of calls.

This issue is similar to CWG2463.

rjmccall commented 2 years ago

In Clang, at least, the generic C++ ABI considers Weird acceptable to pass directly, but that just means that the decision for how to pass it is deferred to the platform ABI (in this case, the x86-64 ABI), which is declining to do so. It's expected that this can happen — among other things, the generic ABI doesn't limit the maximum size of what can be passed directly, but obviously there are limits (hard limits for registers, advisable limits for the stack argument area) — but it's not immediately obvious why it should happen in this case.

The x86-64 ABI says that structs are recursively classified, and then the classifications of fields in overlapping 8-byte chunks are merged. It also says that:

If a C++ object has either a non-trivial copy constructor or a non-trivial destructor, it is passed by invisible reference (the object is replaced in the parameter list by a pointer that has class INTEGER).

This rule wouldn't really be legal under the C++ standard — we also need classes to be passed by reference if they have non-trivial move constructor, and we need to cover the behavior for deleted constructors — but it's reasonable to give the platform ABI some leniency here and just drop in the generic C++ ABI rule.

Now, Weird doesn't match the generic C++ ABI rule. However, somewhat oddly, Clang appears to be applying this rule even in recursive positions, with the modification that the field is classified as MEMORY, and then that gets merged as normal. So in your example, the first (and only) chunk in NonTrivial is classified as MEMORY, and thus the first (and only) chunk in Weird is classified the same way. The x86-64 ABI then says that classes classified as MEMORY are passed as arguments in the stack argument area but returned by invisible reference, and that's what Clang is doing.

That does not seem to be what GCC is doing. You can see this more clearly if you give NonTrivial actual storage (godbolt), but GCC is clearly passing Weird as an argument by reference rather than in the stack argument area. I don't know how GCC is making that decision; it might be implementing a different generic C++ ABI rule, or it might be classifying this as some sort of INDIRECT classification that gets merged up to force passing by reference.

In any case, we have divergence between at least GCC and Clang, which ought to be fixed.

Clang's interpretation of the x86-64 ABI seems unjustifiable. The x86-64 ABI's wording only makes sense at the top level and so should not be applied recursively when classifying field types. Even if there were a good reason to honor this when classifying field types, it should lead to the aggregate being passed indirectly, like GCC does, rather than being trivially copied into the argument area.

I don't know how GCC is reaching this decision, so I can't evaluate whether it is similarly unjustifiable.

In principle, I think the fix that would be most consistent with the ABI rules as written would be for Weird to be passed and returned in registers. Of course, that would be an ABI break for this example for both GCC and Clang, and possibly other compilers.

I think we should see this as a platform ABI issue rather than a generic C++ ABI issue, though. I don't think we want to change the generic C++ ABI rule to be consistent with the possibly-accidental use on one platform.

CC'ing @jicama, @zygoloid