Thread: Corruption after return

  1. #1
    Registered User
    Join Date
    Dec 2003
    Posts
    68

    Corruption after return

    Hi all,

    Firstly, my appologies for the appauling title of this thread - but I couldn't think of what to describe my problem as.

    I have a C++ DLL built, which I'm using code from. Most of it works fine, however I'm experiencing something rather unsual (to me at least).

    I have a method which Is exposing a method in my DLL:
    Code:
    char** dllGetProductKeyInfo( int Type )
    {
    	typedef char** (*GetProductKeyInfo)( int );
    	GetProductKeyInfo _GetProductKeyInfo;
    
    	_GetProductKeyInfo = (GetProductKeyInfo)GetProcAddress(hInstLibrary, "GetProductKeyInfo");
    
    	if (_GetProductKeyInfo)
    	{
    		// Debug.
    		printf("dllGetProductKeyInfo: %s\r\n", _GetProductKeyInfo(Type)[0] );
    		printf("dllGetProductKeyInfo: %s\r\n", _GetProductKeyInfo(Type)[1] );
    		// Debug.
    
    		return _GetProductKeyInfo( Type );
    	}
    	else
    	{
    		return 0;
    	}
    }
    This returns:
    Code:
    dllGetProductKeyInfo: CyberLink PowerDVD DX
    dllGetProductKeyInfo: VDXXXXXXXXXXXXXX
    Great, it works I thought. However, when trying to use dllGetProductKeyInfo(), I'm not so lucky.

    Code:
    void loadKeys( )
    {
    	char** tmpKeyInfo;
    
    	// Size of the Keyfinder vector.
    	int keyFinderSize = dllProductKeyInfoSize();
    
    	for( int a =0; keyFinderSize > a; a++)
    	{
    		tmpKeyInfo = dllGetProductKeyInfo( a );
    		
    		// Debug.
    		printf("loadKeys: %s\r\n", tmpKeyInfo[0]);
    		printf("loadKeys: %s\r\n", tmpKeyInfo[1]);
    		// Debug.
    	}
    }
    Produces:
    Code:
    loadKeys: CyberLink PowerDVD DX
    loadKeys: @dÇw........
    Any thoughts as to what i've missed? - Been looking over it for so long I'm not really looking, if that makes sense.

    Any help would be appreciated, if you know the solution I'd love to know why I've gone wrong

    Cheers!

  2. #2
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,981
    You're calling _GetProductKeyInfo() 3 times - maybe the 3rd call returns a bad value.

    Make sure _GetProductKeyInfo() really is a __cdecl function.

    gg

  3. #3
    Registered User
    Join Date
    Dec 2003
    Posts
    68
    Quote Originally Posted by Codeplug View Post
    You're calling _GetProductKeyInfo() 3 times - maybe the 3rd call returns a bad value.

    Make sure _GetProductKeyInfo() really is a __cdecl function.

    gg
    Thanks for the reply. If I reduce dllGetPoductKeyInfo() to just one call I get the same problem:

    Code:
    har** dllGetProductKeyInfo( int Type )
    {
    	typedef char** (*GetProductKeyInfo)( int );
    	GetProductKeyInfo _GetProductKeyInfo;
    
    	_GetProductKeyInfo = (GetProductKeyInfo)GetProcAddress(hInstLibrary, "GetProductKeyInfo");
    
    
    	char** tmpReturn;
    	tmpReturn = _GetProductKeyInfo(Type);
    
    	if (_GetProductKeyInfo)
    	{
    		printf("dllGetProductKeyInfo: %s\r\n", tmpReturn[0] );
    		printf("dllGetProductKeyInfo: %s\r\n",tmpReturn[1] );
    
    		return tmpReturn;
    	}
    	else
    	{
    		return 0;
    	}
    }
    It looks like it doesn't like me copying the output. I built the DLL myself, other functions seem to work fine.

    The definition of GetProductKeyInfo() is:

    Code:
    char** DLL_EXPORT GetProductKeyInfo( int Type );
    DLL_EXPORT is #defined as _declspec(dllexport). But that's normal for a DLL, isn't it?]

    Thanks for the assistance!

  4. #4
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    My guess - and it's only a guess, as you haven't provided the code to confirm - is that your GetProductKeyInfo() is returning the address of a local variable (i.e. returning the address of something that isn't guaranteed to exist once it returns). If that's the case, any dereferencing (eg evaluating tmpReturn[0] or tmpReturn[1]) yields undefined behaviour.

    One of the frustrating (or entertaining if you're a masochist) characteristics of undefined behaviour is that, sometimes, it gives a result you "intuitively" expect, and sometimes it doesn't. Which appears to be what's happening here ... eg dereferencing the pointer in dllGetProductKeyInfo() gives different visible results to dereferencing it in your loadKeys() function.

  5. #5
    Registered User
    Join Date
    Dec 2003
    Posts
    68
    Quote Originally Posted by grumpy View Post
    My guess - and it's only a guess, as you haven't provided the code to confirm - is that your GetProductKeyInfo() is returning the address of a local variable (i.e. returning the address of something that isn't guaranteed to exist once it returns). If that's the case, any dereferencing (eg evaluating tmpReturn[0] or tmpReturn[1]) yields undefined behaviour.
    Great, that may just be it! Thanks.

    When compiling my DLL, I get:

    Code:
    warning: address of local variable `ReturnArray' returned|
    I'm doing the following.

    Code:
    char** ProductKeyFinder::GetProductKey( int Type, char* subKeyName )
    {
    	/* Setup variables and stuff */
    	char* ReturnArray[2];
    
    	// ... Snip logic.
    
    	return ReturnArray;
    }
    I guess I need to use malloc to allocate the memory? - Something like:
    Code:
    char** ProductKeyFinder::GetProductKey( int Type, char* subKeyName )
    {
    	/* Setup variables and stuff */
    	char* ReturnArray[2];
    
    	// ... Snip logic.
    
    	char**  temp = (char**) malloc ( sizeof(ReturnArray)+1 );
    
    	return temp;
    }
    However, that has even more unpredicted results - any suggestions as to where I went wrong? Thanks for your help!
    Last edited by DaveHope; 04-09-2009 at 05:31 AM.

  6. #6
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Your current code is doing exactly what grumpy hinted at: You are returning the address of a local variable. The array you are returning is gone immediately after you leave the function, and given the right circumstances, the array will be overwritten by other function calls using the same area of memory.

    Generally, the best option is to pass IN an array to be filled by the function, rather than producing the array at the "top" of the stack.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  7. #7
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Quote Originally Posted by DaveHope View Post
    Code:
    char** ProductKeyFinder::GetProductKey( int Type, char* subKeyName )
    {
    	/* Setup variables and stuff */
    	char* ReturnArray[2];
    
    	// ... Snip logic.
    
    	char**  temp = (char**) malloc ( sizeof(ReturnArray)+1 );
    
    	return temp;
    }
    However, that has even more unpredicted results - any suggestions as to where I went wrong? Thanks for your help!
    The code as you've shown it allocates memory, but does not copy the values of ReturnArray to it (in fact, it doesn't initialise the allocated memory at all, so any usage of that array other than setting values gives undefined behaviour). Also, since ReturnArray is an array of pointers, you also need to ensure the elements (pointers) all point at something that will exist when the function returns.

    There are a few other concerns (eg the +1 is possibly unnecessary, use operator new, not malloc()) but those aren't direct contributors to your problem.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  8. #8
    Registered User
    Join Date
    Dec 2003
    Posts
    68
    Quote Originally Posted by grumpy View Post
    The code as you've shown it allocates memory, but does not copy the values of ReturnArray to it (in fact, it doesn't initialise the allocated memory at all, so any usage of that array other than setting values gives undefined behaviour). Also, since ReturnArray is an array of pointers, you also need to ensure the elements (pointers) all point at something that will exist when the function returns.

    There are a few other concerns (eg the +1 is possibly unnecessary, use operator new, not malloc()) but those aren't direct contributors to your problem.
    Thanks for your help, I was copying the values from ReturnArray but forgot to copy the code into the post.

    I malloc'd the two char*'s which formed ReturnArray also, which resolved my issues.

    I'll look into using new rather than malloc too!

    Thanks again - Have a great Easter!

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Another weird error
    By rwmarsh in forum Game Programming
    Replies: 4
    Last Post: 09-24-2006, 10:00 PM
  2. Why only 32x32? (OpenGL) [Please help]
    By Queatrix in forum Game Programming
    Replies: 2
    Last Post: 01-23-2006, 02:39 PM
  3. opengl help
    By heat511 in forum Game Programming
    Replies: 4
    Last Post: 04-05-2004, 01:08 AM
  4. opengl code not working
    By Unregistered in forum Windows Programming
    Replies: 4
    Last Post: 02-14-2002, 10:01 PM
  5. Algorithm to walk through a maze.
    By Nutshell in forum C Programming
    Replies: 30
    Last Post: 01-21-2002, 01:54 AM