diff mbox

Obsolete matherr, _LIB_VERSION, libieee.a

Message ID 1503595246.28672.90.camel@cavium.com
State New, archived
Headers

Commit Message

Steve Ellcey Aug. 24, 2017, 5:20 p.m. UTC 
  On Thu, 2017-08-24 at 12:32 +0000, Joseph Myers wrote:
> 
> > Earlier discussion about tst-mallocstate:
> > https://sourceware.org/ml/libc-alpha/2016-12/msg00527.html
> I think Florian's suggestion from that discussion is a good idea: generate 
> a makefile equivalent of abi-versions.h, so that you can write 
> conditionals in makefiles that are equivalent to SHLIB_COMPAT conditions 
> in C code.

It is not clear to me that I need a separate abi-versions.h file, the
tests are already including shlib-compat.h and that includes the
existing abi-versions.h.  Here is a patch that I have created and
tested, if the approach seems reasonable I can submit it in a seperate
thread with a ChangeLog file.  Basically, I created a TEST_COMPAT macro
that is almost identical to SHLIB_COMPAT and then I used that to ifdef
the test.  I put the macro in shlib-compat.h, maybe it should be in a
different or new header file but I liked having it there so that it was
near SHLIB_COMPAT which it is based on.  Testing on aarch64 looked good
for both ILP32 and LP64 with ILP32 doing the dummy do_test and LP64
running the original do_test.

Steve Ellcey
sellcey@cavium.com

Comments

Joseph Myers Aug. 24, 2017, 5:24 p.m. UTC | #1 
On Thu, 24 Aug 2017, Steve Ellcey wrote:

> It is not clear to me that I need a separate abi-versions.h file, the
> tests are already including shlib-compat.h and that includes the
> existing abi-versions.h.  Here is a patch that I have created and
> tested, if the approach seems reasonable I can submit it in a seperate
> thread with a ChangeLog file.  Basically, I created a TEST_COMPAT macro
> that is almost identical to SHLIB_COMPAT and then I used that to ifdef
> the test.  I put the macro in shlib-compat.h, maybe it should be in a
> different or new header file but I liked having it there so that it was
> near SHLIB_COMPAT which it is based on.  Testing on aarch64 looked good
> for both ILP32 and LP64 with ILP32 doing the dummy do_test and LP64
> running the original do_test.

TEST_COMPAT is plausible, but the patch does of course need to address the 
malloc test as well (with the appropriate version for obsoletion named in 
the TEST_COMPAT call), and I think returning 77 (UNSUPPORTED) rather than 
0 is appropriate in the fallback case, given that the (obsolete) feature 
being tested *is* unsupported by the (new) port.
Steve Ellcey Aug. 24, 2017, 5:44 p.m. UTC | #2 
On Thu, 2017-08-24 at 17:24 +0000, Joseph Myers wrote:
> 
> TEST_COMPAT is plausible, but the patch does of course need to address the 
> malloc test as well (with the appropriate version for obsoletion named in 
> the TEST_COMPAT call), and I think returning 77 (UNSUPPORTED) rather than 
> 0 is appropriate in the fallback case, given that the (obsolete) feature 
> being tested *is* unsupported by the (new) port.

OK, I will submit a patch that includes tst-mallocstate.c and that
returns 77 for the fallback case.  One question, should I update
tst-mallocstate.c from using test-skeleton.c to test-driver.c as
part of this change or is that something that should be done in a
separate patch.

Steve Ellcey
sellcey@cavium.com
Joseph Myers Aug. 24, 2017, 6:08 p.m. UTC | #3 
On Thu, 24 Aug 2017, Steve Ellcey wrote:

> returns 77 for the fallback case.  One question, should I update
> tst-mallocstate.c from using test-skeleton.c to test-driver.c as
> part of this change or is that something that should be done in a
> separate patch.

I think you can do that either way.
Florian Weimer Aug. 24, 2017, 6:53 p.m. UTC | #4 
On 08/24/2017 07:20 PM, Steve Ellcey wrote:
> -#undef _LIB_VERSION
> +
> +#if TEST_COMPAT (libm, GLIBC_2_0, GLIBC_2_27)
> +
> +# undef matherr
> +# undef _LIB_VERSION
>  compat_symbol_reference (libm, matherr, matherr, GLIBC_2_0);
>  compat_symbol_reference (libm, _LIB_VERSION, _LIB_VERSION, GLIBC_2_0);
>  
> @@ -45,5 +48,12 @@ do_test (void)
>    acos (2.0);
>    return fail;
>  }
> +#else
> +static int
> +do_test (void)
> +{
> +  return 0;
> +}
> +#endif

Does the fail to compile, or does it fail to link?

Maybe it is possible to make this slightly more regular.  Although the
expansion of compat_symbol_reference is difficult to remove because GCC
does not see it, so it is not optimized away.

Thanks,
Florian
Steve Ellcey Aug. 24, 2017, 9:06 p.m. UTC | #5 
On Thu, 2017-08-24 at 20:53 +0200, Florian Weimer wrote:
> 
> Does the fail to compile, or does it fail to link?
> 
> Maybe it is possible to make this slightly more regular.  Although the
> expansion of compat_symbol_reference is difficult to remove because GCC
> does not see it, so it is not optimized away.
> 
> Thanks,
> Florian

test-matherr.c failed to link:

/toolchain-ilp32/obj/glibc32/math/test-matherr.o: In function `do_test':
/toolchain-ilp32/src/glibc/math/test-matherr.c:43: undefined reference to `_LIB_VERSION@GLIBC_2.27'
/toolchain-ilp32/src/glibc/math/test-matherr.c:43: undefined reference to `_LIB_VERSION@GLIBC_2.27'



test-matherr-2.c compiled and linked but failed when running because
it did not call the matherr routine defined in the testcase.



tst-mallocstate.c also failed to link:

/toolchain-ilp32/src/glibc/malloc/tst-mallocstate.c:493: undefined reference to `malloc_get_state@GLIBC_2.27'
/toolchain-ilp32/obj-glibc32/malloc/tst-mallocstate.o: In function `init_heap':
/toolchain-ilp32/src/glibc/malloc/tst-mallocstate.c:315: undefined reference to `malloc_set_state@GLIBC_2.27'

Steve Ellcey
sellcey@cavium.com
diff mbox

Patch

diff --git a/include/shlib-compat.h b/include/shlib-compat.h
index 41eb362..d872afc 100644
--- a/include/shlib-compat.h
+++ b/include/shlib-compat.h
@@ -97,4 +97,14 @@ 
   compat_symbol (libc, name, aliasname, version);
 # endif
 
+/* The TEST_COMPAT macro acts just like the SHLIB_COMPAT macro except
+   that it does not check IS_IN.  It is used by tests that are testing
+   functionality that is only available in specific GLIBC versions.  */
+
+# define TEST_COMPAT(lib, introduced, obsoleted)			      \
+  _TEST_COMPAT (lib, introduced, obsoleted)
+# define _TEST_COMPAT(lib, introduced, obsoleted)			      \
+   (!(ABI_##lib##_##obsoleted - 0)					      \
+       || ((ABI_##lib##_##introduced - 0) < (ABI_##lib##_##obsoleted - 0)))
+
 #endif	/* shlib-compat.h */
diff --git a/math/test-matherr-2.c b/math/test-matherr-2.c
index c2fc5e6..667e9be 100644
--- a/math/test-matherr-2.c
+++ b/math/test-matherr-2.c
@@ -22,8 +22,11 @@ 
 
 #include <math-svid-compat.h>
 #include <shlib-compat.h>
-#undef matherr
-#undef _LIB_VERSION
+
+#if TEST_COMPAT (libm, GLIBC_2_0, GLIBC_2_27)
+
+# undef matherr
+# undef _LIB_VERSION
 compat_symbol_reference (libm, matherr, matherr, GLIBC_2_0);
 compat_symbol_reference (libm, _LIB_VERSION, _LIB_VERSION, GLIBC_2_0);
 
@@ -45,5 +48,12 @@  do_test (void)
   acos (2.0);
   return fail;
 }
+#else
+static int
+do_test (void)
+{
+  return 0;
+}
+#endif
 
 #include <support/test-driver.c>
diff --git a/math/test-matherr.c b/math/test-matherr.c
index 34856f1..927433c 100644
--- a/math/test-matherr.c
+++ b/math/test-matherr.c
@@ -22,8 +22,11 @@ 
 
 #include <math-svid-compat.h>
 #include <shlib-compat.h>
-#undef matherr
-#undef _LIB_VERSION
+
+#if TEST_COMPAT (libm, GLIBC_2_0, GLIBC_2_27)
+
+# undef matherr
+# undef _LIB_VERSION
 compat_symbol_reference (libm, matherr, matherr, GLIBC_2_0);
 compat_symbol_reference (libm, _LIB_VERSION, _LIB_VERSION, GLIBC_2_0);
 
@@ -44,5 +47,12 @@  do_test (void)
   acos (2.0);
   return fail;
 }
+#else
+static int
+do_test (void)
+{
+  return 0;
+}
+#endif
 
 #include <support/test-driver.c>