cai567890 / pcsx2

Automatically exported from code.google.com/p/pcsx2
1 stars 0 forks source link

SPU2Init crashes due to inconsistent type sizes under GCC (long != u32) #225

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hi, I just compiled SVN and tried to run it under Ubuntu 9.04. I have the
following graphics card: 01:00.0 VGA compatible controller: ATI
Technologies Inc RV610 video device [Radeon HD 2400 PRO]

I've never used this successfully, but when I run ./pcsx2 the first time it
asks me to configure it. I selected the bios and plugin directory, exit and
run it again. This time I get:

[ Aside: No-where on your website does it explain what the 'bios' is or why
you need it. Not very newbie friendly! ]

t@sage:~/Local/pcsx2/pcsx2/bin$ gdb ./pcsx2 
GNU gdb 6.8-debian
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu"...
(gdb) run
Starting program: /home/t/Local/pcsx2/pcsx2/bin/pcsx2 
[Thread debugging using libthread_db enabled]

[New Thread 0x7f895372a790 (LWP 5008)]
*** buffer overflow detected ***: /home/t/Local/pcsx2/pcsx2/bin/pcsx2
terminated
======= Backtrace: =========
/lib/libc.so.6(__fortify_fail+0x37)[0x7f894febf2c7]
/lib/libc.so.6[0x7f894febd170]
/home/t/Local/pcsx2/pcsx2/bin/plugins/libZeroSPU2.so.0.1.0(_Z8InitADSRv+0x1c)[0x
7f893d3b9a9c]
/home/t/Local/pcsx2/pcsx2/bin/plugins/libZeroSPU2.so.0.1.0(SPU2init+0x159)[0x7f8
93d3bb209]
/home/t/Local/pcsx2/pcsx2/bin/pcsx2[0x4506d3]
/home/t/Local/pcsx2/pcsx2/bin/pcsx2[0x4555b5]
/home/t/Local/pcsx2/pcsx2/bin/pcsx2[0x41c919]
/home/t/Local/pcsx2/pcsx2/bin/pcsx2[0x41ceed]
/lib/libc.so.6(__libc_start_main+0xe6)[0x7f894fdde5a6]
/home/t/Local/pcsx2/pcsx2/bin/pcsx2[0x4068b9]
======= Memory map: ========
00400000-0061b000 r-xp 00000000 08:01 36929863                          
/home/t/Local/pcsx2/pcsx2/bin/pcsx2

(snip)

(gdb) bt
#0  0x00007f894fdf2fb5 in raise () from /lib/libc.so.6
#1  0x00007f894fdf4bc3 in abort () from /lib/libc.so.6
#2  0x00007f894fe32228 in ?? () from /lib/libc.so.6
#3  0x00007f894febf2c7 in __fortify_fail () from /lib/libc.so.6
#4  0x00007f894febd170 in __chk_fail () from /lib/libc.so.6
#5  0x00007f893d3b9a9c in InitADSR () from plugins/libZeroSPU2.so.0.1.0
#6  0x00007f893d3bb209 in SPU2init () from plugins/libZeroSPU2.so.0.1.0
#7  0x00000000004506d3 in InitPlugins ()
#8  0x00000000004555b5 in LoadPlugins ()
#9  0x000000000041c919 in SysInit ()
#10 0x000000000041ceed in main ()

Note that I also tried selecting SPU2null. This changes the backtrace to:

#0  0x00007f3107e77fb5 in raise () from /lib/libc.so.6
#1  0x00007f3107e79bc3 in abort () from /lib/libc.so.6
#2  0x00007f3107eb7228 in ?? () from /lib/libc.so.6
#3  0x00007f3107f442c7 in __fortify_fail () from /lib/libc.so.6
#4  0x00007f3107f42170 in __chk_fail () from /lib/libc.so.6
#5  0x00007f30f5451e5c in InitADSR () from plugins/libSPU2null.so
#6  0x00007f30f5454058 in SPU2init () from plugins/libSPU2null.so
#7  0x00000000004506d3 in InitPlugins ()
#8  0x00000000004555b5 in LoadPlugins ()
#9  0x000000000041c919 in SysInit ()
#10 0x000000000041ceed in main ()

I.e. basically the same.

Original issue reported on code.google.com by tdh...@gmail.com on 17 May 2009 at 3:51

