[PATCH roland/arm-mcount] ARM: Disable compat mcount code when unneeded.

[PATCH roland/arm-mcount] ARM: Disable compat mcount code when unneeded.

[Roland McGrath]

The obsolete _mcount entry point is not needed in a shared library not
supporting any old ABIs.  This change also makes it unavailable for static
linking altogether.  We have never supported linking old object files with
new libraries, so that should not be a problem for existing configurations.


Thanks,
Roland


ports/ChangeLog.arm
2013-08-27  Roland McGrath  <roland@hack.frob.com>

	* sysdeps/arm/arm-mcount.S: #include <shlib-compat.h>.
	(_mcount, mcount):
	Protect under [SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_19)].

--- a/ports/sysdeps/arm/arm-mcount.S
+++ b/ports/sysdeps/arm/arm-mcount.S
@@ -65,6 +65,10 @@ ENTRY(__gnu_mcount_nc)
 END(__gnu_mcount_nc)
 
 
+#include <shlib-compat.h>
+
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_19)
+
 /* Provide old mcount for backwards compatibility.  This requires
    code be compiled with APCS frame pointers.  */
 
@@ -102,3 +106,5 @@ END(_mcount)
    but some old asm code might assume it's `mcount'.  */
 #undef mcount
 weak_alias (_mcount, mcount)
+
+#endif

Re: [PATCH roland/arm-mcount] ARM: Disable compat mcount code when unneeded.

[Joseph S. Myers]

On Tue, 27 Aug 2013, Roland McGrath wrote:

> The obsolete _mcount entry point is not needed in a shared library not
> supporting any old ABIs.  This change also makes it unavailable for static
> linking altogether.  We have never supported linking old object files with
> new libraries, so that should not be a problem for existing configurations.

Isn't this about objects built with old compilers (before GCC 4.4, when 
__gnu_mcount_nc was introduced; arm*-*-linux-gnueabi support was added in 
4.1), which is generally supported, rather than objects built with old 
library headers, which isn't?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH roland/arm-mcount] ARM: Disable compat mcount code when unneeded.

[Roland McGrath]

> Isn't this about objects built with old compilers (before GCC 4.4, when 
> __gnu_mcount_nc was introduced; arm*-*-linux-gnueabi support was added in 
> 4.1), which is generally supported, rather than objects built with old 
> library headers, which isn't?

I see.  I'd still like to find a way to exclude this code from
configurations that will never use it, i.e. a target for which there was
never a compiler that produced _mcount calls.  But perhaps there is no
"generic" way to achieve that.  I suppose I can just move it to a separate
file and let my configuration drop that file from sysdep-routines.

I think the ideal would be if we dropped it from the shared library (except
for a compat_symbol where needed, of course) and had it only in
libc_nonshared.a.  But I can't see a way to e.g. write _mcount in terms of
calling __gnu_mcount_nc (without breaking the profile so that it only
tracks _mcount itself as the callee), and I don't want to expose something
like __mcount_internal in the ABI just so that _mcount can call it.

Can you think of anything better?


Thanks,
Roland

Re: [PATCH roland/arm-mcount] ARM: Disable compat mcount code when unneeded.

[Joseph S. Myers]

On Thu, 29 Aug 2013, Roland McGrath wrote:

> > Isn't this about objects built with old compilers (before GCC 4.4, when 
> > __gnu_mcount_nc was introduced; arm*-*-linux-gnueabi support was added in 
> > 4.1), which is generally supported, rather than objects built with old 
> > library headers, which isn't?
> 
> I see.  I'd still like to find a way to exclude this code from
> configurations that will never use it, i.e. a target for which there was
> never a compiler that produced _mcount calls.  But perhaps there is no
> "generic" way to achieve that.  I suppose I can just move it to a separate
> file and let my configuration drop that file from sysdep-routines.
> 
> I think the ideal would be if we dropped it from the shared library (except
> for a compat_symbol where needed, of course) and had it only in
> libc_nonshared.a.  But I can't see a way to e.g. write _mcount in terms of
> calling __gnu_mcount_nc (without breaking the profile so that it only
> tracks _mcount itself as the callee), and I don't want to expose something
> like __mcount_internal in the ABI just so that _mcount can call it.
> 
> Can you think of anything better?

I don't have anything better in general.

