* Jason Pleau <[email protected]> [141111 22:34]: > Hello ! > > I've been trying to find a way to fix this issue as well for a few days now. > > I managed to get it to work with this following changes (see attached file) > > The rename of usr/bin/zsnesd => usr/bin/zsnes could probably be handled > better (maybe with dh_links ?), for now as long as I can load my game > states I'm happy :) > > Thoughts on this patch? I'm not sure how adding --enable-debug fixes > this, so it could very well lead to other issues.
Hi, Thanks indeed for work! It's indeed the -O0 that fixes the issue in that case. It's also possible to disable hardening while keeping optimizations. I didn't publish my patch when I worked on this last year, but here it is. It's tempting to apply the following hunk to continue the job and eliminate the last copy_func call: > - copy_func(&buffer, &Op02FX, 11*4+3*4+28*8); > + COPY_SHORT(Op02FX); > + COPY_SHORT(Op02FY); > + COPY_SHORT(Op02FZ); > + COPY_SHORT(Op02LFE); > + COPY_SHORT(Op02LES); > + COPY_SHORT(Op02AAS); > + COPY_SHORT(Op02AZS); but unfortunately it makes it crash again and I'm not sure I understand why (especially one year later). -- Etienne Millon
Author: Etienne Millon <[email protected]> Description: Fix loading for DSP1 ROMS with _FORTIFY_SOURCE The original code uses a memory copy scheme that is unsafe and triggers fatal errors under _FORTIFY_SOURCE. More precisely, the scope of this patch is ROMs that use the DSP-1 coprocessor. . The code copies data by assuming that global variables are contiguous in memory: it takes the address of the first variable and uses it to copy several ones. . For variables declared in assembly, a new symbol DSP1state is added so that memory can be copied as an array. . For variables declared in C, it is trickier as it's not possible to alias them with an array. So a macro is used to copy variables of type short. . There is padding at several places: - 2 bytes after each short - 128 bytes after each "block" of variables - extra blocks of 236, 112 and 84 bytes . The purpose of the last ones (declared with "PADn") is unknown, probably other global variables that have been moved in the meantime. . In order to preserve compatibility with existing state files, this padding is kept. Data read from these offsets is discarded when loading state, and zeroes are written when saving state. Bug-Debian: http://bugs.debian.org/727781 Last-Update: 2013-10-27 --- --- a/src/chips/dsp1proc.asm +++ b/src/chips/dsp1proc.asm @@ -307,6 +307,7 @@ ret SECTION .bss +NEWSYM DSP1state NEWSYM DSP1COp, resb 1 NEWSYM DSP1RLeft, resb 1 NEWSYM DSP1WLeft, resb 1 --- a/src/gblvars.h +++ b/src/gblvars.h @@ -35,7 +35,7 @@ extern unsigned int tempesi, tempedi, tempedx, tempebp; extern unsigned int SPCMultA, PHnum2writespc7110reg, PHdspsave2; extern unsigned char sndrot[], SPCRAM[65472], DSPMem[256], SA1Status, *SA1RAMArea, *SPCState; -extern unsigned char DSP1Enable, DSP1COp, prevoamptr, BRRBuffer[], *romdata; +extern unsigned char DSP1Enable, DSP1state[70], DSP1COp, prevoamptr, BRRBuffer[], *romdata; extern unsigned char curcyc, echoon0[], spcnumread, NextLineCache, HIRQNextExe; extern unsigned char vidmemch4[4096], vidmemch8[4096], vidmemch2[4096]; --- a/src/zstate.c +++ b/src/zstate.c @@ -186,22 +186,126 @@ if (DSP1Enable && (method != csm_load_zst_old)) { - copy_func(&buffer, &DSP1COp, 70+128); - copy_func(&buffer, &Op00Multiplicand, 3*4+128); - copy_func(&buffer, &Op10Coefficient, 4*4+128); - copy_func(&buffer, &Op04Angle, 4*4+128); - copy_func(&buffer, &Op08X, 5*4+128); - copy_func(&buffer, &Op18X, 5*4+128); - copy_func(&buffer, &Op28X, 4*4+128); - copy_func(&buffer, &Op0CA, 5*4+128); - copy_func(&buffer, &Op02FX, 11*4+3*4+28*8+128); - copy_func(&buffer, &Op0AVS, 5*4+14*8+128); - copy_func(&buffer, &Op06X, 6*4+10*8+4+128); - copy_func(&buffer, &Op01m, 4*4+128); - copy_func(&buffer, &Op0DX, 6*4+128); - copy_func(&buffer, &Op03F, 6*4+128); - copy_func(&buffer, &Op14Zr, 9*4+128); - copy_func(&buffer, &Op0EH, 4*4+128); + unsigned char padding[128]; + memset(padding, 0, 128); +#define COPY_SHORT(X) do {\ + extern short X;\ + copy_func(&buffer, &(X), sizeof(short));\ + copy_func(&buffer, padding, 2);\ + } while(0) +#define PADn(N) do {\ + copy_func(&buffer, padding, N);\ + } while(0) +#define PAD PADn(128) + + copy_func(&buffer, DSP1state, 70); + PAD; + + COPY_SHORT(Op00Multiplicand); + COPY_SHORT(Op00Multiplier); + COPY_SHORT(Op00Result); + PAD; + + COPY_SHORT(Op10Coefficient); + COPY_SHORT(Op10Exponent); + COPY_SHORT(Op10CoefficientR); + COPY_SHORT(Op10ExponentR); + PAD; + + COPY_SHORT(Op04Angle); + COPY_SHORT(Op04Radius); + COPY_SHORT(Op04Sin); + COPY_SHORT(Op04Cos); + PAD; + + COPY_SHORT(Op08X); + COPY_SHORT(Op08Y); + COPY_SHORT(Op08Z); + COPY_SHORT(Op08Ll); + COPY_SHORT(Op08Lh); + PAD; + + COPY_SHORT(Op18X); + COPY_SHORT(Op18Y); + COPY_SHORT(Op18Z); + COPY_SHORT(Op18R); + COPY_SHORT(Op18D); + PAD; + + COPY_SHORT(Op28X); + COPY_SHORT(Op28Y); + COPY_SHORT(Op28Z); + COPY_SHORT(Op28R); + PAD; + + COPY_SHORT(Op0CA); + COPY_SHORT(Op0CX1); + COPY_SHORT(Op0CY1); + COPY_SHORT(Op0CX2); + COPY_SHORT(Op0CY2); + PAD; + + copy_func(&buffer, &Op02FX, 11*4+3*4+28*8); + PAD; + + COPY_SHORT(Op0AVS); + COPY_SHORT(Op0AA); + COPY_SHORT(Op0AB); + COPY_SHORT(Op0AC); + COPY_SHORT(Op0AD); + PADn(14*8); + PAD; + + COPY_SHORT(Op06X); + COPY_SHORT(Op06Y); + COPY_SHORT(Op06Z); + COPY_SHORT(Op06H); + COPY_SHORT(Op06V); + COPY_SHORT(Op06M); + PADn(10*8+4); + PAD; + + COPY_SHORT(Op01m); + COPY_SHORT(Op01Zr); + COPY_SHORT(Op01Xr); + COPY_SHORT(Op01Yr); + PAD; + + COPY_SHORT(Op0DX); + COPY_SHORT(Op0DY); + COPY_SHORT(Op0DZ); + COPY_SHORT(Op0DF); + COPY_SHORT(Op0DL); + COPY_SHORT(Op0DU); + PAD; + + COPY_SHORT(Op03F); + COPY_SHORT(Op03L); + COPY_SHORT(Op03U); + COPY_SHORT(Op03X); + COPY_SHORT(Op03Y); + COPY_SHORT(Op03Z); + PAD; + + COPY_SHORT(Op14Zr); + COPY_SHORT(Op14Xr); + COPY_SHORT(Op14Yr); + COPY_SHORT(Op14U); + COPY_SHORT(Op14F); + COPY_SHORT(Op14L); + COPY_SHORT(Op14Zrr); + COPY_SHORT(Op14Xrr); + COPY_SHORT(Op14Yrr); + PAD; + + COPY_SHORT(Op0EH); + COPY_SHORT(Op0EV); + COPY_SHORT(Op0EX); + COPY_SHORT(Op0EY); + PAD; +#undef COPY_SHORT +#undef PAD +#undef PADn } if (SETAEnable)
