crash with simple newgrf

[Yexo]

TTDPatch crashes when presented with the following simple newgrf:

Code: Select all

// Automatically generated by GRFCODEC. Do not modify!
// (Info version 7)
// Escapes: 2+ = 71 = D= = DR 2- = 70 = D+ = DF 2< = 7= = D- = DC 2> = 7! = Du* = DM 2u< = 7< 
= D* = DnF 2u> = 7> = Du<< = DnC 2/ = 7G = D<< = DO 2% = 7g = D& 2u/ = 7gG = D| 2u% = 7GG 
= Du/ 2* = 7gg = D/ 2& = 7c = Du% 2| = 7C = D% 2^ 2sto = 2s 2rst = 2r 2+ 2ror = 2rot
// Format: spritenum pcxfile xpos ypos compression ysize xsize xrel yrel

0 * 4 \d3 

1 * 65 08 07 "SER0" "Þ name of generated grf! - nightly-3" 00 "Þno description yet" 00 
2 * 9 0D 0B \D= FF 00 \dxFFFFFFFF 

3 * 5 0D 0B \D/ 0B 0B 

It crashes on sprite 3, but only if the value in param 0B is negative. Tested with ttdpatch 2.6r2321 (if there is a newer version please provide a link, I couldn't find one).

Edit: added linebreaks in the Escapes line and added grf.

[Yexo]

I've looked through the ttdpatch source code and found this piece of code (http://svn.ttdpatch.net/trac/browser/tr ... .asm#L3278)

Code: Select all

.idiv:
        test ebx,ebx
        jz .nozerodiv
        cwd
        idiv ebx
.nozerodiv:
        ret

Shouldn't the CWD be CDQ because the denominator (ebx) is a 32bit value?

[eis_os]

I am sorry, as no one really cares, I simple dump my thoughts on the topic here...

Yes, this sign extend seems wrong, but generally a broken sign extend shouldn't crash TTDPatch, you may get a wrong result.

CWD -> Extend AX to DX:AX
CDQ -> Extend EAX to EDX:EAX

Ok, TTDPatch gets an integer overflow, and it doesn't catch it successfully, hmpf, the compiled code shows clearly to skip the div then...

-edit-
http://svn.ttdpatch.net/trac/changeset/2325

Thanks for reporting

[Yexo]

Thanks, I haven't been able to build my own ttdpatch binaries so far, so I'll test whether it works after the next nightly comes out.

[eis_os]

At least your testgrf should work, after fighting with the debugger, I tested the code, and clearly the overflow handler was executed, but somehow the code after it goes wrong... So while the problem with problematical grfs still crashing TTDPatch isn't fixed, your use case should be fixed now...