Message ID | 1565360776-31536-1-git-send-email-mihailo.stojanovic@rt-rk.com |
---|---|
State | Superseded |
Headers |
Received: (qmail 88669 invoked by alias); 9 Aug 2019 14:26:24 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 88660 invoked by uid 89); 9 Aug 2019 14:26:23 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=ver, H*Ad:U*macro, suppose X-HELO: mail.rt-rk.com From: Mihailo Stojanovic <mihailo.stojanovic@rt-rk.com> To: libc-alpha@sourceware.org Cc: Joseph Myers <joseph@codesourcery.com>, "Maciej W . Rozycki" <macro@wdc.com>, Carlos O'Donell <carlos@redhat.com>, Dragan Mladjenovic <dragan.mladjenovic@rt-rk.com>, Mihailo Stojanovic <mihailo.stojanovic@rt-rk.com> Subject: [PATCH] [MIPS] Raise highest supported EI_ABIVERSION value Date: Fri, 9 Aug 2019 16:26:16 +0200 Message-Id: <1565360776-31536-1-git-send-email-mihailo.stojanovic@rt-rk.com> |
Commit Message
Mihailo Stojanovic
Aug. 9, 2019, 2:26 p.m. UTC
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(-)
Comments
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.)
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
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.
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 */