libexpr-c: fix EvalState pointer passed to primop callbacks by Mic92 · Pull Request #15300 · NixOS/nix

Mic92 · 2026-02-19T14:53:12Z

nix_c_primop_wrapper cast nix::EvalState& directly to the C API wrapper EvalState*, but the wrapper has fetchSettings and settings fields before the inner nix::EvalState member. C API functions like nix_alloc_value() then accessed state->state at the wrong offset, causing a segfault.

Use offsetof to recover the enclosing wrapper from the inner member. Same fix applied to NixCExternalValue::printValueAsJSON/printValueAsXML.

xokdvium · 2026-02-19T15:10:14Z

Hm, shouldn't we instead reorder some fields maybe so that it's the first memeber? Then this won't be UB since you are allowed to do reinterpret_cast between the pointer to first member and the object itself.

xokdvium · 2026-02-19T15:10:56Z

Note that it wouldn't break the C ABI, since it's just an opaque pointer from that PoV.

xokdvium · 2026-02-19T15:37:23Z

+ */
+inline EvalState * nix_capi_eval_state_from_inner(nix::EvalState & state)
+{
+    return reinterpret_cast<EvalState *>(reinterpret_cast<char *>(&state) - offsetof(EvalState, state));


Somewhat skeptical of doing it this way. Let's reorder the members so that the nix::EvalState is the first one of the struct and keep the reinterpret_cast to the first member.

Tried that and this breaks unsafe_new_with_self, see message below.

Mic92 · 2026-02-19T15:40:31Z

Hm, shouldn't we instead reorder some fields maybe so that it's the first memeber? Then this won't be UB since you are allowed to do reinterpret_cast between the pointer to first member and the object itself.

This gives me a segfault:

   Thread 1 "nix-expr-tests" received signal SIGSEGV, Segmentation fault.
   #0  nix::EvalState::createBaseEnv(nix::EvalSettings const&) ()
       from libnixexpr.so
   #1  nix::EvalState::EvalState(nix::LookupPath const&, ...) ()
       from libnixexpr.so
   #2  unsafe_new_with_self<EvalState, ...>()
       from libnixexprc.so
   #3  nix_state_create ()
       from libnixexprc.so
   #4  TestFactoryImpl<...nix_primop_alloc_value_in_callback_Test>::CreateTest()

looks like unsafe_new_with_self needs a specific order and EvalState takes the builder->fetchSettings and builder->settings as arguments.

xokdvium · 2026-02-19T15:49:36Z

takes the builder->fetchSettings and builder->settings as arguments.

Could we put those behind std::unique_ptr and initialise before calling into unsafe_new_with_self? That way the pointers are stable and we can massage the initialisation order to be right.

xokdvium · 2026-02-19T15:50:46Z

~~I guess this solution is also fine if it works with sanitizers...~~ Doesn't work for plugins.

xokdvium · 2026-02-19T15:55:52Z

        nix_string_context ctx{context};
        nix_string_return res{""};
-        desc.printValueAsJSON(v, (EvalState *) &state, strict, &ctx, copyToStore, &res);
+        desc.printValueAsJSON(v, nix_capi_eval_state_from_inner(state), strict, &ctx, copyToStore, &res);


What about the case where the eval isn't called from the C API but by nix itself like with plugins? Seems like we need to support for use-cases. Maybe let's try #15300 (comment)?

xokdvium · 2026-02-19T16:16:03Z

It's still unclear to me how a C plugin registering a builtin via nix_register_primop could receive a valid instance of EvalState. Seems like even in case the nix::EvalState is created by the C++ code it needs something to pass back to a C plugin.

Mic92 · 2026-02-19T17:22:48Z

It's still unclear to me how a C plugin registering a builtin via nix_register_primop could receive a valid instance of EvalState. Seems like even in case the nix::EvalState is created by the C++ code it needs something to pass back to a C plugin.

Don't know how to solve this either...

xokdvium · 2026-02-19T19:51:34Z

Don't know how to solve this either...

I guess we need to agree on the same common initial sequence of members and/or common structure that is available always. Will take a stab at it.

xokdvium · 2026-03-02T14:06:04Z

Sanitizer is not happy: https://github.com/NixOS/nix/actions/runs/22577061201/job/65399161807?pr=15300#step:5:7017

Mic92 · 2026-03-02T14:21:13Z

Sanitizer is not happy: https://github.com/NixOS/nix/actions/runs/22577061201/job/65399161807?pr=15300#step:5:7017

I am also not happy with this code :(

Primop callbacks and external value methods received an inner nix::EvalState* instead of the C API EvalState* wrapper, causing segfaults when passed to functions like nix_alloc_value(). The old code cast (EvalState *) &state directly from the inner nix::EvalState& reference, but the C API wrapper struct has fetchSettings/settings/statePtr fields before the state member, so C API functions accessed memory at wrong offsets. Introduce EvalStateRef, a lightweight non-owning wrapper that constructs a proper EvalState from a nix::EvalState& on the stack. This avoids UB (no pointer arithmetic or reinterpret_cast tricks), works correctly for C plugins where nix::EvalState is created by C++ code rather than nix_eval_state_build(), and has the same lifetime guarantees as the previous approach since the wrapper lives on the stack for the duration of the callback.

roberth · 2026-03-02T20:08:06Z

I think #15383 does a bit better on the surprise factor - no implicit struct layout equivalence requirements.
::EvalState is opaque to C API callers, so let's use that to our advantage.

Mic92 · 2026-03-02T20:38:11Z

This is a better solution to this: #15383

Mic92 requested a review from edolstra as a code owner February 19, 2026 14:53

github-actions Bot added the c api Nix as a C library with a stable interface label Feb 19, 2026

Mic92 force-pushed the fix-eval-state-in-c-primops branch 2 times, most recently from f69be29 to 8d53913 Compare February 19, 2026 15:07

github-actions Bot added the documentation label Feb 19, 2026

xokdvium reviewed Feb 19, 2026

View reviewed changes

Comment thread src/libexpr-tests/nix_api_expr.cc

Mic92 force-pushed the fix-eval-state-in-c-primops branch from 8d53913 to 3879e05 Compare February 19, 2026 15:33

xokdvium reviewed Feb 19, 2026

View reviewed changes

Mic92 force-pushed the fix-eval-state-in-c-primops branch 2 times, most recently from e57e9bb to a6922cc Compare February 19, 2026 16:12

Mic92 force-pushed the fix-eval-state-in-c-primops branch from a6922cc to 2dc31e2 Compare February 19, 2026 16:18

Mic92 force-pushed the fix-eval-state-in-c-primops branch 3 times, most recently from bf2051a to e2c117a Compare March 2, 2026 12:58

Mic92 force-pushed the fix-eval-state-in-c-primops branch from e2c117a to 14d8f74 Compare March 2, 2026 18:10

roberth mentioned this pull request Mar 2, 2026

fix(libexpr-c): pass valid EvalState to primop callback #15383

Merged

Mic92 closed this Mar 2, 2026

Mic92 deleted the fix-eval-state-in-c-primops branch March 2, 2026 20:38

philiptaron mentioned this pull request Mar 12, 2026

nixVersions.nix_2_34: 2.34.0 -> 2.34.1 NixOS/nixpkgs#499291

Merged

13 tasks

Uh oh!

Conversation

Mic92 commented Feb 19, 2026

Uh oh!

xokdvium commented Feb 19, 2026

Uh oh!

xokdvium commented Feb 19, 2026

Uh oh!

Uh oh!

xokdvium Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Mic92 Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Mic92 commented Feb 19, 2026

Uh oh!

xokdvium commented Feb 19, 2026

Uh oh!

xokdvium commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xokdvium Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

xokdvium commented Feb 19, 2026

Uh oh!

Mic92 commented Feb 19, 2026

Uh oh!

xokdvium commented Feb 19, 2026

Uh oh!

xokdvium commented Mar 2, 2026

Uh oh!

Mic92 commented Mar 2, 2026

Uh oh!

roberth commented Mar 2, 2026

Uh oh!

Mic92 commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xokdvium commented Feb 19, 2026 •

edited

Loading