On Fri, Apr 25, 2014 at 2:05 PM, Jonathan Wakely <jwakely....@gmail.com> wrote:
>On 25 April 2014 13:01, Richard Biener wrote:
>> On Fri, Apr 25, 2014 at 1:18 PM, Jonathan Wakely <jwakely....@gmail.com>
>> wrote:
>>> On 25 April 2014 11:22, Andrew Haley wrote:
>>>> Summary: Devirtualization uses type information to determine if a
>>>> virtual method is reachable from a call site. If type information
>>>> indicates that it is not, devirt marks the site as unreachable. I
>>>> think this is wrong, and it breaks some programs. At least, it should
>>>> not do this if the user specifies -fno-strict-aliasing.
>>>>
>>>> Consider this class:
>>>>
>>>> class Container {
>>>> void *buffer[5];
>>>> public:
>>>> EmbeddedObject *obj() { return (EmbeddedObject*)buffer; }
>>>> Container() { new (buffer) EmbeddedObject(); }
>>>> };
>>>>
>>>> Placement new is used to embed an object in a buffer inside another
>>>> object. Its address can be retrieved. This usage of placement new is
>>>> common, and it even appears as the canonical use of placement new in
>>>> the in the C++ FAQ at
>>>> http://www.parashift.com/c++-faq/placement-new.html. (I am aware that
>>>> this is not strictly legal. For one thing, the memory at buffer may
>>>> not be suitably aligned. Please bear with me.)
>>>
>>> I think the program is strictly legal if you define Container like this:
>>>
>>> class Container {
>>> alignas(EmbeddedObject) char buffer[sizeof(EmbeddedObject)];
>>> public:
>>> EmbeddedObject *obj() { return (EmbeddedObject*)buffer; }
>>> Container() { new (buffer) EmbeddedObject(); }
>>> };
>>>
>>> But GCC still does the same transformation and prints nothing, which I
>>> agree is wrong (even with -fstrict-aliasing).
>>
>> I agree, -fstrict-aliasing has nothing to do with this. Sounds simply like
>> a genuine bug (please open a bugzilla).
>>
>> Thanks,
>> Richard.
>>
>>> With ubsan that program gives:
>>>
>>> sa.cc:9:24: runtime error: load of null pointer of type '<unknown> *'
>>> Segmentation fault (core dumped)
>
>I created http://gcc.gnu.org/60963 for the ubsan error. The testcase
>there is a single file which crashes with ubsan and returns the wrong
>value without. Changing the return statement to
>__builtin_printf("%d\n", o.obj()->val()) prints nothing, like Andrew's
>original example.
>
>If it's not really a ubsan problem feel free to re-categorise that as
>a devirt bug rather than open a new one, whatever's most appropriate.
>
Hi Jonathan,
I don't think this is a ubsan bug. I rather think it is an devirt bug.
If you look at the generated code (taking Andrews initial example) you
can see the following:
0000000000400720 <main>:
main():
/tmp/embed-test/embed.cpp:7
400720: 48 83 ec 08 sub $0x8,%rsp
/tmp/embed-test/embed.cpp:9
400724: 48 83 3d b4 07 20 00 cmpq $0x0,0x2007b4(%rip)
# 600ee0 <o>
40072b: 00
40072c: 74 00 je 40072e <main+0xe>
40072e: 31 f6 xor %esi,%esi
400730: bf 80 0d 60 00 mov $0x600d80,%edi
400735: e8 d6 ff ff ff callq 400710
<__ubsan_handle_type_mismatch@plt>
40073a: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
So because of the devirt bug, the main method unconditionally falls
into the ubsan handler and misleadingly prints the warning. Afterwards
it crashes, but again not because of an ubsan error, but because it
wrongly entered the ubsan handler et all because of the
devirtualization bug.
Notice that this bug was first detected while compiling the OpenJDK
with gcc 4.9.0 (see mail threads at
http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-April/thread.html#13577
and http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-April/013614.html).
We as well compiled OpenJDK with -fsanitze=undefined and got some
warnings. But after a closer look it turned out these were false
positives as well, for exactly the same reason. As you can read in
this mail
(http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-April/013602.html)
the OpenJDK code executed several ubsan handlers not because the
corresponding NULL-checks failed, but because the code for the
containing method had nor return at the end and just fell over into
the ubsan handlers.
Could you therefore please re-categorize this as devirt bug.
Thank you and best regards,
Volker
