Use gcc visibility attribute

[phprus]

Description

According to GCC symbols visibility wiki page as such (with __attribute__ ((visibility ("default"))) exported symbols should be explicitly marked, to allow compilating with -fvisibility=hidden flag.

Previous PR: #786, #799

Fixes #779, #713

• - git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details)

Type of change

Choose one or multiple, leave empty if none of the other choices apply

Add a respective label(s) to PR if you have permissions

• bug fix - change that fixes an issue
• new feature - change that adds functionality
• tests - change in tests
• infrastructure - change in infrastructure and CI
• documentation - documentation update

Tests

• added - required for new features and some bug fixes
• not needed

Documentation

• updated in # - add PR number
• needs to be updated
• not needed

Breaks backward compatibility

• Yes
• No
• Unknown

Notify the following users

List users with @ to send notifications

Other information

[phprus]

@pavelkumbrasev, @isaevil what do you think about this PR?

[phprus]

Rebased.

Removed

Replace __TBB_EXPORT with TBBMALLOC_EXPORT.
Revert "Replace __TBB_EXPORT with TBBMALLOC_EXPORT."

experiment.

Move flags into cmake script (Suggested by @ilya-lavrenov).

[phprus]

Append commit form PR #799 (issue #713).
Warning:

ld: warning: cannot export hidden symbol typeinfo for tbb::detail::r1::unsafe_wait from CMakeFiles/tbb.dir/exception.cpp.o

Error:

 64/136 Test  #64: test_global_control ......................Subprocess aborted***Exception:   0.10 sec
libc++abi: terminating with uncaught exception of type doctest::detail::TestFailureException
[doctest] doctest version is "2.4.11"
[doctest] run with "--help" for options
===============================================================================
/Users/runner/work/oneTBB/oneTBB/test/tbb/test_global_control.cpp:239:
TEST CASE:  prolong lifetime advanced

/Users/runner/work/oneTBB/oneTBB/test/tbb/test_global_control.cpp:140: FATAL ERROR: REQUIRE( false ) is NOT correct!
  values: REQUIRE( false )

/Users/runner/work/oneTBB/oneTBB/test/tbb/test_global_control.cpp:239: FATAL ERROR: test case CRASHED: SIGABRT - Abort (abnormal termination) signal

===============================================================================
[doctest] test cases: 3 | 2 passed | 1 failed | 1 skipped
[doctest] assertions: 6 | 5 passed | 1 failed |
[doctest] Status: FAILURE!

Failed test test_global_control:
https://github.com/oneapi-src/oneTBB/blob/ea4e6156a2cde839fc01b21b76cd9b5743ec0cef/test/tbb/test_global_control.cpp#L126-L143

[phprus]

ping?

[phprus]

ping?

[phprus]

Rebased.

[phprus]

Any news on this PR?

[phprus]

ping?

[phprus]

I think the documentation build error is out of scope for this PR:
https://github.com/oneapi-src/oneTBB/actions/runs/5587280735/jobs/10212448857?pr=1114#step:3:83

[isaevil]

Append commit form PR #799 (issue #713). Warning:

ld: warning: cannot export hidden symbol typeinfo for tbb::detail::r1::unsafe_wait from CMakeFiles/tbb.dir/exception.cpp.o

Error:

 64/136 Test  #64: test_global_control ......................Subprocess aborted***Exception:   0.10 sec
libc++abi: terminating with uncaught exception of type doctest::detail::TestFailureException
[doctest] doctest version is "2.4.11"
[doctest] run with "--help" for options
===============================================================================
/Users/runner/work/oneTBB/oneTBB/test/tbb/test_global_control.cpp:239:
TEST CASE:  prolong lifetime advanced

/Users/runner/work/oneTBB/oneTBB/test/tbb/test_global_control.cpp:140: FATAL ERROR: REQUIRE( false ) is NOT correct!
  values: REQUIRE( false )

/Users/runner/work/oneTBB/oneTBB/test/tbb/test_global_control.cpp:239: FATAL ERROR: test case CRASHED: SIGABRT - Abort (abnormal termination) signal

