GetProcaddress returns null but GetLastError indicate no problem

  • Thread starter Thread starter AG
  • Start date Start date
A

AG

Hi all,

I have this classic code : load a dll, and get a pointer on a specific
function in that DLL.

The GetProcAddress() returns NULL, but the GetLastWin32Error()
function says : operation successful.

Then if I try to use the SetHookDll() function, my application
crashes. If I change the call to GetProcAdress with "SetHookDll2"
instead of the correct "SetHookDll", I get the right error message
from GetLastWin32Error() : cannot find the appropriate function.

Any Hint ?

Thanks in advance.

AG.


if((hinstDLL = LoadLibrary((LPCTSTR) FICHIER))==NULL)
{
print_error("Error loading library");
FreeLibrary(hinstDLL);
}
HINSTANCE module_dll;
GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,LPSTR("KHook"),&module_dll);
if(module_dll==NULL)
{
print_error("Error loading module");
FreeLibrary(hinstDLL);
}

if((SetHookDll=(SETHOOK_FCT)GetProcAddress(module_dll,LPSTR("SetHookDll")))==NULL)
{
print_error("Error get SetHookDll pointer");
FreeLibrary(hinstDLL);
}

FreeLibrary(hinstDLL);

with print_error defined like this :

void print_error(LPCSTR title)
{

Win32Exception^ winex = gcnew
Win32Exception(Marshal::GetLastWin32Error());
String^ errorMessage = winex->Message;
String^ myTitle = gcnew String(title);

MessageBox::Show(errorMessage,myTitle,MessageBoxButtons::OK,MessageBoxIcon::Asterisk);
delete myTitle;
delete winex;
}
 
AG said:
Hi all,

I have this classic code : load a dll, and get a pointer on a specific
function in that DLL.

The GetProcAddress() returns NULL, but the GetLastWin32Error()
function says : operation successful.

Then if I try to use the SetHookDll() function, my application
crashes. If I change the call to GetProcAdress with "SetHookDll2"
instead of the correct "SetHookDll", I get the right error message
from GetLastWin32Error() : cannot find the appropriate function.

Any Hint ?
Click to expand...

Yes. Don't mix managed and unmanaged like that. Just calling managed code
may cause other operations that complete successfully, clearing the error
state. Use the GetLastError function from the Win32 API when you are
calling Win32 API functions like GetProcAddress.

Also, you've got a useless ANSI->UNICODE runtime conversion of a literal
string. If you create a System::String from a string literal, make it a
unicode string literal.

After error reporting for failure to get the module handle, instead of
cancelling the operation you then use the NULL handle. Not good.

You don't test the return value from GetModuleHandleEx.

Use RAII to free the library in the presence of failures, instead of placing
many different calls to FreeLibrary.

You don't need the cast to LPSTR in the GetProcAddress call.

Why are you using LoadLibrary at all, if you aren't getting the function
from it? If SetHookDll in one of the import dependencies? In that case it
will be unloaded as soon as you unload the other library, which would cause
your crash.
 
Yes. Don't mix managed and unmanaged like that. Just calling managed code
may cause other operations that complete successfully, clearing the error
state. Use the GetLastError function from the Win32 API when you are
calling Win32 API functions like GetProcAddress.
Click to expand...
Ok, I did the change.

Also, you've got a useless ANSI->UNICODE runtime conversion of a literal
string. If you create a System::String from a string literal, make it a
unicode string literal.
Click to expand...
I never went through the whole ANSI vs UNICODE problem, and thus
always mess every things up. which line are you referring ?
After error reporting for failure to get the module handle, instead of
cancelling the operation you then use the NULL handle. Not good.
Click to expand...
Yes. I had corrected this already.
You don't test the return value from GetModuleHandleEx.
Click to expand...
ok. idem.
Use RAII to free the library in the presence of failures, instead of placing
many different calls to FreeLibrary.
Click to expand...
I don't know what RAII is, but I will have a look on it.

You don't need the cast to LPSTR in the GetProcAddress call. ok.

Why are you using LoadLibrary at all, if you aren't getting the function
from it? If SetHookDll in one of the import dependencies? In that case it
will be unloaded as soon as you unload the other library, which would cause
your crash.
Click to expand...
I did not post the part of the code where the SetHookDll() call is
done, since I got an error before. I but I do use the function which
is an import dependencies. I am well aware that after FreeLibrary()
call, I cannot use the function anymore.

At the end, I did your changes and went back to GetLastError(). Then a
problem arise when it comes to format the error code, using
FormatMessage() api into managed code. I forgot about formatting the
error message to simply the problems, and output the error code
returned by GetLastError() : 0 meaning successful operation.

if((hinstDLL = LoadLibrary((LPCTSTR) FICHIER))==NULL)
{
print_error("Erreur : Chargement de la librairie");
return;
}
HINSTANCE module_dll;


if(GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,"KHook",&module_dll)==FALSE)
{
print_error("Erreur : Chargement de la librairie");
FreeLibrary(hinstDLL);
return;
}


