Hi, Maxim!
Thanks for the review!
Please, consider my answers below.
On 13.10.23, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few nits below.
> On Wed, Oct 11, 2023 at 06:04:09PM +0300, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Analyzed by Sergey Kaplun.
> >
> > (cherry-picked from commit b7a8c7c184257858699454408420dd5f0b6c8a75)
> >
> > Assume we have parent and child traces with the following IRs from the
> > IR dump:
> >
> > Parent:
> > | 0009 rax > tab TNEW 0 0
> > | 0010 p32 FLOAD 0008 tab.node
> > | 0011 p32 HREFK 0010 "Name" @1
> > | 0012 {0008} tab HSTORE 0011 0009
> > | .... SNAP 2 [ ---- 0001 0002 0008 ---- ]
> > | 0013 {sink} tab TNEW 0 0
> > | 0014 {0008} fal HSTORE 0011 false
> > | .... SNAP 3 [ ---- 0001 0002 0008 ---- ]
> >
> > Child:
> > | 0001 r15 tab SLOAD 1 PI
> > | 0002 rbp tab SLOAD 2 PI
> > | 0003 tab PVAL 9
> >
> > As we can see from the trace dump above, the `rax` register is missing
> > in the `0003 PVAL` IR for the side trace -- so it is assumed to be
> > available in the allow RegSet inside `asm_stack_check()` and its value
> > is spoiled during this check, so if we are restoring from the 3rd
> Typo: s/spoiled/spilled/
I mean spoiled (damaged) (with incorrect value) here -- there is no spill, but
incorrect value as `ir->r`.
> > snapshot by stack overflow -- we are in trouble.
> >
> > The moment when IR is spoiled is when we set a hint on the register
> Typo: s/spoiled/spilled/
Ditto.
> > inherited from the parent trace (see `asm_setup_regsp()` for details).
<snipped>
> >
--
Best regards,
Sergey Kaplun