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

Compute static memory requirements of methods. #218

Merged
merged 5 commits into from May 30, 2018

Conversation

@dvander
Copy link
Member

dvander commented May 29, 2018

This series of patches adds a few new verification steps. The first is that the stack must be balanced. The amount of stack space used must be equal across all incoming edges to a block. This has a few important implications:

  1. The compiler is not allowed to leak stack space, which in a loop could easily yield a run-time error.
  2. The amount of stack space used by a method must be static.
  3. The operand of any stack-affecting instruction can be assigned a constant offset from the base of the stack frame. (This will lead to big wins in the JIT).

The algorithm is simple: for each opcode, the stack balance is adjusted based on how much it adds or removes from the stack. When parsing each block, its initial stack balance is inherited from any predecessor, and it must be equal across all predecessors. Loop edges must be checked again later since they are cyclic.

In addition, a second verification step is that stack offsets must now be within each block's stack balance. Even if the stack offset is valid for some other block, it must also be valid locally.

These changes did cause a few new failures in my .smx corpus. Four copies of "THC RPG" [1], three dated 02/15/2014 and one dated 07/14/2013 (both using SourceMod 1.5.x) have an odd bug. One block references a local variable that does not exist, and it is quite clear from the bytecode that something is wrong. Lysis manages to assign a name to this stack offset, but the name isn't anywhere in the debug symbols. I don't know what happened, but this plugin is unsupported so I'm not too worried.

A plugin that used to fail verification, but now passes, is from the Lysis test suite. "anlol.smx" (dated 02/24/2016, SM 1.7.3) has a wildly huge stack offset, -1000000004, which we used to statically determine was impossible to satisfy. I removed this check because now it can be encapsulated in the runtime's stack space check, which I'll do as a separate patch when I go to optimize the JIT. This plugin has been heavily obfuscated so it is certainly not generated by a supported compiler.

Finally, a plugin called "PUNISHER old v34" [2] fails the new requirements. It is again heavily obfuscated by some third-party compiler. Unfortunately this compiler inserted an extra pop.alt instruction on an empty stack, which is nonsensical and not allowed.

[1] Attachments 114946, 130624, 130633, 122854
[2] Attachment 164723 (08/04/2017, SM 1.7)

@dvander dvander merged commit 50a82e9 into master May 30, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dvander dvander deleted the cfg branch May 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.