-
Notifications
You must be signed in to change notification settings - Fork 102
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
add: Runtime.LoadScript #766
Conversation
Add a return value? |
@@ -162,7 +162,7 @@ public EvaluationStack ExecuteTestCaseStandard(int offset, ushort rvcount, NefFi | |||
Console.WriteLine("op:[" + | |||
this.CurrentContext.InstructionPointer.ToString("X04") + | |||
"]" + | |||
this.CurrentContext.CurrentInstruction.OpCode); | |||
this.CurrentContext.CurrentInstruction?.OpCode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@erikzhang the unique weird thing is that, the current instruction was null after the dynamic script
op:[019D] INITSLOT
op:[01A0] PUSHDATA1
op:[01A3] CONVERT
op:[01A5] CONVERT
op:[01A7] STLOC0
op:[01A8] LDARG1
op:[01A9] LDARG0
op:[01AA] PUSH2
op:[01AB] PACK
op:[01AC] PUSH15
op:[01AD] LDLOC0
op:[01AE] SYSCALL
op:[0000] ADD
+ op:[0001]
op: [01B3] JMP
op:[01B5] RET
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After execute "ADD", the current instruction point will auto jump to "Next" point even it is an outrange point. (see here)
internal bool MoveNext()
{
Instruction? current = CurrentInstruction;
if (current is null) return false;
InstructionPointer += current.Size;
return InstructionPointer < Script.Length;
}
Dynamic Script length is 1, InstructionPointer +1, InstructionPointer will be 1.
In the next iteration, the out-range point will get a "null" instruction.
By the way, the vm will auto change "null" to "RET" instruction, so it won't be fault in vm(https://github.com/neo-project/neo-vm/blob/f48de7e595ade7c72be6ccd5cce0ab99708f0afd/src/Neo.VM/ExecutionEngine.cs#L1450).
Merge? |
It's ok for me |
Require neo-project/neo#2756