Skip to main content
GameDev.net gamedev.net
🔒 Locked

C Function Pointers

Started by Crypter Apr 11, 2009 at 8:30 PM 15 replies 2.7k views
Original Post
Crypter
Crypter
Hey everyone, This might be a simple one, but Im not to sure what I am overlooking. I have the following code, stripped down for simplicity:
INISYSAPI
INISTATUS
INIAPI
MountVolume (IN UCHAR Volume, OUT PFILESYSTEM FileSystem) {

//*cut*
	FileSystem->Mount = FsysFatMount;
}
FileSystem is a pointer to a structure that contains function pointers (FileSystem->Mount is a function pointer.) If I call FileSystem->Mount() inside of this function, it properly calls FsysFatMount () as expected thanks to the above assignment. But, if I call FileSystem::Mount from outside this function inside of a different function like the following code:
FILESYSTEM FileSystem;

MountVolume ('a', &FileSystem);
FileSystem.Mount ();
It crashes with an invalid instruction as FileSystem::Mount no longer points to FsysFatMount. Does anyone see what I may be doing wrong? Thanks for any help [smile]
outRider
outRider
If it no longer points to where it should then obviously the pointer is being trashed. And if your snippet is accurate, it's getting trashed between MountVolume() and FileSystem.Mount().

Since there's nothing between them I'm guessing your calling conventions are messed up and the stack gets corrupted somewhere around the return of MountVolume(), and since it looks like FileSystem is the last item on the stack (i.e. very close to where a stack pointer might be currently pointing) its members would make good candidates for corruption in that case.
Crypter
Crypter
Quote:
Original post by outRiderSince there's nothing between them I'm guessing your calling conventions are messed up and the stack gets corrupted somewhere around the return of MountVolume()


Thats what I was thinking to. Except I have no problems with the FileSystem->Name member variable... If the stack was indeed getting corrupted, FileSystem->Name should contain garbage when being used after I call MountVolume(), but it is not. (FileSystem::Name is set inside of MountVolume () along with FileSystem::Mount. Sorry, I should have mentioned that earlier..)

I dont suspect the calling conventions as every one of the functions in the API uses the same calling convention, including the function pointers (_cdecl)

Thanks for your reply.
outRider
outRider
All it takes is a single byte to corrupt the pointer, especially if it's the last member of that struct, every other member could easily be unaffected.

It may not be a calling convention mismatch, but I would still suspect stack corruption. Is "FileSystem->Mount = FsysFatMount;" the last line of MountVolume(), or is there more code after that might trash the stack? Also, is there non-trivial code between VolumeMount() and FileSystem.Mount()? That would be another place to check.
Crypter
Crypter
I commented everything out so now FileSystem->Mount = FsysFatMount; is the only line in that function. There is no code between MountVolume ('a', &FileSystem); and FileSystem.Mount (); either

Here are the entire routines:

