Error on valid code with new string return check.

[Y-Less]

https://github.com/Open-GTO/sa-mp-fixes/issues/78

Caused by not correctly checking the destination is actually a copy:

https://github.com/Zeex/pawn/pull/151

@YashasSamaga

[Zeex]

@YashasSamaga Can you take a look please?

[YashasSamaga]

This is difficult fix without introducing new global constants because the conditional operator and the assignment are processed at different places and extra information between them cannot be exchanged directly.

Assume that the expression is:

new arr[10];
arr = 1 ? "str1" : "str2_longer" ;

Here is exactly what's happening:

    addr.pri ffffffd8
    push.pri
    const.pri 1
    jzer 0
    zero.pri
    jump 1
l.0
    const.pri 14
l.1
    pop.alt
    movs 14
    ;$exp
    stack 28
    zero.pri
    retn

The compiler looks for an expression and starts from hier14 (lowest level of precedence). There are 14 hier functions with each of them processing a different set of syntax. The number suffix in their name indicates the precedence level (lower number indicating higher precedence). Every hier function falls to a lower levels before starts its job. The hier functions take a pointer to a value as an argument. They are only interested in what comes next. The hierarchy falls to primary (14->13 ..... 1 -> primary) which processes arr. A series of returns bring the control back to hier14 which sees an assignment operator. hier14 now calls hier13. hier13 is responsible for handling the conditional operator. The control falls again until the 1 is seen by primary after which control comes back to hier13. Now it notices the ? operator and starts processing it. It loads the constant 1 and adds instructions to 'jmp if zero'. It calls itself to start processing (this handles nested conditional operators). The control is returned back and it finds "str1". It adds instructions to load the address of the literal. It then looks for : token. It finds it and plunges again for resolving "str2_longer". After the control returns, it loads the address of "str2_longer" and adds a unconditional jump (execution comes here if the previous jump if zero had failed; refer to the assembly posted above). Since we have information about both the strings, the previous buggy fix used to check the array sizes here. Now it returns back to hier14 (had called hier13 after it found the assignment operator). The hier14 now sees the statement as arr = processed value provided by hier13. hier13 sends "str1" to hier14 which makes hier14 think that it is assigning "str1" to arr which is why the compiler adds the movs instruction with the operand as size of "str1".

Other than global constants and the lvalue which every hier keeps passing around, there is no other link between different hiers. Therefore, the destination check cannot be done at hier13 because it is not aware of the destination and hier14 cannot check for the array sizes of the two expressions of the conditional operator because it is not aware of it (and it shouldn't be aware of it as it's hier13's job).

[YashasSamaga]

Another solution which doesn't seem nice is to provide movs with the largest size to copy. When the execution goes to the branch where the smaller string had to be copied, it will copy a bit of excess but the null character will indicate till where the data should have been copied.

If the user tries to copy two arrays which aren't strings, then they must ensure that the sizes are equal and the destination is capable of holding enough. Again, this can't be enforced easily (as far as my little brain can think) which is basically what the previous buggy PR tried to do.

Another solution would be to repeat code with different movs instructions in each branch but this is complicated and could introduce new bugs.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.