On Sun, 28 Apr 2024, Michael Schmitz wrote:
In my tests, the fault seen there had been caused by the movesl in the
section above, hence the fixup at 20b.
Well, that was my point. A fault at MOVE.W should get fixed up at 20b, not
50b. So the patch is wrong, isn't it?
Should we put a NOP here to avoid having the unknown next instruction
(label 9) in the exception table? We can't actually fix up a fault
there unless by chance it was the MOVES that caused it.
... As to the unknown instructions following the final exception label:
These functions, though they contain inline assembly code are not
further inlined, and the instructions following the inline assembly are
simple boilerplate register restore stack saves that ought not to fault
(an invalid stack pointer would have faulted on function entry, if at
all).
But that's just luck. IMHO, the asm is a foot-gun even if it has not gone
off yet.
On balance, I am confident the code is correct as-is. You (and in
particular, Geert) may argue though that the NOP approach follows the
principle of least surprises, and can be considered safe to apply
without further testing on 68060 and Coldfire.
If you're saying that your patch addresses a different bug, fair enough.
All I'm saying is that, since you're adding the NOP anyway, you could make
better use of it.
Anyway, like you, I am keen to hear from others regarding the API issue.
