graphitemaster / gmqcc

An Improved Quake C Compiler
Other
160 stars 28 forks source link

entfield expressions and ast_member expressions can overwrite immediates and globals table #181

Open graphitemaster opened 6 years ago

graphitemaster commented 6 years ago

Minimal test case

.vector vec;
void main()
{
    entity e = spawn();
    (e.vec = '0 0 0').x += 1;
    print(vtos(e.vec), "\n");
}

This should print '1 0 0' however the .x += 1 expression binds to the literal '0 0 0' instead of e.vec which overwrites the immediate. Using literal '0 0 0' anywhere now will actually become '1 0 0'. This is true for anything on the right hand side of the entfield assignment, other globals, or entfields will be overwritten too.

graphitemaster commented 6 years ago

Upon further inspection this is actually true for all ast_member this also exibits the same problem

void main()
{
    vector x;
    (x = '0 0 0').x += 1;
    print(vtos(x));
}
graphitemaster commented 6 years ago

Xonotic has a few places where this is miscompiling and they don't even realise it there yet. Just did a coverage check.

graphitemaster commented 6 years ago

This behavior also manifests for arrays too. We shuffle the regular parsing rules for ent.foo[n] to behave as ent.(foo[n]) opposed to (ent.foo)[n], as it should by the binding logic does not follow suit, instead (ent.foo[n] = '0 0 0').x += 1 binds to the literal n and causes a compiler error.

Blub commented 6 years ago

The inner workings of the qcvm warrant the label change to 'feature' ;p