cmd/cgo: gcc_arm.S miscompiled under clang 3.4 #8348

davecheney · 2014-07-09T12:02:31Z

What steps will reproduce the problem?

The linux-arm-arm5 builder runs debian unstable which recently transition to clang 3.4 

This broke the cgo portions of the build

# ../misc/cgo/test
# _/export/builder/workspace/linux-arm-arm5-27eb9d18b77c/go/misc/cgo/test
/tmp/--815d94.s: Assembler messages:
/tmp/--815d94.s:47: Error: selected processor does not support ARM mode `blx r0'
clang: error: assembler command failed with exit code 1 (use -v to see invocation)
FAIL    _/export/builder/workspace/linux-arm-arm5-27eb9d18b77c/go/misc/cgo/test [build
failed]

My diagnosis is clang's internal assembler does not honour the .arch armv5t pragma.
Previous versions of clang did not assemble internally and forked /usr/bin/as to handle
the assembly, and so would honor the arch pragma.

Because .arch is being ignored, clang is assembling using the built in armv4t model
(this is a guess, but gcc on this system also reports armv4t, and this is a common
baseline for pre hardfloat distributions, so I think a safe one).

Unless there is an alternative assembly pragma that clang does honour, we may have to
revert to passing an -march directive on the command line.

Please use labels and text to provide additional information.

diff -r fdd2d7a9dd4c src/cmd/go/build.go
--- a/src/cmd/go/build.go       Wed Jul 09 18:50:38 2014 +1000
+++ b/src/cmd/go/build.go       Wed Jul 09 22:07:39 2014 +1000
@@ -2055,7 +2055,7 @@
        case "6":
                return []string{"-m64"}
        case "5":
-               return []string{"-marm"} // not thumb
+               return []string{"-marm", "-march=armv5t"} // not
thumb
        }
        return nil
 }

This small patch (not suitable for submission) fixes the problem.

minux · 2014-07-09T15:14:15Z

Comment 1:

could you show the output of go test -x ../misc/cgo/test?
I don't think we have "blx r0" in our tree, and judging from
the file name, it seems the instruction is generated by the
c compiler.
the problem of adding "-march=armv5t" is that then all cgo
code will be compiled using only armv5t instructions.

davecheney · 2014-07-09T15:22:28Z

Comment 2:

Is it this one? https://code.google.com/p/go/source/browse/src/pkg/runtime/cgo/gcc_arm.S

minux · 2014-07-09T15:27:14Z

Comment 3:

no. that file uses "blx r5" and "blx r4", but not "blx r0":
/tmp/--815d94.s:47: Error: selected processor does not support ARM mode `blx r0'
also, it's strange that this error manifests itself when testing misc/cgo/test,
because at that time, the runtime/cgo package should have been compiled already.

minux · 2014-07-10T18:40:43Z

Comment 4:

now it seems a clang bug where it generates the blx instruction but its integrated
assembler rejects it.

rsc · 2014-09-15T15:25:49Z

Comment 5:

It sounds like this is a clang bug.

Labels changed: added release-none, removed release-go1.4.

davecheney · 2014-10-20T00:12:36Z

Comment 6:

> It sounds like this is a clang bug.
It looks this way, I have verified that clang does process .arch directives when using
the integrated assembler (clang 3.4+), but it does not appear to change the compilation
model. If we use --no-integrated-assembler then everything works ok.

gopherbot · 2014-10-20T11:49:36Z

Comment 7:

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

davecheney · 2014-10-20T23:30:43Z

Comment 8:

This issue was closed by revision cf9558c.

Status changed to Fixed.

gopherbot · 2014-10-21T23:23:44Z

Comment 9:

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

davecheney · 2014-10-21T23:34:16Z

Comment 10:

reopened

Status changed to Started.

davecheney · 2014-10-21T23:42:42Z

Comment 11:

This issue was closed by revision 4073be8.

Status changed to Fixed.

gopherbot · 2014-10-22T00:14:36Z

Comment 12:

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

gopherbot · 2014-10-22T00:16:36Z

Comment 13:

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

gopherbot · 2014-10-22T00:18:33Z

Comment 14:

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

davecheney · 2014-10-22T01:21:21Z

Comment 15:

This issue was closed by revision d1b2913.

Fixes golang#8348. Clang's internal assembler (introduced by default in clang 3.4) understands the .arch directive, but doesn't change the default value of -march. This causes the build to fail when we use BLX (armv5 and above) when clang is compiled for the default armv4t architecture (which appears to be the default on all the distros I've used). This is probably a clang bug, so work around it for the time being by disabling the integrated assembler when compiling the cgo assembly shim. This CL also includes a small change to ldelf.c which was required as clang 3.4 and above generate more weird symtab entries. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/156430044

Partial undo, changes to ldelf.c retained. Some platforms are still not working even with the integrated assembler disabled, will have to find another solution. ««« original CL description cmd/cgo: disable clang's integrated assembler Fixes golang#8348. Clang's internal assembler (introduced by default in clang 3.4) understands the .arch directive, but doesn't change the default value of -march. This causes the build to fail when we use BLX (armv5 and above) when clang is compiled for the default armv4t architecture (which appears to be the default on all the distros I've used). This is probably a clang bug, so work around it for the time being by disabling the integrated assembler when compiling the cgo assembly shim. This CL also includes a small change to ldelf.c which was required as clang 3.4 and above generate more weird symtab entries. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews https://golang.org/cl/156430044 »»» LGTM=minux R=iant, minux CC=golang-codereviews https://golang.org/cl/162880044

Fixes golang#8348. Trying to work around clang's dodgy support for .arch by reverting to the external assembler didn't work out so well. Minux had a much better solution to encode the instructions we need as .word directives which avoids .arch altogether. I've confirmed with gdb that this form produces the expected machine code Dump of assembler code for function crosscall_arm1: 0x00000000 <+0>: push {r4, r5, r6, r7, r8, r9, r10, r11, r12, lr} 0x00000004 <+4>: mov r4, r0 0x00000008 <+8>: mov r5, r1 0x0000000c <+12>: mov r0, r2 0x00000010 <+16>: blx r5 0x00000014 <+20>: blx r4 0x00000018 <+24>: pop {r4, r5, r6, r7, r8, r9, r10, r11, r12, pc} There is another compilation failure that blocks building Go with clang on arm # ../misc/cgo/test # _/home/dfc/go/misc/cgo/test /tmp/--407b12.s: Assembler messages: /tmp/--407b12.s:59: Error: selected processor does not support ARM mode `blx r0' clang: error: assembler command failed with exit code 1 (use -v to see invocation) FAIL _/home/dfc/go/misc/cgo/test [build failed] I'll open a new issue for that LGTM=iant R=iant, minux CC=golang-codereviews https://golang.org/cl/158180047