Re: master 8c81818673a 6/7: Tune volatile in read_char

[Pip Cet <pipcet@protonmail.com>]

"Paul Eggert" <eggert@cs.ucla.edu> writes:

> On 2024-08-19 13:08, Pip Cet wrote:
>> one can come up with similar examples without a
>> backward jump, which loop unquittably even in emacs-30
>
> Ouch, in that case this issue happens regardless of the recent
> 'volatile'-related changes, and we should address the issue regardless
> of what we do with 'volatile'.

Agreed.

> Would the attached patch fix the issue in master? It can be tuned
> further (and obviously needs comments); I'm just trying to see whether I
> understand the point you're making.

 		pc = bytestr_data;
 		PUSH (c->val);
+		if (BYTE_CODE_SAFE)
+		  quitcounter += ! (op < 0);
 		goto op_branch;
 	      }

Yes, that does work, though I had to fix a bug in my code and it now
reads:

(disassemble #[65535 "\210\3011\0\0\202\013\0\302\211\241\202\010\000"
[argument (wrong-type-argument) 0 t] 65536])

But, as you say, it could be simplified:

 		pc = bytestr_data;
 		PUSH (c->val);
+		if (BYTE_CODE_SAFE)
+		  quitcounter++;
 		goto op_branch;
 	      }

'op' is relative to the beginning of the byte string, so it should never
be less than 0.  If it is, and BYTE_CODE_SAFE is true, we will call
emacs_abort in op_branch.

> This patch affects behavior only if BYTE_CODE_SAFE, because as I
> understand things we trust bytecode anyway otherwise.

(That reminds me of a bug that I hunted down in which it turned out gcc
helpfully generated different code depending on whether -g (not -Og) was
specified or not.  I would prefer to err on the side of making things
reproducible and unconditionally increment quitcounter.)

By the way, I was very surprised that 'signal_or_quit', by way of
'Fmemq', calls 'maybe_quit'.  I hope that cannot result in infinite
recursion.

Pip