public inbox for libc-ports@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [AArch64] Provide symbol versions for mcount and _mcount.
@ 2013-07-25  8:09 Marcus Shawcroft
  2013-07-25 14:52 ` Joseph S. Myers
  0 siblings, 1 reply; 3+ messages in thread
From: Marcus Shawcroft @ 2013-07-25  8:09 UTC (permalink / raw)
  To: libc-ports; +Cc: Marcus Shawcroft

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  <marcus.shawcroft@linaro.org>

        * 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  <marcus.shawcroft@linaro.org>
+
+        * 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  <marcus.shawcroft@linaro.org>
 
 	* 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] [AArch64] Provide symbol versions for mcount and _mcount.
  2013-07-25  8:09 [PATCH] [AArch64] Provide symbol versions for mcount and _mcount Marcus Shawcroft
@ 2013-07-25 14:52 ` Joseph S. Myers
  2013-07-26  7:35   ` Marcus Shawcroft
  0 siblings, 1 reply; 3+ messages in thread
From: Joseph S. Myers @ 2013-07-25 14:52 UTC (permalink / raw)
  To: Marcus Shawcroft; +Cc: libc-ports

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
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] [AArch64] Provide symbol versions for mcount and _mcount.
  2013-07-25 14:52 ` Joseph S. Myers
@ 2013-07-26  7:35   ` Marcus Shawcroft
  0 siblings, 0 replies; 3+ messages in thread
From: Marcus Shawcroft @ 2013-07-26  7:35 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: libc-ports

On 25 July 2013 15:52, Joseph S. Myers <joseph@codesourcery.com> 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
> joseph@codesourcery.com

Thanks for the feedback.

/Marcus

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-07-26  7:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-25  8:09 [PATCH] [AArch64] Provide symbol versions for mcount and _mcount Marcus Shawcroft
2013-07-25 14:52 ` Joseph S. Myers
2013-07-26  7:35   ` Marcus Shawcroft

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).