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

["Joseph S. Myers" <joseph@codesourcery.com>]

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