Skip to content

runtime: windows test compiles dll - cross compile can fail #7513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kardianos opened this issue Mar 11, 2014 · 20 comments
Closed

runtime: windows test compiles dll - cross compile can fail #7513

kardianos opened this issue Mar 11, 2014 · 20 comments

Comments

@kardianos
Copy link
Contributor

In "runtime/syscall_windows_test.go" a DLL is compiled and then run to test
callbacks in test "TestStdcallAndCDeclCallbacks".

This DLL needs to be the same bitness as the calling executable. Should add
"-m32" for 32-bit builds or "-m64" for 64-bit builds. This ensures
the correct executable gets called.
@alexbrainman
Copy link
Member

Comment 1:

You didn't say what happens in TestStdcallAndCDeclCallbacks. Does it PASSes? Does it
fail? How? Can you provide the test output.
Thank you.
Alex

Status changed to WaitingForReply.

@kardianos
Copy link
Contributor Author

Comment 2:

Currently FAILs when compiling on amd64 for 386 when a gcc 64-bit compiler is available.
Fails with:
panic: Failed to load C:\...\test.dll: %1 is not a valid Win32 application.

@davecheney
Copy link
Contributor

Comment 3:

It sounds like you are cross compiling from windows/amd64 to windows/386. We don't
expect the tests to pass in such an environment.

@kardianos
Copy link
Contributor Author

Comment 4:

I know we don't want to guarantee this, but his had a PASS in 1.2, seems like a tiny fix
to continue to get a PASS in 1.3.
Normally I'd agree with you (Linux, OSX), but on Windows, I think it is worth
addressing. Generally, on Windows I have both 32-bit binaries mixed with 64-binaries all
on the same box. It's nice to see the runtime get a PASS without looking into and seeing
it is not a relevant failure.

@alexbrainman
Copy link
Member

Comment 5:

Do you know why the test PASSes on my system? I use:
c:\>gcc --version
gcc (GCC) 4.7.0 20111220 (experimental)
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Thank you.
Alex

@kardianos
Copy link
Contributor Author

Comment 6:

I have a 64 bit GCC version installed. Thus the issue with 64 bit DLLs. If you have a 32
bit GCC on windows (typical) I don't think it will show up.

@alexbrainman
Copy link
Member

Comment 7:

My gcc compiles 64-bit dlls without -m... switch. Why doesn't yours?
Alex

@kardianos
Copy link
Contributor Author

Comment 8:

Sorry for the confusion.
System: 64-bit
w:\>gcc --version
gcc (tdm64-1) 4.6.1
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
My system compiles 64-bit DLLs by default. That is the problem. The 32-bit test
executable can't load a 64-bit DLL.
I suspect it works on your system because you may have a 32-bit gcc on your 64-bit
windows(?).

@alexbrainman
Copy link
Member

Comment 9:

> ... My system compiles 64-bit DLLs by default. 
I use 64-bit version of Windows. I use gcc as per #5. TestStdcallAndCDeclCallbacks
creates 64-bit DLLs on my system too (I checked DLLs are 64-bit). We don't specify any
-m32 or -m64 in TestStdcallAndCDeclCallbacks, and DLLs are 64-bit by default.
> ... That is the problem. 
I don't understand. What is the problem?
> ... The 32-bit test executable can't load a 64-bit DLL.
Sure, but my test program (the one that contains TestStdcallAndCDeclCallbacks) is
64-bit. I run all.bat or make.bat to build that test, and Go scripts can see that I use
64-bit OS, so they use GOOS=windows GOARCH=amd64 by default. That is hy my test is
64-bit.
> ... I suspect it works on your system because you may have a 32-bit gcc on your 64-bit
windows(?).
I don't know what "32-bit gcc" means. My gcc version is as per #5.
Can you, please, try to understand why TestStdcallAndCDeclCallbacks fails on your
system? I suspect it is because exe and dlls are of different bitness. Can you find out
what they are and compare them.
Thank you.
Alex

@kardianos
Copy link
Contributor Author

Comment 10:

Reading back, it looks like I wasn't clear. My workstation is 64-bit. I sometimes need
to deploy to 32-bit servers. I like to make sure the build tree is alright so I will run
the tests if I'm compiling from windows. Technically I'm cross-compiling, but Windows
blurs the line here a bit.
As we both mentioned, they are due to different bitness. My 64-bit host system is
testing 32-bit go, and gcc is trying to make a 64-bit dll in this test.
Again, I know testing while cross-compiling isn't typically done, I think it can make
sense here. If this still doesn't make sense, feel free to close the issue. Probably not
worth spending much more time one.

