Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some dependent projects break: undefined reference to folly::detail::base64_detail::base64Decode_SSE4_2(char const*, char const*, char*) #1823

Closed
yurivict opened this issue Jul 19, 2022 · 8 comments

Comments

@yurivict
Copy link

yurivict commented Jul 19, 2022

proxygen-2022.07.18.00 fails:

ld: error: /usr/local/lib/libfolly.so.0.58.0-dev: undefined reference to folly::detail::base64_detail::base64Decode_SSE4_2(char const*, char const*, char*) [--no-allow-shlib-undefined]
ld: error: /usr/local/lib/libfolly.so.0.58.0-dev: undefined reference to folly::detail::base64_detail::base64URLEncode_SSE4_2(char const*, char const*, char*) [--no-allow-shlib-undefined]
ld: error: /usr/local/lib/libfolly.so.0.58.0-dev: undefined reference to folly::detail::base64_detail::base64Encode_SSE4_2(char const*, char const*, char*) [--no-allow-shlib-undefined]
c++: error: linker command failed with exit code 1 (use -v to see invocation)

See facebook/proxygen#419

clang-13
FreeBSD 13.1

@Orvid
Copy link
Contributor

Orvid commented Jul 19, 2022

I suspect 9d6acb3 probably needed to be included for things to work. Could you try with that applied? If it works, I can release a patch version of folly and get everything else updated to that patch version.

@yurivict
Copy link
Author

It fails the same way with this patch.

@MagRicky
Copy link

I am seeing the same issue when installing CacheLib

make[2]: Leaving directory '/root/Desktop/Ritesh/new/CacheLib/build-folly'
[ 99%] Built target folly_exception_counter
../../../libfolly.so.0.58.0-dev: undefined reference to folly::detail::base64_detail::base64Decode_SSE4_2(char const*, char const*, char*)' ../../../libfolly.so.0.58.0-dev: undefined reference to folly::detail::base64_detail::base64Encode_SSE4_2(char const*, char const*, char*)'
../../../libfolly.so.0.58.0-dev: undefined reference to `folly::detail::base64_detail::base64URLEncode_SSE4_2(char const*, char const*, char*)'
collect2: error: ld returned 1 exit status
make[2]: *** [folly/logging/example/CMakeFiles/logging_example.dir/build.make:113: folly/logging/example/logging_example] Error 1
make[2]: Leaving directory '/root/Desktop/Ritesh/new/CacheLib/build-folly'
make[1]: *** [CMakeFiles/Makefile2:433: folly/logging/example/CMakeFiles/logging_example.dir/all] Error 2
make[1]: Leaving directory '/root/Desktop/Ritesh/new/CacheLib/build-folly'
make: *** [Makefile:130: all] Error 2
build-package.sh: error: make failed
build.sh: error: failed to build dependency 'folly'

@cho-m
Copy link

cho-m commented Jul 22, 2022

I also hit this issue building folly in Homebrew (Homebrew/homebrew-core#106363) as our Linux runner builds folly with -march=core2 (i.e. no SSE4.2).

At least for Homebrew failure with v2022.07.18.00 tag, the following diff worked to avoid compilation of SSE4.2 code when it wasn't supported on target.


diff --git a/folly/detail/base64_detail/Base64Api.cpp b/folly/detail/base64_detail/Base64Api.cpp
index b02148a64..ad074525b 100644
--- a/folly/detail/base64_detail/Base64Api.cpp
+++ b/folly/detail/base64_detail/Base64Api.cpp
@@ -19,13 +19,13 @@
 #include <folly/detail/base64_detail/Base64Api.h>
 #include <folly/detail/base64_detail/Base64SWAR.h>
 
-#if FOLLY_X64
+#if FOLLY_X64 && FOLLY_SSE_PREREQ(4, 2)
 #include <folly/detail/base64_detail/Base64_SSE4_2.h>
 #endif
 
 namespace folly::detail::base64_detail {
 
-#if FOLLY_X64
+#if FOLLY_X64 && FOLLY_SSE_PREREQ(4, 2)
 Base64RuntimeImpl base64EncodeSelectImplementation() {
   if (folly::CpuId().sse42()) {
     return {

@Orvid
Copy link
Contributor

Orvid commented Jul 22, 2022

Sorry it's taken so long to actually get this fix landed, 437b29d should fix this issue.

gleyba pushed a commit to flarebuild/folly that referenced this issue Jul 24, 2022
Summary:
Pull Request resolved: facebook#1776

This is a dangerous trait that we do not really need.

Effectively reverts {D35615227 (facebook@ad7ec0b)}.

Addresses the following build failure observed in the Github Actions build iunder Windows:

```
Test project Z:/build/folly
    Start 1823: traits_test.Traits.is_complete
