Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
graphitemaster opened this issue Nov 27, 2017 · 4 comments

Comments

@graphitemaster
Copy link
Owner

graphitemaster commented Nov 27, 2017

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
Copy link
Owner Author

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 graphitemaster changed the title entfield expressions can overwrite immediates and globals table entfield expressions and ast_member expressions can overwrite immediates and globals table Nov 27, 2017
@graphitemaster
Copy link
Owner Author

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

@graphitemaster
Copy link
Owner Author

graphitemaster commented Dec 1, 2017

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 Blub added A-Feature and removed E-Urgent labels Jan 14, 2018
@Blub
Copy link
Collaborator

Blub commented Jan 14, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants