This commit adds a new helper template function to gc-alloc.hh (which is
probably where you want to look at first, O great reviewer [custom file
ordering in review diffs when]), which uses a type argument to determine
the size to allocate, rather than making the caller use sizeof().
Change-Id: Ib5d138d91a28bdda304a80db24ea9fb08669ad22
The purpose of this function has little to do with immutability. Value's
strings are never mutated, and the point of this function is to
singleton empty strings.
Change-Id: Ifd41dd952409d54e4d3de9ab59064e6928b0e480
This also bans various sneaking of negative numbers from the language
into unsuspecting builtins as was exposed while auditing the
consequences of changing the Nix language integer type to a newtype.
It's unlikely that this change comprehensively ensures correctness when
passing integers out of the Nix language and we should probably add a
checked-narrowing function or something similar, but that's out of scope
for the immediate change.
During the development of this I found a few fun facts about the
language:
- You could overflow integers by converting from unsigned JSON values.
- You could overflow unsigned integers by converting negative numbers
into them when going into Nix config, into fetchTree, and into flake
inputs.
The flake inputs and Nix config cannot actually be tested properly
since they both ban thunks, however, we put in checks anyway because
it's possible these could somehow be used to do such shenanigans some
other way.
Note that Lix has banned Nix language integer overflows since the very
first public beta, but threw a SIGILL about them because we run with
-fsanitize=signed-overflow -fsanitize-undefined-trap-on-error in
production builds. Since the Nix language uses signed integers, overflow
was simply undefined behaviour, and since we defined that to trap, it
did.
Trapping on it was a bad UX, but we didn't even entirely notice
that we had done this at all until it was reported as a bug a couple of
months later (which is, to be fair, that flag working as intended), and
it's got enough production time that, aside from code that is IMHO buggy
(and which is, in any case, not in nixpkgs) such as
lix-project/lix#445, we don't think
anyone doing anything reasonable actually depends on wrapping overflow.
Even for weird use cases such as doing funny bit crimes, it doesn't make
sense IMO to have wrapping behaviour, since two's complement arithmetic
overflow behaviour is so *aggressively* not what you want for *any* kind
of mathematics/algorithms. The Nix language exists for package
management, a domain where bit crimes are already only dubiously in
scope to begin with, and it makes a lot more sense for that domain for
the integers to never lose precision, either by throwing errors if they
would, or by being arbitrary-precision.
This change will be ported to CppNix as well, to maintain language
consistency.
Fixes: lix-project/lix#423
Change-Id: I51f253840c4af2ea5422b8a420aa5fafbf8fae75
Turns errors like this:
let
throwMsg = a: throw (a + " invalid bar");
in throwMsg "bullshit"
error:
… from call site
at «string»:3:4:
2| throwMsg = a: throw (a + " invalid bar");
3| in throwMsg "bullshit"
| ^
… while calling 'throwMsg'
at «string»:2:14:
1| let
2| throwMsg = a: throw (a + " invalid bar");
| ^
3| in throwMsg "bullshit"
… while calling the 'throw' builtin
at «string»:2:17:
1| let
2| throwMsg = a: throw (a + " invalid bar");
| ^
3| in throwMsg "bullshit"
error: bullshit invalid bar
into errors like this:
let
throwMsg = a: throw (a + " invalid bar");
in throwMsg "bullshit"
error:
… from call site
at «string»:3:4:
2| throwMsg = a: throw (a + " invalid bar");
3| in throwMsg "bullshit"
| ^
… while calling 'throwMsg'
at «string»:2:14:
1| let
2| throwMsg = a: throw (a + " invalid bar");
| ^
3| in throwMsg "bullshit"
… caused by explicit throw
at «string»:2:17:
1| let
2| throwMsg = a: throw (a + " invalid bar");
| ^
3| in throwMsg "bullshit"
error: bullshit invalid bar
Change-Id: I593688928ece20f97999d1bf03b2b46d9ac338cb
Turns errors like:
let
somepkg.src = throw "invalid foobar";
in somepkg.src.meta
error:
… while evaluating the attribute 'src.meta'
at «string»:2:3:
1| let
2| somepkg.src = throw "invalid foobar";
| ^
3| in somepkg.src.meta
… while calling the 'throw' builtin
at «string»:2:17:
1| let
2| somepkg.src = throw "invalid foobar";
| ^
3| in somepkg.src.meta
error: invalid foobar
into errors like:
let
somepkg.src = throw "invalid foobar";
in somepkg.src.meta
error:
… while evaluating the attribute 'src.meta'
at «string»:2:3:
1| let
2| somepkg.src = throw "invalid foobar";
| ^
3| in somepkg.src.meta
… while evaluating 'somepkg.src' to select 'meta' on it
at «string»:3:4:
2| somepkg.src = throw "invalid foobar";
3| in somepkg.src.meta
| ^
… while calling the 'throw' builtin
at «string»:2:17:
1| let
2| somepkg.src = throw "invalid foobar";
| ^
3| in somepkg.src.meta
error: invalid foobar
And for type errors, from:
let
somepkg.src = "I'm not an attrset";
in somepkg.src.meta
error:
… while evaluating the attribute 'src.meta'
at «string»:2:3:
1| let
2| somepkg.src = "I'm not an attrset";
| ^
3| in somepkg.src.meta
… while selecting an attribute
at «string»:3:4:
2| somepkg.src = "I'm not an attrset";
3| in somepkg.src.meta
| ^
error: expected a set but found a string: "I'm not an attrset"
into:
let
somepkg.src = "I'm not an attrset";
in somepkg.src.meta
error:
… while evaluating the attribute 'src.meta'
at «string»:2:3:
1| let
2| somepkg.src = "I'm not an attrset";
| ^
3| in somepkg.src.meta
… while selecting 'meta' on 'somepkg.src'
at «string»:3:4:
2| somepkg.src = "I'm not an attrset";
3| in somepkg.src.meta
| ^
error: expected a set but found a string: "I'm not an attrset"
For the low price of an enumerate() and a lambda you too can have the
incorrect line of code actually show up in the trace!
Change-Id: Ic1491c86e33c167891bdac9adad6224784760bd6
Turns errors like:
let
errpkg = throw "invalid foobar";
in errpkg.meta
error:
… while calling the 'throw' builtin
at «string»:2:12:
1| let
2| errpkg = throw "invalid foobar";
| ^
3| in errpkg.meta
error: invalid foobar
into errors like:
let
errpkg = throw "invalid foobar";
in errpkg.meta
error:
… while evaluating 'errpkg' to select 'meta' on it
at «string»:3:4:
2| errpkg = throw "invalid foobar";
3| in errpkg.meta
| ^
… while calling the 'throw' builtin
at «string»:2:12:
1| let
2| errpkg = throw "invalid foobar";
| ^
3| in errpkg.meta
error: invalid foobar
For the low price of one try/catch, you too can have the incorrect line
of code actually show up in the trace!
Change-Id: If8d6200ec1567706669d405c34adcd7e2d2cd29d
this gives about 20% performance improvements on pure parsing. obviously
it will be less on full eval, but depending on how much parsing is to be
done (e.g. including hackage-packages.nix or not) it's more like 4%-10%.
this has been tested (with thousands of core hours of fuzzing) to ensure
that the ASTs produced by the new parser are exactly the same as the old
one would have produced. error messages will change (sometimes by a lot)
and are not yet perfect, but we would rather leave this as is for later.
test results for running only the parser (excluding the variable binding
code) in a tight loop with inputs and parameters as given are promising:
- 40% faster on lix's package.nix at 10000 iterations
- 1.3% faster on nixpkgs all-packages.nix at 1000 iterations
- equivalent on all of nixpkgs concatenated at 100 iterations
(excluding invalid files, each file surrounded with parens)
more realistic benchmarks are somewhere in between the extremes, parsing
once again getting the largest uplift. other realistic workloads improve
by a few percentage points as well, notably system builds are 4% faster.
Benchmarks summary (from ./bench/summarize.jq bench/bench-*.json)
old/bin/nix --extra-experimental-features 'nix-command flakes' eval -f bench/nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix
mean: 0.408s ± 0.025s
user: 0.355s | system: 0.033s
median: 0.389s
range: 0.388s ... 0.442s
relative: 1
new/bin/nix --extra-experimental-features 'nix-command flakes' eval -f bench/nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix
mean: 0.332s ± 0.024s
user: 0.279s | system: 0.033s
median: 0.314s
range: 0.313s ... 0.361s
relative: 0.814
---
old/bin/nix --extra-experimental-features 'nix-command flakes' eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 6.133s ± 0.022s
user: 5.395s | system: 0.437s
median: 6.128s
range: 6.099s ... 6.183s
relative: 1
new/bin/nix --extra-experimental-features 'nix-command flakes' eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 5.925s ± 0.025s
user: 5.176s | system: 0.456s
median: 5.934s
range: 5.861s ... 5.943s
relative: 0.966
---
GC_INITIAL_HEAP_SIZE=10g old/bin/nix eval --extra-experimental-features 'nix-command flakes' --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 4.503s ± 0.027s
user: 3.731s | system: 0.547s
median: 4.499s
range: 4.478s ... 4.541s
relative: 1
GC_INITIAL_HEAP_SIZE=10g new/bin/nix eval --extra-experimental-features 'nix-command flakes' --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 4.285s ± 0.031s
user: 3.504s | system: 0.571s
median: 4.281s
range: 4.221s ... 4.328s
relative: 0.951
---
old/bin/nix --extra-experimental-features 'nix-command flakes' search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello
mean: 16.475s ± 0.07s
user: 14.088s | system: 1.572s
median: 16.495s
range: 16.351s ... 16.536s
relative: 1
new/bin/nix --extra-experimental-features 'nix-command flakes' search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello
mean: 15.973s ± 0.013s
user: 13.558s | system: 1.615s
median: 15.973s
range: 15.946s ... 15.99s
relative: 0.97
---
Change-Id: Ie66ec2d045dec964632c6541e25f8f0797319ee2
We got UBSan working on Lix, so we of course immediately found a bug and
some definitely nonsense behaviour.
Accessing `pureEval` or `restrictEval` from a default setting value is
nonsense, since they would never be actually set by the time that value
is set so they are not going to do anything. The configuration is not
applied in an initializer (and even if it were, it's not going to be in
the right order).
After looking into *that*, we hunted down what actually was applying
these, since clearly this code did not do anything. The EvalState
constructor should have a "search path added and removed here :)" sign
on it, because that's where it is done. We added an explicit
initialization of the optional in there because it was otherwise unclear
why pureEval also has the search path to allowed paths setup code run.
We then realized that the `pureEval` documentation was *also* bogus, and
we rewrote it. In so doing, we realized that we forgot to file a bug to
make `builtins.storePath` work in pure eval mode, so we filed one of
those: lix-project/lix#402
Yaks have been thoroughly shorn.
UBSan report:
../src/libexpr/eval-settings.cc:66:10: runtime error: member call on address 0x752fa9a13060 which does not point to an object of type 'nix::BaseSetting<b
ool>'
0x752fa9a13060: note: object has invalid vptr
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^~~~~~~~~~~~~~~~~~~~~~~
invalid vptr
0 0x752fa95106a6 in nix::EvalSettings::getDefaultNixPath[abi:cxx11]() /home/jade/lix/lix2/build/src/libexpr/eval-settings.cc:66:10
1 0x752fa950e420 in nix::EvalSettings::EvalSettings() /home/jade/lix/lix2/build/src/libexpr/eval-settings.hh:36:15
2 0x752fa9469f1f in __cxx_global_var_init.50 /home/jade/lix/lix2/build/src/libexpr/eval-settings.cc:98:14
3 0x752fa9469f1f in _GLOBAL__sub_I_eval_settings.cc /home/jade/lix/lix2/build/src/libexpr/eval-settings.cc
4 0x752fabbd308d in call_init (/nix/store/k7zgvzp2r31zkg9xqgjim7mbknryv6bs-glibc-2.39-52/lib/ld-linux-x86-64.so.2+0x508d) (BuildId: a5b8228edc9f16078ac3c894af964eeb990ecb4c)
5 0x752fabbd317b in _dl_init (/nix/store/k7zgvzp2r31zkg9xqgjim7mbknryv6bs-glibc-2.39-52/lib/ld-linux-x86-64.so.2+0x517b) (BuildId: a5b8228edc9f16078ac3c894af964eeb990ecb4c)
6 0x752fabbe9c2f in _dl_start_user (/nix/store/k7zgvzp2r31zkg9xqgjim7mbknryv6bs-glibc-2.39-52/lib/ld-linux-x86-64.so.2+0x1bc2f) (BuildId: a5b8228edc9f16078ac3c894af964eeb990ecb4c)
Change-Id: I5d8ffb7bfbe24b6584020ac74eed93d9f2e6d111
with the prepatory work done this mostly means turning plain pointers
into unique_ptrs, with all the associated churn that necessitates. we
might want to change some of these to box_ptrs at some point as well,
but that would be a semantic change that isn't fully appropriate yet.
Change-Id: I0c238c118617420650432f4ed45569baa3e3f413
almost all places where Exprs are passed as pointers expect the pointers
to be non-null. pass them as references to encode this constraint in the
type system as well (and also communicate that Exprs must not be freed).
Change-Id: Ia98f166fec3c23151f906e13acb4a0954a5980a2
sizeof(long) is 4 bytes on i686 GCC.
With ~32 GiB of memory and a page size of 4096, there are 7988420 pages.
(7988420 * 4096) is bigger than INT32_MAX folks.
This has gone unnoticed for 9 years, and only came up thanks to
94ea517db[1] adding integer overflow sensitization checks, which caused
this broken code to emit an illegal instruction, crashing Lix the
instant the buildsystem ran Lix to generate the docs files.
[1]: 94ea517dbe729765b69638190f4bea3f6a632b40
Change-Id: I50bb9ea072aac11b449d79e5d55525887a6e5a99
not doing this exposes the binding name order to the annoying
interference of parse order on symbol order, which wouldn't be so bad if
it didn't make the tests less reliable and, importantly, dependent on
linker behavior (due to primop initialization being done in static
initializer, and the order of static initializers being defined only
within a single translation unit).
fixes#143
Change-Id: I3cf417893fbcf19e9ad3ff8986deb7cbcf3ca511
we now keep not a table of all positions, but a table of all origins and
their sizes. position indices are now direct pointers into the virtual
concatenation of all parsed contents. this slightly reduces memory usage
and time spent in the parser, at the cost of not being able to report
positions if the total input size exceeds 4GiB. this limit is not unique
to nix though, rustc and clang also limit their input to 4GiB (although
at least clang refuses to process inputs that are larger, we will not).
this new 4GiB limit probably will not cause any problems for quite a
while, all of nixpkgs together is less than 100MiB in size and already
needs over 700MiB of memory and multiple seconds just to parse. 4GiB
worth of input will easily take multiple minutes and over 30GiB of
memory without even evaluating anything. if problems *do* arise we can
probably recover the old table-based system by adding some tracking to
Pos::Origin (or increasing the size of PosIdx outright), but for time
being this looks like more complexity than it's worth.
since we now need to read the entire input again to determine the
line/column of a position we'll make unsafeGetAttrPos slightly lazy:
mostly the set it returns is only used to determine the file of origin
of an attribute, not its exact location. the thunks do not add
measurable runtime overhead.
notably this change is necessary to allow changing the parser since
apparently nothing supports nix's very idiosyncratic line ending choice
of "anything goes", making it very hard to calculate line/column
positions in the parser (while byte offsets are very easy).
(cherry picked from commit 5d9fdab3de0ee17c71369ad05806b9ea06dfceda)
Change-Id: Ie0b2430cb120c09097afa8c0101884d94f4bbf34
the parser modifies its inputs, which means that sharing them between
the error context reporting system and the parser itself can confuse the
reporting system. usually this led to early truncation of error context
reports which, while not dangerous, can be quite confusing.
(cherry picked from commit d384ecd553aa997270b79ee98d02f7cf7e1849e6)
Change-Id: I677646b5675b12b2faa787943646aa36dc6e6ee3
These now have equivalents in the standard lib in C++20. This change was
performed with a custom clang-tidy check which I will submit later.
Executed like so:
ninja -C build && run-clang-tidy -checks='-*,nix-*' -load=build/libnix-clang-tidy.so -p .. -fix ../tests | tee -a clang-tidy-result
Change-Id: I62679e315ff9e7ce72a40b91b79c3e9fc01b27e9
it's no longer widely used and has a rather confusing meaning now that
inherit-from is handled very differently.
(cherry picked from commit 1cd87b7042d14aae1fafa47b1c28db4c5bd20de7)
Change-Id: I90bbebddf06762960d8ca4f621cf042ce8ae83f9
desugaring inherit-from to syntactic duplication of the source expr also
duplicates side effects of the source expr (such as trace calls) and
expensive computations (such as derivationStrict).
(cherry picked from commit cefd0302b55b3360dbca59cfcb4bf6a750d6cdcf)
Change-Id: Iff519f991adef2e51683ba2c552d37a3df7a179e
in place of inherited() — not quite useful yet since we don't
distinguish plain and inheritFrom attr kinds so far.
(cherry picked from commit 1f542adb3e18e7078e6a589182a53a47d971748a)
Change-Id: If948c9d43e875de18f213a73a06a36f7c335b536
Do not skip any stack frames when `--show-trace` is given
(cherry picked from commit 0b47783d0a879875d558f0b56e49584f25ceb2d0)
Change-Id: Ia0f18266dbcf97543110110c655c219c7a3e3270
While preparing PRs like #9753, I've had to change error messages in
dozens of code paths. It would be nice if instead of
EvalError("expected 'boolean' but found '%1%'", showType(v))
we could write
TypeError(v, "boolean")
or similar. Then, changing the error message could be a mechanical
refactor with the compiler pointing out places the constructor needs to
be changed, rather than the error-prone process of grepping through the
codebase. Structured errors would also help prevent the "same" error
from having multiple slightly different messages, and could be a first
step towards error codes / an error index.
This PR reworks the exception infrastructure in `libexpr` to
support exception types with different constructor signatures than
`BaseError`. Actually refactoring the exceptions to use structured data
will come in a future PR (this one is big enough already, as it has to
touch every exception in `libexpr`).
The core design is in `eval-error.hh`. Generally, errors like this:
state.error("'%s' is not a string", getAttrPathStr())
.debugThrow<TypeError>()
are transformed like this:
state.error<TypeError>("'%s' is not a string", getAttrPathStr())
.debugThrow()
The type annotation has moved from `ErrorBuilder::debugThrow` to
`EvalState::error`.
(cherry picked from commit c6a89c1a1659b31694c0fbcd21d78a6dd521c732)
Change-Id: Iced91ba4e00ca9e801518071fb43798936cbd05a
Enter debugger more reliably in `let` expressions and function calls
(cherry picked from commit c4ed92fa6f836d3d8eb354a48c37a2f9eeecc3aa)
Change-Id: I16d0cad7e898feecd2399723b92ba8df67222fb4
these symbols are used a *lot*, so it makes sense to cache them. this
mostly increases clarity of the code (however clear one may wish to call
the parser desugaring here), but it also provides a small performance
benefit.
(cherry picked from commit 09a1128d9e2ff0ae6176784938047350d6f8a782)
Change-Id: I73d9f66be4555168e048cb2d542277251580c2d1
there's no reason the parser itself should be doing semantic analysis
like bindVars. split this bit apart (retaining the previous name in
EvalState) and have the parser really do *only* parsing, decoupled from
EvalState.
(cherry picked from commit b596cc9e7960b9256bcd557334d81e9d555be5a2)
Change-Id: I481a7623afc783e9d28a6eb4627552cf8a780986
most EvalState and Expr members defined here could be elsewhere, where
they'd be easier to maintain (not being embedded in a file with arcane
syntax) and *somewhat* more faithfully placed according to the path of
the file they're defined in.
(cherry picked from commit e1aa585964c3d864ebff0030584f3349a539d615)
Change-Id: Ibc704567462bb40f37cda05d8fadd465519db5f5
Print the value in `error: cannot coerce` messages
(cherry picked from commit 5b7bfd2d6b89d7dd5f54c1ca6c8072358d31a84e)
===
test taken from 6e8d5983143ae576e3f4b1d2954a5267f2943a49; it was added
previously (and not backported because its pr was a mostly-revert), but
it's useful to have around.
Change-Id: Icbd14b55e3610ce7b774667bf14b82e6dc717982
libexpr: print value of what is attempted to be called as function
(cherry picked from commit 50e5d7b883042852538371237e32a66bb22f0485)
Change-Id: I7cb6290bd8f244e83bfce3b2eed2a4c8b4f16a83
Print the value in `value is X while a Y is expected` error
(cherry picked from commit 5f72a97092da6af28a7d2b2a50d74e9d34fae7e1)
Change-Id: Idb4bc903ae59a0f5b6fb3b1da4d47970fe0a6efe
Previously, there were two mostly-identical value printers -- one in
`libexpr/eval.cc` (which didn't force values) and one in
`libcmd/repl.cc` (which did force values and also printed ANSI color
codes).
This PR unifies both of these printers into `print.cc` and provides a
`PrintOptions` struct for controlling the output, which allows for
toggling whether values are forced, whether repeated values are tracked,
and whether ANSI color codes are displayed.
Additionally, `PrintOptions` allows tuning the maximum number of
attributes, list items, and bytes in a string that will be displayed;
this makes it ideal for contexts where printing too much output (e.g.
all of Nixpkgs) is distracting. (As requested by @roberth in
https://github.com/NixOS/nix/pull/9554#issuecomment-1845095735)
Please read the tests for example output.
Future work:
- It would be nice to provide this function as a builtin, perhaps
`builtins.toStringDebug` -- a printing function that never fails would
be useful when debugging Nix code.
- It would be nice to support customizing `PrintOptions` members on the
command line, e.g. `--option to-string-max-attrs 1000`.
(cherry picked from commit 0fa08b451682fb3311fe58112ff05c4fe5bee3a4, )
===
Restore ambiguous value printer for `nix-instantiate`
The Nix team has requested that this output format remain unchanged.
I've added a warning to the man page explaining that `nix-instantiate
--eval` output will not parse correctly in many situations.
(cherry picked from commit df84dd4d8dd3fd6381ac2ca3064432ab31a16b79)
Change-Id: I7cca6b4b53cd0642f2d49af657d5676a8554c9f8
Combine `AbstractPos`, `PosAdapter`, and `Pos`
(cherry picked from commit 113499d16fc87d53b73fb62fe6242154909756ed)
===
this is a bit cursed because originally it was based on InputAccessor
code that we don't have and moved/patched features we likewise don't
have (fetchToStore caching, all the individual accessors,
ContentAddressMethod). the commit is adjusted accordingly to
match (remove caching, ignore accessors, use FileIngestionMethod).
note that `state.rootPath . CanonPath == abs` and
computeStorePathForPath works relative to cwd, so the slight rewrite in
the moved fetchToStore is legal.
Change-Id: I05fd340c273f0bcc8ffabfebdc4a88b98083bce5
Add position information to `while evaluating the attribute` errors in the debugger
(cherry picked from commit ffe67c86a8ef3695e5c8b9c9800c192ac633dded)
Change-Id: I177ea5ec60898abe09fb9d80d9602b2a32ff8f44
Qyriad
jade
eldritch horrors
nan-git
Tom Hubrecht
