[PATCH] [AArch64] Provide symbol versions for mcount and _mcount.

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] [AArch64] Provide symbol versions for mcount and _mcount.

Marcus Shawcroft-2
Hi,

This patch fixes the symbol versions associated with mcount and _mcount in the AArch64 port.
The initial AArch64 port in 2.17 omitted this interface.  A patch earlier this year here
http://sourceware.org/ml/libc-ports/2013-05/msg00087.html introduced
the symbols but introduced them with default 2_17 versions instead of 2_18.

This patch versions them correctly.

I'd appreciate a review of this patch.

Cheers
/Marcus

 
2013-07-25  Marcus Shawcroft  <[hidden email]>

        * sysdeps/aarch64/Versions: New file.
        * sysdeps/aarch64/machine-gmon.h: New file.
        * sysdeps/aarch64/mcount.c: New file.
        * sysdeps/unix/sysv/linux/aarch64/nptl/libc.abilist (mcount): Add.
        (_mcount): Likewise.

---
 ports/ChangeLog.aarch64                            |  8 ++++
 ports/sysdeps/aarch64/Versions                     |  5 +++
 ports/sysdeps/aarch64/machine-gmon.h               | 43 ++++++++++++++++++++++
 ports/sysdeps/aarch64/mcount.c                     | 39 ++++++++++++++++++++
 .../unix/sysv/linux/aarch64/nptl/libc.abilist      |  2 +
 5 files changed, 97 insertions(+)
 create mode 100644 ports/sysdeps/aarch64/Versions
 create mode 100644 ports/sysdeps/aarch64/machine-gmon.h
 create mode 100644 ports/sysdeps/aarch64/mcount.c

diff --git a/ports/ChangeLog.aarch64 b/ports/ChangeLog.aarch64
index 3f26cb7..ee7f831 100644
--- a/ports/ChangeLog.aarch64
+++ b/ports/ChangeLog.aarch64
@@ -1,3 +1,11 @@
+2013-07-25  Marcus Shawcroft  <[hidden email]>
+
+        * sysdeps/aarch64/Versions: New file.
+        * sysdeps/aarch64/machine-gmon.h: New file.
+        * sysdeps/aarch64/mcount.c: New file.
+        * sysdeps/unix/sysv/linux/aarch64/nptl/libc.abilist (mcount): Add.
+        (_mcount): Likewise.
+
 2013-07-12  Marcus Shawcroft  <[hidden email]>
 
  * sysdeps/aarch64/Makefile (CFLAGS-backtrace.c): Define.
diff --git a/ports/sysdeps/aarch64/Versions b/ports/sysdeps/aarch64/Versions
new file mode 100644
index 0000000..29bbc86
--- /dev/null
+++ b/ports/sysdeps/aarch64/Versions
@@ -0,0 +1,5 @@
+libc {
+  GLIBC_2.18 {
+    mcount; _mcount;
+  }
+}
diff --git a/ports/sysdeps/aarch64/machine-gmon.h b/ports/sysdeps/aarch64/machine-gmon.h
new file mode 100644
index 0000000..815f7bb
--- /dev/null
+++ b/ports/sysdeps/aarch64/machine-gmon.h
@@ -0,0 +1,43 @@
+/* Machine-dependent definitions for profiling support.  Generic GCC 2 version.
+   Copyright (C) 1996-2013 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/>.  */
+
+/* GCC version 2 gives us a perfect magical function to get
+   just the information we need:
+     void *__builtin_return_address (unsigned int N)
+   returns the return address of the frame N frames up.  */
+
+/* Be warned that GCC cannot usefully compile __builtin_return_address(N)
+   for N != 0 on all machines.  In this case, you may have to write
+   your own version of _mcount().  */
+
+#if __GNUC__ < 2
+ #error "This file uses __builtin_return_address, a GCC 2 extension."
+#endif
+
+#include <sysdep.h>
+
+static void mcount_internal (u_long frompc, u_long selfpc);
+
+#define _MCOUNT_DECL(frompc, selfpc) \
+static inline void mcount_internal (u_long frompc, u_long selfpc)
+
+#define MCOUNT \
+void __mcount (void)      \
+{      \
+  mcount_internal ((u_long) RETURN_ADDRESS (1), (u_long) RETURN_ADDRESS (0)); \
+}
diff --git a/ports/sysdeps/aarch64/mcount.c b/ports/sysdeps/aarch64/mcount.c
new file mode 100644
index 0000000..8871a4f
--- /dev/null
+++ b/ports/sysdeps/aarch64/mcount.c
@@ -0,0 +1,39 @@
+/* Copyright (C) 2013 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/>.  */
+
+#include <shlib-compat.h>
+
+#include <gmon/mcount.c>
+
+/* We forgot to add _mcount and mcount in glibc 2.17.  We added them in 2.18
+   therefore we want them to be added with version GLIBC_2_18.  However, setting
+   then version is not straight forward because generic Version files include
+   an earlier 2.xx version for each of these symbols and the linker uses the
+   first version it sees.  */
+
+#if SHLIB_COMPAT (libc, GLIBC_2_17, GLIBC_2_18)
+/* The canonical name for the function is `_mcount' in both C and asm,
+   but some old asm code might assume it's `mcount'.  */
+weak_alias (__mcount, __weak_mcount)
+versioned_symbol (libc, __weak_mcount,  mcount, GLIBC_2_18);
+
+versioned_symbol (libc, __mcount, _mcount, GLIBC_2_18);
+#else
+strong_alias (__mcount, _mcount);
+weak_alias (__mcount, mcount)
+#endif
diff --git a/ports/sysdeps/unix/sysv/linux/aarch64/nptl/libc.abilist b/ports/sysdeps/unix/sysv/linux/aarch64/nptl/libc.abilist
index b04a761..b99dc36 100644
--- a/ports/sysdeps/unix/sysv/linux/aarch64/nptl/libc.abilist
+++ b/ports/sysdeps/unix/sysv/linux/aarch64/nptl/libc.abilist
@@ -2080,3 +2080,5 @@ GLIBC_2.17
 GLIBC_2.18
  GLIBC_2.18 A
  __cxa_thread_atexit_impl F
+ _mcount F
+ mcount F
--
1.8.1.2

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [AArch64] Provide symbol versions for mcount and _mcount.

Joseph Myers
On Thu, 25 Jul 2013, Marcus Shawcroft wrote:

> +/* Machine-dependent definitions for profiling support.  Generic GCC 2 version.

You mean "AArch64 version".

> +/* GCC version 2 gives us a perfect magical function to get
> +   just the information we need:
> +     void *__builtin_return_address (unsigned int N)
> +   returns the return address of the frame N frames up.  */
> +
> +/* Be warned that GCC cannot usefully compile __builtin_return_address(N)
> +   for N != 0 on all machines.  In this case, you may have to write
> +   your own version of _mcount().  */

"all machines" comment irrelevant.

> +#if __GNUC__ < 2
> + #error "This file uses __builtin_return_address, a GCC 2 extension."
> +#endif

No need for such conditionals except in installed headers and files shared
with gnulib.  (Yes, there are various cases still needing cleaning up
across glibc.)

> +/* We forgot to add _mcount and mcount in glibc 2.17.  We added them in 2.18
> +   therefore we want them to be added with version GLIBC_2_18.  However, setting
> +   then version is not straight forward because generic Version files include
> +   an earlier 2.xx version for each of these symbols and the linker uses the
> +   first version it sees.  */
> +
> +#if SHLIB_COMPAT (libc, GLIBC_2_17, GLIBC_2_18)
> +/* The canonical name for the function is `_mcount' in both C and asm,
> +   but some old asm code might assume it's `mcount'.  */

You shouldn't have such "old asm" for a new architecture, which would
indicate that you shouldn't need the "mcount" name at all.

But, given that you want that name, the patch seems correct for adding the
versions desired, subject to my other comments.

> +weak_alias (__mcount, __weak_mcount)
> +versioned_symbol (libc, __weak_mcount,  mcount, GLIBC_2_18);

Two spaces after comma, should be one.

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [AArch64] Provide symbol versions for mcount and _mcount.

Marcus Shawcroft-2
On 25 July 2013 15:52, Joseph S. Myers <[hidden email]> wrote:


> You shouldn't have such "old asm" for a new architecture, which would
> indicate that you shouldn't need the "mcount" name at all.

Good point, I've dropped mcount completely in the re-spun patch.

> --
> Joseph S. Myers
> [hidden email]

Thanks for the feedback.

/Marcus