* [PATCH] [MIPS] Raise highest supported EI_ABIVERSION value
@ 2019-08-09 14:26 Mihailo Stojanovic
2019-08-09 14:29 ` Joseph Myers
0 siblings, 1 reply; 4+ messages in thread
From: Mihailo Stojanovic @ 2019-08-09 14:26 UTC (permalink / raw)
To: libc-alpha
Cc: Joseph Myers, Maciej W . Rozycki, Carlos O'Donell,
Dragan Mladjenovic, Mihailo Stojanovic
Hello everyone,
As suggested by Joseph here [1], this bumps the highest valid ABIVERSION
value for ABSOLUTE ABI, which was, I suppose, overlooked by Maciej in
[2].
Cheers,
Mihailo
[1] https://sourceware.org/ml/libc-alpha/2019-07/msg00548.html
[2] https://sourceware.org/ml/libc-alpha/2018-07/msg00131.html
* sysdeps/unix/sysv/linux/mips/ldsodefs.h: Increment highest valid
ABIVERSION value.
---
sysdeps/unix/sysv/linux/mips/ldsodefs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sysdeps/unix/sysv/linux/mips/ldsodefs.h b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
index 8dde855..28257f8 100644
--- a/sysdeps/unix/sysv/linux/mips/ldsodefs.h
+++ b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
@@ -34,7 +34,7 @@ extern void _dl_static_init (struct link_map *map);
#undef VALID_ELF_ABIVERSION
#define VALID_ELF_ABIVERSION(osabi,ver) \
(ver == 0 \
- || (osabi == ELFOSABI_SYSV && ver < 4) \
+ || (osabi == ELFOSABI_SYSV && ver < 5) \
|| (osabi == ELFOSABI_GNU && ver < LIBC_ABI_MAX))
#endif /* ldsodefs.h */
--
2.7.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [MIPS] Raise highest supported EI_ABIVERSION value
2019-08-09 14:26 [PATCH] [MIPS] Raise highest supported EI_ABIVERSION value Mihailo Stojanovic
@ 2019-08-09 14:29 ` Joseph Myers
2019-08-13 9:22 ` Mihailo Stojanović
0 siblings, 1 reply; 4+ messages in thread
From: Joseph Myers @ 2019-08-09 14:29 UTC (permalink / raw)
To: Mihailo Stojanovic
Cc: libc-alpha, Maciej W . Rozycki, Carlos O'Donell, Dragan Mladjenovic
On Fri, 9 Aug 2019, Mihailo Stojanovic wrote:
> Hello everyone,
>
> As suggested by Joseph here [1], this bumps the highest valid ABIVERSION
> value for ABSOLUTE ABI, which was, I suppose, overlooked by Maciej in
> [2].
Please see what I said there about either adding a testcase (that fails
before and passes after the patch), or including an explanation in the
proposed commit message of why this is hard enough to test in the
testsuite that it's appropriate not to include a test. (Stating "this
fixes test X that is already in the testsuite" is also an option if true.)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [MIPS] Raise highest supported EI_ABIVERSION value
2019-08-09 14:29 ` Joseph Myers
@ 2019-08-13 9:22 ` Mihailo Stojanović
2019-08-13 15:35 ` Joseph Myers
0 siblings, 1 reply; 4+ messages in thread
From: Mihailo Stojanović @ 2019-08-13 9:22 UTC (permalink / raw)
To: Joseph Myers
Cc: libc-alpha, Maciej W . Rozycki, Carlos O'Donell, Dragan Mladjenovic
Hello,
I wanted to discuss this a bit more before proposing another
version of the patch.
Seeing as this is a trivial ABI version increment proposition,
I believe that including a testcase is superfluous (even more
so considering that the original issue [1] has a corresponding
testcase).
Furthermore, testing this patch requires static linker cooperation,
which means the undefined hidden and internal weak symbol
handling in static linker must be checked during glibc configuration.
If you think that the testcase is needed anyway, what I had in
mind was checking the static linker in the configure script, and
then enabling/disabling the test based on the result. The test
would just need to execute without "ABI version invalid" error
message.
Cheers,
Mihailo
[1] https://sourceware.org/ml/libc-alpha/2018-06/msg00509.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [MIPS] Raise highest supported EI_ABIVERSION value
2019-08-13 9:22 ` Mihailo Stojanović
@ 2019-08-13 15:35 ` Joseph Myers
0 siblings, 0 replies; 4+ messages in thread
From: Joseph Myers @ 2019-08-13 15:35 UTC (permalink / raw)
To: Mihailo Stojanović
Cc: libc-alpha, Maciej W . Rozycki, Carlos O'Donell, Dragan Mladjenovic
[-- Attachment #1: Type: text/plain, Size: 2202 bytes --]
On Tue, 13 Aug 2019, Mihailo StojanoviÄ wrote:
> Seeing as this is a trivial ABI version increment proposition,
> I believe that including a testcase is superfluous (even more
> so considering that the original issue [1] has a corresponding
> testcase).
A corresponding testcase that did not discover this bug. (I should add:
as a bug that was user-visible in a release, it should also be filed in
Bugzilla; then, once fixed, the bug should be marked RESOLVED / FIXED with
the target milestone set to the first mainline release that will have the
fix, so that it then appears in the automatically-generated list of fixed
bugs in the NEWS file for that release.)
When we discover a bug in some feature / previous bug fix, that was not
shown up by the tests added with that feature / bug fix, that indicates
missing test coverage, and so a new test should typically be added along
with the fix.
> Furthermore, testing this patch requires static linker cooperation,
> which means the undefined hidden and internal weak symbol
> handling in static linker must be checked during glibc configuration.
>
> If you think that the testcase is needed anyway, what I had in
> mind was checking the static linker in the configure script, and
> then enabling/disabling the test based on the result. The test
> would just need to execute without "ABI version invalid" error
> message.
Yes, a test with such configure test support seems appropriate, if the
test would fail when using an older static linker. Please make sure a
comment on the configure test says what binutils release was the first one
with the fix, so that it's obvious at what point we can remove the
configure test as no longer needed.
What exactly would go wrong when using an older static linker? If the
test would fail to link, then a configure test is needed. If the test
would simply wrongly PASS even without the rest of this glibc patch,
because the older linker doesn't set EI_ABIVERSION for this, I don't think
the configure test is needed. I'm guessing this test does not need to
check the values of symbols at runtime because the existing tests deal
with that.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-13 15:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-09 14:26 [PATCH] [MIPS] Raise highest supported EI_ABIVERSION value Mihailo Stojanovic
2019-08-09 14:29 ` Joseph Myers
2019-08-13 9:22 ` Mihailo Stojanović
2019-08-13 15:35 ` Joseph Myers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).