This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2] [MIPS] Raise highest supported EI_ABIVERSION value
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Mihailo Stojanovic <mihailo dot stojanovic at rt-rk dot com>
- Cc: <libc-alpha at sourceware dot org>, "Maciej W . Rozycki" <macro at linux-mips dot org>, Carlos O'Donell <carlos at redhat dot com>, Dragan Mladjenovic <dragan dot mladjenovic at rt-rk dot com>
- Date: Fri, 16 Aug 2019 16:21:35 +0000
- Subject: Re: [PATCH v2] [MIPS] Raise highest supported EI_ABIVERSION value
- Ironport-sdr: LJ9b0/5AGqYKr29wvQmRqBQL2nfo+LYtn0cAKCFikXAmV15fCI3jUxmtWQNSYJyJ3waoXz/6rT 6E6vcI5vtIvhnwQQ5Eq3K73H5ivGSdsvnoUiUi13I8awRzCGls3WTGgAeYloN2aZOPqesVMEHt mSeESp8B9C0LA89oPH1EklHlNDP4+lEO3rDzfJ6lqgohcMr9alJ/tQ5JVj4pON0nuVy5LOW5D+ 2a+4MePUF4FDbTOxuX0uALsKznYlbhPfK6xDr6VD+kgexgWrFHmbmQOnqUPEjYcglLqyI9h836 OF4=
- Ironport-sdr: Es0jPAIhBOwRzPBb89BGKpXcSfMo6UrOl3zQyt4aa2bjO6sHuvtwsJ2HsMiB0p0Pq8TtDJwA92 470YjUnvJfRgqXqjPrG3fQnej/94+x7aIo0M6LHo5xqxXMbrk6/9b2HYcAu+u5sVqYztqNoFmV TYS2ftCCeh30w7KMIq6CvDld52Hu4v7J3gtOduOJ4a19/E/0iBgmaAaQag5vFMEy57zz/+b2DA 34B7Tjr6z3W5pg+jL6bH0L3Xj1gtkrpf7u5t71+NIF6t3KtQCpci6k+rxIXYpxV+N/DdPEJ+44 24g=
- References: <1565960509-29381-1-git-send-email-mihailo.stojanovic@rt-rk.com>
On Fri, 16 Aug 2019, Mihailo Stojanovic wrote:
> New testcase doesn't actually check the value of the symbol, it just
> makes sure that it is executed without "ABI version invalid" error.
The test also doesn't execute the load of the symbol address from the GOT
at all (the load is after the return from the function - because it
doesn't use ".set noreorder", the assembler puts a nop in the branch delay
slot). Is that intentional? And for n32 / n64 it doesn't set up the $gp
address either (.cpload doesn't do anything for non-o32 ABIs), which in
this case is harmless because the load isn't executed.
I.e., the test does in fact verify the library is loaded without error,
and does pass for all three ABIs, but if it's intended that the address
doesn't get loaded and $gp doesn't get set up for the load for n32 / n64,
there ought to be comments saying this is deliberate.
> +++ b/sysdeps/mips/tst-undefined-weak-lib.S
> @@ -0,0 +1,33 @@
> +/* Copyright (C) 2019 Free Software Foundation, Inc.
Files should start with a one-line comment above the copyright notice
(saying what this is testing, or something like that).
As noted, a bug should be filed in Bugzilla for this issue (then the
comments could give the bug number, as well as putting [BZ #N] in the
ChangeLog entry).
> +#define TEST_FUNCTION do_test ()
> +#include "../../test-skeleton.c"
The modern approach for tests is to use <support/test-driver.c> rather
than defining TEST_FUNCTION and using test-skeleton.c.
--
Joseph S. Myers
joseph@codesourcery.com