I just ran across an evil little bug in the WINSPOOL_GetPrinter_2 function. It looks like this type of bug could be hiding in other API functions too. It causes a segmentation fault because of an unaligned access on Solaris (sparc).
Yikes. That's a bad one. The trouble is MS loves this sort of return value. Even if the dll itself doesn't dereference an unaligned pointer, the caller might depending on how things are layed out. The trouble is i386 allows unaligned memory access, so we don't see this sort of thing.
If you happen to know how MS handles alignment on platforms that require it, we might be able to fix it. Otherwise it'll have to be case-by-case I'm afraid.
--Juan
_______________________________ Do you Yahoo!? Declare Yourself - Register online to vote today! http://vote.yahoo.com
Juan Lang a écrit :
I just ran across an evil little bug in the WINSPOOL_GetPrinter_2 function. It looks like this type of bug could be hiding in other API functions too. It causes a segmentation fault because of an unaligned access on Solaris (sparc).
Yikes. That's a bad one. The trouble is MS loves this sort of return value. Even if the dll itself doesn't dereference an unaligned pointer, the caller might depending on how things are layed out. The trouble is i386 allows unaligned memory access, so we don't see this sort of thing.
If you happen to know how MS handles alignment on platforms that require it, we might be able to fix it. Otherwise it'll have to be case-by-case I'm afraid.
MSVC supports __unaligned as pointer attribute. So declaring *(__unaligned long*)ptr will force the compiler (on CPUs/OSes which requires it) to generate the relevant code for this. It doesn't seem however that gcc supports such keyword.
A+
Juan Lang a écrit :
Yikes. That's a bad one. The trouble is MS loves this sort of return value. Even if the dll itself doesn't dereference an unaligned pointer, the caller might depending on how things are layed out. The trouble is i386 allows unaligned memory access, so we don't see this sort of thing.
I did some more reading, and found there's a way to get the kernel to install a handler for unaligned accesses which will synthesize the unaligned access with two aligned accesses, and then jump back to the original point in the code. It's slow, but you only pay the price if you do an unaligned access, where we would currently crash. The code to enable this handler is nice and simple: asm("ta 6"); /* 6 == ST_FIX_ALIGN defined in sys/trap.h */ I tossed this code in SIGNAL_Init() in signal_sparc.c, and it prevented the crash (when I tested, disabling my previous fix).
Since it sounds like there may be more of these bugs lurking deep within wine, it seems like the best solution would be to have this automatic code enabled so the application doesn't crash, but to find a way to report it with ERR or WARN so the problem can be fixed -- even on i386, aligned memory reads should be faster (I think).
I don't know if there's an easy way to print a message and then jump to the system's FIX_ALIGN handler. I did find some code that is supposed to do the same thing as the system's handler: http://groups.google.com/groups?selm=D9to5q.531%40beaver.cs.washington.edu&a... which could be useful if it wasn't possible to chain to the system's handler.
Do you feel it's reasonable just to turn on ST_FIX_ALIGN, which *could* mask errors both in wine and in the user's code?
Eric
--- Eric Frias efrias@syncad.com wrote:
I did some more reading, and found there's a way to get the kernel to install a handler for unaligned accesses which will synthesize the unaligned access with two aligned accesses, and then jump back to the original point in the code.
Interesting.
It's slow, but you only pay the price if you do an unaligned access, where we would currently crash.
This may be the right thing to do, but let me suggest some alternatives:
There's the TYPE_ALIGNMENT macro in winnt.h. We could try and change Wine to properly align strings and structs and whatnot using this. I can try to provide a list of APIs I know about that may have similar issues as what you reported, though it wouldn't be exhaustive.
As Eric Pouech mentioned before, MSVC supports the __unaligned attribute, and winnt.h defines UNALIGNED to __unaligned when using MSVC. So, on platforms that require aligned memory access, maybe using the UNALIGNED macro is the right thing, if in doing so you could insert the proper asm to emulate unaligned memory access.
Do you feel it's reasonable just to turn on ST_FIX_ALIGN, which *could* mask errors both in wine and in the user's code?
It may indeed. The masking problems in user code bothers me, though, since you're using winelib and you may not have a program which "should work." If you have the time and energy to try other approaches, we might get this right for other platforms too.
Thanks, --Juan
_______________________________ Do you Yahoo!? Declare Yourself - Register online to vote today! http://vote.yahoo.com
Juan Lang juan_lang@yahoo.com writes:
Do you feel it's reasonable just to turn on ST_FIX_ALIGN, which *could* mask errors both in wine and in the user's code?
It may indeed. The masking problems in user code bothers me, though, since you're using winelib and you may not have a program which "should work."
I think masking problems in the user code is exactly what we want, Windows code doesn't care about alignment, and there isn't much sense in making users fix their code, the Win32 API is simply broken in that respect.
-
Alexandre Julliard -
Eric Frias -
Eric Pouech -
Juan Lang