Submitted a patch at https://go-review.googlesource.com/#/c/18057/

CL https://golang.org/cl/18057 mentions this issue.

I've added 32-bit support to this code, however it no longer builds at all when I set GOARCH=386.

I get:

$ go build -v -x -buildmode=c-archive -o test.a calc.go
can't load package: no buildable Go source files in <path>

GOARCH=amd64 works perfectly fine.

Any ideas?

I can see rt0_windows_amd64.s changed in CL 18057. For 386 support, I suspect you want to make similar changes to rt0_windows_386.s ay the very least. No?

Alex

I'm sorry, I should have been more clear. I have not pushed the 32-bit changes into the CL. I can do that, if you like. I made numerous changes, including rt0_windows_386.s. I admit that it is hard to diagnose a problem blind. I was thinking I just made some obvious beginner's mistake and perhaps you would might have seen something like this before.

It's just a guess, but perhaps you are getting no buildable Go source files in <path> because you have set GOARCH such that you are cross-compiling, but you did not explicitly set CGO_ENABLED=1.

That was it exactly. The 32-bit Windows API does not support some of the runtime features I wrote for initialization. I will have to rewrite parts of that code.

I am encountering very strange errors. Basically, hundreds of errors that look like:

'blah' referenced in section '.text' of 'blah' defined in discarded section '.text' of 'blah.../libkernel.a

I've tried to look through why this may be happening, but I'm afraid that I haven't come up with anything. Any ideas?

If implementing 32-bit is a problem, do not do it. Lets implement windows-amd64, and we'll worry about windows-386 later.

Alex

Okay. I will disable the build commands for 32-bit, but I feel like I should leave the code in. The problem happens during the final link of the library. The support code should be correct, including the runtime and relocation bits. There's just something about the link command that needs to be figured out. Does that sound okay?

32-bit is also important to me. I'd like to help. I'm seeing those same link errors when I enable it with your patch.

Many errors like the following:

