diff mbox

Add hidden visibility to internal function prototypes

Message ID CAMe9rOpOR3=Vf1FoViQuJ1o95RNwkHF-MnuKr7GQhT4p-wVOuw@mail.gmail.com
State New, archived
Headers

Commit Message

H.J. Lu Aug. 22, 2017, 12:16 p.m. UTC 
  On Tue, Aug 22, 2017 at 4:45 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 22 Aug 2017, Florian Weimer wrote:
>
>> On 08/22/2017 12:47 PM, Florian Weimer wrote:
>> > I think the attached approach will work for the math subdirectory.
>> > Tested on aarch64 and ppc64le.
>> >
>> > I see additional aarch64 failures, though (outside math).
>>
>> The failure was spurious (probably clock skew).
>>
>> I have since tried “make xcheck” and “make bench-build” on both
>> architectures and don't see any failures anymore with the patch I posted
>> (and the revert reverted).
>
> Thanks.  This patch (and restoring HJ's patch) is OK to allow continued
> progress on the visibility changes (although the alternative of using
> mini-gmp in these tests is attractive as well, in line with the general
> principle of limiting tests to using public interfaces when they have no
> real need for internal interfaces).

I tried mini-gmp from gmp 6.1.2.  Some GMP functions are missing from
mini-gmp.  It seems that these math tests depend on the GMP in glibc.

I didn't see the issue on x86 because

./sysdeps/x86_64/rshift.S
./sysdeps/powerpc/powerpc32/rshift.S
./sysdeps/m68k/m680x0/rshift.S
./sysdeps/hppa/rshift.S
./sysdeps/i386/rshift.S
./sysdeps/i386/i586/rshift.S
./sysdeps/sparc/sparc32/rshift.S
./sysdeps/sparc/sparc64/rshift.S
./sysdeps/mips/rshift.S
./sysdeps/mips/mips64/rshift.S
./sysdeps/alpha/rshift.S
./sysdeps/alpha/alphaev5/rshift.S

Only targets which use stdlib/rshift.c have this issue.  This patch
copies the debug approach from stdlib/lshift.c and works on aarch64,

Comments

Florian Weimer Aug. 22, 2017, 12:22 p.m. UTC | #1 
On 08/22/2017 02:16 PM, H.J. Lu wrote:
> Only targets which use stdlib/rshift.c have this issue.  This patch
> copies the debug approach from stdlib/lshift.c and works on aarch64,

I think the existing assert better reflects the actual intent here.  We
should use assert in stdlib/lshift.c as well.

With staticaly linked tests, the use of __assert_fail is a non-issue.

Thanks,
Florian
diff mbox

Patch

From 3478fe3a92a8c85fa342a38ef3bf9bcc704bea94 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 21 Aug 2017 17:07:36 -0700
Subject: [PATCH] rshift.c: Replace assert with DEBUG and abort

assert in stdlib/rshift.c should be for debug purpose only and there is
no such check in any rshift assembly implementations nor in lshift.c.
This patch replaces assert with DEBUG and abort, similar to lshift.c
so that generic GMP codes from libc.a can be linked with libc.so in
atest-exp, atest-exp2 and atest-sincos, which depend on the GMP
implementation in glibc.

	* stdlib/rshift.c (mpn_rshift): Replace ssert with DEBUG and
	abort.
---
 stdlib/rshift.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/stdlib/rshift.c b/stdlib/rshift.c
index d4c7f77769..ab69dd7915 100644
--- a/stdlib/rshift.c
+++ b/stdlib/rshift.c
@@ -42,7 +42,10 @@  mpn_rshift (register mp_ptr wp,
   register mp_size_t i;
   mp_limb_t retval;
 
-  assert (usize != 0 && cnt != 0);
+#ifdef DEBUG
+  if (usize == 0 || cnt == 0)
+    abort ();
+#endif
 
   sh_1 = cnt;
 
-- 
2.13.5