darknesswind / NutCracker

fork from DamianXVI's squirrel decompiler
22 stars 11 forks source link

'break' is not emitted in some cases #20

Open AdamMil opened 5 years ago

AdamMil commented 5 years ago

Consider this small script:

while(true) {
  if(!this.condition) {
    if(--n <= 0) { break; }
  }
}

When compiled and decompiled, the output is:

while (true) {
        if (!this.condition) {
                if (--this.n <= 0) {
                }
        }
}

The 'break' statement is missing, resulting in an infinite loop.

AdamMil commented 5 years ago

While debugging, I notice this:

void NutFunction::DecompileJCMP(VMState& state, int condVar, int offsetIp, int iterVar, int cmpOp) const
{
    ...
    bool bCanBreak = (state.m_BlockState.inLoop || state.m_BlockState.inSwitch);

Here, m_BlockState refers to the 'if' statement and .inLoop is false. However, m_BlockState.parent refers to the loop and has .inLoop true. I think either bCanBreak needs to examine the chain of parent blocks, or inLoop needs to be propagated from parent to child.

I changed the code to:

bool bCanBreak = false;
for (BlockState *p = &state.m_BlockState; !bCanBreak && p; p = p->parent)
{
    bCanBreak = (p->inLoop || p->inSwitch);
}

And got the correct output for my example. But I'm not sure that's the correct change.

AdamMil commented 5 years ago

Perhaps elseEnd > prevBlockState.blockEnd also needs to be fixed to compare against the containing loop or switch end?