It does occur to me that for the hard-float ABI (__ARM_PCS_VFP) it would 
be OK in principle to make the old entry point into a compat symbol in the 
shared library and remove it from the static library, so not available for 
new links, given that the hard-float ABI wasn't supported before GCC 4.5 
(and GCC has never supported marking objects that don't use floating point 
as "don't care" for the ABI, so soft-float objects built with older GCC 
couldn't be used with a hard-float libc).  But that's probably not 
worthwhile (and conditioning something that's nothing to do with floating 
point on the floating-point ABI would at least be rather confusing).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH roland/arm-mcount] ARM: Disable compat mcount code when unneeded.

[Roland McGrath]

[This new patch is on the replaced branch roland/arm-mcount.]

> It does occur to me that for the hard-float ABI (__ARM_PCS_VFP) it would 
> be OK in principle to make the old entry point into a compat symbol in the 
> shared library and remove it from the static library, so not available for 
> new links, given that the hard-float ABI wasn't supported before GCC 4.5 
> (and GCC has never supported marking objects that don't use floating point 
> as "don't care" for the ABI, so soft-float objects built with older GCC 
> couldn't be used with a hard-float libc).  But that's probably not 
> worthwhile (and conditioning something that's nothing to do with floating 
> point on the floating-point ABI would at least be rather confusing).

I've finally gotten back to this.  I made it a bit cleaner than the minimal
requirement, by introducing gcc-compat.h and GCC_COMPAT as a rough analogue
to shlib-compat.h and SHLIB_COMPAT.

I've done an arm-linux-gnueabi build and verified that arm-mcount.o{,s}
have no differences except for the addition of the __mcount_arm_compat
symbol, and that 'make check-abi' still passes.

I've done an arm-linux-gnueabihf build and verified that arm-mcount.o has
no differences except the removal of the _mcount code and the mcount alias,
and that arm-mcount.os has no differences except in its symbol table, and
that 'make check-abi' still passes.  That is, the one user-visible change
here is that the symbols _mcount and mcount are no longer available at link
time (either static or dynamic) for arm-linux-gnueabihf.  Does that merit a
NEWS mention?

I've also done a build of my unfinished arm-nacl port (which has earliest
symbol set GLIBC_2.19), and verified that the _mcount code and the symbols
_mcount and mcount are entirely gone from everywhere.

OK?

I have a mild preference for putting this in before the freeze, just to
avoid having to push the SHLIB_COMPAT tests to GLIBC_2.20.


Thanks,
Roland


2014-01-10  Roland McGrath  <roland@hack.frob.com>

	* sysdeps/generic/gcc-compat.h: New file.

ports/ChangeLog.arm
2014-01-10  Roland McGrath  <roland@hack.frob.com>

	* sysdeps/arm/arm-mcount.S:
	#include <shlib-compat.h> and <gcc-compat.h>.
	(_mcount): Renamed to __mcount_arm_compat.  Made conditional on
	[GCC_COMPAT (4, 3) || SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_19)].
	(_mcount, mcount): Define (as aliases) only under [GCC_COMPAT (4, 3)],
	with compat_symbol under [SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_19)].

	* sysdeps/arm/gcc-compat.h: New file.

--- a/ports/sysdeps/arm/arm-mcount.S
+++ b/ports/sysdeps/arm/arm-mcount.S
@@ -21,6 +21,7 @@
 
 #include <sysdep.h>
 
+#undef mcount
 
 #ifdef __thumb2__
 	.thumb
@@ -65,10 +66,20 @@ ENTRY(__gnu_mcount_nc)
 END(__gnu_mcount_nc)
 
 
+#include <gcc-compat.h>
+#include <shlib-compat.h>
+
+/* The new __gnu_mcount_nc entry point was introduced in 4.4, so the
+   static library needs the old one only to support older compilers.
+   Even in a configuration that only cares about newer compilers, the
+   shared library might need it only for strict ABI compatibility.  */
+
+#if GCC_COMPAT (4, 3) || SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_19)
+
 /* Provide old mcount for backwards compatibility.  This requires
    code be compiled with APCS frame pointers.  */
 
-ENTRY(_mcount)
+ENTRY(__mcount_arm_compat)
 	push	{r0, r1, r2, r3, fp, lr}
 	cfi_adjust_cfa_offset (24)
 	cfi_rel_offset (r0, 0)
