Skip to content

[Bazel] Set HAVE_MALLINFO2=1 #125681

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aaronmondal
Copy link
Member

This silences the annoying warning from llvm/lib/Support/Process.cpp.

This silences the annoying warning from `llvm/lib/Support/Process.cpp`.
@llvmbot llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label Feb 4, 2025
@rupprecht
Copy link
Collaborator

mallinfo2 is relatively new, so while it should be available in a lot of places, it won't work on all versions of glibc. I don't know exact years/versions, but I think it's recent enough that we need to care about it. It'd be nice if we could autodetect this from bazel.

The bazelrc file in llvm sets -Wno-deprecated for both --config=generic_gcc and --config=generic_clang. Maybe we could add copts = ["-Wno-deprecated-declarations"] to this build target?

@rupprecht
Copy link
Collaborator

We could probably replace uses of #if HAVE_MALLINFO2 with #if __GLIBC_PREREQ(2, 33), which would mirror the same condition that sanitizers use:

INTERCEPTOR(__sanitizer_struct_mallinfo2, mallinfo2) {

@aaronmondal
Copy link
Member Author

Aargh what a rabbithole xDxD

So __GLIBC_PREREQ is defined by glibc's features.h. This means that if we substituted HAVE_MALLINFO2 with it we'd make compilation dependent on this dynamic value after configuration stage, making the build irreproducible, given the same configuration. I'd argue that this is actually somewhat suboptimal for the sanitizers as well, but at least those are not "core" libraries.

In the HAVE_MALLINFO2 case however we'd be opening up libllvm to this nondeterminism which would harm portability of the build (the support lib built on a glibc2.33 system wouldn't work or even crash on a glibc2.31 system without an easy way to opt out, forcing users to either modify the source or to use crosscompilation sysroots).

Ignoring the deprecation warnings also seems suboptimal as it increases the "surprise factor" of breaking changes (IMO this is also an issue witht the current -Wno-deprecated).

Fortunately, the easy way out seems to be to just make this as an actual Bazel config.

Regarding the default I think it should be fine to make it true by default. According to https://reviews.llvm.org/D117916 mallinfo2 was released if Feb 2021. In terms of mainstream distros Ubuntu 22.04 runs on glibc2.35 so that should be fine. Users of Ubuntu 20.04 (glibc2.31) would have to flip the flag but that reaches EOL in May 2025 anyways.

... is what i would've said, but now it turns out that we're not actually using platforms for config settings. So I gess that needs fixing first lol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants