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.

Sign up for GitHub

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

Jump to bottom

tcmalloc build failure when buiding envoy 1.31.5 #38230

Closed
VivekSubr opened this issue Jan 28, 2025 · 11 comments · May be fixed by #38235
Closed

tcmalloc build failure when buiding envoy 1.31.5 #38230

VivekSubr opened this issue Jan 28, 2025 · 11 comments · May be fixed by #38235
Labels
bug

Comments

@VivekSubr
Copy link

VivekSubr commented Jan 28, 2025 •
edited
Loading

Title: tcmalloc build failure

Description:
We were building envoy 1.31.5 release, for using with istio 1.23, but hit this build failure for it's tcmalloc dependency ->

ERROR: /mount-build/bazel_out/external/com_github_google_tcmalloc/tcmalloc/BUILD:296:11: Compiling tcmalloc/arena.cc failed: (Exit 1): clang failed: error executing command (from target @com_github_google_tcmalloc//tcmalloc:common)
  (cd /mount-build/bazel_out/sandbox/processwrapper-sandbox/2180/execroot/io_istio_proxy && \
  exec env - \
    BAZEL_COMPILER=clang \
    BAZEL_CXXOPTS='-stdlib=libc++' \
    BAZEL_LINKLIBS=-l%:libc++.a:-l%:libc++abi.a \
    BAZEL_LINKOPTS=-lm:-pthread \
    CC=/usr/bin/clang \
    CPP=/usr/bin/clang-cpp \
    CXX=/usr/bin/clang++ \
    CXXFLAGS='-stdlib=libc++' \
    LD=/usr/bin/ld.lld \
    LDFLAGS='-stdlib=libc++' \
    PATH=/bin:/usr/bin:/usr/local/bin \
    PWD=/proc/self/cwd \
  /usr/lib/llvm-18/bin/clang -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -Wunused-but-set-parameter -Wno-free-nonheap-object -fcolor-diagnostics -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections -fdata-sections '-stdlib=libc++' -MD -MF bazel-out/k8-opt/bin/external/com_github_google_tcmalloc/tcmalloc/_objs/common/arena.d '-frandom-seed=bazel-out/k8-opt/bin/external/com_github_google_tcmalloc/tcmalloc/_objs/common/arena.o' -gsplit-dwarf -g '-DBAZEL_CURRENT_REPOSITORY="com_github_google_tcmalloc"' -iquote external/com_github_google_tcmalloc -iquote bazel-out/k8-opt/bin/external/com_github_google_tcmalloc -iquote external/com_google_absl -iquote bazel-out/k8-opt/bin/external/com_google_absl '-DABSL_MIN_LOG_LEVEL=4' -fdebug-types-section -fPIC -Wno-deprecated-declarations '-Wno-error=deprecated-enum-enum-conversion' -DNULL_PLUGIN '-std=c++20' -fsized-deallocation -Wformat -Wformat-security -Werror -Wno-deprecated-declarations -Wno-deprecated-volatile -Wno-implicit-int-float-conversion -Wno-sign-compare -Wno-uninitialized -Wno-unused-function -Wno-unused-variable -no-canonical-prefixes -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' -c external/com_github_google_tcmalloc/tcmalloc/arena.cc -o bazel-out/k8-opt/bin/external/com_github_google_tcmalloc/tcmalloc/_objs/common/arena.o)
# Configuration: 1fb1bc580b88ab4ae7fd27f19aa6f78238ea08ee16d9cdeb7c0f63e696bed59e
# Execution platform: @local_config_platform//:host

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from external/com_github_google_tcmalloc/tcmalloc/arena.cc:18:
In file included from external/com_github_google_tcmalloc/tcmalloc/static_vars.h:40:
In file included from external/com_github_google_tcmalloc/tcmalloc/page_allocator.h:25:
In file included from external/com_github_google_tcmalloc/tcmalloc/huge_page_aware_allocator.h:30:
external/com_github_google_tcmalloc/tcmalloc/page_allocator_interface.h:83:46: error: returning variable 'info_' by reference requires holding mutex 'pageheap_lock' [-Werror,-Wthread-safety-reference-return]
   83 |   const PageAllocInfo& info() const { return info_; }

Is there any easy way to fix this failure?
@VivekSubr VivekSubr added bug triage Issue requires triage labels Jan 28, 2025
@VivekSubr VivekSubr changed the title tcmalloc build failure when buiding envoy 1.31 tcmalloc build failure when buiding envoy 1.31.5 Jan 28, 2025
@alyssawilk alyssawilk removed the triage Issue requires triage label Jan 28, 2025
@alyssawilk
Copy link
Contributor

This should be fixable locally if you use the exact build change we do in CI, but it looks like that issue was fixed in tcmalloc a few years ago and we're not using the latest version. cc @phlax any reason to not bump our tcmalloc dep to make Envoy a little easier to build here?

@yanavlasov
Copy link
Contributor

I will try to upgrade tcmalloc to see what happens.
But this specific build issue should be addressed by this patch: bazel/tcmalloc.patch

@yanavlasov
Copy link
Contributor

It looks like it might not be an easy update, as new tcmalloc fails bazel analysis.

@yanavlasov yanavlasov mentioned this issue Jan 28, 2025
Draft
@phlax
Copy link
Member

phlax commented Jan 28, 2025

yeah - @krinkinmu has been looking at updating tcmalloc recently as part of the llvm update - i forget the exact issue but there was some blocker using recent versions - its possible we could update to something in between - or as @yanavlasov is trying above - patch it up-to-date

@yanavlasov
Copy link
Contributor

Is it possible patch for tcmalloc is lost when fetching dependencies for istio-proxy build?

In any case I have #38235 which should remove the need for patching this part of code.

@krinkinmu
Copy link
Contributor

krinkinmu commented Jan 28, 2025 •
edited
Loading

Thanks @phlax for cc-ing me. Yes, that's the same issue I hit, I ended up working around it with #38120.

The issue was fixed upstream as well, but updating tcmalloc caused some other problems, so I decided to patch for now and return to it once we finish with the toolchain update.

I think #38235 is hitting one of those problems now. Basically the story is that newer tcmalloc version relies on some relatively new version of assembly directive that was not supported by version we have in our CI image.

I'm pretty sure that the issue can be fixed by either the updating binutils or replacing the binutils version of assembly with an llvm one. Probably there are simpler ways to deal with the issue, but I didn't spend much time figuring out what to do with that yet.

@krinkinmu
Copy link
Contributor

FWIW, the issue does not happen on the llvm-14 that is currently used by Envoy CI image, so if it's possible, the easiest fix is to use clang-14 instead of clang-18 to build Envoy.

@phlax
Copy link
Member

phlax commented Jan 28, 2025

But this specific build issue should be addressed by this patch: bazel/tcmalloc.patch

k - so probably its llvm version related

i guess the question is whether we want to backport the patch

@krinkinmu
Copy link
Contributor

I'm happy to backport the patch to the supported Envoy release branches if it helps. Though, if we are talking about builds on clang-18, I think we might need to backport a few more changes besides that one.

@phlax
Copy link
Member

phlax commented Jan 28, 2025

yeah was thinking similar

@VivekSubr
Copy link
Author

Thanks @krinkinmu , it builds with clang-14, although not with simple 'apt install clang'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update tcmalloc to 5da4a882 (2024-12-27) yanavlasov/envoy
5 participants
@phlax @krinkinmu @yanavlasov @VivekSubr @alyssawilk