Skip to content
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

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

Closed
davecheney opened this issue Jul 9, 2014 · 15 comments

Comments

@davecheney
Copy link
Contributor

commented Jul 9, 2014

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

This comment has been minimized.

Copy link
Member

commented Jul 9, 2014

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

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2014

@minux

This comment has been minimized.

Copy link
Member

commented Jul 9, 2014

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

This comment has been minimized.

Copy link
Member

commented Jul 10, 2014

Comment 4:

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

This comment has been minimized.

Copy link
Contributor

commented Sep 15, 2014

Comment 5:

It sounds like this is a clang bug.

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

@davecheney

This comment has been minimized.

Copy link
Contributor Author

commented Oct 20, 2014

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

This comment has been minimized.

Copy link

commented Oct 20, 2014

Comment 7:

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

This comment has been minimized.

Copy link
Contributor Author

commented Oct 20, 2014

Comment 8:

This issue was closed by revision cf9558c.

Status changed to Fixed.

@gopherbot

This comment has been minimized.

Copy link

commented Oct 21, 2014

Comment 9:

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

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2014

Comment 10:

reopened

Status changed to Started.

@davecheney

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2014

Comment 11:

This issue was closed by revision 4073be8.

Status changed to Fixed.

@gopherbot

This comment has been minimized.

Copy link

commented Oct 22, 2014

Comment 12:

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

This comment has been minimized.

Copy link

commented Oct 22, 2014

Comment 13:

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

This comment has been minimized.

Copy link

commented Oct 22, 2014

Comment 14:

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

This comment has been minimized.

Copy link
Contributor Author

commented Oct 22, 2014

Comment 15:

This issue was closed by revision d1b2913.

@golang golang locked and limited conversation to collaborators Jun 25, 2016

wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
undo CL 156430044 / 5d69cad4faaf
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
runtime/cgo: encode BLX directly, fixes one clang build error on arm
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
undo CL 156430044 / 5d69cad4faaf
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
runtime/cgo: encode BLX directly, fixes one clang build error on arm
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
undo CL 156430044 / 5d69cad4faaf
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
runtime/cgo: encode BLX directly, fixes one clang build error on arm
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
undo CL 156430044 / 5d69cad4faaf
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
runtime/cgo: encode BLX directly, fixes one clang build error on arm
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
undo CL 156430044 / 5d69cad4faaf
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
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
runtime/cgo: encode BLX directly, fixes one clang build error on arm
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
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.