[v2,MIPS] Raise highest supported EI_ABIVERSION value

Message ID 1565960509-29381-1-git-send-email-mihailo.stojanovic@rt-rk.com
State Superseded
Headers

Commit Message

Mihailo Stojanovic Aug. 16, 2019, 1:01 p.m. UTC
  Hello everyone,

As suggested by Joseph here [1], this bumps the highest valid ABIVERSION
value to ABSOLUTE ABI, which was, I suppose, overlooked by Maciej in
[2].

New testcase doesn't actually check the value of the symbol, it just
makes sure that it is executed without "ABI version invalid" error.

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/mips/Makefile: New test.
    * sysdeps/mips/tst-undefined-weak-lib.S: Ditto.
    * sysdeps/mips/tst-undefined-weak.c: Ditto.
    * sysdeps/unix/sysv/linux/mips/ldsodefs.h: Increment highest valid
      ABIVERSION value.
---
 sysdeps/mips/Makefile                   |  8 ++++++++
 sysdeps/mips/tst-undefined-weak-lib.S   | 33 +++++++++++++++++++++++++++++++++
 sysdeps/mips/tst-undefined-weak.c       | 28 ++++++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/mips/ldsodefs.h |  2 +-
 4 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 sysdeps/mips/tst-undefined-weak-lib.S
 create mode 100644 sysdeps/mips/tst-undefined-weak.c
  

Comments

Joseph Myers Aug. 16, 2019, 4:21 p.m. UTC | #1
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.
  

Patch

diff --git a/sysdeps/mips/Makefile b/sysdeps/mips/Makefile
index 7ac6fa5..eb52966 100644
--- a/sysdeps/mips/Makefile
+++ b/sysdeps/mips/Makefile
@@ -82,3 +82,11 @@  $(objpfx)tst-mode-switch-2: $(shared-thread-library)
 endif
 endif
 endif
+
+ifeq ($(subdir),elf)
+tests += tst-undefined-weak
+modules-names += tst-undefined-weak-lib
+
+CFLAGS-tst-undefined-weak.c: -O0
+$(objpfx)tst-undefined-weak: $(objpfx)tst-undefined-weak-lib.so
+endif
diff --git a/sysdeps/mips/tst-undefined-weak-lib.S b/sysdeps/mips/tst-undefined-weak-lib.S
new file mode 100644
index 0000000..9f33729
--- /dev/null
+++ b/sysdeps/mips/tst-undefined-weak-lib.S
@@ -0,0 +1,33 @@ 
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+	.text
+	.globl	x
+	.set	nomips16
+	.set	nomicromips
+	.ent	x
+	.type	x, @function
+x:
+	.frame	$sp,0,$31		# vars= 0, regs= 0/0, args= 0, gp= 0
+	.cpload $25
+	jr	$31
+	lb	$2,%got(a)($28)
+
+	.end	x
+	.size	x, .-x
+	.weak	a
+	.hidden	a
diff --git a/sysdeps/mips/tst-undefined-weak.c b/sysdeps/mips/tst-undefined-weak.c
new file mode 100644
index 0000000..d32334d
--- /dev/null
+++ b/sysdeps/mips/tst-undefined-weak.c
@@ -0,0 +1,28 @@ 
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+int *x (void);
+
+int
+do_test (void)
+{
+  x ();
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../../test-skeleton.c"
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 */