gcc12, gcc13: always mangle FILE into invalid store path #255192

trofi · 2023-09-14T22:06:39Z

Before the change macros like assert() embedded paths to source file into an executable binary. Examples are nlohmann_json embedding into nix:

$ nix shell nixpkgs#nix nixpkgs#binutils-unwrapped nixpkgs#which
$ strings $(which nix) | fgrep nlohmann_json
/nix/store/5xih6daf5g3hpa0wc5vs2cgrhakn4s0j-nlohmann_json-3.11.2/include/nlohmann/json.hpp
/nix/store/5xih6daf5g3hpa0wc5vs2cgrhakn4s0j-nlohmann_json-3.11.2/include/nlohmann/detail/output/serializer.hpp
/nix/store/5xih6daf5g3hpa0wc5vs2cgrhakn4s0j-nlohmann_json-3.11.2/include/nlohmann/detail/conversions/to_chars.hpp
/nix/store/5xih6daf5g3hpa0wc5vs2cgrhakn4s0j-nlohmann_json-3.11.2/include/nlohmann/detail/input/lexer.hpp
/nix/store/5xih6daf5g3hpa0wc5vs2cgrhakn4s0j-nlohmann_json-3.11.2/include/nlohmann/detail/iterators/iter_impl.hpp
/nix/store/5xih6daf5g3hpa0wc5vs2cgrhakn4s0j-nlohmann_json-3.11.2/include/nlohmann/detail/input/json_sax.hpp
/nix/store/5xih6daf5g3hpa0wc5vs2cgrhakn4s0j-nlohmann_json-3.11.2/include/nlohmann/detail/iterators/iteration_proxy.hpp
/nix/store/5xih6daf5g3hpa0wc5vs2cgrhakn4s0j-nlohmann_json-3.11.2/include/nlohmann/detail/input/parser.hpp

Sometimes these dependencies are not too large: nlohmann_json is "just" 1MB of headers code.

But sometimes the -dev outputs contain python scripts and other build-only dependencies. The example is lttng-ust.

Description of changes

Things done

trofi · 2023-09-14T22:12:18Z

And examples like pipewire reduced their closure down for about 20% (~90MB):

Before:

$ nix path-info -rsSh $(nix-build '<nixpkgs>' -A pipewire.out) | nl | tail -n1
   219  /nix/store/ff0w34nr807in3b1swmqklxy9g9v5hg9-pipewire-0.3.79 1.4M  543.4M

After:

$ nix path-info -rsSh $(nix-build ~/n -A pipewire.out) | nl | tail -n1
   207  /nix/store/hl4dffvc73nsh3zfbji0y7h9lcnrk14b-pipewire-0.3.79 1.3M  452.0M

Mindavi · 2023-09-15T06:26:41Z

This seems like a pretty safe change to me and will reduce closure size in (presumably) all sorts of places. My bash-foo is not up to speed, so I'll leave that to others.

trofi · 2023-09-15T06:57:26Z

Even dedupe is still able to fail qemu build, but a bit later:

-----------
Command line: `gcc -m64 -mcx16 /build/qemu-8.1.0/build/meson-private/tmpbyikv8nc/testfile.c -o /build/qemu-8.1.0/build/meson-private/tmpbyikv8nc/output.exe -D_FILE_OFFSET_BITS=64 -O0 -Wl,--start-group -laio -Wl,--end-group -Wl,--allow-shlib-undefined` -> 1
stderr:
gcc: fatal error: cannot execute '/nix/store/yabgsv2ahyxk7v8n1msp4z8ybaw12ham-gcc-14.0.0/libexec/gcc/x86_64-unknown-linux-gnu/14.0.0/cc1': execv: Argument list too long
compilation terminated.
-----------

Will try a response file instead. Converting to draft.

trofi · 2023-09-15T17:28:05Z

An attempt to use response files populated in setup hooks has at least 3 problems:

expandResponseParams is empty in early bootstrap and that triggers a deficiency in gcc-8
setupHook can't create files as nix develop executes setupHooks in a separate derivation
something in response file file presence breaks -lstdc++ file detection in audiofile package

trofi · 2023-09-21T18:43:44Z

Response files would not help shorter term either: https://trofi.github.io/posts/299-maximum-argument-count-on-linux-and-in-gcc.html

It might be simpler to patch gcc and clang directly until response file infrastructure is ready.

trofi · 2023-09-23T21:56:46Z

Filed https://gcc.gnu.org/PR111527 for longer term solution in gcc. Switched to direct gcc patching to mangle NIX_STORE references out of __FILE__ macros.

The patches still are able to reduce nix and pipewire closure. Ready for review.

Mindavi · 2023-09-24T09:35:29Z

pkgs/development/compilers/gcc/patches/12/mangle-NIX_STORE-in-__FILE__.patch

+it's not a problem, but for headers in `/nix/store` this causes `-dev`
+inputs to be retained in runtime closure.
+
+Typical examples are `nix` -> `nlonhamm_json` and `pipewire` ->


Typo: nlohmann

Oh, good catch! Fixed typo in 3 places.

Mindavi

I think this is a fine solution and it probably won't hurt any users outside. FILE is mostly an indication anyway.

trofi · 2023-09-24T19:31:34Z

As a longer-term workaround proposed limit increase in linux kernel as well: https://lkml.org/lkml/2023/9/24/381

RaitoBezarius · 2023-10-04T19:36:39Z

pkgs/development/compilers/gcc/patches/12/mangle-NIX_STORE-in-__FILE__.patch

+   maps = map;
+ }
+
+/* Forward declatration for a $NIX_STORE remap hack below. */


Suggested change

+/* Forward declatration for a $NIX_STORE remap hack below. */

+/* Forward declaration for a $NIX_STORE remap hack below. */

Updated the patch. Thank you!

RaitoBezarius · 2023-10-04T19:37:35Z

pkgs/development/compilers/gcc/patches/13/mangle-NIX_STORE-in-__FILE__.patch

+   maps = map;
+ }
+
+/* Forward declatration for a $NIX_STORE remap hack below. */


Suggested change

+/* Forward declatration for a $NIX_STORE remap hack below. */

+/* Forward declaration for a $NIX_STORE remap hack below. */

Updated the patch. Thank you!

RaitoBezarius

I do not have enough expertise in GCC to take responsibility. But the patch seems sane, the solution seems reasonable. I am in favor of this change.

Artturin · 2023-10-04T19:41:08Z

Building pkgsCross.aarch64-multiplatform.sway

Ericson2314 · 2023-10-04T19:49:35Z

@trofi Have you tried to submit this patch upstream? I am recalling how I never got around to that with the Binutils patch, and then it became hard to maintain and that patch was puled out (by you no less :P).

I sorely miss that patch (now, because LLVM depends on libbfd for the LTO plugin, I always end up recompiling LLVM for new arches even though it is multitarget). I don't want you to lose this patch and miss it too. So I think the lesson is to submit upstream first.

To be clear I am definitely not saying we should block doing this on them accepting the patch. Rather that there should at least be an email thread we can link in Nixpkgs, and have the upstreaming process be started. Perfectly fine if it a takes a while for it to resolve itself.

Without the change `__FILE__` used in static inline functions in headers embed paths to header files into executable images. For local headers it's not a problem, but for headers in `/nix/store` this causes `-dev` inputs to be retained in runtime closure. Typical examples are `nix` -> `nlonhmann_json` and `pipewire` -> `lttng-ust.dev`. Ideally we would like to use `-fmacro-prefix-map=` feature of `gcc` as: -fmacro-prefix-map=/nix/store/$hash1-nlohmann-json-ver=/nix/store/eeee.eee-nlohmann-json-ver -fmacro-prefix-map=/nix/... In practice it quickly exhausts argument lengtth limit due to `gcc` deficiency: https://gcc.gnu.org/PR111527 Until it;s fixed let's hardcode header mangling if $NIX_STORE variable is present in the environment. Tested as: $ printf "# 0 \"/nix/store/01234567890123456789012345678901-pppppp-vvvvvvv\" \nconst char * f(void) { return __FILE__; }" | NIX_STORE=/nix/store ./gcc/xgcc -Bgcc -x c - -S -o - ... .string "/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-pppppp-vvvvvvv" ... Mangled successfully.

trofi · 2023-10-04T20:30:31Z

I am recalling how I never got around to that with the Binutils patch, and then it became hard to maintain and that patch was puled out (by you no less :P).

I sorely miss that patch (now, because LLVM depends on libbfd for the LTO plugin, I always end up recompiling LLVM for new arches even though it is multitarget). I don't want you to lose this patch and miss it too. So I think the lesson is to submit upstream first.

binutils patch context: Heh, it's still there for outdated 2.38 if you want to have a look: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/tools/misc/binutils/2.38/build-components-separately.patch . In case of this patch it's very invasive to the build system. binutils introduced extra internal libraries (and nixpkgs attributes) which does not make it easy to port and switch back/forth with the patch.

