-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
stdenv: introduce -frandom-seed #102251
stdenv: introduce -frandom-seed #102251
Conversation
@@ -0,0 +1,2 @@ | |||
#export NIX_SET_BUILD_ID=${SOURCE_DATE_EPOCH:1} | |||
export NIX_CFLAGS_COMPILE+=" -frandom-seed=${SOURCE_DATE_EPOCH:1} " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about
export NIX_CFLAGS_COMPILE+=" -frandom-seed=${SOURCE_DATE_EPOCH:1} " | |
export NIX_CFLAGS_COMPILE+=" -frandom-seed=$out " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good question. We also had that idea.
Andi's argument was that it would prevent content-addressable outputs since the output will ten always change if $out changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I even think that $SOURCE_DATE_EPOCH has that problem. I've not yet come up with a good idea here. Apparently, based on random googling, a single, static value that is the same across all builds isn't great. This went to the extend of being different per input file (e.g. it's hash). I am not convinced or an expert here :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up going with the bash equivalent of basename $out
as I want to avoid embedding a store path into the binary (and in e.g. multi-output derivations carrying a reference).
This adds -frandom-seed to each compiler invocation in stdenv. The object here is to make the compierl invocations produce the same output every time they are called (for the same derivation). When the -frandom-seed option is not set the compiler will use a combination of random numbers (in GCC's case from /dev/urandom) and the durrent time to produce a "random" input per file. This can (among other things) lead to different ordering of symbols in the produced object files. For reason of reproducibility we prefer having the same derivation produce the exact same outputs. This is not a silver bullet but one way to tame the compiler.
@Ericson2314 Would be nice if you can give us some feedback on this. This effectively made derivations more reproducible. It solves all of the cases where e.g. symbol ordering in the build result would be random across rebuilds. This was discovered why looking at the current state of https://r13y.com and the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rsync is now reproducible
I think this broke GCC cross builds (i.e. when build != host == target) somehow, by causing cyclical references in the outputs (out <-> lib)
I bisected the issue with pkgsCross.powernv.gcc down to this merge, and it looks like it might have caused similar failures for other platforms' cross builds: I'm not sure why this is happening - any ideas? |
Looks like the out path ends up in the random seed, which is stored along with the rest of the command line args:
|
local randomseed="${out##*/}"
export NIX_CFLAGS_COMPILE+=" -frandom-seed=${randomseed:0:10}" should fix this issue, because the full hash is not present anymore. |
On 02:05 13.12.20, r-burns wrote:
Looks like the out path ends up in the random seed, which is stored along with the rest of the command line args:
```
strings /nix/store/89ircz9c1930mald5pqji1vb25ds1jsz-gcc-debug-9.3.0-powerpc64le-unknown-linux-gnu-lib/lib/libasan.so.5.0.0 | grep 2vbd2sf8mmkpv4z6fx4wd90qhka1hfdw
GNU C++11 9.3.0 -g -g -O2 -O2 -O2 -std=gnu++11 -fstack-protector-strong -fno-strict-overflow -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -fno-ipa-icf -fPIC -frandom-seed=2vbd2sf8mmkpv4z6fx4wd90qhka1hfdw-gcc-debug-9.3.0-powerpc64le-unknown-linux-gnu --param ssp-buffer-size=4
GNU C++11 9.3.0 -g -g -O2 -O2 -O2 -std=gnu++11 -fstack-protector-strong -fno-strict-overflow -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -fPIC -frandom-seed=2vbd2sf8mmkpv4z6fx4wd90qhka1hfdw-gcc-debug-9.3.0-powerpc64le-unknown-linux-gnu --param ssp-buffer-size=4
GNU C17 9.3.0 -g -g -O2 -O2 -O2 -fstack-protector-strong -fno-strict-overflow -fPIC -frandom-seed=2vbd2sf8mmkpv4z6fx4wd90qhka1hfdw-gcc-debug-9.3.0-powerpc64le-unknown-linux-gnu --param ssp-buffer-size=4
GNU C++11 9.3.0 -g -g -O2 -O2 -O2 -std=gnu++11 -fstack-protector-strong -fno-strict-overflow -fno-rtti -fno-exceptions -fPIC -frandom-seed=2vbd2sf8mmkpv4z6fx4wd90qhka1hfdw-gcc-debug-9.3.0-powerpc64le-unknown-linux-gnu --param ssp-buffer-size=4
```
It really shouldn't as we are not using the full store path for these
(notice the lack of /nix/store in the random seed).
|
It looks like the hash is enough:
|
On 09:40 14.12.20, Guillaume Girol wrote:
It looks like the hash is enough:
That sounds like a flaw in Nix but appears to be on purpose:
https://github.com/NixOS/nix/blame/1b79b5b983a6c775766bd0d1c7881042188998b8/src/libstore/references.cc#L91
While writing the code in the setup-hook I did expect this to be
implemented proper aka. scanning for the entire store path not just some
substring.
|
I fail to see how this is a flaw in nix - checking the hash is more robust to scripts joining individual path components, no? |
I had an idea for a different approach - what if this logic is moved to the cc-wrapper script, so that the random seed can depend on the input files? Then we can follow GCC's recommendation to use a different seed for each file. Anyway, the suggested trimmed seed seems to work for me so I will submit a patch. |
Motivation for this change
This is some random work that I did while looking at r13y.com and rsync.
Apparently the
gcc-unwrapped
attribute reproduces.cc @zimbatm
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)