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

Refactor stackTrace in the vm implementation #160

Closed
haxscramper opened this issue Jan 16, 2022 · 0 comments · Fixed by #268
Closed

Refactor stackTrace in the vm implementation #160

haxscramper opened this issue Jan 16, 2022 · 0 comments · Fixed by #268
Labels
good first issue Easy-to-start-with issue with no in-depth knowledge or complex implementation required. refactor Implementation refactor

Comments

@haxscramper
Copy link
Collaborator

VM rawExecute uses an implicitly injected return statement that is added by template stackTrace(). This makes code much harder to read, since you can't properly see where the loop is terminated.

template stackTrace(
    c: PCtx,
    tos: PStackFrame,
    pc: int,
    sem: ReportTypes,
  ) =
  stackTraceImpl(c, tos, pc, c.debug[pc], instLoc())
  localReport(c.config, c.debug[pc], sem)
- return

Remove implicitly injected return and add it in a way that is visible to the reader.

    of opcLdArr:
      # a = b[c]
      decodeBC(rkNode)
      if regs[rc].intVal > high(int):
        stackTrace(c, tos, pc, reportVmIdx(regs[rc].intVal, high(int)))
+       return

Hint: in order to track all the usages of the stackTrace template, you can add {.hint: "used here".} or {.warning: "used here".}

@haxscramper haxscramper added good first issue Easy-to-start-with issue with no in-depth knowledge or complex implementation required. refactor Implementation refactor labels Jan 16, 2022
bors bot added a commit that referenced this issue Apr 8, 2022
268: Improve internal reporting of errors in `vm`/`vmgen` r=saem a=zerbina

Reporting of guest code related errors was done in an inconsistent
manner via either `stackTrace` or `globalReport`, where `stackTrace` used
`localReport` and an injected `return` statement to terminate the
execution loop. `stackTrace` was sometimes used in nested functions,
thus not terminating execution at all.

### Changes
Both the execution engine and `vmgen` now use exceptions for error
propagation internally, translating them into a result value at their
respective interface edges. In case of an error, the result value stores the
error report. Otherwise, it stores the actual result.

In addition, some usages of `globalReport` are also replaced with the
 new error propagation mechanism.

Depending on whether or not a VM invocation is expected to return
something, errors reported from VM execution and/or `vmgen` are either
turned into a `nkError` node or handled directly via `handleReport`

Fixes #160



Co-authored-by: zerbina <[email protected]>
@bors bors bot closed this as completed in #268 Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Easy-to-start-with issue with no in-depth knowledge or complex implementation required. refactor Implementation refactor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant