-
Notifications
You must be signed in to change notification settings - Fork 18k
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
Labels
Comments
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. |
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 |
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(?). |
> ... 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 |
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. |
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. |
@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 |
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. |
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 |
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. |
@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. |
CL https://golang.org/cl/74160043 references this issue. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The text was updated successfully, but these errors were encountered: