Skip to content

libexpr: Fix invalid handling of errors for imported functions (backport #13450)#13453

Merged
xokdvium merged 1 commit into2.30-maintenancefrom
mergify/bp/2.30-maintenance/pr-13450
Jul 11, 2025
Merged

libexpr: Fix invalid handling of errors for imported functions (backport #13450)#13453
xokdvium merged 1 commit into2.30-maintenancefrom
mergify/bp/2.30-maintenance/pr-13450

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Jul 11, 2025

Motivation

c39cc00 has added assertions for all Value accesses and the following case has started failing with an unreachable:

(/tmp/fun.nix):

{a}: a
$ nix eval --impure --expr 'import /tmp/fun.nix {a="a";b="b";}'

This would crash:

terminating due to unexpected unrecoverable internal error: Unexpected condition in getStorage at ../include/nix/expr/value.hh:844

This is not a regression, but rather surfaces an existing problem, which previously was left undiagnosed. In the case of an import fun is the import primOp, so that read is invalid and previously this resulted in an access into an inactive union member, which is UB. The correct thing to use is vCur. Identical problem also affected the case of a missing argument.

Add previously failing test cases to the functional/lang test suite.

Context

Fixes #13448.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.


This is an automatic backport of pull request #13450 done by [Mergify](https://mergify.com).

c39cc00 has added assertions for
all Value accesses and the following case has started failing with
an `unreachable`:

(/tmp/fun.nix):

```nix
{a}: a
```

```
$ nix eval --impure --expr 'import /tmp/fun.nix {a="a";b="b";}'
```

This would crash:

```
terminating due to unexpected unrecoverable internal error: Unexpected condition in getStorage at ../include/nix/expr/value.hh:844
```

This is not a regression, but rather surfaces an existing problem, which previously
was left undiagnosed. In the case of an import `fun` is the `import` primOp, so that read is invalid
and previously this resulted in an access into an inactive union member, which is UB.
The correct thing to use is `vCur`. Identical problem also affected the case of a missing argument.

Add previously failing test cases to the functional/lang test suite.

Fixes #13448.

(cherry picked from commit 6e78cc9)
@mergify mergify bot requested a review from edolstra as a code owner July 11, 2025 18:26
@mergify mergify bot added automatic backport This PR is a backport produced by automation (does not trigger backporting) merge-queue labels Jul 11, 2025
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Jul 11, 2025
@mergify
Copy link
Contributor Author

mergify bot commented Jul 11, 2025

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #13453 has been dequeued. The pull request could not be merged. This could be related to an activated branch protection or ruleset rule that prevents us from merging. (details: Required status check "tests on ubuntu" is in progress.).

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.
If you do update this pull request, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@xokdvium xokdvium merged commit eb3c004 into 2.30-maintenance Jul 11, 2025
25 of 27 checks passed
@xokdvium xokdvium deleted the mergify/bp/2.30-maintenance/pr-13450 branch July 11, 2025 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automatic backport This PR is a backport produced by automation (does not trigger backporting) merge-queue with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant