Skip to content

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

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

libexpr: Fix invalid handling of errors for imported functions (backport #13450)#13452
xokdvium merged 1 commit into2.29-maintenancefrom
mergify/bp/2.29-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).

@mergify mergify bot added automatic backport This PR is a backport produced by automation (does not trigger backporting) conflicts merge-queue labels Jul 11, 2025
@mergify mergify bot requested a review from edolstra as a code owner July 11, 2025 18:26
@mergify mergify bot added merge-queue conflicts automatic backport This PR is a backport produced by automation (does not trigger backporting) labels Jul 11, 2025
@mergify
Copy link
Contributor Author

mergify bot commented Jul 11, 2025

Cherry-pick of 6e78cc9 has failed:

On branch mergify/bp/2.29-maintenance/pr-13450
Your branch is up to date with 'origin/2.29-maintenance'.

You are currently cherry-picking commit 6e78cc90d.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   tests/functional/lang/eval-fail-missing-arg-import.err.exp
	new file:   tests/functional/lang/eval-fail-missing-arg-import.nix
	new file:   tests/functional/lang/eval-fail-undeclared-arg-import.err.exp
	new file:   tests/functional/lang/eval-fail-undeclared-arg-import.nix
	new file:   tests/functional/lang/non-eval-trivial-lambda-formals.nix

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   src/libexpr/eval.cc

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Jul 11, 2025
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)
@xokdvium xokdvium force-pushed the mergify/bp/2.29-maintenance/pr-13450 branch from 65ca32e to 8736cb5 Compare July 11, 2025 19:52
@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 #13452 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 failing.).

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 4b3bbf8 into 2.29-maintenance Jul 11, 2025
32 of 35 checks passed
@xokdvium xokdvium deleted the mergify/bp/2.29-maintenance/pr-13450 branch July 11, 2025 21:12
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) conflicts 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