===============================================================================
[doctest] test cases: 3 | 2 passed | 1 failed | 1 skipped
[doctest] assertions: 6 | 5 passed | 1 failed |
[doctest] Status: FAILURE!

Failed test test_global_control:

https://github.com/oneapi-src/oneTBB/blob/ea4e6156a2cde839fc01b21b76cd9b5743ec0cef/test/tbb/test_global_control.cpp#L126-L143

@phprus Looks like the changes break ABI on MacOS, because symbols are now different:

$ diff patch_tbb_mac.txt vanilla_tbb_mac.txt
119c119
< __ZTIN3tbb6detail2r111unsafe_waitE S
---
> __ZTIN3tbb6detail2r111unsafe_waitE D
132c132
< __ZTVN3tbb6detail2r111unsafe_waitE S
---
> __ZTVN3tbb6detail2r111unsafe_waitE D

And test_global_control linked against v2021.1.0 fails with the same issue you mentioned above (btw, how did you get this error?)

[phprus]

@isaevil

Demangled:

phprus@mbp tmp % c++filt __ZTIN3tbb6detail2r111unsafe_waitE
typeinfo for tbb::detail::r1::unsafe_wait

phprus@mbp tmp % c++filt __ZTVN3tbb6detail2r111unsafe_waitE
vtable for tbb::detail::r1::unsafe_wait

Symbols with this PR:

cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=20 ../..
make -j8


# Get typeinfo for all exception:
nm -g appleclang_14.0_cxx20_64_release/libtbb.12.dylib | grep __ZTIN3tbb6detail2r
0000000000020610 S __ZTIN3tbb6detail2r110user_abortE
0000000000020590 S __ZTIN3tbb6detail2r111unsafe_waitE
0000000000020650 S __ZTIN3tbb6detail2r112missing_waitE
00000000000205d0 S __ZTIN3tbb6detail2r114bad_last_allocE


# Get vtable for all exception:
nm -g appleclang_14.0_cxx20_64_release/libtbb.12.dylib | grep __ZTVN3tbb6detail2r
00000000000205e8 S __ZTVN3tbb6detail2r110user_abortE
0000000000020568 S __ZTVN3tbb6detail2r111unsafe_waitE
0000000000020628 S __ZTVN3tbb6detail2r112missing_waitE
00000000000205a8 S __ZTVN3tbb6detail2r114bad_last_allocE

Symbols without this PR (current master, commit 9314fcf):

cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=20 ../..
make -j8

# Get typeinfo for all exception:
nm -g appleclang_14.0_cxx20_64_release/libtbb.12.dylib | grep __ZTIN3tbb6detail2r
00000000000205d0 S __ZTIN3tbb6detail2r110user_abortE
0000000000020610 S __ZTIN3tbb6detail2r112missing_waitE
0000000000020590 S __ZTIN3tbb6detail2r114bad_last_allocE

# Get vtable for all exception:
nm -g appleclang_14.0_cxx20_64_release/libtbb.12.dylib | grep __ZTVN3tbb6detail2r
00000000000205a8 S __ZTVN3tbb6detail2r110user_abortE
0000000000020640 S __ZTVN3tbb6detail2r111unsafe_waitE
00000000000205e8 S __ZTVN3tbb6detail2r112missing_waitE
0000000000020568 S __ZTVN3tbb6detail2r114bad_last_allocE

Without patch typeinfo for tbb::detail::r1::unsafe_wait is not exported and this exception is not captured correctly.

Clang:

phprus@mbp cxx20 % clang++ --version
Apple clang version 14.0.3 (clang-1403.0.22.14.1)
Target: arm64-apple-darwin22.5.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

For get this error build this PR without commit "Fix: ld: warning: cannot export hidden symbol typeinfo for tbb::detail::r1::unsafe_wait" (36f0108)

[phprus]

@isaevil

Symbols from release binary:

