oilshell / oil

Oils is our upgrade path from bash to a better language and runtime. It's also for Python and JavaScript users who avoid shell!
http://www.oilshell.org/
Other
2.78k stars 150 forks source link

intermittent crash running amd-test script -- reproducible in dbg, opt #1986

Open andychu opened 4 weeks ago

andychu commented 4 weeks ago

Somehow I don't hit anything in ASAN, but GDB reveals the same problem in dbg/opt

[Detaching after fork from child process 629535]
[Detaching after fork from child process 629536]
[Detaching after fork from child process 629537]
[Detaching after fork from child process 629538]
[Detaching after fork from child process 629706]

Program received signal SIGSEGV, Segmentation fault.
0x00005555555d9a55 in state::Mem::SetNamed (this=0x5555557ce318, lval=<optimized out>, val=<optimized out>, which_scopes=which_scopes@entry=runtime_asdl::scope_e::LocalOnly, flags=flags@entry=0) at _gen/bin/oils_for_unix.mycpp.cc:26233
26233     if (((flags & SetNameref) or (flags & ClearNameref))) {
(gdb) 
(gdb) quit
A debugging session is active.
jtb20 commented 4 weeks ago

It's a GC issue. The "restore" field in ctx_Shvar gets collected, and that makes ctx_Shvar::_Pop read uninitialised data and thus crash.

So the problem is sort of in Shvar::Run. I can fix it temporarily and badly by hacking the generated C++ source:

  {  // with
    ctx_Shvar *ctx = Alloc<ctx_Shvar>(this->mem, pairs);

    unused = this->cmd_ev->EvalCommand(cmd);
  }

i.e. because the pointers in ctx are never marked, so considered fair game to be swept. (I haven't audited anywhere else that might have the same problem, guess there might be though.)

(Edit: this appears to be a mycpp issue, see below comment.)

HTH!

jtb20 commented 4 weeks ago

(This "with ...:" idiom is used three times in pure_ysh.py, AFAICT.)

andychu commented 4 weeks ago

Thank you! This is very useful

I just reproduced the bug easily, by running replace() in a loop, which uses ctx_Shvar

We do test with a gcalways variant, not sure why that didn't catch this earlier, but we'll see


Also there seems to be a perf bug I also want to look into

andychu commented 4 weeks ago

I have a small reproducible case now, outside Oils -- this is indeed a bug in the mycpp compiler and/or runtime

https://github.com/oilshell/oil/commit/cddb9f701e600f890151c840e2bfaa4fa2e4dbd1

OK good news is that this bug is not all that subtle ... not sure what exactly the fix is yet