_free' referenced in section `.text' of C:/Program Files/mingw-w64/i686-5.1.0-p
osix-dwarf-rt_v4-rev0/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.1.0/../../../../
i686-w64-mingw32/lib/../lib/dllcrt2.o: defined in discarded section `.text' of C
:/Program Files/mingw-w64/i686-5.1.0-posix-dwarf-rt_v4-rev0/mingw32/bin/../lib/g
cc/i686-w64-mingw32/5.1.0/../../../../i686-w64-mingw32/lib/../lib/libmsvcrt.a(dk
tbs01101.o)
`__initterm' referenced in section `.text' of C:/Program Files/mingw-w64/i686-5.
1.0-posix-dwarf-rt_v4-rev0/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.1.0/../../.
./../i686-w64-mingw32/lib/../lib/dllcrt2.o: defined in discarded section `.text'
 of C:/Program Files/mingw-w64/i686-5.1.0-posix-dwarf-rt_v4-rev0/mingw32/bin/../
lib/gcc/i686-w64-mingw32/5.1.0/../../../../i686-w64-mingw32/lib/../lib/libmsvcrt
.a(dktbs00350.o)
`__amsg_exit' referenced in section `.text' of C:/Program Files/mingw-w64/i686-5
.1.0-posix-dwarf-rt_v4-rev0/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.1.0/../../
../../i686-w64-mingw32/lib/../lib/dllcrt2.o: defined in discarded section `.text
' of C:/Program Files/mingw-w64/i686-5.1.0-posix-dwarf-rt_v4-rev0/mingw32/bin/..
/lib/gcc/i686-w64-mingw32/5.1.0/../../../../i686-w64-mingw32/lib/../lib/libmsvcr
t.a(dktbs00145.o)
`__initterm' referenced in section `.text' of C:/Program Files/mingw-w64/i686-5.
1.0-posix-dwarf-rt_v4-rev0/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.1.0/../../.
./../i686-w64-mingw32/lib/../lib/dllcrt2.o: defined in discarded section `.text'
 of C:/Program Files/mingw-w64/i686-5.1.0-posix-dwarf-rt_v4-rev0/mingw32/bin/../
lib/gcc/i686-w64-mingw32/5.1.0/../../../../i686-w64-mingw32/lib/../lib/libmsvcrt
.a(dktbs00350.o)

Can anyone shed some light on what might be causing the 32-bit link errors so others can look into it? From searching I found hints that it might be the order the libs are linked in.. but I had no luck changing the linking order of the libs to resolve the errors.

32-bit Windows uses a leading underscore on symbols. 64-bit WIndows does not. It may be related to that. Or it may not, it's just a guess.

before those errors I do see the following

Cannot export bad_proc_msg: symbol not found
Cannot export gosave: symbol not found
Cannot export masks: symbol not found
Cannot export setg_gcc: symbol not found
Cannot export shifts: symbol not found
Error: 0-bit reloc in dll

32-bit is also important to me. I'd like to help.

64-bit version does not work at this moment as far as I am concerned. We need testcarchive to PASS for us to even consider CL for inclusion. No point discussing 32-bit - it is only distracting everyone. If you want to help, help testcarchive to PASS.

Alex

testcarchive is only failing because the test is set up as a .bash file, which won't work on Windows.

I'll work on a test.bat now

@alexbrainman The c-archive tests were rewritten by @ianlancetaylor and now they don't even compile on Windows. So... I'm really not sure how to make the tests pass at this point. I started rewriting the test.bash file in Go, but now that the signal handlers exist there's kind of no point.

1. If it was rewritten in Go should it just be a "go run" file?
2. What do you suggest we do about the test files?

Sorry about that. Signal handling is pretty hard to get right for c-archive and c-shared, so we needed the tests. I didn't really think about the effect on Windows. It's fine with me if you want to put in a main_windows.c.

Probably test.bash should be replaced by a archive_test.go file that is run using go test, like the one in misc/cgo/testshared.

I still prefer we use Go to write tests (in $GOROOT/src/cmd/dist/test.go or what Ian suggested). We can write test to do whatever is required for different OSes. But I will let you do what you want.

Alex

I have added a main_windows.c, which contains the contents of main.c before @ianlancetaylor committed the signal handling changes. I updated test.bat to use that file instead of the other one.

Note that I am willing to rewrite the tests in Go, but I would like to get this patch accepted first. The c-archive tests pass for me on 32 and 64-bit.

I can also verify that c-archive now works for me on both 32 and 64 bit Windows

I removed the InitOnce based synchronization for the runtime init. This allows the patch to compile on older versions of mingw.

Just an update. The only problems left appear to be link errors with the 32-bit mode. This is causing an undefined reference to WinMain(). I am investigating this issue, but it doesn't appear related to my patch other than that cgo may not have worked properly at all on Windows 32-bit, and now we would like it to.

Anyone who is looking to help get this patch done sooner could investigate TestCgoExternalThreadPanic, TestCgoCrashHandler, TestCgoTraceback, etc.

I am looking into these failures too, but more eyes are better.

I have verified the cause of the problem:

If I create a file:

//test.c
int main(int argc, char **argv) {
return _main(argc, argv);
}

and I include it in the build of the failing tests, the build completes normally.

This verifies that it is the underscore problem.

I have tried to adjust the code in pe.go which prepends the underscore to avoid doing that for main(), but it doesn't seem to have any effect.

@ianlancetaylor , @alexbrainman I am not sure where main() is defined for a cgo-enabled program on 32-bit windows. If someone could point me to that location, the fix appears to be trivial.

@minux, thank you. This bug has been fixed. The failing tests now pass. Please re-evaluate the patch for correctness.

@alexbrainman I have addresses the issues you raised in the patch. Please re-evaluate it. Thanks!

@alexbrainman I have addresses the issues you raised in the patch.

Thank you for letting me know. But you don't need to comment about it on this issue. I can see all your new changes on Gerrit. Anyone who posts on your CL (and all nominated reviewers and CC) will get email every time you comment there or upload new patch.

Alex

@minux, @ianlancetaylor: There is some question about the right thing to do here:

// os1_windows.go
// Used by the C library build mode. On Linux this function would allocate a
// stack, but that's not necessary for Windows.
//go:nosplit
func newosproc0(mp *m, stk unsafe.Pointer) {
newosproc(mp, stk)
}

Some similar implementations of this function use //go:nosplit, others use //go:nowritebarrier
Neither @alexbrainman nor I know what the right thing to do here is. Can you please advise?

On Unix systems, newosproc0 is called at process startup time, with no stack guards installed. I assume the same is true in your Windows implementation. Therefore, it must be //go:nosplit. This is not optional. Also, it must only call //go:nosplit functions. This is unlike newosproc, which does not need to be //go:nosplit. But if you are going to have newosproc0 call newosproc, as implied above, then newosproc must also be //go:nosplit.

Similarly, newosproc0 is called before the GC is installed, and therefore any write barriers will fail, so //go:nowritebarrierrec is a good choice (nowritebarrierrec is better than nowritebarrier, as it means that not only must this function have no write barriers, every function it calls must have no write barriers). This is a warning option that does not affect code generation, so it's OK if it's not there. But since you are writing new code, you should add it. newosproc is normally //go:nowritebarrier because it is called from newm, where the comment explains why no write barriers are permitted.

@alexbrainman or anyone else:

I rebased onto origin/master and I am getting a new error when I run all, but only on 64-bit architectures:

ok      cmd/fix 0.125s
--- FAIL: TestShadowingLogic (0.77s)
        go_test.go:244: running testgo [list -f ({{.ImportPath}}) ({{.ConflictDir}}) ./testdata/shadow/root1/src/math]
        go_test.go:259: standard output:
        go_test.go:260: (_/C_/Users/Christopher_Nelson/t/src/cmd/go/testdata/shadow/root1/src/math) (C:\Users\Christopher Nelson\t\src\math)

        go_test.go:1884: shadowed math is not shadowed; looking for (_/C_/Users/Christopher Nelson/t/src/cmd/go/testdata/shadow/root1/src/math) (C:\Users\Christopher Nelson\t\src\math)
        go_test.go:244: running testgo [list -f ({{.ImportPath}}) ({{.ConflictDir}}) ./testdata/shadow/root1/src/foo]
        go_test.go:259: standard output:
        go_test.go:260: (foo) ()

        go_test.go:244: running testgo [list -f ({{.ImportPath}}) ({{.ConflictDir}}) ./testdata/shadow/root2/src/foo]
        go_test.go:259: standard output:
        go_test.go:260: (_/C_/Users/Christopher_Nelson/t/src/cmd/go/testdata/shadow/root2/src/foo) (C:\Users\Christopher Nelson\t\src\cmd\go\testdata\shadow\root1\src\foo)

        go_test.go:1897: shadowed foo is not shadowed; looking for (_/C_/Users/Christopher Nelson/t/src/cmd/go/testdata/shadow/root2/src/foo) (C:\Users\Christopher Nelson\t\src\cmd\go\testdata\shadow\root1\src\foo)
        go_test.go:244: running testgo [install ./testdata/shadow/root2/src/foo]
        go_test.go:263: standard error:
        go_test.go:264: go install: no install location for C:\Users\Christopher Nelson\t\src\cmd\go\testdata\shadow\root2\src\foo: hidden by C:\Users\Christopher Nelson\t\src\cmd\go\testdata\shadow\root1\src\foo

        go_test.go:283: testgo failed as expected: exit status 1
FAIL
FAIL    cmd/go  92.031s

Any guidance, would be appreciated.

I doubt the cmd/go test failure has anything to do with your patch. It should probably be a separate issue.

CL https://golang.org/cl/20892 mentions this issue.