Subject: bug#76180: [feature/igc] Remaining known tracing issues

From: Pip Cet <[email-masked]>
Date: Fri, 14 Feb 2025 12:51:37 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Tue, 11 Feb 2025 20:27:23 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked], [email-masked], [email-masked], [email-masked], [email-masked]
>>
>> Pip Cet <[email-masked]> writes:
>>
>> > diff --git a/src/lread.c b/src/lread.c
>> > index e8cd689d794..67c1bbe5f7b 100644
>> > --- a/src/lread.c
>> > +++ b/src/lread.c
>> > @@ -3483,6 +3483,11 @@ bytecode_from_rev_list (Lisp_Object elems, Lisp_Object readcharfun)
>> >  	            || NILP (vec[CLOSURE_CONSTANTS]))))))
>> >      invalid_syntax ("Invalid byte-code object", readcharfun);
>> >
>> > +#ifdef HAVE_MPS
>> > +  if (XFIXNAT (vec[CLOSURE_STACK_DEPTH]) >= 1022)
>> > +    error ("Byte-code object uses too much stack, increase HORRIBLE_ESTIMATE!");
>> > +#endif
>> > +
>> >    if (STRINGP (vec[CLOSURE_CODE]))
>> >      {
>> >        if (STRING_MULTIBYTE (vec[CLOSURE_CODE]))
>> > --
>> > 2.48.1
>>
>> Sorry, this hunk wasn't the one I meant to send.  This one needs to be
>> applied after it:
>>
>> diff --git a/src/lread.c b/src/lread.c
>> index a638b6eec8e..ae312874574 100644
>> --- a/src/lread.c
>> +++ b/src/lread.c
>> @@ -3576,7 +3576,7 @@ bytecode_from_rev_list (Lisp_Object elems, Lisp_Object readcharfun)
>>      invalid_syntax ("Invalid byte-code object", readcharfun);
>>
>>  #ifdef HAVE_MPS
>> -  if (XFIXNAT (vec[CLOSURE_STACK_DEPTH]) >= 1022)
>> +  if (size > CLOSURE_STACK_DEPTH && XFIXNAT (vec[CLOSURE_STACK_DEPTH]) >= 1022)
>>      error ("Byte-code object uses too much stack, increase HORRIBLE_ESTIMATE!");
>>  #endif
>>
>>
>> to fix the obvious problem :-)
>
> Can't we figure out how far to scan, instead of using some
> more-or-less arbitrary constant?  The comments in scan_bc seem to
> imply that figuring out the distance to scan should be possible?

I've put a lot of debugging statements in, and there are bytecode stacks
in excess of 1024 bytes, but they're rare (org mode and cedet, in
particular).  It also depends on how you build: nativecomp builds seem
to have larger bytecode stacks...

The problem is that the top of stack is usually kept in a register while
exec_byte_code executes, and we don't know which one.  However, I don't
really understand why struct bc_frame can't be expanded to include the
max_stack value when we set up the frame; that way, we'd still be
scanning too much in some cases (since max_stack may not be reached),
but never too little.

We could scan the bytecode stack non-conservatively, but that would
require clearing the stack when we build a new frame.  However, even if
we scan conservatively, it's a lot easier to skip zero words than to
look up random words left over from a previous iteration which are most
likely no longer valid.

I must confess that I've locally changed scan_bc to scan the entire
bytecode stack.  It may be easier to do that and use memclear when we're
popping a frame...

Pip