Re: Looking for some help with GCC 6 and bad code generation

[Zan Lynx <zlynx at acm dot org>]

This is kind of stream-of-conciousness but I swear there's an
interesting new bit about 8 paragraphs down. :)

On 04/20/2016 07:18 PM, Andrew Pinski wrote:
> On Thu, Apr 21, 2016 at 9:11 AM, Zan Lynx <zlynx@acm.org> wrote:
>> > I would like someone to look at this and tell me this is an already
>> > fixed bug. Or that recent GCC patches may have fixed it. :-)
>> >
>> > Or it would also be great to get some advice on building a reproducer
>> > without needing to include many megabytes of proprietary code plus Boost.
>> >
>> > I've been using Fedora 24 Alpha and of course I've somehow managed to
>> > write some C++ code even more confusing for GCC than Firefox. Heh.
>> >
>> > The problem is a crash when std::string tries to free memory from a
>> > temporary std::string that was bound to a const reference.
>> >
>> > And it only crashes when I pass -O2 or -O3 and the -fprofile-generate flags.
> 
> What is the return type of URLDatabaseReader_1::identifier() ?
> Also does -fsanitize=undefined say anything about your code?

The -fsanitize=undefined run is pretty interesting.

The debug build test runs fine, with only some expected unaligned access
messages (due to reading 8 byte values from a memory map, where they
were put on 4 byte boundaries a long time ago by 32-bit code and can't
be moved)

However, when I skip the profile guided build step (which is the part
that crashes) and go to release, I get a sanitize runtime failure at
that same Delete1 test function that crashes in profiling. It is an
unreachable code error. Which it seems to jump to in a virtual function
dispatch to the "lookup" function of URLDatabaseLookup.

So sanitize is somehow jumping through the virtual table to an
unreachable code marker.

It is a fairly complicated virtual function override with multiple
virtual inheritance and templated parent classes to assemble it. I
believe this is sometimes called a "mixin" pattern. Which is part of why
making a nice test case is being so difficult.

But it has always worked in the past. This isn't new code, it has been
in use since 2008.

****
And an interesting, possibly the same problem, is that when I skip
profile-generate and go straight to my release build WITHOUT ubsan GCC
is simply removing all the code from the time I call url_lookup to the
end of the test function. And then crashing in the exception unwinder.

It's WEIRD though. If I add a std::cout << "Test output" to the
url_lookup function then it WORKS.

It almost has to be some interaction of inlining, virtual functions and
devirtualization.

And YES if I change -O3 to -O3 -fno-devirtualize then it WORKS again.

So somehow when it devirtualizes the call to an inlined virtual
function, that function disappears. Somehow.

Maybe I can build a simplified reproducer with this hint...
****

Here's a snip of the implementation class
> class
> URLDatabaseReader_1 :
>     protected virtual URLDatabaseStorage,
>     public URLDatabaseReaderCommon,
>     public URLDatabaseLookup<CompressionTable_1, ComponentTable, IPLookupTableNull, IPLookupTableNull>,
>     public virtual URLDatabaseReader
> {

And a snip of the interface class:
> class
> DLLEXPORT
> URLDatabaseReader
> {
>     public:
>     virtual uint32_t lookup(const char *url) const = 0;
>     virtual uint32_t lookup(const char *url, url_options &url_flags) const = 0;
>     virtual uint32_t lookup(
>         const comp_list_type &domain_parts,
>         const comp_list_type &path_parts,
>         const comp_list_type &query_parts,
>         url_options &url_flags
>     ) const = 0;

And a snip of the class that does the lookups:
> template<class CompressionTable, class ComponentTable, class IPLookupTable, class IPv6LookupTable>
> class URLDatabaseLookup : 
>     protected virtual URLDatabaseStorage,
>     public virtual URLDatabaseReader
> {       
>     public:
>     uint32_t lookup(const char *url) const
>     {
>         url_options url_flags = URL_NONE;
>         return lookup(url, url_flags);
>     }
>     uint32_t lookup(
>         const char *url,
>         url_options &url_flags
>     ) const;

And then there's the whole Writer side which goes into a ReaderWriter
> class
> DLLEXPORT
> URLDatabaseReaderWriter
>     : public virtual URLDatabaseWriter,
>       public virtual URLDatabaseReader
> {

Attachment: signature.asc
Description: OpenPGP digital signature