1/1 Test facebook#1823: traits_test.Traits.is_complete ...***Failed    0.02 sec
Note: Google Test filter = Traits.is_complete
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from Traits
[ RUN      ] Traits.is_complete
D:\a\folly\folly\folly\test\TraitsTest.cpp(355): error: Value of: IncompleteEnum::IS_COMPLETE
  Actual: true
Expected: false
[  FAILED  ] Traits.is_complete (0 ms)
[----------] 1 test from Traits (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (0 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] Traits.is_complete

 1 FAILED TEST
```

Reviewed By: Orvid

Differential Revision: D36426055

fbshipit-source-id: c74a9709bbcdd579e3e943ff8cb151060e33d0e9
@cho-m
Copy link

cho-m commented Jul 26, 2022

v2022.07.25.00 fails to build for me in Homebrew environment on macOS (with -march=nehalem so SSE4.2 supported, AVX not):

Undefined symbols for architecture x86_64:
  "folly::detail::base64_detail::base64Decode_SSE4_2(char const*, char const*, char*)", referenced from:
      folly::detail::base64_detail::base64EncodeSelectImplementation() in Base64Api.cpp.o
...

Issues appear to be due to CMake logic that was previously referenced.

  • One issue is CMAKE_LIBRARY_ARCHITECTURE is not guaranteed to be set. Testing on macOS, this appears to be empty.
  • More important issue is the corresponding CMake logic that uses above value doesn't look right. The conditional uses IS_X86_64_ARCH but the actual behavior is for non-x86_64:

    folly/CMakeLists.txt

    Lines 235 to 241 in 6198adf

    if (${IS_X86_64_ARCH})
    message(
    STATUS
    "arch ${CMAKE_LIBRARY_ARCHITECTURE} does not match x86_64, "
    "skipping building SSE4.2 version of base64"
    )
    list(REMOVE_ITEM files ${FOLLY_DIR}/detail/base64_detail/Base64_SSE4_2.cpp)

Also, I wanted to check how -m compiled code will handle running on non-SSE/AVX-compatible architectures:

folly/CMakeLists.txt

Lines 254 to 255 in 6198adf

COMPILE_FLAGS
-msse4.2

Does folly handle CPU runtime detection for all scenarios?


EDIT: When built without -march and manually setting CMAKE_LIBRARY_ARCHITECTURE=x86_64, just built without SSE/AVX:

-- arch x86_64 does not match x86_64, skipping building SSE4.2 version of base64
-- arch x86_64 does not match x86_64, skipping setting SSE2/AVX2 compile flags for LtHash SIMD code

@rleshchinskiy
Copy link

It seems this test is the wrong way round? https://github.com/facebook/folly/blob/main/CMakeLists.txt#L233

facebook-github-bot pushed a commit that referenced this issue Aug 12, 2022
Summary:
See #1823

* CMAKE_LIBRARY_ARCHITECTURE is not always defined
* This doesn't work: `set(IS_X86_64_ARCH NOT(IS_X86_64_ARCH STREQUAL "-1"))`
* Two conditionals for `IS_X86_64_ARCH` were reversed

Reviewed By: bochko

Differential Revision: D38653631

fbshipit-source-id: c4b6f2820a2280356a7eb69bf0e9253434b5e750
@Orvid
Copy link
Contributor

Orvid commented Aug 17, 2022

Everything around this should be fixed at this point as far as I know.

@Orvid Orvid closed this as completed Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants