code review 13261055: cmd/go: fix missing __mingw_fprintf symbol for cgo on w... (issue 13261055)

[minu...@gmail.com]

Reviewers: golang-dev1,

Message:
Hello golan...@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go


Description:
cmd/go: fix missing __mingw_fprintf symbol for cgo on windows

Fixes issue 5986.

Please review this at https://codereview.appspot.com/13261055/

Affected files (+36, -2 lines):
M misc/cgo/test/cgo_test.go
A misc/cgo/test/issue5986.go
M src/cmd/go/build.go


Index: misc/cgo/test/cgo_test.go
===================================================================
--- a/misc/cgo/test/cgo_test.go
+++ b/misc/cgo/test/cgo_test.go
@@ -47,5 +47,6 @@
func TestFpVar(t *testing.T) { testFpVar(t) }
func Test4339(t *testing.T) { test4339(t) }
func Test6390(t *testing.T) { test6390(t) }
+func Test5986(t *testing.T) { test5986(t) }

func BenchmarkCgoCall(b *testing.B) { benchCgoCall(b) }
Index: misc/cgo/test/issue5986.go
===================================================================
new file mode 100644
--- /dev/null
+++ b/misc/cgo/test/issue5986.go
@@ -0,0 +1,32 @@
+// Copyright 2013 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package cgotest
+
+/*
+#cgo LDFLAGS: -lm
+#include <stdio.h>
+#include <math.h>
+
+static void output5986()
+{
+ int current_row = 0, row_count = 0;
+ double sum_squares = 0;
+ do {
+ if (current_row == 10) {
+ current_row = 0;
+ }
+ ++row_count;
+ }
+ while (current_row++ != 1);
+ double d = sqrt(sum_squares / row_count);
+ printf("sqrt is: %g\n", d);
+}
+*/
+import "C"
+import "testing"
+
+func test5986(t *testing.T) {
+ C.output5986()
+}
Index: src/cmd/go/build.go
===================================================================
--- a/src/cmd/go/build.go
+++ b/src/cmd/go/build.go
@@ -2034,8 +2034,9 @@

var staticLibs []string
if goos == "windows" {
- // libmingw32 and libmingwex might also use libgcc, so libgcc must come
last
- staticLibs = []string{"-lmingwex", "-lmingw32"}
+ // libmingw32 and libmingwex might also use libgcc, so libgcc must come
last,
+ // and they also have some inter-dependencies, so must use linker groups.
+ staticLibs =
[]string{"-Wl,--start-group", "-lmingwex", "-lmingw32", "-Wl,--end-group"}
}
if cgoLibGccFile != "" {
staticLibs = append(staticLibs, cgoLibGccFile)

[Russ Cox]

LGTM

[alex.b...@gmail.com]

LGTM

Thank you.

Alex

https://codereview.appspot.com/13261055/

[minux]

Hi Russ and Alex,

To complete the fix of known windows cgo problems, I still have a patch like this:

diff -r e3fb358fb3c6 src/cmd/ld/ldpe.c

--- a/src/cmd/ld/ldpe.c Wed Sep 18 22:27:25 2013 -0400

+++ b/src/cmd/ld/ldpe.c Thu Sep 19 00:55:29 2013 -0400

@@ -468,6 +468,7 @@

  break;

  case IMAGE_SYM_CLASS_NULL:

  case IMAGE_SYM_CLASS_STATIC:

+ case IMAGE_SYM_CLASS_LABEL:

  s = lookup(name, version);

  s->dupok = 1;

  break;

but as of now, i don't come up with a small standalone test for the problem.

is it ok to main a CL with this patch without a proper test?

the reporter of the problem have verified with all three patches applied, the

problem is fixed.

[alex.b...@gmail.com]

On 2013/09/19 04:58:35, minux wrote:

> ... I still have a patch
> like this:

I take it, the diff is unrelated to this CL? If yes, then, please, send
it in a new CL with proper description.

> @@ -468,6 +468,7 @@
> break;
> case IMAGE_SYM_CLASS_NULL:
> case IMAGE_SYM_CLASS_STATIC:
> + case IMAGE_SYM_CLASS_LABEL:
> s = lookup(name, version);
> s->dupok = 1;
> break;

I am not familiar with that code, but I will look. Thank you.

Alex

https://codereview.appspot.com/13261055/

[minux]

On Thu, Sep 19, 2013 at 1:03 AM, <alex.b...@gmail.com> wrote:

I take it, the diff is unrelated to this CL? If yes, then, please, send
it in a new CL with proper description.

just sent https://codereview.appspot.com/13722050. 

[minu...@gmail.com]

*** Submitted as
https://code.google.com/p/go/source/detail?r=45d005b47fc0 ***

cmd/go: fix missing __mingw_fprintf symbol for cgo on windows

Fixes issue 5986.

R=golang-dev, rsc, alex.brainman
CC=golang-dev
https://codereview.appspot.com/13261055


https://codereview.appspot.com/13261055/