I agree downstream changes are a problem. I hoped gcc hack would be more self-contained: it's ok to drop the patch and get larger closures (no extra changes need to be proliferated to the rest of nixpkgs).

To be clear I am definitely not saying we should block doing this on them accepting the patch. Rather that there should at least be an email thread we can link in Nixpkgs, and have the upstreaming process be started. Perfectly fine if it a takes a while for it to resolve itself.

@trofi Have you tried to submit this patch upstream?

This patch is a bit hacky as it hardcodes NIX_STORE_DIR variable and assumes rigid hash structure. More generic mangling mechanism might be more acceptable (some kind of template-based or regex-based mangling).

If you think it's fine as is I can propose to send it upstream as well. I can also do it optional upstream with a compile-time "on" flag.

Longer term I would prefer to be able to pass megabytes of options to gcc as is (and to clang as a byproduct):

trofi · 2023-10-04T21:20:30Z

Sent the problem statement with this proof-of-concept patch to gcc mailing list: https://gcc.gnu.org/pipermail/gcc/2023-October/242636.html

Artturin · 2023-10-05T00:32:38Z

pkgsCross.aarch64-multiplatform.sway built, no runtime closure size diff.

trofi · 2023-10-05T05:39:25Z

pkgsCross.aarch64-multiplatform.sway built, no runtime closure size diff.

