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

don't pass CFLAGS and fix _FORTIFY_SOURCE errors #116

Merged
merged 3 commits into from
Feb 6, 2025

Conversation

th7nder
Copy link

@th7nder th7nder commented Jan 31, 2025

Fixes NixOS/nixpkgs#370494 and #108.
Long-ass explanation is here: NixOS/nixpkgs#370494 (comment).

When running under nix environment _FORTIFY_SOURCE=3 is set by default.
Currently the CFLAGS are passed down to the underlying jemalloc compiler.

If we're running cargo build, it means that something along the lines:
-O0 -ffunction-sections -fdata-sections -fPIC -m64 -Wall is passed down (optimisation disabled, because debug mode).
Because optimisations are disabled, we're getting errors:

In file included from /nix/store/hn4s3cq7368jkb2db02gzxhdzfa3g9zp-glibc-2.40-36-dev/include/errno.h:25,
                 from conftest.c:101:
/nix/store/hn4s3cq7368jkb2db02gzxhdzfa3g9zp-glibc-2.40-36-dev/include/features.h:422:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
  422 | #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
      |    ^~~~~~~

This simply removes passing down the CFLAGS, so they can be set automatically by underlying autoconfigures and not dependent on the build profile.
Plus jemalloc/jemalloc#1196 was solved long time ago, so I think the reason those were added is gone.

P.S this issue is reproducible by running the master branch, flake.nix in the root repo

{
  inputs = {
    nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
    rust-overlay = {
      url = "github:oxalica/rust-overlay";
      inputs = {
        nixpkgs.follows = "nixpkgs";
      };
    };
  };
  outputs = { self, nixpkgs, flake-utils, rust-overlay }:
    flake-utils.lib.eachDefaultSystem (system:
      let
        overlays = [ (import rust-overlay) ];
        pkgs = import nixpkgs {
          inherit system overlays;
        };
        rustToolchain = pkgs.pkgsBuildHost.rust-bin.stable.latest.default;
        buildInputs = with pkgs; [
          rustToolchain
        ];
      in
      with pkgs;
      {
        devShells.default = mkShell {
          inherit buildInputs;
        };
      }
    );
}

and running

$ git add flake.nix
$ nix develop
$ cargo -vv build

Copy link

ti-chi-bot bot commented Jan 31, 2025

Welcome @th7nder! It looks like this is your first PR to tikv/jemallocator 🎉

@th7nder th7nder force-pushed the fix-not-building-with-fortify-source branch from 52accb4 to b8885e4 Compare January 31, 2025 01:32
@th7nder th7nder force-pushed the fix-not-building-with-fortify-source branch from b8885e4 to 7abada8 Compare January 31, 2025 01:36
Copy link
Member

@BusyJay BusyJay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM

Konrad Stepniak added 2 commits January 31, 2025 16:17
Signed-off-by: Konrad Stepniak <[email protected]>
Signed-off-by: Konrad Stepniak <[email protected]>
@th7nder th7nder requested a review from BusyJay January 31, 2025 15:19
@BusyJay BusyJay merged commit fa4486d into tikv:main Feb 6, 2025
10 checks passed
@BusyJay
Copy link
Member

BusyJay commented Feb 6, 2025

Thank you!

BusyJay added a commit that referenced this pull request Feb 6, 2025
BusyJay added a commit that referenced this pull request Feb 6, 2025
@BusyJay
Copy link
Member

BusyJay commented Feb 6, 2025

When running under nix environment _FORTIFY_SOURCE=3 is set by default.

warning _FORTIFY_SOURCE requires compiling with optimization (-O)

I think this means it's wrong to set _FORTIFY_SOURCE=3 by default.

@javierhonduco
Copy link

Thanks so much for the fix! Would really appreciate a release if possible 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jemalloc (jemallocator) C toolchain / compilation issues
3 participants