"mcount" is deprecated for arm backend

[llvmbot]

Bugzilla Link	23969
Resolution	FIXED
Resolved on	Apr 10, 2016 11:23
Version	trunk
OS	Linux
Blocks	#4440 #24719
Reporter	LLVM Bugzilla Contributor
CC	@compnerd,@echristo,@rengolin

Extended Description

Current ARM bpapi (Base Platform Application Binary Interface) no longer uses "mcount" as function profiler name, instead it uses "__gnu_mcount_nc".

(Refer to trunk gcc - gcc/config/arm/bpapi.h ARM_FUNCTION_PROFILER.)

Also checking the system libc.so, __gnu_mcount_nc is properly defined. Notwithstanding the fact that "_mcount" and "mcount" are defined in libc.so, they are not the default-version symbol, to link against these symbols, a version script file has to be provided to the linker, otherwise we get an undefined symbol error.

May I suggest override this value in ARMTaretInfo as -
this->MCountName = "__gnu_mcount_nc";

[llvmbot]

assigned to @compnerd

[rengolin]

Hi Han,

This is a duplicate of #​19317, but since that one is closed, I'll keep this open.

Repeating what was said there (and threads on mailing lists), the ARM bpabi does NOT mandate __gnu_mcount_nc, as that would be non-sense. The GNU ARM BABI, however, uses it, as it's more robust, especially on ARM.

I don't think we should be linking against __gnu_mcount_nc just because GCC does it, but I also don't have a better option, if current _mcount we end up linking against is broken (see that bug).

It's possible that we could "implement" _mcount ourselves, in compiler-rt, at least as an alias to __gnu_mcount_nc, but I'm not sure how much better that would be.

Do you have any strong reason to suggest that we move the MCountName? Remember, LLVM is used in a lot more environments than just GNU, and just doing the change you suggest would probably break a lot of (if not all) other environments.

[llvmbot]

Hi Renato, thanks.

I don't think my reason is strong enough to make such a change that impacts lots of others, but let me state our situation - we are building ChromeOs using clang, which fails at one particular package punybench (https://chromium.googlesource.com/chromiumos/platform/punybench/) on ARM. So currently we bypass clang for this package.

Yes I think implementing mcount in compiler_rt arm backend would be a good choice.

[rengolin]

The kernel has a similar problem, and assumes gnu_mcount_nc is used with the ARM unwinder (EHABI). This may be reason enough to choose it, at least for EHABI.

[compnerd]

The kernel has a similar problem, and assumes gnu_mcount_nc is used with the
ARM unwinder (EHABI). This may be reason enough to choose it, at least for
EHABI.

We can predicate the selection of the instrumentation entry point on the -meabi parameter (gnu -> __gnu_mcount_nc, eabi -> _mcount).

[rengolin]

We can predicate the selection of the instrumentation entry point on the
-meabi parameter (gnu -> __gnu_mcount_nc, eabi -> _mcount).

I think that's the sanest idea, yes. I'm ok with this change.

[rengolin]

*** Bug llvm/llvm-bugzilla-archive#27248 has been marked as a duplicate of this bug. ***

[rengolin]

Saleem, I believe the same goes for AArch64, so if you don't mind, the "gnu" triple for AArch64 should also have the same results.

[compnerd]

Saleem, I believe the same goes for AArch64, so if you don't mind, the "gnu"
triple for AArch64 should also have the same results.

Yes, except for the fact, that do we want do it based on the triple (gnueabi) or the -meabi parameter? I think that this should be part of the -meabi flag as the "gnueabi" environment really is indicative of a GNU (libc) environment which conforms to EABI. This allows us to to say that we want to use the GNU libc with a pure EABI BPAPI (generate mcount) vs a GNU libc and a GNU BPAPI (generate __gnu_mcount_nc). This would also allow us to properly handle things like armv7--none-musleabi and still use the GNU version of the instrumentation hooks.

[rengolin]

-meabi exists because we have odd triples, yes, but IIRC, GNU triples force -meabi=gnu on ARM, too. That's what I meant... :)

[compnerd]

SVN r265888.

Use -meabi gnu in addition to the -target {armv7,aarch64}-*-{linux,unknown}-{gnu,}eabi{,hf}.

[rengolin]

Saleem, for AArch64, GCC emits "_mcount", not "__gnu_mcount_nc".

[compnerd]

Saleem, for AArch64, GCC emits "_mcount", not "__gnu_mcount_nc".

Ah, so your previous comment was incorrect? I suppose that I should've double checked. I'll fix that :)

[rengolin]

Right, "same results" was the wrong choice of words. :)

[compnerd]

Addressed review comments in SVN r265899.

[rengolin]

Saleem, I'm confused...

Isn't the new commit going to call "mcount" instead of "_mcount"?

[compnerd]

Yes, it does, but, that is an existing issue. Ive restored the previous behavior. I am happy enough to fix it, but that shouldn't really be considered a regression caused by the support for __gnu_mcount_nc. The question then is, what is the spelling on all the other OSes (primarily CloudABI, FreeBSD, NetBSD). Can you file a separate bug for that, and assign it to me?

[rengolin]

ok

[llvmbot]

mentioned in issue #24719

[rengolin]

mentioned in issue llvm/llvm-bugzilla-archive#27248

[edwintorok]

mentioned in issue #4440