neverwinter.nim icon indicating copy to clipboard operation
neverwinter.nim copied to clipboard

scriptcomp: Do-while loop emits invalid opcodes when variable is declared but not initialized before loop

Open nelak2 opened this issue 4 months ago • 6 comments

Do while loops appear to fail when working with object iterators.

See the simplified example below:

void main()
{
    object oArea;
    
    do
    {
        oArea = OBJECT_INVALID;
        object oTarget = GetFirstPC();
        while (oTarget != OBJECT_INVALID)
        {
            if (GetIsObjectValid(oTarget))
            {
                oArea = GetArea(oTarget);
            }
            oTarget = GetNextPC();
        }
    }
    while (oArea != OBJECT_INVALID);
}

The error is INVALID OP CODE.

Tested on v89.8193.37-15

nelak2 avatar Jul 08 '25 04:07 nelak2

This is an infinite loop. What's the goal here?

tinygiant98 avatar Jul 08 '25 06:07 tinygiant98

Yes, the examples are infinite loops. They're just the simplest examples I've found to reproduce the behavior. The original description of the issue isn't correct though.

It seems the issue is with the scoping of oTest. If it isn't initialized prior to the loop, when the while condition is executed the generated code is not valid. Though its not always consistent... I know I've run into it without an infinite loop, but it seems they're the easiest way to reproduce it.

// Runs once and fails with an ERROR: INVALID OP CODE
void main()
{
    object oTest;
    do
    {
        oTest = OBJECT_INVALID;
        oTest = GetFirstPC(); // oTest should be a valid object now
        SendMessageToPC(oTest, "Hello World!"); // Confirm it by sending a message
    }
    while (oTest != OBJECT_INVALID); // We should infinitely loop now but instead we error out
}

// This one loops infinitely as expected
void main()
{
    object oTest = OBJECT_INVALID;
    do
    {
        // oTest = OBJECT_INVALID;
        oTest = GetFirstPC(); // oTest should be a valid object now
        SendMessageToPC(oTest, "Hello World!"); // Confirm it by sending a message
    }
    while (oTest != OBJECT_INVALID); // loops forever
}

// Fails with invalid op code
void main()
{
    int iCount;
    do
    {
        iCount = 5;
        iCount = iCount - 1;
    }
    while (iCount > 0);  // Should run once then error
}

nelak2 avatar Jul 09 '25 06:07 nelak2

Okay. After some experimentation here are some other examples. Infinite loop vs Invalid op code depending on the order of things even though that shouldn't impact the actual logic at all.

// Fails with invalid op code
void main()
{
    int iCount;
    do
    {
        iCount = 5;
    }
    while (iCount > 0);
}

These two loop infinitely or fail with invalid op code depending on if I do object oPC = GetFirstPC() before or after the iCount stuff.

// Fails after 1 iteration
void main()
{
    int iCount;
    do
    {
        iCount = 5;
        object oPC = GetFirstPC();
        SendMessageToPC(oPC, "Hello World");
    }
    while (iCount > 0);  // Should run once then error
}

// Loops infinitely
void main()
{
    int iCount;
    do
    {
        object oPC = GetFirstPC();
        SendMessageToPC(oPC, "Hello World");
        iCount = 5;
    }
    while (iCount > 0); 
}

// Fails after 1 iteration
void main()
{
    int iCount;
    do
    {
        iCount = 5;
        int iCount2 = 10;
    }
    while (iCount > 0);
}

// Infinite loop
void main()
{
    int iCount;
    do
    {
        int iCount2 = 10;
        iCount = 5;
    }
    while (iCount > 0);
}

nelak2 avatar Jul 09 '25 06:07 nelak2

Decompiled


void main()
{
    int iCount;
    do
    {
        iCount = 5;
    }
    while (iCount > 0);
}
// Compiled O2
  0   JSR      8
  6   RET
  8   CONST.I  5
  14  CPTOPSP  -4, 4
  22  CONST.I  0
  28  GT.II
  30  JZ       42
  36  JMP      10
  42  MOVSP    -4
  48  RET

// Compiled O0 
```as
  0   JSR       8
  6   RET
  8   RSADD.I
  10  CONST.I   5
  16  CPDOWNSP  -8, 4
  24  MOVSP     -4
  30  CPTOPSP   -4, 4
  38  CONST.I   0
  44  GT.II
  46  JZ        58
  52  JMP       10
  58  MOVSP     -4
  64  RET

nelak2 avatar Jul 09 '25 07:07 nelak2

Ok, thanks for that legwork. I'll (or maybe somene else!) take a look next time I'm in the compiler code.

tinygiant98 avatar Jul 09 '25 15:07 tinygiant98

Fixed the title and leaving clippy's summary of the issue for whoever looks at this again in a few months or whenever:

"This is the "don't auto initialize variables if it will be initialized immediately afterwards" optimization interacting poorly with the do-while loop being a backwards jump."

nelak2 avatar Jul 09 '25 16:07 nelak2