phprus@mbp oneapi-tbb-2021.9.0 % nm -g lib/libtbb.12.9.dylib | grep __ZTIN3tbb6detail2r
0000000000067b20 D __ZTIN3tbb6detail2r110user_abortE
0000000000067aa8 D __ZTIN3tbb6detail2r111unsafe_waitE
0000000000067b58 D __ZTIN3tbb6detail2r112missing_waitE
0000000000067ae0 D __ZTIN3tbb6detail2r114bad_last_allocE
phprus@mbp oneapi-tbb-2021.9.0 % nm -g lib/libtbb.12.9.dylib | grep __ZTVN3tbb6detail2r
00000000000616f0 S __ZTVN3tbb6detail2r110user_abortE
00000000000616a0 S __ZTVN3tbb6detail2r111unsafe_waitE
0000000000061718 S __ZTVN3tbb6detail2r112missing_waitE
00000000000616c8 S __ZTVN3tbb6detail2r114bad_last_allocE

Default build (RelWithDebInfo):

cmake -DCMAKE_VERBOSE_MAKEFILE=ON ../..
make -j8

Without PR:

phprus@mbp cxx % nm -g appleclang_14.0_cxx11_64_relwithdebinfo/libtbb.12.10.dylib | grep __ZTIN3tbb6detail2r
00000000000245c8 S __ZTIN3tbb6detail2r110user_abortE
0000000000024608 S __ZTIN3tbb6detail2r112missing_waitE
0000000000024588 S __ZTIN3tbb6detail2r114bad_last_allocE
phprus@mbp cxx % nm -g appleclang_14.0_cxx11_64_relwithdebinfo/libtbb.12.10.dylib | grep __ZTVN3tbb6detail2r
00000000000245a0 S __ZTVN3tbb6detail2r110user_abortE
0000000000024638 S __ZTVN3tbb6detail2r111unsafe_waitE
00000000000245e0 S __ZTVN3tbb6detail2r112missing_waitE
0000000000024560 S __ZTVN3tbb6detail2r114bad_last_allocE

With PR:

phprus@mbp cxx % nm -g appleclang_14.0_cxx11_64_relwithdebinfo/libtbb.12.10.dylib | grep __ZTIN3tbb6detail2r
0000000000024608 S __ZTIN3tbb6detail2r110user_abortE
0000000000024588 S __ZTIN3tbb6detail2r111unsafe_waitE
0000000000024648 S __ZTIN3tbb6detail2r112missing_waitE
00000000000245c8 S __ZTIN3tbb6detail2r114bad_last_allocE
phprus@mbp cxx % nm -g appleclang_14.0_cxx11_64_relwithdebinfo/libtbb.12.10.dylib | grep __ZTVN3tbb6detail2r
00000000000245e0 S __ZTVN3tbb6detail2r110user_abortE
0000000000024560 S __ZTVN3tbb6detail2r111unsafe_waitE
0000000000024620 S __ZTVN3tbb6detail2r112missing_waitE
00000000000245a0 S __ZTVN3tbb6detail2r114bad_last_allocE

I can't reproduce your environment and bug on local.
I don't see changing symbol types, only exporting a new symbol.

[isaevil]

@phprus When you refer to "Without PR" you still mean this PR but without changes to unsafe_wait? It looks like when we have these changes (-fvisibility=hidden global flag and and export attributes for entry points) it completely ignores *.def files we set because there is typeinfo for unsafe_wait in 2021.9 release binary.

The diff I've sent is current master built with Apple Clang vs your patch and the difference is that unsafe_wait was in initialized data segment and now it is in sbss segment.

Can you try compare symbols on Mac, but disable global visibility flags in CMakeLists.txt?

Notify: @pavelkumbrasev

[phprus]

