Suggestion: cgo should not assume $CC is gcc

[Tianyi Cui]

Today, I tried "CC=clang ./all.bash" and everything works fine except
cgo. I found out the reason is that cgo assumes the C compiler we are
using is gcc.

I think cgo should support other compilers simply because people don't
always use gcc (for example I am currently a fan of building
everything with clang now).

After investigation, I found out there is a file named go/src/cmd/cgo/
gcc.go and the only func that assume the C compiler is gcc is "func (p
*Package) guessKinds(f *File) []*Name", which called $CC and interpret
the error message assuming the format is gcc's.

So, in order to make cgo with other compilers such as clang, icc.
There are two kinds of plans: A) use different ways to "guessKinds"
other than current "tricks gcc into revealing ..." way. B) Make
guessKinds be sure it executes gcc even if $CC is not set to gcc; when
user sets $CC as something other than gcc, the func guessKinds should
look for $GCC (or use "gcc" directly if it's not set).

I personally support plan B, because 1) I don't know how to implement
plan A in easy/portable/elegant way; 2) gcc is available on all
platforms, the user who does not want to use gcc can simply use
something like "CC=clang-2.7 GCC=gcc-4.2 make", or just "CC=clang
make" without noticing cgo executes gcc in $PATH once.

Here is a quick-and-dirty/proof-of-concept patch which simply let
gccErrors (only called by guessKinds) uses "gcc" in $PATH, which makes
"CC=clang ./all.bash" successful.

diff -r 768afe3769a8 src/cmd/cgo/gcc.go
--- a/src/cmd/cgo/gcc.go Tue Aug 17 20:01:50 2010 -0400
+++ b/src/cmd/cgo/gcc.go Wed Aug 18 10:01:13 2010 +0800
@@ -526,7 +526,9 @@
// gcc to fail.
func (p *Package) gccErrors(stdin []byte) string {
// TODO(rsc): require failure
- args := concat(p.gccCmd(), p.GccOptions)
+ cmd := p.gccCmd()
+ cmd[0] = "gcc";
+ args := concat(cmd, p.GccOptions)
if *debugGcc {
fmt.Fprintf(os.Stderr, "$ %s <<EOF\n",
strings.Join(args, " "))
os.Stderr.Write(stdin)

If my suggestion is accepted, I'd like to work on it and submit a
patch for review.

Tianyi Cui

[Russ Cox]

On Tue, Aug 17, 2010 at 22:09, Tianyi Cui <tian...@gmail.com> wrote:
> Today, I tried "CC=clang ./all.bash" and everything works fine except
> cgo.

I'm surprised by this. In some makefiles CC means the
host C compiler and in others it means the Go C compiler (6c or 8c).
The fact that this works suggests that you managed to override
only the first kind of use. Might be a good day to buy a lottery ticket too!
In all seriousness, it would help if you could look and see why it
worked and whether there's anything we should be doing differently.
I wonder if you ended up just using gcc despite setting the variable.

> I think cgo should support other compilers simply because people don't
> always use gcc (for example I am currently a fan of building
> everything with clang now).

Okay. But your description below doesn't do that.
It just forces cgo to use gcc. That's fine too.

Usnig $CC in cgo has caused other problems too.
The next release will change this to $GCC, which will
have the same effect as your proposed patch.
So I think you can just sit back and the problem will fix itself.

Russ

[Tianyi Cui]

Sorry I made mistakes in the original post. That patch didn't really
work. Below is the quick-and-dirty patch to make "CC=clang ./all.bash"
work correctly (gcc is used only internally in cgo when necessary). It
does the following things:

1) In makefiles, change the variable name for 6c/8c from CC to C9 (I
wanted to make 9C but failed ;-), and all $(CC) in makefiles now
means $(QUOTED_GOBIN)/quietgcc, the actual compiler called by quietgcc
is set by Make.bash by checking the environment varaible CC, that's
how "CC=clang ./all.bash" work.
2) In makefiles, change several direct mention of gcc to $(CC).
3) Changed src/cmd/cgo/gcc.go to make sure cgo excutes gcc instead of
quietgcc when necessary (for me it means when a quietgcc which calls
clang inside won't work).

I'm still not sure whether I should make this quick-and-dirty fix more
sophisticated and submit for code review, any suggestions?

diff -r 768afe3769a8 src/Make.386
--- a/src/Make.386 Tue Aug 17 20:01:50 2010 -0400
+++ b/src/Make.386 Wed Aug 18 12:14:21 2010 +0800
@@ -4,7 +4,7 @@

O=8
AS=${O}a
-CC=${O}c
+C9=${O}c
GC=${O}g
LD=${O}l
OS=568vq
diff -r 768afe3769a8 src/Make.amd64
--- a/src/Make.amd64 Tue Aug 17 20:01:50 2010 -0400
+++ b/src/Make.amd64 Wed Aug 18 12:14:21 2010 +0800
@@ -4,7 +4,7 @@

O=6
AS=${O}a
-CC=${O}c
+C9=${O}c
GC=${O}g
LD=${O}l
OS=568vq
diff -r 768afe3769a8 src/Make.arm
--- a/src/Make.arm Tue Aug 17 20:01:50 2010 -0400
+++ b/src/Make.arm Wed Aug 18 12:14:21 2010 +0800
@@ -4,7 +4,7 @@

O=5
AS=${O}a
-CC=${O}c
+C9=${O}c
GC=${O}g
LD=${O}l
OS=568vq
diff -r 768afe3769a8 src/Make.pkg
--- a/src/Make.pkg Tue Aug 17 20:01:50 2010 -0400
+++ b/src/Make.pkg Wed Aug 18 12:14:21 2010 +0800
@@ -117,11 +117,13 @@
%.cgo2.c: _cgo_defun.c _cgo_gotypes.go
@true

+CC=$(QUOTED_GOBIN)/quietgcc
+
%.cgo2.o: %.cgo2.c
- gcc $(_CGO_CFLAGS_$(GOARCH)) -fPIC -O2 -o $@ -c $(CGO_CFLAGS)
$*.cgo2.c
+ $(CC) $(_CGO_CFLAGS_$(GOARCH)) -fPIC -O2 -o $@ -c $(CGO_CFLAGS)
$*.cgo2.c

_cgo_export.o: _cgo_export.c _cgo_export.h
- gcc $(_CGO_CFLAGS_$(GOARCH)) -fPIC -O2 -o $@ -c $(CGO_CFLAGS)
_cgo_export.c
+ $(CC) $(_CGO_CFLAGS_$(GOARCH)) -fPIC -O2 -o $@ -c $(CGO_CFLAGS)
_cgo_export.c

# The rules above added x.cgo1.go and _cgo_gotypes.go to $(GOFILES),
# added _cgo_defun.$O to $OFILES, and added the installed copy of
@@ -142,10 +144,10 @@

# Compile _cgo_defun.c with 6c; needs access to the runtime headers.
_cgo_defun.$O: _cgo_defun.c
- $(QUOTED_GOBIN)/$(CC) $(CFLAGS) $(RUNTIME_CFLAGS) _cgo_defun.c
+ $(QUOTED_GOBIN)/$(C9) $(CFLAGS) $(RUNTIME_CFLAGS) _cgo_defun.c

_cgo_.so: $(GCC_OFILES) $(CGO_DEPS)
- gcc $(_CGO_CFLAGS_$(GOARCH)) -o $@ $(GCC_OFILES) $(CGO_LDFLAGS) $
(_CGO_LDFLAGS_$(GOOS))
+ $(CC) $(_CGO_CFLAGS_$(GOARCH)) -o $@ $(GCC_OFILES) $(CGO_LDFLAGS) $
(_CGO_LDFLAGS_$(GOOS))

$(pkgdir)/$(CGOTARG).so: _cgo_.so
@test -d $(QUOTED_GOROOT)/pkg && mkdir -p $(pkgdir)/$(dir)
@@ -155,7 +157,7 @@
# These come last so that the rules above can override them
# for more specific file names.
%.$O: %.c
- $(QUOTED_GOBIN)/$(CC) $(CFLAGS) $*.c
+ $(QUOTED_GOBIN)/$(C9) $(CFLAGS) $*.c

%.$O: %.s
$(QUOTED_GOBIN)/$(AS) $*.s

diff -r 768afe3769a8 src/cmd/cgo/gcc.go
--- a/src/cmd/cgo/gcc.go Tue Aug 17 20:01:50 2010 -0400

+++ b/src/cmd/cgo/gcc.go Wed Aug 18 12:14:21 2010 +0800
@@ -491,7 +491,9 @@
// returns the corresponding DWARF data and any messages
// printed to standard error.
func (p *Package) gccDebug(stdin []byte) *dwarf.Data {
- runGcc(stdin, concat(p.gccCmd(), p.GccOptions))

+ cmd := p.gccCmd()
+ cmd[0] = "gcc"

+ runGcc(stdin, concat(cmd, p.GccOptions))

// Try to parse f as ELF and Mach-O and hope one works.
var f interface {
@@ -526,7 +528,9 @@

// gcc to fail.
func (p *Package) gccErrors(stdin []byte) string {
// TODO(rsc): require failure
- args := concat(p.gccCmd(), p.GccOptions)
+ cmd := p.gccCmd()
+ cmd[0] = "gcc";
+ args := concat(cmd, p.GccOptions)
if *debugGcc {
fmt.Fprintf(os.Stderr, "$ %s <<EOF\n", strings.Join(args, " "))
os.Stderr.Write(stdin)

diff -r 768afe3769a8 src/pkg/runtime/Makefile
--- a/src/pkg/runtime/Makefile Tue Aug 17 20:01:50 2010 -0400
+++ b/src/pkg/runtime/Makefile Wed Aug 18 12:14:21 2010 +0800
@@ -136,13 +136,13 @@
mv -f $@.tmp $@

%.$O: $(GOARCH)/%.c
- $(QUOTED_GOBIN)/$(CC) $(CFLAGS) $<
+ $(QUOTED_GOBIN)/$(C9) $(CFLAGS) $<

%.$O: $(GOOS)/%.c
- $(QUOTED_GOBIN)/$(CC) $(CFLAGS) $<
+ $(QUOTED_GOBIN)/$(C9) $(CFLAGS) $<

%.$O: $(GOOS)/$(GOARCH)/%.c
- $(QUOTED_GOBIN)/$(CC) $(CFLAGS) $<
+ $(QUOTED_GOBIN)/$(C9) $(CFLAGS) $<

%.$O: $(GOARCH)/%.s $(GOARCH)/asm.h
$(QUOTED_GOBIN)/$(AS) $<
@@ -152,16 +152,16 @@

# for discovering offsets inside structs when debugging
runtime.acid.$(GOARCH): runtime.h proc.c
- $(QUOTED_GOBIN)/$(CC) $(CFLAGS) -a proc.c >$@
+ $(QUOTED_GOBIN)/$(C9) $(CFLAGS) -a proc.c >$@

# 386 traceback is really amd64 traceback
ifeq ($(GOARCH),386)
traceback.$O: amd64/traceback.c
- $(QUOTED_GOBIN)/$(CC) $(CFLAGS) $<
+ $(QUOTED_GOBIN)/$(C9) $(CFLAGS) $<
endif

# NaCl closure is special.
ifeq ($(GOOS),nacl)
closure.$O: nacl/$(GOARCH)/closure.c
- $(QUOTED_GOBIN)/$(CC) $(CFLAGS) $<
+ $(QUOTED_GOBIN)/$(C9) $(CFLAGS) $<
endif

[Tianyi Cui]

Some explanation about my patch:

On Aug 18, 10:23 am, Russ Cox <r...@golang.org> wrote:

> Okay.  But your description below doesn't do that.
> It just forces cgo to use gcc.  That's fine too.

It only forces cgo to use gcc when it's used to gather information
rather than compile things, while output text format is relevant.

> Usnig $CC in cgo has caused other problems too.
> The next release will change this to $GCC, which will
> have the same effect as your proposed patch.
> So I think you can just sit back and the problem will fix itself.
>

My new patch has the effect to make "CC=other-cc ./all.bash"
from impossible to possible, should I just sit back and wait for next
release now?

Tianyi Cui

[Russ Cox]

>> Usnig $CC in cgo has caused other problems too.
>> The next release will change this to $GCC, which will
>> have the same effect as your proposed patch.
>> So I think you can just sit back and the problem will fix itself.
>
> My new patch has the effect to make "CC=other-cc ./all.bash"
> from impossible to possible, should I just sit back and wait for next
> release now?

Yes, please. The makefiles are undergoing a bit of churn
right now anyway. I'm also not convinced that we have to
support changing the compiler via setting CC. I know that
is the GNU Way, but this is not a GNU project. If I were
going to build Go using clang I would do it by editing
src/Make.conf instead. It exists to hold that kind of
configuration.

Russ

[Tianyi Cui]

On Aug 18, 9:23 pm, Russ Cox <r...@golang.org> wrote:
> Yes, please.  The makefiles are undergoing a bit of churn
> right now anyway.  I'm also not convinced that we have to
> support changing the compiler via setting CC.  I know that
> is the GNU Way, but this is not a GNU project.  If I were
> going to build Go using clang I would do it by editing
> src/Make.conf instead.  It exists to hold that kind of
> configuration.

OK, sure, I see your point. It's my fault to ignorantly assume the
"GNU way" is the only way, in fact it used to be the only "way" I'm
familiar with before knowing Go. Thank you and the Go project as a
whole for teaching me more "ways"!

[Benny Siegert]

On Wed, Aug 18, 2010 at 15:23, Russ Cox <r...@golang.org> wrote:
> Yes, please.  The makefiles are undergoing a bit of churn
> right now anyway.  I'm also not convinced that we have to
> support changing the compiler via setting CC.  I know that
> is the GNU Way, but this is not a GNU project.  If I were
> going to build Go using clang I would do it by editing
> src/Make.conf instead.  It exists to hold that kind of
> configuration.

Please do support at least the CC and CFLAGS environment variables.
They make the job much easier for the people who want to create binary
packages.

--Benny.

--
The first essential in chemistry is that you should perform practical
work and conduct experiments, for he who performs not practical work
nor makes experiments will never attain the least degree of mastery.
        -- Abu Musa Jabir ibn Hayyan (721-815)