Unbalanced stack push/pop when processing chained input events

[matseh]

InputHandler ; A0-InputEvent, A1-Data Area
	MOVEM.L	D1/A0-A3,-(SP)
	TST.B	StopInputFlag
	BEQ.B	inphend
	SUB.L	A2,A2
	MOVE.L	A0,A1
	MOVE.B	4(A1),D0	; ie_Class
	CMP.B	#1,D0		; RAWKEY
	BEQ.B	InpRawkey
	CMP.B	#2,D0		; RAWMOUSE
	BEQ.W	xInpRawMouse
	MOVE.L	A1,A2
InpNext	MOVE.L	(A1),A1
	MOVE.L	A1,D0
	BNE.B	InputHandler  <---- This ends up pushing registers to stack again for every input event in the chain.
inphend	MOVE.L	A0,D0
	MOVEM.L	(SP)+,D1/A0-A3
	RTS

[8bitbubsy]

I don't think I've had a crash (or weird behavior) related to input before, kind of odd how the RTS seems to read the right jump address from the stack. Maybe the last branch is never (or extremely rarely) taken? I will of course fix this anyway, it's really nasty. Thanks for reporting it!

[8bitbubsy]

Hm, ProTracker 1.2 from 1991 also has the same code at line 1042, so this is probably fine: https://16-bits.org/pt_src/tracker/PT1.2A.s

[matseh]

It works fine as long as you only get one input event at a time but this is not always the case. Especially if you're using additional tools like commodities that insert events into the chain. I have no problem crashing ProTracker. Sometimes with just quick keyboard input. I have made a build with only a change in the loop label that should solve the problem. I haven't been able to reproduce crashes with this build.

InputHandler ; A0-InputEvent, A1-Data Area
	MOVEM.L	D1/A0-A3,-(SP)
	TST.B	StopInputFlag
	BEQ.B	inphend
	SUB.L	A2,A2		; A2.L -> Previous InputEvent in output InputEvent chain.
	MOVE.L	A0,A1		; A1.L -> Current InputEvent in input InputEvent chain.
inploop	MOVE.B	4(A1),D0	; ie_Class
	CMP.B	#1,D0		; RAWKEY
	BEQ.B	InpRawkey
	CMP.B	#2,D0		; RAWMOUSE
	BEQ.W	xInpRawMouse
	MOVE.L	A1,A2
InpNext	MOVE.L	(A1),A1		; Move to the next InputEvent in the input InputEvent chain.
	MOVE.L	A1,D0		; Check if the end of the input InputEven chain is reached.
	BNE.B	inploop
inphend	MOVE.L	A0,D0
	MOVEM.L	(SP)+,D1/A0-A3
	RTS

[8bitbubsy]

Chances are no reg pushing is needed at all for input handlers, I’ll find out soon.

[8bitbubsy]

I didn't find out if it's safe to not preserve registers in an input handler, so I decided to add your fix (with some tweaks).
Can you test the current code in the github repo to see if it's crash-free? Thanks. 👍🏻

[matseh]

I have now tested in the setup where I previously could easily produce crashes and I haven't had a single crash.

[8bitbubsy]

Thanks. I think this one is good to go then. I had forgotten to push some extra registers (which where used for retrieving the mouse coords), so this fix also fixed that.