Wrap const_eval_stack_frame_limit in a LockCell

[Aaron1011]

This allows Miri to set const_eval_stack_frame_limit as needed,
while still keeping Session thread-safe

[rust-highfive]

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

[Aaron1011]

r? @Zoxc

[Zoxc]

This doesn't look correct since Miri could be used on multiple threads.

[Aaron1011]

Miri currently doesn't support multiple threads. I don't believe anyone is currently working on adding multithreading support, so I think this change should be fine for the forseeable future.

[Zoxc]

Miri can't emulate multiple threads, but you can have Miri executions on multiple threads in rustc currently.

[Aaron1011]

@Zoxc: I've switched const_eval_stack_frame_limit to use a LockCell

[Zoxc]

I'd like to know how you plan on changing const_eval_stack_frame_limit in Miri in order to check that it's actually thread-safe. I don't think you could change const_eval_stack_frame_limit using LockCell in the rustc driver in a race-free way.

[Aaron1011]

@Zoxc: I'll be changing it here, before Miri starting running the target function.

[Zoxc]

That doesn't seem correct either, assuming that the Miri driver will use rustc's const_eval for consistency. I think the correct move would be to add a stack_limit field to EvalContext and have the Miri driver populate that differently from rustc's const_eval. cc @oli-obk

[oli-obk]

cc @RalfJung

We should probably remove this field entirely and make the engine depend on the recursion limit that is also used for macros and such. This way users can control the recursion depth directly from the source

[RalfJung]

@oli-obk Actually I think miri-the-tool wants no limit at all here, TBH.

For CTFE, the "normal" recursion limit seems fine. What is its default value?

[oli-obk]

What is its default value?

rust/src/librustc/middle/recursion_limit.rs

Lines 24 to 25 in b7c6e8f

	update_limit(sess, krate, &sess.recursion_limit, "recursion_limit",
	"recursion limit", 64);

Actually I think miri-the-tool wants no limit at all here, TBH.

That seems like an easy change to make by making the check a Machine-method

[RalfJung]

That seems like an easy change to make by making the check a Machine-method

Actually, wouldn't the stack_push hook added by #56094 suffice for that?

[oli-obk]

Yes that seems like the correct place to check that

[TimNN]

Ping from triage @Aaron1011: It looks like some changes have been requested to your PR.

[Dylan-DPC-zz]

Ping from triage. @Aaron1011 Unfortunately we haven't heard from you on this in a while, so I'm closing the PR to keep things tidy. Don't worry though, if you'll have time again in the future please reopen this PR, we'll be happy to review it again!