-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
WIP: make variable types in simulate_unbind match OP_unbind #381
base: master
Are you sure you want to change the base?
Conversation
Did you look at where you are changing the signedness of the variables that are being right-shifted? When you go from signed to unsigned you will lose the sign-extension -- are you sure that that is what is needed? |
The code for the unbind implementation isn't N_OP_unbind, it's the code in inlineC.h --
|
I'm doing this since i'm not sure what's going on. |
Yeah, I've seen way too much cargo-cult programming, mostly on the VLC dev list, where the practitioners are often roundly excoriated for it. I'm not sure that the problem here is actually in the The BIND/UNBIND in the regular opcode stream aren't making new stack frames, are they? They should be stuffing a value into, or marking as unused, slots that are in the current frame. My understanding from the code is that the
is simulating the return of the lower procedure and skipping back to where the \MVLIST would have returned to, if it were actually being called, and if there were any UNBIND operations between here and there, the PVAR entries in those frames need to be unbound (i.e., have the binding smashed to the unbound mark). So, if the stack isn't being properly set up it's not a problem in the simulated unbind, but more likely in the way the return is being faked when the PC of the caller is adjusted, and the lower procedure's stack frame isn't being freed. |
Rereading, a question: shouldn't it only need to simulate unbinds in frames that will still exist after we fake the return? Stack frames that (should) have been released don't need to have adjustments made. |
I've annotated what I think is going on here -- the "outer" code is stackbug, the indented is stkbug1.
|
The next thing to investigate is exactly how it implements the return from STKBUG1 to the point following the \MVLIST... does it actually execute a RETURN opcode to clean up the stack frames? Is it faked and thus (bugily) missing a necessary stack adjustment? |
the problem only happens when returning multiple values to an UNBIND followed by a FN1 \MVLIST. No UNBIND, no problem. |
I did that long ago. It makes no difference whether it's 32- or 64- bit, or little- or big-endian. |
Even if simulate_unbind() does nothing but print the stack pointer and return... the stack pointer passed is increasing by 4 bytes each time, and I don't think that that is correct. I surmise that the problem is not in simulate_unbind(). |
In the case we are having trouble with, the way it returns from
|
and again, with the stack pointer included in the trace -- so what opcode is not having the expected effect on the stack (this notes stack pointer at the start of the opcode)
So when we arrive back at STACKBUG+0x33, we've got 4 more bytes on the stack than when we started. |
it's the missing FN1 \MVLIST, which SHOULD pop the stack with its parameter and PUSH the return value. |
Wouldn't the combination of POP the parameter and PUSH the return value be stack-neutral, so no adjustment should be necessary? What am I missing? I don't think an argument got pushed for \MVLIST, and the result of the MISCN should (?) have been on the stack as the argument for CAR...? |
The call decrements the stack and the return pushes the return value. Yes, FN1 is neutral. |
I've been thinking of ways of doing multiple-value-return in a way that is straightforward but doesn't require mass recompiling / macro changes. I have an idea but it requires changing one Lisp function, \MVLIST as well as maiko -- a coordinated change. The way it's set up now, there are to opcodes, Values and ValuesList. Both return one value (the first for Values and CAR of the list for ValuesList) UNLESS the next opcode is (a) a call to \MVLIST or a RETURN, JUMP, or UNBIND (being the only cases that pass along multiple values). In those cases it keeps on looking for \MVLIST, following up the control stack or the jumps. There are 4 cases: the first is ordinary return to ordinary returnee, MV return to MVLIST returnee , MV return to 1-value returnee, ordinary return to MVLIST returnee. in case 2 it finds the returnees MVLIST call and forces a return with the PC set to skip the call to \MVLIST. In case 3 it doesn't find a MVLIST call and just returns the single value as if cl:values was PROG1. In case 4, you wind up calling \MVLIST which is just (lambda (x) (list x)). The solution I was going to propose was not to modify the PC or frame pointer or simulate any unbinds, but instead pass on a return value (cons '\mvlist values-list) in the case where a fn1 \mvlist was found. at the same change \mvlist (lambda (x) (cond ((and (listp x) (eq (car x) '\mvlist)) (cdr x)) (t (list x))))) But now that I've written this, i want to go back and check to see if simulate_unbind properly pops the bind mark off the stack (if I can remember what BIND does). |
simulate_unbind (and the unbind opcode itself) don't pop the bind mark off the stack, or manipulate the stack pointer at all, they just smash the binding slot with a value that indicates unbound. |
Just posting looking for ssome help. When I stepped through the code in values / simulate_unbind, there were some cases where "value" was a negative int instead of a LispPTR*. So I tried (cargo cult programming) making simulate_unbind look like OP_unbind,
renaming variables and declarations to match. I left the MAKEFREEBLOCK commented out.