GCC binary compatibility / bug 23034

Hi,

Regarding <https://llvm.org/bugs/show_bug.cgi?id=23034&gt; - is the intent (still?) to be binary compatible with GCC? I.e. is this a bug or is this something that we'll have to try to solve on our side?

It's pretty visible for us because any call with std::string temporaries to map<string,...>::op(string&&) across the linker causes it and we seem to like those a lot :frowning:

I checked with r204218 and it seems to fail the same way, so if it's an issue it's not a new one (on clang's side).

And finally: should this go to llvm's bugs or stay with clang's?

Cheers, Axel.

Hi,

Regarding <https://llvm.org/bugs/show_bug.cgi?id=23034&gt; - is the intent
(still?) to be binary compatible with GCC?

ABI compatible, yes.

I.e. is this a bug or is this something that we'll have to try to solve on
our side?

If you have a function defined in GCC that isn't callable from Clang (or
the other way around) that's a bug, but it doesn't indicate where the bug
is. It could be in GCC, Clang, or the ABI specification itself (the ABI
might be underspecified, vague, or contradictory). Also the bug may've been
fixed (sometimes this is due to an ABI fix - ABI spec gets updated, one
compiler takes the fix while the other lags a bit potentially)

It's pretty visible for us because any call with std::string temporaries
to map<string,...>::op(string&&) across the linker causes it and we
seem to like those a lot :frowning:

I checked with r204218 and it seems to fail the same way, so if it's an
issue it's not a new one (on clang's side).

And finally: should this go to llvm's bugs or stay with clang's?

I'm not sure - would depend on where the bug is & I don't know enough about
this stuff to say.

A reduced test case would be helpful (the smallest standalone program (no
headers, etc) that demonstrates the inconsistency).

- David

The test case in the PR is interesting. Clang thinks that the result of forward_as_tuplestd::string should be returned directly, but GCC thinks it should be returned indirectly. The return type is apparently a std::tuplestd::string&&; without looking it up in the libstdc++ sources, I assume that has a non-static data member of type std::string&& and that its copy and move constructors are explicitly defaulted. I think the standard itself may have wavered about this, but the current state is that the copy constructor should be defined as deleted, the move constructor is okay, and that both are technically trivial.

Anyway, it is not surprising that you can find Clang and GCC versions that disagree about the ABI there. You can almost surely find two Clang versions that disagree about the ABI there. The ABI rule we’ve settled on is that this should be returned directly, since all the copy and move constructors are either deleted or trivial and at least one (the move constructor) is not deleted. It is likely that you are using a version of GCC that does not implement this rule correctly yet.

John.

Hi John,

see below...

    Hi,

    Regarding <https://llvm.org/bugs/show___bug.cgi?id=23034
    <https://llvm.org/bugs/show_bug.cgi?id=23034&gt;&gt; - is the intent
    (still?) to be binary compatible with GCC?

ABI compatible, yes.

    I.e. is this a bug or is this something that we'll have to try to
    solve on our side?