if((SetHookDll=(SETHOOK_FCT)GetProcAddress(module_dll,"SetHookDll"))==NULL)
{
print_error("Erreur : Obtention de l'adresse de SetHookDll");
FreeLibrary(hinstDLL);
return;
}
int HookKeyOn=SetHookDll(TRUE,(HWND)this-
 
AG said:
Ok, I did the change.
Click to expand...

It seems you are still going to managed code before calling GetLastError,
because I don't see any GetLastError calls. When you call managed code, the
CLR may need to call type initializers, load dependent assemblies, and all
sorts of other stuff that may overwrite the error code.

This is especially likely to be a problem because the print_error function
is likely to be called here for the first time since app startup, meaning
the CLR still has to compile it from MSIL to machine code.

Passing GetLastError() as an argument to print_error should solve this. You
can use a native inline helper function to avoid the messiness of writing
GetLastError every time.
 
Hi Ben,

Thank you for the help and free time your are spending on this. Though
I am still stucked to the same point. Here is the last version of my
code :

---------------------8<-----------------------------------------------------------------------------

if((hinstDLL = LoadLibrary((LPCTSTR) FICHIER))==NULL)
{
DWORD errorCode = GetLastError();
String^ errorMessage = "Error code :" + errorCode;
MessageBox::Show(errorMessage,"Problem with
LoadLibrary",MessageBoxButtons::OK,MessageBoxIcon::Error);
return;
}

HINSTANCE module_dll;
if(GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,"KHook",&module_dll)==FALSE)
{
DWORD errorCode = GetLastError();
String^ errorMessage = "Error code :" + errorCode;
MessageBox::Show(errorMessage,"Problem with
GetModuleHandleEx",MessageBoxButtons::OK,MessageBoxIcon::Error);
FreeLibrary(hinstDLL);
return;
}

if((SetHookDll=(SETHOOK_FCT)GetProcAddress(module_dll,"SetHookDll"))==NULL)
{
DWORD errorCode = GetLastError(); // it comes up to here, and
GetLastError() return 0.
String^ errorMessage = "Error code :" + errorCode;
MessageBox::Show(errorMessage,"Problem with
GetProcAdress",MessageBoxButtons::OK,MessageBoxIcon::Error);
FreeLibrary(hinstDLL);
return;
}

int HookKeyOn=SetHookDll(TRUE,(HWND)this-
Handle.ToPointer(),WH_KEYBOARD);
Click to expand...

---------------------8<-----------------------------------------------------------------------------

I forgot to mention that :

- It is included in a function Form1_Load() of my windows form
application which is executed when the form loads.
- SetHookDll is of type SETHOOK_FCT which is a pointer on a function.
This is a private member of my Form. I don't know if it is allowed to
set pointers on functions which are part of managed code with an
unmanaged API, but the compiler doesn't complain.
- I compile with /clr:pure option
- I have Visual C++ 2008 express edition, but I think it is not a
problem.
The Error code returned by GetLastError() is still 0.

When I position my mouse on GetLastError(), the intellisens says that
GetLastError() is #define'd with GetLastWin32Error(), which confuse me
since the goal is not to call GetLastWin32Error().

AG.
 
AG said:
Hi Ben,

Thank you for the help and free time your are spending on this. Though
I am still stucked to the same point. Here is the last version of my
code :
Click to expand...

Do you get the same result without /clr:pure (i.e. just /clr)?

Native function calls are made very differently when /clr:pure or /clr:safe
is in effect.
---------------------8<-----------------------------------------------------------------------------

if((hinstDLL = LoadLibrary((LPCTSTR) FICHIER))==NULL)
{
DWORD errorCode = GetLastError();
String^ errorMessage = "Error code :" + errorCode;
MessageBox::Show(errorMessage,"Problem with
LoadLibrary",MessageBoxButtons::OK,MessageBoxIcon::Error);
return;
}
Click to expand...

I was suggesting something more along the lines of:

DWORD errorCode = GetLastError();
ReportError("Problem with LoadLibrary", errorCode);

instead of duplicating all the MessageBox stuff. But for troubleshooting
what you've done is not unreasonable.
HINSTANCE module_dll;
if(GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,"KHook",&module_dll)==FALSE)
{
DWORD errorCode = GetLastError();
String^ errorMessage = "Error code :" + errorCode;
MessageBox::Show(errorMessage,"Problem with
GetModuleHandleEx",MessageBoxButtons::OK,MessageBoxIcon::Error);
FreeLibrary(hinstDLL);
return;
}

if((SetHookDll=(SETHOOK_FCT)GetProcAddress(module_dll,"SetHookDll"))==NULL)
{
DWORD errorCode = GetLastError(); // it comes up to here, and
GetLastError() return 0.
String^ errorMessage = "Error code :" + errorCode;
MessageBox::Show(errorMessage,"Problem with
GetProcAdress",MessageBoxButtons::OK,MessageBoxIcon::Error);
FreeLibrary(hinstDLL);
return;
}
Click to expand...

Hmm, let's try:

FARPROC procAddress = ::GetProcAddress(module_dll,"SetHookDll");
DWORD errorCode = ::GetLastError();
if (NULL == procAddress) { ... }
SetHookDll=static_cast<SETHOOK_FCT>(procAddress);

Now there is nothing that could happen between GetProcAddress and
GetLastError.
 
Hi Ben,

It is finally working with your suggestion. I thank you for this, I
would have spends weeks to figure this out by myself.
The line :

SetHookDll=static_cast<SETHOOK_FCT>(procAddresse);

did not compile (needed a C-Style, function-style or reinterpret
cast) , so I replaced it by :

SetHookDll = (SETHOOK_FCT)procAddress;

which is fine.

Thanks again,

Alexandre.
 
AG said:
Hi Ben,

It is finally working with your suggestion. I thank you for this, I
would have spends weeks to figure this out by myself.
The line :

SetHookDll=static_cast<SETHOOK_FCT>(procAddresse);

did not compile (needed a C-Style, function-style or reinterpret
cast) , so I replaced it by :
Click to expand...

Ahh, the compiler suggested the solution there:

SetHookDll=reinterpret_cast said:
SetHookDll = (SETHOOK_FCT)procAddress;

which is fine.
Click to expand...

It's syntactically correct, but using the template-style casts is preferable
because it allows you to immediately find locations where a potentially
dangerous cast is used with simple search commands.
 
Back
Top