Re: D3D8_GetCreationParameters fix
MSDN says that if it's given an invalid pointer it should return D3DERR_INVALIDCALL, so that's what it does now. MSDN: http://msdn.microsoft.com/archive/default.asp?url=/archive/en-us/directx9_c/... So I'm guessing this is right now. I don't have a windows computer to check this against, but it does comply with the docs. Andrei On Fri, 2004-07-09 at 11:19, Mike McCormack wrote:
This patch is better than the first one, but...
Windows doesn't usually check pointers like this. Can you try testing the Windows implementation of IDirect3DDevice8->GetCreationParameters() to see whether it crashes when passed an invalid pointer such as NULL or 0x00000001? It may be that the invalid pointer is coming from somewhere else in Wine's code, so you may be just hiding another bug.
Mike
Andrei Barbu wrote:
And this is the correct patch.
@@ -380,6 +381,12 @@ HRESULT WINAPI IDirect3DDevice8Impl_GetCreationParameters(LPDIRECT3DDEVICE8 iface, D3DDEVICE_CREATION_PARAMETERS *pParameters) { ICOM_THIS(IDirect3DDevice8Impl,iface); TRACE("(%p) copying to %p\n", This, pParameters); + + if( IsBadWritePtr( pParameters , sizeof( D3DDEVICE_CREATION_PARAMETERS ) ) ) + { + WARN("(%p) : bad pointer in device caps (%p)\n", This, pParameters); + return D3DERR_INVALIDCALL; + } memcpy(pParameters, &This->CreateParms, sizeof(D3DDEVICE_CREATION_PARAMETERS)); return D3D_OK; }
Andrei Barbu wrote:
So I'm guessing this is right now.
I don't have a windows computer to check this against, but it does comply with the docs.
I have a feeling you'll need to be a bit more thorough than that to get your patch applied. Mike
Point is.. it works, it's compliant letter for letter with the MSDN docs, and it doesn't break anything. Don't see why it would need more testing. On Fri, 2004-07-09 at 13:41, Mike McCormack wrote:
Andrei Barbu wrote:
So I'm guessing this is right now.
I don't have a windows computer to check this against, but it does comply with the docs.
I have a feeling you'll need to be a bit more thorough than that to get your patch applied.
Mike
Andrei Barbu wrote:
Point is.. it works, it's compliant letter for letter with the MSDN docs, and it doesn't break anything.
Don't see why it would need more testing.
It needs more testing because at least three other developers believe that you're not fixing a bug, you're masking another bug. You need to prove to us that the bad pointer is not being passed in because of another wine bug, and that Windows really does check for bad pointers using IsBadWritePtr. Sorry, to be a pain, but Wine development is hard... Mike
Don't be sorry, We're both trying to help the wine project. Wine itself never calls IDirect3DDevice8Impl_GetCreationParameters. I greped through the sources for that. So it can't be a straightforward call from inside wine that would be passing that bad pointer. And MSDN says: " Return Value If the method succeeds, the return value is D3D_OK. D3DERR_INVALIDCALL is returned if the argument is invalid. " Clearly here the argument is invalid so it should be returning D3DERR_INVALIDCALL. I can't think of a better way that it would check for the argument being invalid. Even if it does it a different way, this is a reliable way of testing to see if we can write to a pointer, so if it's valid. I understand concerns that this might be hiding a different wine bug, that's why I added the warning. Like that we can still see if it's getting a bad pointer. I think even if a wine bug might be generating that bad pointer, which I don't think it is since Chessmaster works fine after that, this is how DX works, so we should implement it. If it doesn't crash on windows, why should it in wine? Thanks, Andrei On Fri, 2004-07-09 at 13:54, Mike McCormack wrote:
Andrei Barbu wrote:
Point is.. it works, it's compliant letter for letter with the MSDN docs, and it doesn't break anything.
Don't see why it would need more testing.
It needs more testing because at least three other developers believe that you're not fixing a bug, you're masking another bug.
You need to prove to us that the bad pointer is not being passed in because of another wine bug, and that Windows really does check for bad pointers using IsBadWritePtr.
Sorry, to be a pain, but Wine development is hard...
Mike
On Fri, 09 Jul 2004 17:08:05 -0400, Andrei Barbu wrote:
Even if it does it a different way, this is a reliable way of testing to see if we can write to a pointer, so if it's valid.
Actually it's not - IsBadWritePtr isn't thread safe, as it only tells you whether the pointer was valid at the instant you called it. The docs warn against using it for exactly that reason, hence the fact that Windows itself uses it only rarely. thanks -mike
On Fri, 09 Jul 2004 16:14:31 -0400, Andrei Barbu wrote:
Point is.. it works, it's compliant letter for letter with the MSDN docs, and it doesn't break anything.
The easiest way to advance this is to write a simple test case which gets an instance of that object and then calls it with a bad pointer - does it crash? A test case can be written in C pretty quick, the hard part is (a) compiling it as an EXE file, and (b) locating a suitable Windows machine to run it on. OK so you would not think b is hard given that 95% of the worlds computers run Windows, but often it can be anyway especially if you don't have Windows yourself :) So, because I have some XP boxes here, if you send me an EXE file I will run it for you and let you know the results. That leaves, how do I make an EXE file? The best way is to install the mingw cross compiler package. It should be available for your distro somewhere. It's in Debian and there are RPMs lying around. If anybody knows the URL speak up! The mingw headers are sadly quite incomplete, so you may need to compile it with the Wine headers. Good luck! thanks -mike
Hi, On Fri, Jul 09, 2004 at 04:14:31PM -0400, Andrei Barbu wrote:
Point is.. it works, it's compliant letter for letter with the MSDN docs, and it doesn't break anything. That's EXACTLY the problem - if it's compliant to MSDN letter for letter, then you can bet your whole household that it is even more BOUND to be wrong somewhere, given our comprehensive previous experience with MSDN correctness ;-) I just had to mention this.
Anyway, just wanted to point this out, I don't want to shatter your confidence. Simply locate e.g. some DX sample code on the internet which uses that function, modify it to call it with a "1" parameter and observe Windows behaviour. Good luck, Andreas Mohr
participants (4)
-
Andreas Mohr -
Andrei Barbu -
Mike Hearn -
Mike McCormack