@isaevil
No, "Wihtout PR" - current master (commit 88f73bb

See comment #1114 (comment) for symbol comparation.

[phprus]

@isaevil , @pavelkumbrasev

Legacy macOS 10.15 (x86_64), AppleClang 12.0.0.12000032:

Commit 88f73bb (current master without this PR):

cmake -DCMAKE_VERBOSE_MAKEFILE=ON ../..
make -j8


phprus@mbp-phprus cxx % nm -g appleclang_12.0_cxx11_64_relwithdebinfo/libtbb.12.10.dylib | grep __ZTIN3tbb6detail2r
000000000002c3f0 S __ZTIN3tbb6detail2r110user_abortE
000000000002c448 S __ZTIN3tbb6detail2r111unsafe_waitE
000000000002c430 S __ZTIN3tbb6detail2r112missing_waitE
000000000002c3b0 S __ZTIN3tbb6detail2r114bad_last_allocE

phprus@mbp-phprus cxx % nm -g appleclang_12.0_cxx11_64_relwithdebinfo/libtbb.12.10.dylib | grep __ZTVN3tbb6detail2r
000000000002c3c8 S __ZTVN3tbb6detail2r110user_abortE
000000000002c460 S __ZTVN3tbb6detail2r111unsafe_waitE
000000000002c408 S __ZTVN3tbb6detail2r112missing_waitE
000000000002c388 S __ZTVN3tbb6detail2r114bad_last_allocE

__ZTIN3tbb6detail2r111unsafe_waitE - is exported, but begin from Xcode 13.2.1 is NOT exported.
I can't check Xcode 13.2.1 on x86_64, because my legacy mac does not support this version.

As you can see, the symbols type is S.

[phprus]

Rebased to resolve conflicts with current master.

@isaevil, @pavelkumbrasev

[jdumas]

I admire your patience @phprus 😅 . I would also love to see this feature merged one day...

[pavelkumbrasev]

@phprus It seems like that these two issues we are trying to solve here are orthogonal. First one about just adding export attributes doesn't bring any ABI breaks at first glance and can be merged, but the current second issue's resolution about fvisibility=hidden on application and unsafe_wait does bring an ABI break for existing application which is not desired. I think the second issue requires additional time and effort to investigate, but while being separated from the main content of this PR. So I would suggest doing these things:

1. Revert tbb::unsafe_wait changes in this PR
2. Remove setting of -fvisilibilty=hidden globally in root CMakelists.txt

I believe this comment is still on the table. oneTBB should preserve backward compatibility.

[phprus]

@pavelkumbrasev

I believe this comment is still on the table. oneTBB should preserve backward compatibility.

You are right!

On my tests, commit 2887a2f (Workaround for tbb::detail::r1::unsafe_wait broken ABI on macOS) fix this ABI backward compatibility issue.

My previous comment about it:
#1114 (comment)

Please check ABI compatibility in your environment.

[pavelkumbrasev]

@isaevil could you please take a look ?

[phprus]

@isaevil could you please check ABI on macOS for the current version of PR?

[phprus]

CI errors is not related with this PR:

The following tests FAILED:
     33 - test_buffer_node (Timeout)

[saschasc]

I have the same issue. I'm trying to build onetbb with gcc 14.x on macOS. So, I really appreciate the effort to get this in the next version.

What about the failing test? Is this known to be flaky? Can the CI be rerun?

[saschasc]

@isaevil Anything blocking this PR for being merged beside the reviews? Would be really great if this would make it to the next version.

[isaevil]

@saschasc I am going to validate the patch in terms of testing failures/ABI compatibility this or next week.

[isaevil]

@phprus Could you please rebase your PR once again so it would include #1489?

[phprus]

@isaevil rebased.

[isaevil]

I've compared ABI between current master branch and this PR on Linux and MacOS and seems like it is now the same. Workaround for unsafe_wait on MacOS seems to work as well.
@pavelkumbrasev what do you think?

[pavelkumbrasev]

Since visibility attribute in a sense duplicate Def files we will need to discuss removal of Def files.

[phprus]

Since visibility attribute in a sense duplicate Def files we will need to discuss removal of Def files.

During the review of this PR or separately?

[pavelkumbrasev]

I think separately, we will need to discuss it within the team.

[phprus]

@pavelkumbrasev what do you think about this PR?

[pavelkumbrasev]

@pavelkumbrasev what do you think about this PR?

It looks good, we just need to define a plan for removing the Def files.

[saschasc]

Thanks a lot for this PR and the reviews. Is it already known when the next release of oneTBB will be available. For macOS it would be great to have a fixed version as soon as possible. Thanks!

[isaevil]

Thanks a lot for this PR and the reviews. Is it already known when the next release of oneTBB will be available. For macOS it would be great to have a fixed version as soon as possible. Thanks!

FYI @omalyshe