code review 114420043: cmd/ld: fix operator precedence

cmd/ld: fix operator precedence

[minux, 2014-08-06 03:28:49]

Hello rsc@golang.org, iant@golang.org (cc: golang-codereviews@googlegroups.com),

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

[rsc, 2014-08-06 03:35:38]

LGTM

[minux, 2014-08-06 04:25:08]

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

cmd/ld: fix operator precedence

LGTM=rsc
R=rsc, iant
CC=golang-codereviews
https://codereview.appspot.com/114420043

[gobot, 2014-08-06 05:15:46]

This CL appears to have broken the freebsd-amd64 builder.
See http://build.golang.org/log/ae502c5cfde2c7b734feb986ccb344ff69e059ba

[dave_cheney.net, 2014-08-06 05:18:50]

This is a real failure,

ok   _/tmp/gobuilder/freebsd-amd64-b613f2acdf69/go/misc/cgo/test 1.538s
# testmain
_/tmp/gobuilder/freebsd-amd64-b613f2acdf69/go/misc/cgo/test(.text):
compilerrt_abort_impl: not defined
FAIL _/tmp/gobuilder/freebsd-amd64-b613f2acdf69/go/misc/cgo/test [build failed]
Build complete, duration 5m17.708335978s. Result: error: exit status 1

On Wed, Aug 6, 2014 at 3:15 PM,  <gobot@golang.org> wrote:
> This CL appears to have broken the freebsd-amd64 builder.
> See http://build.golang.org/log/ae502c5cfde2c7b734feb986ccb344ff69e059ba
>
>
> https://codereview.appspot.com/114420043/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[minux, 2014-08-06 05:48:12]

On Wed, Aug 6, 2014 at 1:18 AM, Dave Cheney <dave@cheney.net> wrote:

> This is a real failure,
>
> ok   _/tmp/gobuilder/freebsd-amd64-b613f2acdf69/go/misc/cgo/test 1.538s
> # testmain
> _/tmp/gobuilder/freebsd-amd64-b613f2acdf69/go/misc/cgo/test(.text):
> compilerrt_abort_impl: not defined
> FAIL _/tmp/gobuilder/freebsd-amd64-b613f2acdf69/go/misc/cgo/test [build
> failed]
> Build complete, duration 5m17.708335978s. Result: error: exit status 1

This must be latent bug. Because previous to this CL, that conditional is
always
false so that diagnosis will never be emitted.

compilerrt_abort_impl is supposed to be hidden symbol, probably that's
reason.

I don't have access to FreeBSD machines at the moment, and I can't reproduce
it on linux with clang (presumably because i'm not using compiler-rt) so if
nobody
is interested in figuring why the code breaks, i will back off this CL.
(but the bug is still there.)

[dave_cheney.net, 2014-08-06 05:54:38]

I can't reproduce the issue with linux/amd64 + clang either, but I
agree with your diagnosis.

I agree that you should roll back this change for the moment, at least
so the build isn't broken overnight.

On Wed, Aug 6, 2014 at 3:47 PM, minux <minux@golang.org> wrote:
>
> On Wed, Aug 6, 2014 at 1:18 AM, Dave Cheney <dave@cheney.net> wrote:
>>
>> This is a real failure,
>>
>> ok   _/tmp/gobuilder/freebsd-amd64-b613f2acdf69/go/misc/cgo/test 1.538s
>> # testmain
>> _/tmp/gobuilder/freebsd-amd64-b613f2acdf69/go/misc/cgo/test(.text):
>> compilerrt_abort_impl: not defined
>> FAIL _/tmp/gobuilder/freebsd-amd64-b613f2acdf69/go/misc/cgo/test [build
>> failed]
>> Build complete, duration 5m17.708335978s. Result: error: exit status 1
>
> This must be latent bug. Because previous to this CL, that conditional is
> always
> false so that diagnosis will never be emitted.
>
> compilerrt_abort_impl is supposed to be hidden symbol, probably that's
> reason.
>
> I don't have access to FreeBSD machines at the moment, and I can't reproduce
> it on linux with clang (presumably because i'm not using compiler-rt) so if
> nobody
> is interested in figuring why the code breaks, i will back off this CL.
> (but the bug is still there.)
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[minux, 2014-08-06 06:09:05]

On Wed, Aug 6, 2014 at 1:54 AM, Dave Cheney <dave@cheney.net> wrote:

> I can't reproduce the issue with linux/amd64 + clang either, but I
> agree with your diagnosis.
>
> I agree that you should roll back this change for the moment, at least
> so the build isn't broken overnight.

the change is rolled back (CL 120630043).

[dave_cheney.net, 2014-08-06 06:14:09]

I've been able to reproduce this issue on a freebsd 9.1 machine.

On Wed, Aug 6, 2014 at 4:08 PM, minux <minux@golang.org> wrote:
>
> On Wed, Aug 6, 2014 at 1:54 AM, Dave Cheney <dave@cheney.net> wrote:
>>
>> I can't reproduce the issue with linux/amd64 + clang either, but I
>> agree with your diagnosis.
>>
>> I agree that you should roll back this change for the moment, at least
>> so the build isn't broken overnight.
>
> the change is rolled back (CL 120630043).

[dave_cheney.net, 2014-08-16 04:04:30]

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

cmd/ld: fix operator precedence

LGTM=rsc
R=gobot, dave
CC=golang-codereviews, iant, rsc
https://codereview.appspot.com/114420043

Committer: Dave Cheney <dave@cheney.net>

[dave_cheney.net, 2014-08-16 04:08:07]

On 2014/08/16 04:04:30, dfc wrote:
> *** Submitted as https://code.google.com/p/go/source/detail?r=e90c6c19ab19 ***
> 
> cmd/ld: fix operator precedence
> 
> LGTM=rsc
> R=gobot, dave
> CC=golang-codereviews, iant, rsc
> https://codereview.appspot.com/114420043
> 
> Committer: Dave Cheney <mailto:dave@cheney.net>

I'm sorry, I accidentally resubmitted this CL.

[gobot, 2014-08-16 04:10:01]

This CL appears to have broken the freebsd-amd64 builder.
See http://build.golang.org/log/3339cb66678ddb201dc235de9dca1c77e66dddd8

[dave_cheney.net, 2014-08-16 04:12:02]

This CL was accidentally submitted twice, the second time as e90c6c19ab19, I
submitted 128310043 as a followup which corrects the build failure on freebsd
machines.