If you have a function defined in GCC that isn't callable from Clang
(or the other way around) that's a bug, but it doesn't indicate where
the bug is. It could be in GCC, Clang, or the ABI specification itself
(the ABI might be underspecified, vague, or contradictory). Also the
bug may've been fixed (sometimes this is due to an ABI fix - ABI spec
gets updated, one compiler takes the fix while the other lags a bit
potentially)

    It's pretty visible for us because any call with std::string
    temporaries to map<string,...>::op(string&&__) across the linker
    causes it and we seem to like those a lot :frowning:

    I checked with r204218 and it seems to fail the same way, so if
    it's an issue it's not a new one (on clang's side).

    And finally: should this go to llvm's bugs or stay with clang's?

I'm not sure - would depend on where the bug is & I don't know enough
about this stuff to say.

A reduced test case would be helpful (the smallest standalone program
(no headers, etc) that demonstrates the inconsistency).

The test case in the PR is interesting. Clang thinks that the result of
forward_as_tuple<std::string> should be returned directly, but GCC
thinks it should be returned indirectly. The return type is apparently
a std::tuple<std::string&&>; without looking it up in the libstdc++
sources, I assume that has a non-static data member of type
std::string&& and that its copy and move constructors are explicitly
defaulted. I think the standard itself may have wavered about this, but
the current state is that the copy constructor should be defined as
deleted, the move constructor is okay, and that both are technically
trivial.

Anyway, it is not surprising that you can find Clang and GCC versions
that disagree about the ABI there. You can almost surely find two Clang
versions that disagree about the ABI there. The ABI rule we’ve settled
on is that this should be returned directly, since all the copy and move
constructors are either deleted or trivial and at least one (the move
constructor) is not deleted. It is likely that you are using a version
of GCC that does not implement this rule correctly yet.

It took me a while to get the GCCs I wanted. I can now confirm that this fails with GCC 4.8.2, GCC 4.9.1 and GCC 5-20150405 (with their respective libstdc++). I don't see which version could work...

Shall I continue to reduce the example, would that help?

I can also test your hypothesis by switching the behavior of clang - if you can point me to where this direct versus indirect return happens.

Cheers, Axel

The place to hack would be ItaniumCXXABI::classifyReturnType.

If that’s indeed the problem, the appropriate place to file the bug would be GCC. If they decide not to implement the revised ABI rule because of backward compatibility, we’ll need to hash that out somehow on the C++ ABI list.

John.

Hi John,

wrote: Hi John,

see below...

Hi,

Regarding <https://llvm.org/bugs/show___bug.cgi?id=23034
<https://llvm.org/bugs/show_bug.cgi?id=23034&gt;&gt; - is the intent
(still?) to be binary compatible with GCC?

ABI compatible, yes.

I.e. is this a bug or is this something that we'll have to try
to solve on our side?

If you have a function defined in GCC that isn't callable from
Clang (or the other way around) that's a bug, but it doesn't
indicate where the bug is. It could be in GCC, Clang, or the
ABI specification itself (the ABI might be underspecified,
vague, or contradictory). Also the bug may've been fixed
(sometimes this is due to an ABI fix - ABI spec gets updated,
one compiler takes the fix while the other lags a bit
potentially)

It's pretty visible for us because any call with std::string
temporaries to map<string,...>::op(string&&__) across the
linker causes it and we seem to like those a lot :frowning:

I checked with r204218 and it seems to fail the same way, so
if it's an issue it's not a new one (on clang's side).

And finally: should this go to llvm's bugs or stay with
clang's?

I'm not sure - would depend on where the bug is & I don't know
enough about this stuff to say.

A reduced test case would be helpful (the smallest standalone
program (no headers, etc) that demonstrates the
inconsistency).

The test case in the PR is interesting. Clang thinks that the
result of forward_as_tuple<std::string> should be returned
directly, but GCC thinks it should be returned indirectly. The
return type is apparently a std::tuple<std::string&&>; without
looking it up in the libstdc++ sources, I assume that has a
non-static data member of type std::string&& and that its copy
and move constructors are explicitly defaulted. I think the
standard itself may have wavered about this, but the current
state is that the copy constructor should be defined as deleted,
the move constructor is okay, and that both are technically
trivial.

Anyway, it is not surprising that you can find Clang and GCC
versions that disagree about the ABI there. You can almost
surely find two Clang versions that disagree about the ABI there.
The ABI rule we’ve settled on is that this should be returned
directly, since all the copy and move constructors are either
deleted or trivial and at least one (the move constructor) is not
deleted. It is likely that you are using a version of GCC that
does not implement this rule correctly yet.

It took me a while to get the GCCs I wanted. I can now confirm that
this fails with GCC 4.8.2, GCC 4.9.1 and GCC 5-20150405 (with their
respective libstdc++). I don't see which version could work...

Shall I continue to reduce the example, would that help?

I can also test your hypothesis by switching the behavior of clang
- if you can point me to where this direct versus indirect return
happens.

The place to hack would be ItaniumCXXABI::classifyReturnType.

If that’s indeed the problem, the appropriate place to file the bug
would be GCC. If they decide not to implement the revised ABI rule
because of backward compatibility, we’ll need to hash that out
somehow on the C++ ABI list.

Sorry, I am not smart enough to test this, or it's just not it.

In the call to ItaniumCXXABI::classifyReturnType() for my_forward_as_tuple of the (new) reproducer (see below) I changed data().HasTrivialSpecialMembers & SMF_Destructor to be 0 (the value of data().HasTrivialSpecialMembers was 58, I set it to 26). The binary still crashes, even though classifyReturnType() now set the FI.getReturnInfo() to Indirect and returned true.

I have uploaded a much smaller reproducer without external dependencies <https://llvm.org/bugs/show_bug.cgi?id=23034#c1&gt;\. Could you help me and have a look, please? This repro enters ItaniumCXXABI::classifyReturnType() exactly once with an RD != 0.

Cheers, Axel.