GoogleCodeExporter commented 9 years ago
OK well I found the bug. In SPU2.cpp of SPU2null and in the other SPU2 plugins 
we
have this code:

u32 RateTable[160];

...

void InitADSR()                                    // INIT ADSR
{
    unsigned long r,rs,rd;
    int i;
    memset(RateTable,0,sizeof(unsigned long)*160);

Obviously this doesn't work so well on 64 bit machines where sizeof(unsigned 
long) is
8! I fixed it and now it doesn't crash.

Are none of the developers on 64 bit machines? Bet you wish all bug reports 
were like
this!

Original comment by tdh...@gmail.com on 17 May 2009 at 4:03

GoogleCodeExporter commented 9 years ago
Ah, yup.  That'd do it.  I'll assign to arcum for fixing.

>> Are none of the developers on 64 bit machines?

Not all, but most.  However that doesn't matter since two reasons:

 * With the exception of Arum we're Windows developers, and windows provisions us with a 
foolproof 32-bit environment without any hassle or even a first or second 
thought.

 * MSVC compiles C code using uniform type sizes.  'long' in 64 bit modes is still 4 bytes, 
which is the *LOGICAL* thing to do since compilers have long (aha pun) since 
introduced 
special 64 and 128 bit data types.  GCC deciding to upgrade 'long' to 64 bits 
just for x64 
builds is useless from a programmer's standpoint, and only serves to break old 
code.

For that matter, gcc in -m32 mode *should* be using a 32 bit value for longs.  
Even if there 
is some justifiable reason for using 64 bit longs when in 64 bit target mode, 
there's no 
reason to do so with the -m32 option specified.

Original comment by Jake.Stine on 17 May 2009 at 5:54

GoogleCodeExporter commented 9 years ago
Ah cool. I checked and -m32 does give 4-byte longs. I'm kind of with you on the 
long
issue, but then again it has always been emphasized that long has a *minimum* 
size of
4 bytes. C++0x should fix this issue anyway by introducing fixed-size types.

Anyway, it would still be good to have a configure flag. I think all it needs 
to do
is add -m32 to all the C*FLAGS and make sure that cpu64=yes never happens (see 
the
configure.ac file). This should be fairly easy to do. It won't completely solve
things because it is still a hassle to actually install the 32 bit libs. I 
think you
can fairly easily do most of them using ia32-apt-get, but the nvidia-cg-toolkit 
is a
pain.

I have given up for now and will install windows on a spare partition! Keep up 
the
good work.

Original comment by tdh...@gmail.com on 17 May 2009 at 6:41

GoogleCodeExporter commented 9 years ago
-m32's already on all the flags, and I've taken out the stuff in configure.ac
throughout svn now, as well as fixed the section you mentioned in zerospu2 and 
SPU2Null. 

I'll go over some of the other plugins and pcsx2 a few times and change what I 
can as
far as longs and unsigned longs. A 32-bit chroots still the easiest method, and 
it's
what I personally use. I was able to build pcsx2 with plugins outside of a 
chroot and
boot into the bios and KH1 today as of r1214, though I cheated a little. 

(Since I had the chroot handy, I copied a few of the 32 bit libraries from it to
where my distribution keeps 32 bit libraries, /usr/lib32. Easier then trying to 
find
them...)

Original comment by arcum42@gmail.com on 18 May 2009 at 9:41

GoogleCodeExporter commented 9 years ago
I'd consider this one fixed, since I've made the changes to SPU2null & 
ZeroSPU2, and
removed the cpu64 code in the various configure.a files, though I'll continue
checking other files for unsigned longs...

Original comment by arcum42@gmail.com on 19 May 2009 at 9:31

GoogleCodeExporter commented 9 years ago
Sounds good. It will still be possible to compile in 64 bit mode right? I might 
try
my hand at making it work as well as the windows version (with the one PS2 game 
I
have)... I have a feeling it is a problem with the GS plugin - the zeroGS one 
is the
only available one on Linux and it didn't work too well when I tried it in 
windows
either.

Original comment by tdh...@gmail.com on 19 May 2009 at 10:05

GoogleCodeExporter commented 9 years ago
Yeah, I don't even recommend ZeroGS on pcsx2 despite the fact that it's the 
only one
in the trunk that works on Linux. Use ZZogl on Linux:
http://forums.pcsx2.net/thread-4108.html

And on Windows, I'd use GSdx.

Original comment by arcum42@gmail.com on 19 May 2009 at 10:08