Incorrect codegen with returns inside nested ifs

[osyu]

This snippet's control flow does not get compiled correctly, and both messages are printed when run.

public void OnPluginStart()
{
  if (1)
  {
    if (0)
    {
      return;
    }

    PrintToServer("hi");
  }
  else
  {
    PrintToServer("this should never happen");
  }
}

hi
this should never happen

Disassembly:

0xda0: proc
0xda4: break
0xda8: const.pri 0x1
0xdb0: jzer 0xdf0         ; if (1) jump to 0xdf0
0xdb8: break
0xdbc: zero.pri
0xdc0: jzer 0xdd4         ; if (0) jump to 0xdd4
0xdc8: break
0xdcc: zero.pri
0xdd0: retn
0xdd4: break
0xdd8: const.pri 0x8e8
0xde0: push.pri
0xde4: sysreq.n 2 0x1     ; PrintToServer("hi")
                          ; *** There should be a jump to 0xe0c here, but there isn't so it falls through ***
0xdf0: break
0xdf4: const.pri 0x8ec
0xdfc: push.pri
0xe00: sysreq.n 2 0x1     ; PrintToServer("this should never happen")
0xe0c: break
0xe10: zero.pri
0xe14: retn
0xe18: endproc

---

If the return statement inside the nested if is removed, it compiles properly and only the first message is printed.

public void OnPluginStart()
{
  if (1)
  {
    if (0)
    {
      // no return
    }

    PrintToServer("hi");
  }
  else
  {
    PrintToServer("this should never happen");
  }
}

hi

Disassembly:

0xda0: proc
0xda4: break
0xda8: const.pri 0x1
0xdb0: jzer 0xdec         ; if (1) jump to 0xdec
0xdb8: break
0xdbc: zero.pri
0xdc0: jzer 0xdcc         ; if (0) jump to 0xdcc
0xdc8: break
0xdcc: const.pri 0x8e8
0xdd4: push.pri
0xdd8: sysreq.n 2 0x1     ; PrintToServer("hi")
0xde4: jump 0xe08         ; *** There is a jump here, so it's correct ***
0xdec: break
0xdf0: const.pri 0x8ec
0xdf8: push.pri
0xdfc: sysreq.n 2 0x1     ; PrintToServer("this should never happen")
0xe08: break
0xe0c: zero.pri
0xe10: retn
0xe14: endproc

---

This issue started in SourceMod with alliedmodders/sourcemod@cb85415 and currently affects basebans.sp::LoadBanReasons, causing the plugin to always fail even if the banreasons.txt file exists.

[osyu]

The same issue happens with switch statements, where having a return inside an if causes a drop into the next case.

public void OnPluginStart()
{
  switch (0)
  {
    case 0:
    {
      if (0)
      {
        return;
      }

      PrintToServer("hi");
    }
    case 1:
    {
      if (0)
      {
        return;
      }

      PrintToServer("this should never happen");
    }
    default:
    {
      PrintToServer("this should never happen either");
    }
  }
}

hi
this should never happen
this should never happen either

Disassembly:

0xda0: proc
0xda4: break
0xda8: zero.pri
0xdac: switch 0xe48
0xdb4: break              ; <case 0 start>
0xdb8: zero.pri
0xdbc: jzer 0xdd0         ; if (0) jump to 0xdd0
0xdc4: break
0xdc8: zero.pri
0xdcc: retn
0xdd0: break
0xdd4: const.pri 0x8e8
0xddc: push.pri
0xde0: sysreq.n 2 0x1     ; PrintToServer("hi")
                          ; *** There should be a jump to 0xe64 here, but there isn't so it falls through ***
0xdec: break              ; <case 1 start>
0xdf0: zero.pri
0xdf4: jzer 0xe08         ; if (0) jump to 0xe08
0xdfc: break
0xe00: zero.pri
0xe04: retn
0xe08: break
0xe0c: const.pri 0x8ec
0xe14: push.pri
0xe18: sysreq.n 2 0x1     ; PrintToServer("this should never happen")
                          ; *** There should be a jump to 0xe64 here, but there isn't so it falls through (again) ***
0xe24: break              ; <default start>
0xe28: const.pri 0x908
0xe30: push.pri
0xe34: sysreq.n 2 0x1     ; PrintToServer("this should never happen either")
0xe40: jump 0xe64
0xe48: casetbl 0x2 0xe24
0xe54: case 0 0xdb4
0xe5c: case 1 0xdec
0xe64: break
0xe68: zero.pri
0xe6c: retn
0xe70: endproc

---

Without return statements it compiles fine.

public void OnPluginStart()
{
  switch (0)
  {
    case 0:
    {
      if (0)
      {
        // no return
      }

      PrintToServer("hi");
    }
    case 1:
    {
      if (0)
      {
        // no return
      }

      PrintToServer("this should never happen");
    }
    default:
    {
      PrintToServer("this should never happen either");
    }
  }
}

hi

Disassembly:

0xda0: proc
0xda4: break
0xda8: zero.pri
0xdac: switch 0xe40
0xdb4: break              ; <case 0 start>
0xdb8: zero.pri
0xdbc: jzer 0xdc8         ; if (0) jump to 0xdc8
0xdc4: break
0xdc8: const.pri 0x8e8
0xdd0: push.pri
0xdd4: sysreq.n 2 0x1     ; PrintToServer("hi")
0xde0: jump 0xe5c         ; *** There is a jump here, so it's correct ***
0xde8: break              ; <case 1 start>
0xdec: zero.pri
0xdf0: jzer 0xdfc         ; if (0) jump to 0xdfc
0xdf8: break
0xdfc: const.pri 0x8ec
0xe04: push.pri
0xe08: sysreq.n 2 0x1     ; PrintToServer("this should never happen")
0xe14: jump 0xe5c         ; *** There is a jump here, so it's correct (although this won't run anyway) ***
0xe1c: break              ; <default start>
0xe20: const.pri 0x908
0xe28: push.pri
0xe2c: sysreq.n 2 0x1     ; PrintToServer("this should never happen either")
0xe38: jump 0xe5c
0xe40: casetbl 0x2 0xe1c
0xe4c: case 0 0xdb4
0xe54: case 1 0xde8
0xe5c: break
0xe60: zero.pri
0xe64: retn
0xe68: endproc

---

This currently affects tf2_stocks.inc::TF2_IsPlayerInCondition, causing it to throw an error even if the TFCond value is valid.

[nulldg]

this bug is affecting my plugin. control is leaking and causing the plugin to malfunction. as stated before, this regression affects the latest stable build too.

[peace-maker]

this bug is affecting my plugin. control is leaking and causing the plugin to malfunction. as stated before, this regression affects the latest stable build too.

It should only affect the dev builds, not the stable builds.

[nulldg]

oh my bad, i could have sworn alliedmodders/sourcemod@cb85415 was merged into the 1.11 branch. either way, my plugin has started acting spuriously when built with the dev compiler so i've been using a pre-cb85415 build of the compiler in the meantime.

[dvander]

Thanks for the great test cases and analysis. This has been fixed on master.