@@ -83,7 +94,7 @@ ENTRY(_mcount)
 	ldrne r0, [\B, #-4]
 	movsne r1, lr
 	blne __mcount_internal
-#if defined (__ARM_ARCH_4T__) && defined (__THUMB_INTERWORK__)
+# if defined (__ARM_ARCH_4T__) && defined (__THUMB_INTERWORK__)
 	pop	{r0, r1, r2, r3, fp, lr}
 	cfi_adjust_cfa_offset (-24)
 	cfi_restore (r0)
@@ -93,12 +104,26 @@ ENTRY(_mcount)
 	cfi_restore (fp)
 	cfi_restore (lr)
 	bx lr
-#else
+# else
 	pop	{r0, r1, r2, r3, fp, pc}
+# endif
+END(__mcount_arm_compat)
+
 #endif
-END(_mcount)
+
+#if GCC_COMPAT (4, 3)
+
+strong_alias (__mcount_arm_compat, _mcount)
 
 /* The canonical name for the function is `_mcount' in both C and asm,
    but some old asm code might assume it's `mcount'.  */
-#undef mcount
 weak_alias (_mcount, mcount)
+
+#elif SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_19)
+
+compat_symbol (libc, __mcount_arm_compat, _mcount, GLIBC_2_0)
+
+strong_alias (__mcount_arm_compat, __mcount_arm_compat_1)
+compat_symbol (libc, __mcount_arm_compat_1, mcount, GLIBC_2_0)
+
+#endif
--- /dev/null
+++ b/ports/sysdeps/arm/gcc-compat.h
@@ -0,0 +1,35 @@
+/* Macros for checking required GCC compatibility.  ARM version.
+   Copyright (C) 2014 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/>.  */
+
+#ifndef _ARM_GCC_COMPAT_H
+#define _ARM_GCC_COMPAT_H 1
+
+#ifndef GCC_COMPAT_VERSION
+# ifdef __ARM_PCS_VFP
+/* The hard-float ABI was first supported in 4.5.  */
+#  define GCC_COMPAT_VERSION    GCC_VERSION (4, 5)
+# else
+/* The EABI configurations (the only ones we handle) were first supported
+   in 4.1.  */
+#  define GCC_COMPAT_VERSION    GCC_VERSION (4, 1)
+# endif
+#endif
+
+#include_next <gcc-compat.h>
+
+#endif
--- /dev/null
+++ b/sysdeps/generic/gcc-compat.h
@@ -0,0 +1,42 @@
+/* Macros for checking required GCC compatibility.  Generic version.
+   Copyright (C) 2014 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/>.  */
+
+/* This is the base file.  More-specific sysdeps/.../gcc-compat.h files
+   can define GCC_COMPAT_VERSION and then #include_next this file.  */
+
+#ifndef _GENERIC_GCC_COMPAT_H
+#define _GENERIC_GCC_COMPAT_H 1
+
+/* This is the macro that gets used in #if tests in code: true iff
+   the library we build must be compatible with user code built by
+   GCC version MAJOR.MINOR.  */
+#define GCC_COMPAT(major, minor)        \
+  (GCC_COMPAT_VERSION <= GCC_VERSION (major, minor))
+
+/* This is how we compose an integer from major and minor version
+   numbers, for comparison.  */
+#define GCC_VERSION(major, minor)       \
+  (((major) << 16) + (minor))
+
+#ifndef GCC_COMPAT_VERSION
+/* GCC 2.7.2 was current at the time of the glibc-2.0 release.
+   We assume nothing before that ever mattered.  */
+# define GCC_COMPAT_VERSION     GCC_VERSION (2, 7)
+#endif
+
+#endif

Re: [PATCH roland/arm-mcount] ARM: Disable compat mcount code when unneeded.

[Joseph S. Myers]

On Fri, 10 Jan 2014, Roland McGrath wrote:

> I've done an arm-linux-gnueabi build and verified that arm-mcount.o{,s}
> have no differences except for the addition of the __mcount_arm_compat
> symbol, and that 'make check-abi' still passes.
> 
> I've done an arm-linux-gnueabihf build and verified that arm-mcount.o has
> no differences except the removal of the _mcount code and the mcount alias,
> and that arm-mcount.os has no differences except in its symbol table, and
> that 'make check-abi' still passes.  That is, the one user-visible change
> here is that the symbols _mcount and mcount are no longer available at link
> time (either static or dynamic) for arm-linux-gnueabihf.  Does that merit a
> NEWS mention?

I don't think a NEWS mention is needed; making symbols no longer available 
at link time should get a mention if users might have used them directly, 
but I don't see a need where the symbols couldn't sensibly have been 
called directly from user C code.

The change is OK with me for 2.19, since there's still another ARM issue 
pending before doing ARM release testing for 2.19.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH roland/arm-mcount] ARM: Disable compat mcount code when unneeded.

[Roland McGrath]

Committed.

Thanks,
Roland