Bug 18818 - abidw aborts on a class with a non-complete base class
Summary: abidw aborts on a class with a non-complete base class
Status: RESOLVED FIXED
Alias: None
Product: libabigail
Classification: Unclassified
Component: default (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Dodji Seketeli
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-12 16:14 UTC by andrew.c.morrow
Modified: 2015-08-14 15:25 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
reproducing shared library (172.53 KB, application/x-sharedlib)
2015-08-12 16:14 UTC, andrew.c.morrow
Details
Candidate work-around patch (2.97 KB, patch)
2015-08-12 22:27 UTC, Dodji Seketeli
Details | Diff
GCC 4.9.2 built reproducer (216.85 KB, application/x-sharedlib)
2015-08-13 19:24 UTC, andrew.c.morrow
Details

Note You need to log in before you can comment on or make changes to this bug.
Description andrew.c.morrow 2015-08-12 16:14:15 UTC
Created attachment 8511 [details]
reproducing shared library

At git hash 242e49a321845ca30785594f3b00b15bf3f6b555, libabigail's 'abidw' utility currently crashes with the following assertion failure against the attached shared library:

abidw: abg-hash.cc:627: size_t abigail::ir::class_decl::hash::operator()(const abigail::ir::class_decl&) const: Assertion `cl && (!cl->get_is_declaration_only() || cl->get_definition_of_declaration())' failed.

Please see this email to the libabigail list for additional details: https://sourceware.org/ml/libabigail/2015-q3/msg00000.html
Comment 1 Dodji Seketeli 2015-08-12 20:27:14 UTC
Thanks for the bug report.

This appears to be a binary generated by Clang.  From what I am seeing in the debug info the violated assertion is about the type "class boost::system::system_error" that inherits a type "std::runtime_error" that is not defined.  By not defined, I mean the "std::runtime_error" is just declared in the debug info, and not defined.

So, before I start thinking about the possible work-arounds for this erratic debug info that seems to be emitted by Clang, could you please file the binary generated by GCC that is triggering the same assertion?  I am curious.

Thanks!
Comment 2 Dodji Seketeli 2015-08-12 22:27:33 UTC
Created attachment 8514 [details]
Candidate work-around patch

So, attached is a a patch that works around the (IMHO) ill-formed debug info of the binary attached to the bug.

Below is some context about what is happening and what the patch does.

On some binaries with debug info emitted by "Ubuntu clang version
3.6.0-2ubuntu1" (as the value of the DW_AT_producer property), it
seems some classes can have a base class that is not complete.  E.g,
the debug info (that I have extracted using the command
eu-readelf --debug-dump=info <the-binary-attached-to-the-bug>) has
these relevant pieces:

    [...]

     [  5ff7]        class_type
		     containing_type      (ref4) [  7485]
		     name                 (strp) "system_error"
		     byte_size            (data1) 40
		     decl_file            (data1) 46
		     decl_line            (data1) 22
     [  6003]          inheritance
		       type                 (ref4) [  7480]
    [...]

Here, we are looking at the type system_error (actually
boost::system::system_error) that inherits the type which DIE is
referred to as offset '7480'.

Then the definition of the DIE at offset 7480 is:

    [...]

     [  7480]      class_type
		   name                 (strp) "runtime_error"
		   declaration          (flag_present)
     [  7485]      class_type
		   name                 (strp) "exception"
		   declaration          (flag_present)
    [...]

You can see that the type "runtime_error" (actually
std::runtime_error) has the flag DW_AT_declaration set, marking it as
a declaration (with no definition yet).  And no other DIE in the same
translation unit
(src/third_party/boost-1.56.0/libs/filesystem/src/codecvt_error_category.cpp)
or in the same DSO provides the definition for that declaration.

I believe this is ill-formed.  A base class should be defined and have
a layout completed expressed and accessible from the translation unit
it's used in.

The patch I am proposing detects that the base class is still
incomplete when we finish loading the current binary.  In that case,
the base class is made complete with a size of 1.  Meaning it's an
empty class (with no data member and no base class).  This works as a
viable work-around *if* the producer only omitted definitions for
empty classes.  We'd need to contact the upstream of the DWARF
producer with a reproducer for this issue to be sure.

I haven't applied the patch to the development tree yet, as I'd like to understand more the issue you are having with the GCC-generated binary.

It'd be very useful for me to have the binary generated by GCC which makes abidw abort as well. Either GCC has the same behaviour as Clang and that would need fixing too, or the issue is different and I'd need to address it in a different way.
Comment 3 andrew.c.morrow 2015-08-13 19:24:04 UTC
Created attachment 8518 [details]
GCC 4.9.2 built reproducer

Attached is a version of the library built with GCC 4.9.2, which gives the same assertion failure.
Comment 4 Dodji Seketeli 2015-08-13 20:47:03 UTC
> --- Comment #3 from andrew.c.morrow at gmail dot com ---
>   --> https://sourceware.org/bugzilla/attachment.cgi?id=8518&action=edit
> GCC 4.9.2 built reproducer

Thanks!

And indeed, the binary built with GCC 4.9.2 does have the same pattern
which, IMHO, represents an ill-formed DWARF construct.

I am thus going to apply the work-around patch into master, for
libabigail.

But then, now, I'd like to learn more about these binaries, as I'd like
to include them in the test suite of libabigail, as non-regression
tests for the workaround.

Do you think it'd be possible to just tell me in a comment on this bug
how to build that binary and if all the sources are Free Software?  If
not, I'll commit the patch without the binary for now and we'll work out
a way to try and forge (or build another software) that would exhibit
the same behaviour for libabigail's test-suite.

Thank you for following up on this issue, and sorry for the
inconvenience.
Comment 5 Dodji Seketeli 2015-08-13 20:50:45 UTC
Just to be clear, my intent is not to try and rebuild the binary myself.  It's just to document the binary (when it's added to the libabigail test-suite) so that we know where it comes from and ensure it's all Free Software that we can execute, modify, re-compile and re-distribute the modified source code it originates from.

Thank you again.
Comment 6 Dodji Seketeli 2015-08-14 14:32:59 UTC
OK, so this issue should be fixed by commit https://sourceware.org/git/gitweb.cgi?p=libabigail.git;a=commit;h=160961f3cb5f717ed8260ff210fc5efd19e1204e.

For now I am adding the two binaries attached to this bug to the non-regression test suite of libabigail.  Should you think that there is an issue there, please lemme know by commenting on this bug and we'll proceed appropriately.

Thank you for submitting this problem report, and sorry for the inconvenience.
Comment 7 andrew.c.morrow 2015-08-14 15:02:22 UTC
It should be fine to add those binaries to the test suite. The particular sources used to build those libraries are Boost licensed.

The libraries are built as part of the MongoDB build process, which bundles a private copy of the Boost sources. MongoDB offers a free and open source version of its product, licensed under the AGPL v3. The binaries attached above were built from the freely available sources, and anyway those particular libraries link to none of our code.

https://github.com/mongodb/mongo
https://github.com/mongodb/mongo/blob/master/GNU-AGPL-3.0.txt

Also, I've pulled your latest changes and I can confirm that the issue is resolved for me. Thank you for the quick patch.

I'm a little unclear on whether you have issued stable releases (the RPM world seems to call its current libabigail package as 1.0, but I don't see an associated git tag). If that is correct, is there any possibility of rolling a 1.0.1 with this fix so that it might get picked up by stable releases as a bug fix package update?
Comment 8 Dodji Seketeli 2015-08-14 15:25:36 UTC
"andrew.c.morrow at gmail dot com" <sourceware-bugzilla@sourceware.org>
a écrit:

> https://sourceware.org/bugzilla/show_bug.cgi?id=18818
>
> --- Comment #7 from andrew.c.morrow at gmail dot com ---
> It should be fine to add those binaries to the test suite. The particular
> sources used to build those libraries are Boost licensed.

Great!  I suspected that from the debug info, but having you say it
clearly is just excellent.

> The libraries are built as part of the MongoDB build process, which bundles a
> private copy of the Boost sources. MongoDB offers a free and open source
> version of its product, licensed under the AGPL v3. The binaries attached above
> were built from the freely available sources, and anyway those particular
> libraries link to none of our code.
>
> https://github.com/mongodb/mongo
> https://github.com/mongodb/mongo/blob/master/GNU-AGPL-3.0.txt

There are times when I realize even more how much I love Free Software :-)

> Also, I've pulled your latest changes and I can confirm that the issue
> is resolved for me.

Oh, I am glad to hear that.

> Thank you for the quick patch.

My pleasure.  And thank you for having taken the time to report the
issue.

> I'm a little unclear on whether you have issued stable releases (the RPM world
> seems to call its current libabigail package as 1.0, but I don't see an
> associated git tag).

Ah, right.  Right now, we are "close" to releasing a 1.0 proper :-) So
that is why we (in Fedora at least [1]) are releasing pre-release
binaries (the git hash is included in the package name) of 1.0 regularly
to let users be exposed to the fixes that are flowing in.

Debian and Ubuntu are doing the same pre-release binaries dance.  We
have good relationship relations with the Debian/Ubuntu maintainer.

> is there any possibility of rolling a 1.0.1 with this fix so that it
> might get picked up by stable releases as a bug fix package update?

Of course.  I think we can get Fedora RPMs out this coming week.  I'll
talk to the Debian maintainer; this seems to be a good time as
abipkgdiff just got support for .deb files and that was contributed by
the Debian maintainer.

[1]: http://koji.fedoraproject.org/koji/packageinfo?packageID=20298