@davecheney
Copy link
Contributor

Comment 11:

I can see what you are trying to do, but this has never been supported, and more
specifically has never been tested. 
The general policy is, for this class of change, is there has to be a builder on the
build.golang.org dashboard that exercises this mode, otherwise we're apt to break it
very quickly.

@kardianos
Copy link
Contributor Author

Comment 12:

@dave : I agree on the theory, but the only reason this method broke was the manual
invoking of gcc that doesn't go through the normal cgo method that already ensures this.
Practically this isn't much of an issue on Windows. Due to the method windows uses to
call system DLLs, and due to the multi-lib magic, I've always had this just work. This
just seams like a silly thing to "break" the method over. 
I'm fine discarding the issue and CL.
Regardless, thanks for the time.
-Daniel

@alexbrainman
Copy link
Member

Comment 13:

I agree with Dave. Our tests are designed to be executed by all.bat script. And that, I
suspect, works on your system (Please confirm). If you run tests out of context (with
some GO... variables set in a particular way), then some tests might fail.
I have no problem with your change https://golang.org/cl/74160043/ in general,
but I don't know enough about how gcc works. I'm worried it might break in some
scenarios. Given that nothing is broken (if you confirm all.bat runs to success on your
system), I would suggest we do nothing.
Closing issue for now.
Alex

Status changed to Invalid.

@kardianos
Copy link
Contributor Author

Comment 14:

This first showed up for me when running all.bat. I found a "break" when I was prepping
the 386 compiler chain for initial 1.3 testing now that the builders are green(-ish),
then looked into it. Found it was a false issue (the build would be fine, but thought
I'd look into fixing the test before 1.3 got baked.
So for me, all.bat fails when I build my 386 tools on my amd64 system.
Thanks again,
-Daniel

@alexbrainman
Copy link
Member

Comment 15:

Do you mean you run all.bat with no GO* environment variables set and the
TestStdcallAndCDeclCallbacks fails?
Alex

Status changed to WaitingForReply.

@kardianos
Copy link
Contributor Author

Comment 16:

No, I don't mean that.
It only fails with I compile and test 32-bit go when I have a 64-bit GCC installed and
default. I have set the GOARCH=386 prior to this.
> set GOARCh=386
> all.bat
.. wait ..
 TEST FAIL
Don't worry about it.
-Daniel

@alexbrainman
Copy link
Member

Comment 17:

Thank you. At least we're on the same page now.
Alex

Status changed to Invalid.

@davecheney
Copy link
Contributor

Comment 18:

Two things.
1. set GOHOSTARCH=386; set GOARCH=386 may work better.
2. Really, this isn't guaranteed to work. Despite the amd64 processor
having support for running 32 bit code the fact that your 64bit
operating system can run 32bit code shouldn't be taken as given. This
doesn't work across all platforms, not even linux (unless you install
a 32bit libc development pkgs), and certainly not by default for other
bsds.

@kardianos
Copy link
Contributor Author

Comment 19:

@dave :
1. setting GOHOSTARCH doesn't help (just tried it) because the gcc invoke doesn't use
the same build flags or environment variables. It just invokes the default compiler for
the platform. In this case a 64-bit compiling for windows. I might argue the test is
flawed in that it invokes gcc without this, but I don't care that much (the test would
probably fail on a normal 64-bit environment if a 32-bit gcc was installed, probably it
should first test if gcc CAN invoke the correct bitness, and skip it if it can't, but
that's untested).
2. Again I agree; this isn't guaranteed to work. I would not expect it to work on Linux
on almost all systems. But, all the other tests and runtimes DO work on windows this
way. While I have agreed again that it isn't guaranteed, it corrects the behavior of the
test in a particular situation I found myself. Despite everything, I don't think I'd
need to dig through many tests that work around things builders don't regularly test,
but are useful.
Regardless of this test, this pattern will be continued to be used (by me at least) on
windows, and it will continue to work for the foreseeable future simply due to the
architecture of Windows and Go DLL calling behavior on windows. I'll just note this one
test as a known false positive and continue on my way. Not an issue.

@gopherbot
Copy link
Contributor

Comment 20:

CL https://golang.org/cl/74160043 references this issue.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants