bmx-ng / bmk

The enhanced BlitzMax build program.
zlib License
28 stars 13 forks source link

Generated DLLs not loadable #53

Closed GWRon closed 5 years ago

GWRon commented 6 years ago

Dunno what changed ... but with current BMK, BCC, pub.mod and brl.mod I am no longer able to load the generated DLLs.

It works when using 3rd party DLL files - so something must be "mixed up".

It also has worked some weeks ago - means I am a bit clueless what could be the culprit now. There might have been Windows updates on that computer (3rd party, not under my control).

dll.bmx

SuperStrict

Framework brl.standardIO

Global versionPtr:Byte Ptr = "Version 0.1.2.3".ToCString()

Function TEST_GetVersion:Byte Ptr() Export "Win32"
    Return versionPtr
End Function

dll_test.bmx

SuperStrict

Framework Brl.StandardIO
Import pub.win32
Import brl.systemDefault 'Notify()

Local dllName:String = "dll.dll"
Local dllHandle:Byte Ptr = LoadLibraryW(dllName)
If Not dllHandle Then Notify dllName + " not loaded properly."; End
Print "loaded dll"

Global TEST_GetVersion$z() "Win32" = GetProcAddress(dllHandle, "TEST_GetVersion")

Print TEST_GetVersion()

"loaded dll" is never printed - nor is there a notification for a failed LoadLibraryW call.

GWRon commented 6 years ago

Tried with a fresh installation of BlitzMax_win32_0.98.3.27.7z - same issue.

Replacing "dll.dll" (or another BMXNG generated dll with a 3rd party dll results in "loaded dll" (and of course an EAV because of an invalid procedure address).

The odd thing is, that I am pretty sure to have it seen working some weeks ago. So I extracted a DLL of an archive I did backup at the begin of september. This older dll file also does not load.

Older test files of the very same date (they tested if importing of the DLL was possible) now won't start: "libgcc_s_sjlj-1.dll" missing on the computer... The binary is a release-build! The debug build of this application shows the same error message (and misses the same file).

I am pretty... puzzled now as it of course worked at that time (else I would not have sent it to the 3rd party/client).

Edit: ok tried the new binaries/dll again on another computer - and it works as expected. Compiling the binaries/dll on this computer (with its BlitzMaxNG) leads to a non-working variant.

I assume the "managed OS" computer has issues ... with the "managing software" doing odd things in the background. That computer does some updates now

Will try again when at home this evening - and report back then.

GWRon commented 6 years ago

Hmm ... odd stuff. So I execute "dll_test.exe" from command line ... and it prints "loaded". I compile+run "dll_test.bmx" from with MaxIDE and program does not print stuff and stays "running".

But this only, if the used "dll" is the backup-dll and not a freshly created one.

That is surely a bug on my side and not NGs one ... or at least I am part of the issue ;-)

woollybah commented 6 years ago

There's some stuff about static linking of dlls here. I believe bmk is applying a -static for some dll - can't remember which atm.

Sounds like you have a issue with your paths? Or something. You are building everything with the same architecture? x86/x64

GWRon commented 6 years ago

I will make sure at home... But basically I used the same release-download and updated it. Both contained the 32 and 64 variants of minGW.

Will try at home with a VM.

It just was annoying as it worked last time I touched a file...and then it stopped...

@ static libs Maybe it would be a good idea to have BMK print out statically linked stuff (so you know what to package aside of your binary)?

woollybah commented 6 years ago

This may also be useful :

--add-stdcall-alias If given, symbols with a stdcall suffix (@nn) will be exported as-is and also with the suffix stripped. [This option is specific to the i386 PE targeted port of the linker]

GWRon commented 6 years ago

Ok.

Does above given example code work for you?

woollybah commented 6 years ago

Nope :-)

woollybah commented 6 years ago

Does the app finish when you execute it, or just hang?

woollybah commented 6 years ago

However, I've just now managed to get something to work.

After much digging, and debugging (with gdb), I believe I've narrowed the issue down.

The problem appears to be related to the GC. The test app was, for me, sticking on LoadLibrary. Basically it appears to be deadlocking because of the way the GC is initialising when the library loads - LoadLibrary runs DLL_main().

So, I tried tweaking blitz.bmx...

?win32
ModuleInfo "CC_OPTS: -DATOMIC_UNCOLLECTABLE -DLARGE_CONFIG"
'ModuleInfo "CC_OPTS: -DGC_THREADS -DPARALLEL_MARK -DATOMIC_UNCOLLECTABLE -DLARGE_CONFIG"
?osx

and I had to comment out a line in blitz_gc.c

// GC_allow_register_threads();

I rebuilt brl.blitz, and then rebuilt the dll.

The test program then ran as expected.

If you can try that and let me know how you get on :-)

GWRon commented 6 years ago

Will try when I finished reading aloud the desired fairy tale ...aka kid in bed.

I wonder why it worked in the past. Did not recognize changes in these parts of bmxng.

GWRon commented 6 years ago

Ok ... your changes were applied - and it works.

In my "real world" project I have still issues. There is one file - which wraps a 3rd party DLL (the file imports another bmx file which does the whole handling). The resulting DLL works. Then there is another file - which imports a "serial.bmx" (providing serial-communication via usb - utilizing some WinAPI stuff). As soon as I import "serial.bmx" the resulting DLL fails to get loaded ...

I will email you the project folder so you can operate on a "live object".

woollybah commented 6 years ago

Morning :-)

I have the viewer running and loading both dlls for x86, after some tinkering....

I recommend this dependency viewer for looking at dlls. It's a bit nicer than the old dependency walker. It showed you need to include a couple of dlls :

That got the calibration dll loading. For the lightcontrol dll, I was getting a 1114 error when LoadLibrary failed :

    If Not handle
        Local error:Int = GetLastError()
        Print "Failed to load library ~q"+url+"~q (" + error + ")"
...

which actually wasn't very helpful, but it only occurred when serial.bmx was imported.

Firstly, but unrelated to the load failure, you need to add __stdcall to your extern functions, because they are win32 apis - unless you are 64-bit only, in which case you can either add them or not...

    Function GetCommState:Int( handle:Byte Ptr, DCB:Byte Ptr )="WINBOOL __stdcall GetCommState (HANDLE, LPDCB)!"

If these are not already declared correctly in pub.win32, you could raise an issue for the missing ones ;-)

After sorting out your exports, the thing that immediately concerned me was the brl.retro import. Removing that, and fixing the single line that actually required it :

        For Local i:Int = 0 Until Len(command)
            PokeByte(W_buffer,Size_T(i),command[i])
        Next

seems to have allowed the libraries to load and the app to print the versions of both.

YMMV :-)

GWRon commented 6 years ago

Yes, old dependency walker did just "hang" during process execution - so was no help ;-)

So the named DLLs need to get provided next to the lightcontrol/calibration DLLs to use them? I hoped for self-contained DLLs to avoid this trouble. So I hope "need to look into it" results in "got it working" ;-)

@ brl.retro Why did it concern you? I mean, shouldn't it be possible to import modules? What happens if I want to import another module - another rewrite to use "more basic stuff"? In other words: there is a "bug" (or undesired behaviour) going on which makes using modules a hassle?

@ __stdcall I just did it similar to all others - and as it worked I just thought it's okay to do so. If I miss to add them does it define them a second time?

Will try now - and report asap.

woollybah commented 6 years ago

@ dlls Hopefully, statically importing them would make the runtime linkage requirement go away. Again, this only appears to be an x86 issue

@ brl.retro Probably because it imports brl.basic :-) But otherwise, no particular reason. But it seems I was correct to be concerned anyway!

@ externs It's fine to add them every time you want to use them, but if they are already in Pub.Win32, why add more risk of introducing errors into your code? - if pub.win32 is wrong, it's easy to fix that.

GWRon commented 6 years ago

First of all: thanks for putting so much efforts into it.

The serial.bmx is (see sources) a copy of Nigel Brown's code - in which I included some small fixes here and there (isOpen checks versus 0 instead of the byte pointer to the invalid-handle-constant, and some other annoyancies). Once I get my hands again at the real hardware device I will try your updated bah.serial - or is this not possible (for now) without further issues now?

@ your proposed fix Modified the sources now - works as a charm (thanks Mr. Magician!).

@ brl.basic So this is because it then does some dll_main() thing? In other words: once this particular "original issue" is fixed we could use brl.basic (and brl.retro, brl.xyz) with less problems?

@ externs Hope your upcoming DLL-documentation (bmx site) will clarify some stuff for me. Normally I would have thought that I just need to import pub.win32 to use this stuff directly but this didn't work out somehow when I tried the last time. (In other words: I thought I can use CreateFileA() etc. directly when importing Pub.Win32)

GWRon commented 6 years ago

When adding an import brl.threads (without even using them) I get an EAV as soon as I want to call a function out of the DLL importing BRL.threads.

That DLL also contains a simple print "Creation of thread here" which is printed with commented-out import brl.threads while it isn't printed when the module is imported.

Also for DLL loading your proposed changes (https://github.com/bmx-ng/bmk/issues/53#issuecomment-434005663) are needed - do they change behaviour in "Non-dll-situations"? else they should be made official!

woollybah commented 6 years ago

I think GC_THREADS is okay, and it is PARALLEL_MARK that was causing the problem - however you'd probably want to use it for normal apps... because it is presumably, more efficient. which complicates things - having a different build for dll/non dll stuff... which we don't currently support.

GWRon commented 6 years ago

Hmm couldn't we have "brl.blitz" and "brl.blitz_sharedLib" or something like this?

OR have a conditional like "?debug" - so it precompiles a shared-lib-variant (blitz.win32dll instead of blitz.win32) - this is used then when compiling a DLL (of course this creates a lot of files as it is needed for every module then, similar to release/debug/...)

Is it relevant for ".so" (linux, mac) too or a win32-only problem?

woollybah commented 6 years ago

Well, "makelib" is an app-level build option, so only concerns the final part of putting all the modules together into a shared library.

Shared libraries are not currently supported on other platforms.

woollybah commented 5 years ago

I believe DLLs should (at least) always load now.