Future of libio vtable compatibility

Future of libio vtable compatibility

[Florian Weimer]

The libio implementation of streams (behind FILE *) was originally
written so that it was ABI-compatible with the C++ implementation of
filebuf and streambuf in GCC, so that the implementation could be
shared, and C functions printf etc. could write directly C++ streams.

This did not work out as planned because the C++ ABI changed a couple
of times since then (until settling on the Itanium ABI).

If you remember, we put in some complicated code to detect when
binaries brought along their own vtables, and disable the hardening
(so that exploit writers could no longer trivially inject code).  This
worked in some cases, such as historic APT versions (which I verified
at the time).

Now some other unsupported cases have surfaced.  It's quite difficult
to get hold of binaries which fail, but here is one documented case
with binaries still available (through snapshot.debian.org):

  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=861116

I was able to get run yadex to run to the point of the crash on Debian
unstable.  The crash happens here:

(gdb) where
#0  0xf7fd4c89 in __kernel_vsyscall ()
#1  0xf7b885b2 in __libc_signal_restore_set (set=0xffffcd1c) at ../sysdeps/unix/sysv/linux/nptl-signals.h:80
#2  __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:48
#3  0xf7b899d1 in __GI_abort () at abort.c:79
#4  0xf7bca273 in __libc_message (action=(do_abort | do_backtrace), fmt=<optimized out>) at ../sysdeps/posix/libc_fatal.c:181
#5  0xf7bca30c in __GI___libc_fatal (message=0xf7cdac38 "Fatal error: glibc detected an invalid stdio handle\n") at ../sysdeps/posix/libc_fatal.c:191
#6  0xf7bcacda in _IO_vtable_check () at vtables.c:72
#7  0xf7ba3c7b in IO_validate_vtable (vtable=0xf7e6c720 <filebuf virtual table>) at ../libio/libioP.h:876
#8  _IO_vfprintf_internal (s=0xf7e6c480 <_IO_2_1_stdout_>, format=0x808c040 "%s\nThis program is derived from DEU 5.21 by Rapha\353l Quinet and Brendon Wyber.\n", ap=0xffffd584 "\200\254\t\b\001") at vfprintf.c:1328
#9  0xf7babc66 in __printf (format=0x808c040 "%s\nThis program is derived from DEU 5.21 by Rapha\353l Quinet and Brendon Wyber.\n") at printf.c:33
#10 0x0807b1a9 in ?? ()
#11 0xf7b73e81 in __libc_start_main (main=0x807aff0, argc=1, argv=0xffffd644, init=0x8049dec <_init>, fini=0x8081844 <_fini>, rtld_fini=0xf7fe59a0 <_dl_fini>, stack_end=0xffffd63c) at ../csu/libc-start.c:310
#12 0x0804a671 in ?? ()

glibc's printf uses this stdout value to call _IO_vfprintf_internal:

(gdb) info symb 0xf7e6c480
_IO_2_1_stdout_ in section .data of /usr/lib/libstdc++-libc6.2-2.so.3

What happened here is that this version of libstdc++ has interposed
_IO_2_1_stdout_, and it supplied its own vtable:

(gdb) info symb _IO_2_1_stdout_.vtable 
filebuf virtual table in section .data of /usr/lib/libstdc++-libc6.2-2.so.3

