Set ARM ASM symbol visibility to `hidden`

hebasto commented at 10:14 pm on March 13, 2023: member

Solves one item in #1181.

To test on arm-32bit hardware, run:

0$ ./autogen.sh && ./configure --enable-experimental --with-asm=arm && make

On master branch (427bc3cdcfbc74778070494daab1ae5108c71368):

0$ nm -D .libs/libsecp256k1.so | grep secp256k1_fe
10000e2bc T secp256k1_fe_mul_inner
20000e8dc T secp256k1_fe_sqr_inner

With this PR:

0$ nm -D .libs/libsecp256k1.so | grep secp256k1_fe | wc -l
10

For reference, see https://sourceware.org/binutils/docs/as/Hidden.html.

theuni commented at 9:46 pm on March 14, 2023: contributor

Mmm, concept NACK. This is relying on a side-effect of linker behavior rather than avoiding exporting them in the first place. The problem is that the visibility flag doesn’t get set for asm.

This should really be:

0set(CMAKE_ASM_VISIBILITY_PRESET hidden)

However cmake doesn’t understand that :(

Instead I guess we should add the flag ourselves after testing that it’ll work?

theuni commented at 10:03 pm on March 14, 2023: contributor

Proposed alternative: just always make it hidden.

Since this is already a special-cased platform using gas syntax, I don’t see any problem with adding another directive.

 0diff --git a/src/asm/field_10x26_arm.s b/src/asm/field_10x26_arm.s
 1index 5f68cefc..f7cc1f26 100644
 2--- a/src/asm/field_10x26_arm.s
 3+++ b/src/asm/field_10x26_arm.s
 4@@ -29,6 +29,7 @@ Note:
 5        .align  2
 6        .global secp256k1_fe_mul_inner
 7        .type   secp256k1_fe_mul_inner, %function
 8+       .hidden secp256k1_fe_mul_inner
 9        @ Arguments:
10        @  r0  r      Restrict: can overlap with a, not with b
11        @  r1  a
12@@ -516,6 +517,7 @@ secp256k1_fe_mul_inner:
13        .align  2
14        .global secp256k1_fe_sqr_inner
15        .type   secp256k1_fe_sqr_inner, %function
16+       .hidden secp256k1_fe_sqr_inner
17        @ Arguments:
18        @  r0  r         Can overlap with a
19        @  r1  a

Set ARM ASM symbol visibility to `hidden`

Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>

fd2a408647

hebasto force-pushed on Mar 15, 2023

hebasto renamed this:
~~[POC] build: Do not export symbols from compiled ARM assembly~~
Set ARM ASM symbol visibility to `hidden`
on Mar 15, 2023

hebasto commented at 9:11 am on March 15, 2023: member

Implemented @theuni’s suggestion.

Thank you!

hebasto cross-referenced this on Mar 15, 2023 from issue Symbol visibility by real-or-random

theuni commented at 12:28 pm on March 15, 2023: contributor

Thanks! I’ve tested this locally and it indeed gets the symbols hidden.

Since all the other tools seem to consider it non-standard to pass visibility flags to an asm file, rather than trying to force -fvisibility=hidden in tooling that doesn’t want it, I think it’s perfectly reasonable to embed the property in the file instead.

ACK fd2a408647ba0f999b7b217977cc68773fa35257.

theuni approved

sipa commented at 9:07 pm on March 15, 2023: contributor

ACK fd2a408647ba0f999b7b217977cc68773fa35257

real-or-random merged this on Mar 26, 2023

real-or-random closed this on Mar 26, 2023

hebasto deleted the branch on Mar 26, 2023

sipa referenced this in commit e1552d578e on Apr 11, 2023

sipa referenced this in commit c981671e9b on Apr 14, 2023

hebasto referenced this in commit 49c52ea2b1 on May 13, 2023

RandyMcMillan referenced this in commit 3cc75121b3 on May 27, 2023

vmta referenced this in commit e1120c94a1 on Jun 4, 2023

vmta referenced this in commit 8f03457eed on Jul 1, 2023

jonasnick cross-referenced this on Jul 21, 2023 from issue Upstream PRs 1228, 1236, 1243, 1238, 1246, 1247, 1242, 1250, 1244, 1241, 1257, 1226, 1252, 1118, 1245, 1266, 1269 by jonasnick

Set ARM ASM symbol visibility to hidden #1242

Set ARM ASM symbol visibility to `hidden` #1242