-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
VM callbacks can now raise (and be caught, or show their stacktrace) #13714
Changes from all commits
7ba7afa
9f14c71
b0fba13
caa5ed5
ff76bd1
7a7d877
317de6e
d38481a
16b78f4
65368f8
6117e08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -534,6 +534,27 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = | |
"ra", regDescr("ra", ra), "rb", regDescr("rb", instr.regB), | ||
"rc", regDescr("rc", instr.regC)] | ||
|
||
proc onProcCall(prc: PSym, isClosure: bool, rb = 0, rc = 0, regs2: var seq[TFullReg]) = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note to reviewer: this code blocks is mostly moved without change; the only change involves instances of |
||
let newPc = compile(c, prc) | ||
# tricky: a recursion is also a jump back, so we use the same | ||
# logic as for loops: | ||
if newPc < pc: handleJmpBack() | ||
#echo "new pc ", newPc, " calling: ", prc.name.s | ||
var newFrame = PStackFrame(prc: prc, comesFrom: pc, next: tos) | ||
newSeq(newFrame.slots, prc.offset+ord(isClosure)) | ||
# echo0b (prc.name.s, prc.offset+ord(isClosure),isClosure, rb, rc) | ||
if not isEmptyType(prc.typ[0]): | ||
putIntoReg(newFrame.slots[0], getNullValue(prc.typ[0], prc.info, c.config)) | ||
for i in 1..rc-1: | ||
newFrame.slots[i] = regs2[rb+i] | ||
# echo0b (i, newFrame.slots[i].kind, rc) | ||
if isClosure: | ||
newFrame.slots[rc] = TFullReg(kind: rkNode, node: regs2[rb].node[1]) | ||
tos = newFrame | ||
move(regs, newFrame.slots) | ||
# -1 for the following 'inc pc' | ||
pc = newPc-1 | ||
|
||
case instr.opcode | ||
of opcEof: return regs[ra] | ||
of opcRet: | ||
|
@@ -1193,10 +1214,34 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = | |
let prc = if not isClosure: bb.sym else: bb[0].sym | ||
if prc.offset < -1: | ||
# it's a callback: | ||
c.callbacks[-prc.offset-2].value( | ||
let cb = c.callbacks[-prc.offset-2].addr | ||
template call() = cb[].value( | ||
VmArgs(ra: ra, rb: rb, rc: rc, slots: cast[ptr UncheckedArray[TFullReg]](addr regs[0]), | ||
currentException: c.currentExceptionA, | ||
currentLineInfo: c.debug[pc])) | ||
if cb[].mayRaise: | ||
let info = c.debug[pc] | ||
try: | ||
call() | ||
except Exception as e: | ||
var traceMsg: string | ||
var trace = e.getStackTraceEntries | ||
if trace.len > 0: | ||
# we truncate the stacktrace to only keep the useful portion | ||
var first = 0 | ||
for i in countdown(trace.len-1, 0): | ||
if trace[i].procname == "rawExecute": | ||
first = i | ||
break | ||
trace = trace[first..^1] | ||
traceMsg = toStrDefault(trace) | ||
let vars = [$e.name, e.msg, traceMsg] | ||
var regs2 = newSeq[TFullReg](vars.len) | ||
for i, ai in [$e.name, e.msg, traceMsg]: | ||
putIntoReg(regs2[i], newStrNode(ai, info)) | ||
onProcCall(c.graph.nimInternalNewException, isClosure = false, rb = -1, rc = 1 + regs2.len, regs2) | ||
else: | ||
call() | ||
elif importcCond(prc): | ||
if compiletimeFFI notin c.config.features: | ||
globalError(c.config, c.debug[pc], "VM not allowed to do FFI, see `compiletimeFFI`") | ||
|
@@ -1221,23 +1266,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = | |
else: | ||
globalError(c.config, c.debug[pc], "VM not built with FFI support") | ||
elif prc.kind != skTemplate: | ||
let newPc = compile(c, prc) | ||
# tricky: a recursion is also a jump back, so we use the same | ||
# logic as for loops: | ||
if newPc < pc: handleJmpBack() | ||
#echo "new pc ", newPc, " calling: ", prc.name.s | ||
var newFrame = PStackFrame(prc: prc, comesFrom: pc, next: tos) | ||
newSeq(newFrame.slots, prc.offset+ord(isClosure)) | ||
if not isEmptyType(prc.typ[0]): | ||
putIntoReg(newFrame.slots[0], getNullValue(prc.typ[0], prc.info, c.config)) | ||
for i in 1..rc-1: | ||
newFrame.slots[i] = regs[rb+i] | ||
if isClosure: | ||
newFrame.slots[rc] = TFullReg(kind: rkNode, node: regs[rb].node[1]) | ||
tos = newFrame | ||
move(regs, newFrame.slots) | ||
# -1 for the following 'inc pc' | ||
pc = newPc-1 | ||
onProcCall(prc, isClosure, rb = rb, rc = rc, regs) | ||
else: | ||
# for 'getAst' support we need to support template expansion here: | ||
let genSymOwner = if tos.next != nil and tos.next.prc != nil: | ||
|
@@ -1513,6 +1542,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = | |
rc = instr.regC | ||
idx = int(regs[rb+rc-1].intVal) | ||
callback = c.callbacks[idx].value | ||
# xxx: consider using same treatment as for other VmArgs callback | ||
args = VmArgs(ra: ra, rb: rb, rc: rc, slots: cast[ptr UncheckedArray[TFullReg]](addr regs[0]), | ||
currentException: c.currentExceptionA, | ||
currentLineInfo: c.debug[pc]) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2224,7 +2224,6 @@ when notJSnotNims: | |
include "system/arithm" | ||
{.pop.} | ||
|
||
|
||
when not defined(js): | ||
# this is a hack: without this when statement, you would get: | ||
# Error: system module needs: nimGCvisit | ||
|
@@ -3011,3 +3010,25 @@ export io | |
|
||
when not defined(createNimHcr): | ||
include nimhcr | ||
|
||
when defined(nimHasInternalProc): | ||
## {.internalproc.} is analog to {.compilerproc.} but is used for semantic | ||
## phase instead of codegen. It simplifies implementation by using nim code | ||
## instead of building the AST by hand, while also avoiding making these | ||
## public. | ||
proc nimInternalNewException(name: string, msg: string, traceMsg: string) {.internalproc.} = | ||
## internal proc for raising from vmpops | ||
template fun(T) = | ||
if name == astToStr(T): | ||
var msg = msg | ||
if traceMsg.len > 0: | ||
# we could query for whether nim was build with --stacktrace to | ||
# provide more accurate msg, but not sure if it's exposed. | ||
msg.add "\nVM callback stacktrace (compile nim with --stacktrace if needed)\n" & traceMsg & "\n" | ||
raise newException(T, msg) | ||
# can add more hardcoded standard types here | ||
fun(OSError) | ||
fun(IOError) | ||
fun(AssertionError) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does thins mean that only There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this bit needs improvement. Can't you just pass already created object and raise it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it is not possible to use compilerProc here then it needs to be moved into compiler There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is horrible. |
||
# if hits here, should be easy to handle | ||
doAssert false, "`nimInternalNewException` does not yet handle " & name |
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.
Important to avoid introducing new pragmas