This is also visibile in LD_DEBUG=bindings logging:

     21425:  binding file /lib/i386-linux-gnu/libc.so.6 [0] to /usr/lib/libstdc++-libc6.2-2.so.3 [0]: normal symbol `_IO_2_1_stderr_' [GLIBC_2.1]
     21425:  binding file /lib/i386-linux-gnu/libc.so.6 [0] to /usr/lib/libstdc++-libc6.2-2.so.3 [0]: normal symbol `_IO_2_1_stdout_' [GLIBC_2.1]
     21425:  binding file /lib/i386-linux-gnu/libc.so.6 [0] to /usr/lib/libstdc++-libc6.2-2.so.3 [0]: normal symbol `_IO_2_1_stdin_' [GLIBC_2.1]

Needless to say, I did not expect that.

In order to fix that, we could accept any vtable if it is loaded from
the symbols _IO_2_1_stderr_, _IO_2_1_stdout_, _IO_2_1_stdin_.  If we
do that after a vtable verification failure, there should not be any
performance impact for current binaries, on top of the overhead for
vtable verification.

But I do wonder if it is worth the effort to make this change.  It has
been rather difficult to get hold of old binaries which are currently
broken, and I don't know if the sketched workaround addresses the
issues users have encountered.

Should we instead remove the compatibility logic altogether?

Re: Future of libio vtable compatibility

[Carlos O'Donell]

On 06/15/2018 03:50 AM, Florian Weimer wrote:
> Should we instead remove the compatibility logic altogether?

The libio vtable internal ABI compatibility has been a real source 
of pain over the years, and it has prevented the general cleanup 
of the libio code.

We learned that first hand in the malloc code during the removal of
the dump code for Emacs which also had a similar internal ABI
dependency.

However, in the case of the dumper, only emacs was known to be
affected. In the case of the vtable compat it is potentially any
number of old applications.

I'm inclined to argue that glibc 2.x should remain compatible with
the old internal ABI of the vtables, but that perhaps a future glibc
with a new major version might drop it entirely.

Should we start seriously talking about glibc 3.x?

Cheers,
Carlos.

Re: Future of libio vtable compatibility

[Florian Weimer]

* Carlos O'Donell:

> On 06/15/2018 03:50 AM, Florian Weimer wrote:
>> Should we instead remove the compatibility logic altogether?
>
> The libio vtable internal ABI compatibility has been a real source 
> of pain over the years, and it has prevented the general cleanup 
> of the libio code.

> I'm inclined to argue that glibc 2.x should remain compatible with
> the old internal ABI of the vtables, but that perhaps a future glibc
> with a new major version might drop it entirely.

But how do we test this?  We learned about the incompatibility only
because there's an unambiguous error message about it.  Remember that
none of the affected people tried to report this upstream.  The two
internal cases we had end up with the users upgrading their obsolete
binaries anyway (we didn't get the binaries that reproduced the
issue).

It almost looks to me as if nobody really wants that level of
backwards compatibility.

We could require that vtable compatibility requires setting an
environment variable in glibc 2.28.  This might finally allow us to
gather some data.  Either nobody needs backwards compatibility, or our
backwards compatibility is just too perfect.  It's difficult to tell
why we don't see more bug reports in this area.

Re: Future of libio vtable compatibility

[Zack Weinberg]

On Mon, Jun 18, 2018 at 1:13 PM, Florian Weimer <fw@deneb.enyo.de> wrote:
>
> We could require that vtable compatibility requires setting an
> environment variable in glibc 2.28.  This might finally allow us to
> gather some data.  Either nobody needs backwards compatibility, or our
> backwards compatibility is just too perfect.  It's difficult to tell
> why we don't see more bug reports in this area.

This is not quite the same thing as vtable compatibility, but based on
having had to read a bunch of the relevant code for the bits/types/
work, I suspect that programs that require the "old" FILE struct have
been broken for some time.

zw

Re: Future of libio vtable compatibility

[Adhemerval Zanella]

On 18/06/2018 14:41, Zack Weinberg wrote:
> On Mon, Jun 18, 2018 at 1:13 PM, Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> We could require that vtable compatibility requires setting an
>> environment variable in glibc 2.28.  This might finally allow us to
>> gather some data.  Either nobody needs backwards compatibility, or our
>> backwards compatibility is just too perfect.  It's difficult to tell
>> why we don't see more bug reports in this area.
> 
> This is not quite the same thing as vtable compatibility, but based on
> having had to read a bunch of the relevant code for the bits/types/
> work, I suspect that programs that require the "old" FILE struct have
> been broken for some time.
> 
> zw
> 

I do think the libio compatibility is really more a burden than a 
solution. If I recall correctly, we already have it broken in some
occasions (for instance [1]) to fix actual reported bugs. And I 
think it is quite unproductive to keep compatibility to quite broken
API which does not seem to used anywhere and which requires an
extra burden in both testing and development for the sake of
compatibility (which we can't effectively test by the way).

[1] https://sourceware.org/ml/libc-alpha/2017-04/msg00356.html 

Re: Future of libio vtable compatibility

[Florian Weimer]

* Zack Weinberg:

> On Mon, Jun 18, 2018 at 1:13 PM, Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> We could require that vtable compatibility requires setting an
>> environment variable in glibc 2.28.  This might finally allow us to
>> gather some data.  Either nobody needs backwards compatibility, or our
>> backwards compatibility is just too perfect.  It's difficult to tell
>> why we don't see more bug reports in this area.
>
> This is not quite the same thing as vtable compatibility, but based on
> having had to read a bunch of the relevant code for the bits/types/
> work, I suspect that programs that require the "old" FILE struct have
> been broken for some time.

Do you mean source-level brokenness, or something that affects
compiled binaries?

Re: Future of libio vtable compatibility

[Zack Weinberg]

On Mon, Jun 18, 2018 at 3:08 PM, Florian Weimer <fw@deneb.enyo.de> wrote:
> * Zack Weinberg:
>>
>> This is not quite the same thing as vtable compatibility, but based on
>> having had to read a bunch of the relevant code for the bits/types/
>> work, I suspect that programs that require the "old" FILE struct have
>> been broken for some time.
>
> Do you mean source-level brokenness, or something that affects
> compiled binaries?

Something that affects compiled binaries.  Basically, there were so
many casts back and forth among the _IO_FILE_* variations that I could
not persuade myself that the code was strict-aliasing safe nor that it
actually guaranteed "old" FILEs would never get passed to functions
that expected "new" FILEs.  And because we have no tests for any of
those code paths, we only know that that stuff still compiles, not
that it still works.

zw

Re: Future of libio vtable compatibility

[Carlos O'Donell]

On 06/18/2018 01:13 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> On 06/15/2018 03:50 AM, Florian Weimer wrote:
>>> Should we instead remove the compatibility logic altogether?
>>
>> The libio vtable internal ABI compatibility has been a real source 
>> of pain over the years, and it has prevented the general cleanup 
>> of the libio code.
> 
>> I'm inclined to argue that glibc 2.x should remain compatible with
>> the old internal ABI of the vtables, but that perhaps a future glibc
>> with a new major version might drop it entirely.
> 
> But how do we test this?  We learned about the incompatibility only
> because there's an unambiguous error message about it.  Remember that
> none of the affected people tried to report this upstream.  The two
> internal cases we had end up with the users upgrading their obsolete
> binaries anyway (we didn't get the binaries that reproduced the
> issue).

Let me pose another question...
 
> It almost looks to me as if nobody really wants that level of
> backwards compatibility.

Assume you had reliable test system with 100 tests for the backwards
compatibility support.

Given the tests, would you still make the same argument for removal?

Is the argument about poor testing semi-independent of the argument for
removal?

Should we focus our discussion on the core issue?

Is this a part of a broader discussion to rid glibc of external projects
which depend on implementation internals?

Is "libio vtable compat removal" the best value?

As opposed to fixing the Rust dependency on TLS size API.

Or the libasan issues too.

I don't have a good answer to this, but it's the kind of internal
reflection I'm making about the various parts of our APIs that are
being used by other projects.

> We could require that vtable compatibility requires setting an
> environment variable in glibc 2.28.  This might finally allow us to
> gather some data.  Either nobody needs backwards compatibility, or our
> backwards compatibility is just too perfect.  It's difficult to tell
> why we don't see more bug reports in this area.

We all suspect that nobody needs it. That everyone upgraded through
the libstdc++ timeline in which it existed, or doesn't run those
binaries on new systems because of other unrelated dependencies.

The reason behind my suggestion for making this change in glibc 3.0
is completely one of optics. To ensure that our users see us making
big compatibility changes only on major project boundary changes.

To be honest I don't care if we call the *next* release glibc 3.0,
and remove the vtable libio compat code *today* :-)

I also do not think we need to line up dozens of ABI breaking changes
to make glibc 3.0 valuable. For example in glibc 3.0 we can drop all
of the libio vtable compat, and get DJ's patch in to remove the
malloc hooks (which now use a distinct prelodable library). Call it
done. Then in glibc 4.0 we might unify all the libraries into one
if that's what we decide.

Cheers,
Carlos.

Re: Future of libio vtable compatibility

[Florian Weimer]

* Carlos O'Donell:

> Let me pose another question...
>  
>> It almost looks to me as if nobody really wants that level of
>> backwards compatibility.
>
> Assume you had reliable test system with 100 tests for the backwards
> compatibility support.
>
> Given the tests, would you still make the same argument for removal?

Probably not.  Depends how good the tests are.  If lack of testing of
those internal interfaces does not prevent library cleanups and other
changes (such as the implement of fgetln, biased locking for stdio
streams, or *printf speed-ups),

> Is the argument about poor testing semi-independent of the argument for
> removal?

I think the lack of a testsuite is a huge upfront cost if we ever
tackle libio modernization.  And if we treat vtables as an internal
implementation detail, it's significantly easier to achieve some
decent level of coverage.

If give up the notion of vtable compatibility (or internal, data
structure layout compatibility), it will be somewhat easier to
convince that certain fixes are acceptable.

For example, with virtual methods, the call graph between virtual
methods is part of the API, and also the relative order of internal
calls.  Or look at the marker support (see struct _IO_marker).  I'm
not sure which of the streams are compatible with that.

> Is this a part of a broader discussion to rid glibc of external projects
> which depend on implementation internals?

I don't think so.  I'm not sure such a project even exists.  The
issues are far too disparate for that.

> Is "libio vtable compat removal" the best value?

I'm not even sure to what extent the vtables constrain us in terms of
the changes we want to make.  Sure, code cleanups are basically
impossible right now, but what are the changes we want to make and
people want to work on, and which are at risk due to the vtable
complexity?

>> We could require that vtable compatibility requires setting an
>> environment variable in glibc 2.28.  This might finally allow us to
>> gather some data.  Either nobody needs backwards compatibility, or our
>> backwards compatibility is just too perfect.  It's difficult to tell
>> why we don't see more bug reports in this area.
>
> We all suspect that nobody needs it.

There is this one glibc bug I mentioned. 8-)

> The reason behind my suggestion for making this change in glibc 3.0
> is completely one of optics. To ensure that our users see us making
> big compatibility changes only on major project boundary changes.
>
> To be honest I don't care if we call the *next* release glibc 3.0,
> and remove the vtable libio compat code *today* :-)

I'm worried that it would lead to the hospitalization of a few people
if we did that *now*. 8-/  The first major version change will
certainly be hardest, based on the Linux experience.

I considered the “do not flush on abort or assertion failure” a glibc
3.0 project too.

Hmm, it looks like never posted the glibc 3.0 notes from last year's
Cauldron session to the wiki.  I should really upload the notes you
sent to me.

Re: Future of libio vtable compatibility

[Carlos O'Donell]

On 06/18/2018 04:18 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> Let me pose another question...
>>  
>>> It almost looks to me as if nobody really wants that level of
>>> backwards compatibility.
>>
>> Assume you had reliable test system with 100 tests for the backwards
>> compatibility support.
>>
>> Given the tests, would you still make the same argument for removal?
> 
> Probably not.  Depends how good the tests are.  If lack of testing of
> those internal interfaces does not prevent library cleanups and other
> changes (such as the implement of fgetln, biased locking for stdio
> streams, or *printf speed-ups),
> 
>> Is the argument about poor testing semi-independent of the argument for
>> removal?
> 
> I think the lack of a testsuite is a huge upfront cost if we ever
> tackle libio modernization.  And if we treat vtables as an internal
> implementation detail, it's significantly easier to achieve some
> decent level of coverage.
> 
> If give up the notion of vtable compatibility (or internal, data
> structure layout compatibility), it will be somewhat easier to
> convince that certain fixes are acceptable.
> 
> For example, with virtual methods, the call graph between virtual
> methods is part of the API, and also the relative order of internal
> calls.  Or look at the marker support (see struct _IO_marker).  I'm
> not sure which of the streams are compatible with that.

So either...

(a) Improve libio vtable testing, covering the bug in question,
    and fixing the bug for compat.

and/or

(b) Remove libio vtable compat.

But given the dubious value of the libio vtable compat, I think we
should be deprecating it in a future release. I agree with you that
no complaints upstream is the strongest indicator that nobody is
using old binaries with really new glibc. We usually see breakage
very quickly when we touch features people care about.

It still feels like to me that this is glibc 3.0 material where we
kill the compat, and then enable everyone to start restructuring
the code immediately after 3.0 releases with the expectation set
that we aren't supporting ancient libstdc++ binaries. This can be
done without aprioiri trying to figure out which changes are possible
and which are not (your question about complexity).

My comment about making the next release 3.0 was not meant to give
anyone a heart attack, but just to indicate my willingness to move
this issue forward. Should 2.29 be 3.0?

Cheers,
Carlos.

Re: Future of libio vtable compatibility

[Florian Weimer]

* Carlos O'Donell:

>>> But given the dubious value of the libio vtable compat, I think we
>>> should be deprecating it in a future release.
>> 
>> We alread did that in glibc 2.27.
>
> We removed the public symbols, but we haven't really touched the vtable
> layout?

We deprecated.  We didn't make any real changes.

Re: Future of libio vtable compatibility

[Carlos O'Donell]

On 06/19/2018 03:11 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>>>> But given the dubious value of the libio vtable compat, I think we
>>>> should be deprecating it in a future release.
>>>
>>> We alread did that in glibc 2.27.
>>
>> We removed the public symbols, but we haven't really touched the vtable
>> layout?
> 
> We deprecated.  We didn't make any real changes.
> 

Sorry, correct. Everything is deprecated in 2.27, but no changes are made
yet for any of the interfaces.

We should restart this conversation in the next development cycle, and
talk again about glibc 3.0 and everyone's epectations about what we might
do.

Cheers,
Carlos.

Re: Future of libio vtable compatibility

[Joseph Myers]

On Mon, 18 Jun 2018, Carlos O'Donell wrote:

> I also do not think we need to line up dozens of ABI breaking changes
> to make glibc 3.0 valuable. For example in glibc 3.0 we can drop all
> of the libio vtable compat, and get DJ's patch in to remove the
> malloc hooks (which now use a distinct prelodable library). Call it
> done. Then in glibc 4.0 we might unify all the libraries into one
> if that's what we decide.

I don't think this subjective notion of what is or is not a major change 
is a suitable basis for changing the version number.  (After all, unifying 
all libraries into one would presumably be done in a fully compatible way 
that keeps usages such as dlopen of libm.so.6, and dlsym having opened 
such a library, and linking with -lm, -lpthread etc., and existing 
binaries, working.  Anything it breaks would probably be similar to code 
doing dlopen of libm.so as opposed to LIBM_SO from gnu/lib-names.h, which 
we already broke by making libm.so into a linker script.)

I think changing the major version number should correspond to a SONAME 
change on at least one platform (and even for the case of changing SONAME 
for some but not all platforms, it's unclear if the major version should 
change).

This is not an objection to any particular proposal for some change that 
might have compatibility concerns (though I've already said we should 
provide the alternative preloadable library to keep mtrace working before 
doing anything that would stop it working with the default libc), just to 
arbitrarily saying some such change, that doesn't change SONAME for libc, 
should make it glibc 3.0.

I also think it's clear that future changes that obsolete a symbol version 
and provide a new version of the same symbol with some incompatible 
changes need to add tests for the obsoleted symbol to make sure it 
continues to behave as expected (both general behavior and the specific 
differences between the versions), given the difficulty we're having with 
telling exactly what does or doesn't work for both vtable compatibility 
and C programs linked with glibc 2.0 using the old FILE objects.  (That's 
not needed in the trivial case where the semantics of the new symbol are a 
refinement of the semantics of the old one, so the new version is only 
provided to stop new binaries running incorrectly with old libraries and 
the two versions are aliases - for example, when the return types of 
fenv.h functions changed from void to int in C99 TC1.)  Adhemerval, this 
means I think your fcntl64 patch adding a new version of fcntl on 32-bit 
platforms should also have tests for the old compat version of fcntl.

Ideally we'd add such tests for past compat symbol versions, but in 
practice we have so many non-compat public symbols lacking test coverage - 
see scripts at 
<https://sourceware.org/ml/libc-alpha/2013-07/msg00386.html> to find such 
symbols - and adding tests of such symbols accessible to new programs is 
generally more valuable, and probably less effort, than adding tests of 
compat symbols.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Future of libio vtable compatibility

[Adhemerval Zanella]

On 19/06/2018 09:18, Joseph Myers wrote:
> On Mon, 18 Jun 2018, Carlos O'Donell wrote:
> 
>  Adhemerval, this 
> means I think your fcntl64 patch adding a new version of fcntl on 32-bit 
> platforms should also have tests for the old compat version of fcntl.

Fair enough, I will update the patch.