sway does not have a very big runtime closure. I found pipewire (and it's users) a bit different.

Artturin

Build-wise it works

dotlambda · 2023-11-21T09:13:51Z

This broke python3.pkgs.deltachat: #267870 (comment)

dotlambda · 2023-11-21T09:19:51Z

pkgs/development/compilers/gcc/patches/12/mangle-NIX_STORE-in-__FILE__.patch

+  -fmacro-prefix-map=/nix/store/$hash1-nlohmann-json-ver=/nix/store/eeee.eee-nlohmann-json-ver
+  -fmacro-prefix-map=/nix/...
+
+In practice it quickly exhausts argument lengtth limit due to `gcc`


Suggested change

In practice it quickly exhausts argument lengtth limit due to `gcc`

In practice it quickly exhausts argument length limit due to `gcc`

Good catch! Proposed the fix as #268923

dotlambda · 2023-11-21T09:20:33Z

pkgs/development/compilers/gcc/patches/12/mangle-NIX_STORE-in-__FILE__.patch

+In practice it quickly exhausts argument lengtth limit due to `gcc`
+deficiency: https://gcc.gnu.org/PR111527
+
+Until it;s fixed let's hardcode header mangling if $NIX_STORE variable


Suggested change

Until it;s fixed let's hardcode header mangling if $NIX_STORE variable

Until it's fixed let's hardcode header mangling if $NIX_STORE variable

Good catch! Proposed the fix as #268923

trofi · 2023-11-21T09:59:27Z

This broke python3.pkgs.deltachat: #267870 (comment)

Good find! Proposed the fix as #268920

The 23.11 release of NixOS has caused GCC to mangle __FILE__ names withing the nix-store. See <NixOS/nixpkgs#255192>. This causes gcov to fail with file-not-found errors: (DEBUG) Running gcov: 'gcov /build/source/rsort.gcda --branch-counts --branch-probabilities --demangled-names --hash-filenames --object-directory /build/source' in '/build/source' (ERROR) GCOV produced the following errors processing /build/source/rsort.gcda: Cannot open source file /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-glibc-2.38-27-dev/include/bits/string_fortified.h To work around this issue we add the --relative-only flag to gcov instucting it to not consider files with absoulte paths, thus excluding header files in the nix-store. Note: these files were already filtered out by gcovr, so the resulting documentation hasn't really changed.

trofi requested a review from Ericson2314 as a code owner September 14, 2023 22:06

trofi requested a review from a user September 14, 2023 22:06

ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux-stdenv This PR causes stdenv to rebuild 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Sep 15, 2023

trofi marked this pull request as draft September 15, 2023 06:57

trofi mentioned this pull request Sep 15, 2023

bootstrapFiles should be updated periodically to avoid arcane bugs in stale binaries #253713

Closed

trofi force-pushed the cc-wrapper-no-__FILE_-refs branch from a3fc014 to 20b60ba Compare September 15, 2023 11:17

trofi mentioned this pull request Sep 16, 2023

nix develop silently breaks file side-effects in hooks NixOS/nix#8987

Open

trofi force-pushed the cc-wrapper-no-__FILE_-refs branch from 20b60ba to ad1c188 Compare September 23, 2023 19:33

trofi changed the title ~~cc-wrapper/setup-hook.sh: mangle __FILE__ macros to not point to store~~ gcc12, gcc13: always mangle __FILE__ into invalid store path Sep 23, 2023

trofi marked this pull request as ready for review September 23, 2023 21:56

ofborg bot removed the 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild label Sep 23, 2023

Mindavi reviewed Sep 24, 2023

View reviewed changes

Mindavi approved these changes Sep 24, 2023

View reviewed changes

trofi force-pushed the cc-wrapper-no-__FILE_-refs branch from ad1c188 to bea823c Compare September 24, 2023 09:59

RaitoBezarius reviewed Oct 4, 2023

View reviewed changes

RaitoBezarius approved these changes Oct 4, 2023

View reviewed changes

trofi force-pushed the cc-wrapper-no-__FILE_-refs branch from bea823c to 5371767 Compare October 4, 2023 20:16

Artturin approved these changes Oct 5, 2023

View reviewed changes

Artturin merged commit b64b1ed into NixOS:staging Oct 8, 2023
9 checks passed

trofi deleted the cc-wrapper-no-__FILE_-refs branch October 8, 2023 06:00

dotlambda reviewed Nov 21, 2023

View reviewed changes

roconnor-blockstream mentioned this pull request Nov 30, 2023

Only process local files with gcov BlockstreamResearch/simplicity#208

Merged

symphorien mentioned this pull request Jan 7, 2024

gcc: prevent runtime references via __FILE__ but in a reversible manner #279455

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gcc12, gcc13: always mangle FILE into invalid store path #255192

gcc12, gcc13: always mangle FILE into invalid store path #255192

trofi commented Sep 14, 2023 •

edited

Loading

trofi commented Sep 14, 2023

Mindavi commented Sep 15, 2023

trofi commented Sep 15, 2023

trofi commented Sep 15, 2023

trofi commented Sep 21, 2023

trofi commented Sep 23, 2023

Mindavi Sep 24, 2023

trofi Sep 24, 2023

Mindavi left a comment

trofi commented Sep 24, 2023

RaitoBezarius Oct 4, 2023

trofi Oct 4, 2023

RaitoBezarius Oct 4, 2023

trofi Oct 4, 2023

RaitoBezarius left a comment

Artturin commented Oct 4, 2023

Ericson2314 commented Oct 4, 2023

trofi commented Oct 4, 2023

trofi commented Oct 4, 2023

Artturin commented Oct 5, 2023

trofi commented Oct 5, 2023

Artturin left a comment

dotlambda commented Nov 21, 2023

dotlambda Nov 21, 2023

trofi Nov 21, 2023

dotlambda Nov 21, 2023

trofi Nov 21, 2023

trofi commented Nov 21, 2023

	+/* Forward declatration for a $NIX_STORE remap hack below. */
	+/* Forward declaration for a $NIX_STORE remap hack below. */

	In practice it quickly exhausts argument lengtth limit due to `gcc`
	In practice it quickly exhausts argument length limit due to `gcc`

	Until it;s fixed let's hardcode header mangling if $NIX_STORE variable
	Until it's fixed let's hardcode header mangling if $NIX_STORE variable

gcc12, gcc13: always mangle __FILE__ into invalid store path #255192

gcc12, gcc13: always mangle __FILE__ into invalid store path #255192

Conversation

trofi commented Sep 14, 2023 • edited Loading

Description of changes

Things done

trofi commented Sep 14, 2023

Mindavi commented Sep 15, 2023

trofi commented Sep 15, 2023

trofi commented Sep 15, 2023

trofi commented Sep 21, 2023

trofi commented Sep 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mindavi left a comment

Choose a reason for hiding this comment

trofi commented Sep 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RaitoBezarius left a comment

Choose a reason for hiding this comment

Artturin commented Oct 4, 2023

Ericson2314 commented Oct 4, 2023

trofi commented Oct 4, 2023

trofi commented Oct 4, 2023

Artturin commented Oct 5, 2023

trofi commented Oct 5, 2023

Artturin left a comment

Choose a reason for hiding this comment

dotlambda commented Nov 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trofi commented Nov 21, 2023

gcc12, gcc13: always mangle FILE into invalid store path #255192

gcc12, gcc13: always mangle FILE into invalid store path #255192

trofi commented Sep 14, 2023 •

edited

Loading