INISYSAPIINISTATUSINIAPIMountVolume (IN UCHAR Volume, OUT PFILESYSTEM FileSystem) {	FileSystem->Mount = FsysFatMount;}INISYSAPIINISTATUSINIAPIcoreRun () {	FILESYSTEM FileSystem;	BlpDisplayClear (0x0f);	coreSplash ();	if (GetLogicalDrives () == 0)		BmFatalError (ERROR_LOGICAL_DRIVE);	MountVolume ('a', &FileSystem);	FileSystem.Mount ();  //! Crashes, invalid instructions..	return 0;}

Ill single step and see if there is anything going on with the stack..
Crypter
Crypter
Oh, rats. I think its the structure itself. I commented all members (besides the mount function pointer) in the structure and it works now.

Im going to have to look more into that...

Thanks for your help! [smile]
Crypter
Crypter
Its working now. FileSystem->Name was an array of 10 bytes. I changed it to 8 and now everything works fine. I suspect this has to do with the structure on the stack.

Everything is working fine now. Thanks again for your help!
Zahlman
Zahlman
Quote:
Original post by Crypter
Its working now. FileSystem->Name was an array of 10 bytes. I changed it to 8 and now everything works fine. I suspect this has to do with the structure on the stack.


O_O

That smells like deep voodoo. Is this structure supposed to be aligned with some other structure? Could we see the FileSystem declaration?
Crypter
Crypter
Quote:
Original post by Zahlman
That smells like deep voodoo. Is this structure supposed to be aligned with some other structure? Could we see the FileSystem declaration?

Here you go. No, there is no rules at all about how the structure is supposed to be used (So, it does not have to be aligned).

When I tested it just now again, if Name is 10 bytes, the software generated a double fault (Its self hosting software). After I set it back to 8 bytes, it worked fine again without problem.

I am thinking that you are right though. I suspect the language itself is fine, but something else may be causing the error.

INISTATUS is just an UINT for a return type, INIAPI is _cdecl.
typedef struct _FILE_SYSTEM {	UCHAR Name [8];	INISYSAPI INISTATUS (INIAPI *Directory) (IN OPT LPSTR DirectoryName);	INISYSAPI INISTATUS (INIAPI *Mount) ();	INISYSAPI INISTATUS (INIAPI *Read) (OUT OPT PBYTE Buffer,		IN UINT Length);	INISYSAPI INISTATUS (INIAPI *Close) ();	INISYSAPI INISTATUS (INIAPI *Open) (IN OPT LPSTR FileName);	INISYSAPI INISTATUS (INIAPI *Seek) (IN UINT Offset);	INISYSAPI UINT	    (INIAPI *Eof) ();}FILESYSTEM, *PFILESYSTEM;

Do you have any thoughts?

I will look at it at the assembly level to see if there is anything strange going on...
SiCrane
SiCrane
What architecture are you running on?
Crypter
Crypter
Quote:
Original post by SiCrane
What architecture are you running on?

Just the i86 architecture. Most PC's and laptops.
Crypter
Crypter
It is indeed stack related. With the following code:
MountVolume ('a', &FileSystem);_asm cli_asm hltFileSystem.Mount ();

When the system halts, and Name is 8 bytes, ESP=0x4fd0. But, when the system halts, and Name is 10 bytes, ESP=0x4fcc. 4 bytes difference. Not sure why though...
SiCrane
SiCrane
Is the structure packed?
Crypter
Crypter
Quote:
Original post by SiCrane
Is the structure packed?

It is not, no.
gsg
gsg
Quote:
Original post by Crypter
If the stack was indeed getting corrupted, FileSystem->Name should contain garbage when being used after I call MountVolume(), but it is not.


Unless the stack corruption is caused by writes to that field. Considering that it is perfectly placed to smash the function pointer you are seeing problems with, I'd consider it likely.

Double check the appropriate code. Also consider verifying that buffer overflow is occurring by placing a guard field after the buffer, initialising it to a distinctive value and verifying its value before and after the suspicious function call.
Crypter
Crypter
Quote:
Original post by gsg
Unless the stack corruption is caused by writes to that field. Considering that it is perfectly placed to smash the function pointer you are seeing problems with, I'd consider it likely.

That makes sense. I commented everything in MountVolume() to only contain a FileSystem->Mount = FsysFatMount; instruction just now to test (so no other members in the struct are being set.) Same problem though.. with Name[10] I get an Invalid Instruction; but works fine with Name[8]...

I think that also takes the possibility of a buffer overflow out as well sense only FileSystem::Mount is set; nothing else...
gsg
gsg
Quote:
Original post by Crypter
That makes sense. I commented everything in MountVolume() to only contain a FileSystem->Mount = FsysFatMount; instruction just now to test (so no other members in the struct are being set.) Same problem though.. with Name[10] I get an Invalid Instruction; but works fine with Name[8]...

Right. Well, it would be a little odd to only get errors when you increase the buffer size - you'd have to be doing something funky with pointer arithmetic.

Strange bug.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.