* Re: /srv/bzr/emacs/trunk r103781: * src/bytecode.c (Fbyte_code): CAR and CDR can GC.
[not found] <E1Q4zyk-0006Tl-UI@colonialone.fsf.org>
@ 2011-03-30 19:41 ` Daniel Colascione
2011-03-30 21:49 ` Stefan Monnier
0 siblings, 1 reply; 2+ messages in thread
From: Daniel Colascione @ 2011-03-30 19:41 UTC (permalink / raw)
To: emacs-devel
On 3/30/2011 11:04 AM, Stefan Monnier wrote:
> ------------------------------------------------------------
> revno: 103781
> committer: Stefan Monnier<monnier@iro.umontreal.ca>
> branch nick: trunk
> timestamp: Wed 2011-03-30 14:04:11 -0400
> message:
> * src/bytecode.c (Fbyte_code): CAR and CDR can GC.
Wait, what? How? wrong_type_argument never returns. If we run into a
problem, we'll throw up to some higher context and never hit the
AFTER_POTENTIAL_GC case. BEFORE_POTENTIAL_GC (if it's not a noop) sets
the stack top field to point to the top of the stack, but because we
never return from wrong_type_argument, we don't care whether the stack
is lost. v1 might be on the stack, so the only thing keeping it alive
might be stack.top.
Now, Fsignal *does* run signal-hook-function, which of course might GC.
But that's done using call2, which will keep error_symbol and data
alive even if we do GC, so a GC in this case shouldn't actually cause
any problems. Likewise for the debugging hooks. So GC shouldn't
actually cause a problem, IIUC.
What am I missing?
=== modified file 'src/ChangeLog'
--- a/src/ChangeLog 2011-03-30 13:35:37 +0000
+++ b/src/ChangeLog 2011-03-30 18:04:11 +0000
@@ -1,3 +1,7 @@
+2011-03-30 Stefan Monnier <monnier@iro.umontreal.ca>
+
+ * bytecode.c (Fbyte_code): CAR and CDR can GC.
+
2011-03-30 Zachary Kanfer <zkanfer@gmail.com> (tiny change)
* keyboard.c (Fexecute_extended_command): Do log the "suggest key
=== modified file 'src/bytecode.c'
--- a/src/bytecode.c 2011-03-17 02:18:00 +0000
+++ b/src/bytecode.c 2011-03-30 18:04:11 +0000
@@ -554,7 +554,16 @@
{
Lisp_Object v1;
v1 = TOP;
- TOP = CAR (v1);
+ if (CONSP (v1))
+ TOP = XCAR (v1);
+ else if (NILP (v1))
+ TOP = Qnil;
+ else
+ {
+ BEFORE_POTENTIAL_GC ();
+ wrong_type_argument (Qlistp, v1);
+ AFTER_POTENTIAL_GC ();
+ }
break;
}
@@ -580,7 +589,17 @@
{
Lisp_Object v1;
v1 = TOP;
- TOP = CDR (v1);
+ if (CONSP (v1))
+ TOP = XCDR (v1);
+ else if (NILP (v1))
+ TOP = Qnil;
+ else
+ {
+ BEFORE_POTENTIAL_GC ();
+ wrong_type_argument (Qlistp, v1);
+ AFTER_POTENTIAL_GC ();
+ }
+ break;
break;
}
@@ -911,13 +930,13 @@
v1 = POP;
v2 = TOP;
CHECK_NUMBER (v2);
- AFTER_POTENTIAL_GC ();
op = XINT (v2);
immediate_quit = 1;
while (--op >= 0 && CONSP (v1))
v1 = XCDR (v1);
immediate_quit = 0;
TOP = CAR (v1);
+ AFTER_POTENTIAL_GC ();
break;
}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: /srv/bzr/emacs/trunk r103781: * src/bytecode.c (Fbyte_code): CAR and CDR can GC.
2011-03-30 19:41 ` /srv/bzr/emacs/trunk r103781: * src/bytecode.c (Fbyte_code): CAR and CDR can GC Daniel Colascione
@ 2011-03-30 21:49 ` Stefan Monnier
0 siblings, 0 replies; 2+ messages in thread
From: Stefan Monnier @ 2011-03-30 21:49 UTC (permalink / raw)
To: Daniel Colascione; +Cc: emacs-devel
> Wait, what? How? wrong_type_argument never returns. If we run
> into a problem, we'll throw up to some higher context and never hit the
> AFTER_POTENTIAL_GC case.
That's true the AFTER_POTENTIAL_GC will never be executed.
> BEFORE_POTENTIAL_GC (if it's not a noop) sets the
> stack top field to point to the top of the stack, but because we never
> return from wrong_type_argument, we don't care whether the stack is lost.
But even if we never return to this code, the GC will still want to
trace through the byte_code_stacks and will need the `top' field
for that.
> What am I missing?
Without the BEFORE_POTENTIAL_GC you hit the following check in
mark_byte_stack:
/* If STACK->top is null here, this means there's an opcode in
Fbyte_code that wasn't expected to GC, but did. To find out
which opcode this is, record the value of `stack', and walk
up the stack in a debugger, stopping in frames of Fbyte_code.
The culprit is found in the frame of Fbyte_code where the
address of its local variable `stack' is equal to the
recorded value of `stack' here. */
eassert (stack->top);
You could argue that the check is conservative, but it's actually
a useful check and the BEFORE_POTENTIAL_GC I added is very cheap (`car'
and `cdr' very rarely signal errors and the cost of such signalling
dwarfs the cost of BEFORE_POTENTIAL_GC). You could also argue that instead
of BEFORE_POTENTIAL_GC we should simply set .top to .bottom so that the
elements on the stack can be GC'd. If so, I'd agree and would encourage
you to make a patch for it.
Oh, and BTW, currently BEFORE_POTENTIAL_GC and AFTER_POTENTIAL_GC are
usually no-ops because the byte-code stack is stack-allocated and the
stack is already conservatively scanned on most systems. But we could
significantly speed up Elisp function calls if we manage the byte-code
stacks differently (bigger chunks rather than one per function
invacation), so BEFORE/AFTER_POTENTIAL_GC may lose their no-op status in
the future.
Stefan
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-03-30 21:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <E1Q4zyk-0006Tl-UI@colonialone.fsf.org>
2011-03-30 19:41 ` /srv/bzr/emacs/trunk r103781: * src/bytecode.c (Fbyte_code): CAR and CDR can GC Daniel Colascione
2011-03-30 21:49 ` Stefan Monnier
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.