Use bundled gold by default when building for Android.

Use bundled gold by default when building for Android.

The bundled gold contains a number of bug fixes (mostly LTO-related, but also
http://crbug.com/161942), so use it instead of the NDK gold. Both versions
of gold are based on the same version of binutils (2.24), so this should be
relatively safe.

BUG=161942, 469376
R=thakis@chromium.org
TEST=Built libchromeshell.so with default GYP_DEFINES with and without this change, verified that binaries are equivalent

Committed: https://crrev.com/f5bae669c981b6dabd2df4ffdaf40e0c45b4c6bb
Cr-Commit-Position: refs/heads/master@{#325334}

[Nico, 2015-04-15 16:23:32]

lgtm

Do you want to do a similar change in the gn files?

cc'ing a few folks who might be interested

[pasko, 2015-04-15 17:28:41]

pasko@chromium.org changed reviewers:
+ pasko@chromium.org

[pasko, 2015-04-15 17:28:42]

thanks for context, -fsanitize=cfi-vptr is cute.

lgtm

https://codereview.chromium.org/1084133002/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1084133002/diff/1/build/common.gypi#newcode863
build/common.gypi:863: ['(OS=="linux" or OS=="android") and (target_arch=="x64"
or target_arch=="arm")', {
since it is now also used for android, shall we rename it to just
'use_bundled_gold'?

[Fabrice (no longer in Chrome), 2015-04-15 17:37:35]

fdegans@chromium.org changed reviewers:
+ fdegans@chromium.org

[Fabrice (no longer in Chrome), 2015-04-15 17:37:35]

lgtm, thanks for the heads-up!
There seems to be some references in v8 too but I think that's for a stand-alone
v8 build.

[pcc1, 2015-04-15 21:18:50]

On 2015/04/15 16:23:32, Nico (away until Wed Apr 15) wrote:
> lgtm
> 
> Do you want to do a similar change in the gn files?

I might look into that later. I think I still need to figure out how to target
Android with gn. (I tried adding

current_os = "android"

to gn args, but I received missing file errors from Ninja when I try to build.
It looks like at least some of those files are in a different location, so the
gn build may just be out of date.)

[pcc1, 2015-04-15 21:19:01]

https://codereview.chromium.org/1084133002/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1084133002/diff/1/build/common.gypi#newcode863
build/common.gypi:863: ['(OS=="linux" or OS=="android") and (target_arch=="x64"
or target_arch=="arm")', {
On 2015/04/15 17:28:42, pasko wrote:
> since it is now also used for android, shall we rename it to just
> 'use_bundled_gold'?

I'll see if I can rename it in a follow-up change in case the rename on its own
breaks anything.

[pcc1, 2015-04-15 21:19:24]

The CQ bit was checked by pcc@chromium.org

[commit-bot: I haz the power, 2015-04-15 21:20:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1084133002/1

[Nico, 2015-04-15 21:31:03]

On Wed, Apr 15, 2015 at 2:18 PM, <pcc@chromium.org> wrote:

> On 2015/04/15 16:23:32, Nico (away until Wed Apr 15) wrote:
>
>> lgtm
>>
>
>  Do you want to do a similar change in the gn files?
>>
>
> I might look into that later. I think I still need to figure out how to
> target
> Android with gn. (I tried adding
>
> current_os = "android"
>
> to gn args, but I received missing file errors from Ninja when I try to
> build.
> It looks like at least some of those files are in a different location, so
> the
> gn build may just be out of date.)
>

The gn android build should work, we have bots taking care of this:
http://build.chromium.org/p/chromium.linux/builders/Android%20GN (from
build.chromium.org). From the bot output, you can learn how to run gn:

http://build.chromium.org/p/chromium.linux/builders/Android%20GN/builds/22187...

'--args=symbol_level=1 is_debug=false target_os="android" target_cpu="arm"
use_goma=true goma_dir="/b/build/goma"'

(you probably don't need all of these)


>
> https://codereview.chromium.org/1084133002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2015-04-15 21:46:40]

Ah, I got chrome_shell_apk to start building using some variation of that.
(Looks like it's only the test targets that are missing files, and the bot
only appears to cover chrome_shell_apk. I'll try to fix the missing file
problem.)

Peter

On Wed, Apr 15, 2015 at 2:31 PM, Nico Weber <thakis@chromium.org> wrote:

> On Wed, Apr 15, 2015 at 2:18 PM, <pcc@chromium.org> wrote:
>
>> On 2015/04/15 16:23:32, Nico (away until Wed Apr 15) wrote:
>>
>>> lgtm
>>>
>>
>>  Do you want to do a similar change in the gn files?
>>>
>>
>> I might look into that later. I think I still need to figure out how to
>> target
>> Android with gn. (I tried adding
>>
>> current_os = "android"
>>
>> to gn args, but I received missing file errors from Ninja when I try to
>> build.
>> It looks like at least some of those files are in a different location,
>> so the
>> gn build may just be out of date.)
>>
>
> The gn android build should work, we have bots taking care of this:
> http://build.chromium.org/p/chromium.linux/builders/Android%20GN (from
> build.chromium.org). From the bot output, you can learn how to run gn:
>
>
>
http://build.chromium.org/p/chromium.linux/builders/Android%20GN/builds/22187...
>
> '--args=symbol_level=1 is_debug=false target_os="android" target_cpu="arm"
> use_goma=true goma_dir="/b/build/goma"'
>
> (you probably don't need all of these)
>
>
>>
>> https://codereview.chromium.org/1084133002/
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2015-04-15 23:10:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-04-15 23:10:57]

Patchset 1 (id:??) landed as
https://crrev.com/f5bae669c981b6dabd2df4ffdaf40e0c45b4c6bb
Cr-Commit-Position: refs/heads/master@{#325334}

[Fabrice (no longer in Chrome), 2015-04-16 11:51:40]

It looks like the bundled gold actually introduced issues with LTO:
Assembler messages:
Fatal error: invalid -march= option: `armv7-a'

Are you sure the bundled gold includes everything needed?

[pcc, 2015-04-16 18:51:33]

On 2015/04/16 11:51:40, Fabrice wrote:
> It looks like the bundled gold actually introduced issues with LTO:
> Assembler messages:
> Fatal error: invalid -march= option: `armv7-a'
> 
> Are you sure the bundled gold includes everything needed?

I've investigated this. The GCC LTO plugin finds "as" by searching the -B path,
which is also how GCC finds gold. It appears that binutils "as", unlike gold,
can only support one target architecture at once. The bundled "as" only supports
x86, so we receive this message at LTO time when targeting ARM.

I can see two possible solutions:

1) Don't use the bundled gold by default when using GCC LTO.
2) Don't ship "as" in our bundled binutils. This will cause GCC to continue
along its search path and pick up the NDK "as".

1 is probably the safest option for now, as there may be other dependencies on
the bundled "as". I'll prepare a CL to do that.

[pcc, 2015-04-16 19:15:31]

On 2015/04/16 18:51:33, pcc wrote:
> On 2015/04/16 11:51:40, Fabrice wrote:
> > It looks like the bundled gold actually introduced issues with LTO:
> > Assembler messages:
> > Fatal error: invalid -march= option: `armv7-a'
> > 
> > Are you sure the bundled gold includes everything needed?
> 
> I've investigated this. The GCC LTO plugin finds "as" by searching the -B
path,
> which is also how GCC finds gold. It appears that binutils "as", unlike gold,
> can only support one target architecture at once. The bundled "as" only
supports
> x86, so we receive this message at LTO time when targeting ARM.
> 
> I can see two possible solutions:
> 
> 1) Don't use the bundled gold by default when using GCC LTO.
> 2) Don't ship "as" in our bundled binutils. This will cause GCC to continue
> along its search path and pick up the NDK "as".
> 
> 1 is probably the safest option for now, as there may be other dependencies on
> the bundled "as". I'll prepare a CL to do that.

https://codereview.chromium.org/1096553002/