New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 621581 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 621972
issue 625353

Blocking:
issue 542853



Sign in to add a comment

Flip LKGR builders to GN

Project Member Reported by infe...@chromium.org, Jun 20 2016

Issue description

1. UBSAN vptr is broken.
https://build.chromium.org/p/chromium.lkgr/builders/UBSan%20vptr%20Release/builds/3881/steps/compile/logs/stdio
FAILED: obj/third_party/mesa/mesa/scissor.o 
clang: error: argument unused during compilation: '-fsanitize-coverage=edge'

I think we need to exclude sanitizer flags in gn, just like in gyp - https://cs.chromium.org/chromium/src/third_party/mesa/mesa.gyp?rcl=0&l=81

2. ChromeOS bots are broken
https://build.chromium.org/p/chromium.lkgr/builders/ChromiumOS%20ASAN%20Release/builds/2949/steps/generate_build_files/logs/stdio
/mnt/data/b/build/slave/ChromiumOS_ASAN_Release/build/src/buildtools/linux64/gn gen //out/Release --check
  -> returned 1
ERROR at //tools/ipc_fuzzer/fuzzer/BUILD.gn:26:5: Undefined variable for -=.
    config -= [ "//build/config/sanitizers:default_sanitizer_flags" ]
    ^-----
I don't have something with this name in scope now.
See //tools/ipc_fuzzer/BUILD.gn:27:7: which caused the file to be included.
      "//tools/ipc_fuzzer/fuzzer:ipc_fuzzer",
      ^-------------------------------------

3. ASAN v8 arm bots broken
https://build.chromium.org/p/chromium.lkgr/builders/ASan%20Release%20Media%20%2832-bit%20x86%20with%20V8-ARM%29/builds/1720/steps/compile/logs/stdio
In file included from ../../v8/src/globals.h:13:
../../v8/src/base/build_config.h:156:2: error: Target architecture arm is only supported on arm and ia32 host
#error Target architecture arm is only supported on arm and ia32 host

 
Cc: machenb...@chromium.org
1. The sanitize-coverage=edge failure was supposed to have been fixed in  bug 618534 , but I think we haven't rolled clang yet.

2. ChromeOS wasn't supposed to have been flipped.

3. Looks like v8_target_cpu is being used even in the host toolchain, which is not so good :(.

I'll revert this CL for now.
Blocking: 432959
Cc: jochen@chromium.org
Re 3) It looks like when it compiles the v8 "shell" target with the host toolchain, it uses v8_target_arch arm with it (bad). The host shell shouldn't simulate arm at all. Only compile native x64.

Comment 4 by jochen@chromium.org, Jun 21 2016

Hum, no, that's how it's supposed to be. The v8_target_cpu is supposed to be set both on host and target. For host ia32 and target arm, we'd use the arm simulator on the host.
But the host is x64. So the v8 shell for host is x64, right? The target is ia32 and v8 simulates arm on that target.

Comment 6 by jochen@chromium.org, Jun 21 2016

If the target is arm, we force the host for v8 to ia32
Got it. Then this part has a bug in gn as the compile output generates:
clang_x64/obj/v8/v8_base/module-decoder.o 

Comment 8 by jochen@chromium.org, Jun 21 2016

I guess the right solution for the bot would be to set v8_snapshot_toolchain 
@jochen - you're using "host" in a way that is confusing me (and it's inconsistent with the way that GN uses it).

My understanding is that the snapshot needs to run 32-bit x86 code in order to generate a 32-bit snapshot (for either 32-bit arm or 32-bit x86 targets). While that code runs on the host machine, it is not using the host_toolchain, which has host_os=linux and host_cpu=x64.

My understanding of the "32-bit x86 w/ v8-arm" code is that you want to build code for a 32-bit x86 target, but you want the v8 code to generate 32-bit arm code that will then be run under the simulator. So, the "snapshot" also needs to be 32-bit. However, the actual host_toolchain remains 64-bit.

Given the above, should we be building the v8 shell with the v8_snapshot_toolchain, or the host_toolchain, or this new hybrid x86+arm toolchain concept ? A related question is, do we ever want v8_target_cpu=x64 (or x86) for any targets on this particular builder?
Components: Build
Labels: -Pri-1 Proj-GN-Migration Pri-2
downgrading to P2 since this won't block M53 ...
Labels: OS-All
Blocking: 542853
Owner: infe...@chromium.org
Okay, as far as  issue #3  goes, the issue is that "chromium_builder_asan" is currently configured to build v8_shell with the $host_toolchain (i.e., an x64 binary).

I'm not sure why it's configured to do that, except that the corresponding GYP target also specified "v8_shell#host". 

This, however, isn't going to work the way the current toolchains and args are set up, because it attempts to build a 64-bit version of v8 but with v8_target_cpu set to arm, which, unsurprisingly, isn't going to work.

inferno/marbella - do we actually need v8_shell to be built for CF? If so, which architecture should it be built against? target_cpu=x86/v8_target_cpu=arm? or x64/x64? If the latter, I'm curious why that is is needed on this particular builder?

If it isn't needed, the easiest fix is to simple remove the target (or build it in the default/target toolchain, which'll work fine).

reassigning to inferno@ for guidance. Please reassign back to me after.

(On a related note, even if I fix that I still hit other errors, linking Chrome against NaCl, so there's still more work to do here. Apparently there's some interaction between enable_ipc_fuzzer=true and enable_nacl=false that doesn't quite work right).

Comment 14 by aarya@google.com, Jun 23 2016

Cc: infe...@chromium.org
Owner: mbarbe...@chromium.org
Marty, can you check c#13 for v8 and comment.
Blockedon: 621972
marking as blocked on the clang roll --  bug 621972  -- for the sanitize=edge issue (alternatively we can fix mesa to not use sanitize=edge).
Owner: dpranke@chromium.org
Re v8 shell: The target is not needed by clusterfuzz or for testing. It is used by some chromium tools during the compilation process only. It needs to run on the host. We can either:

1) Do what Jochen says is the status quo in gyp: Force shell to compile with target=x86 and v8_target=arm for the x64 host. The x86 binary will run on the x64 host (it will run slower as it runs the arm simulator, but it doesn't really matter for the small tasks it's doing).

2) Compile it with target=x64, v8_target=x64. This would mean we need to compile (nearly) whole v8 twice (rather undesired I guess).
In the grand scheme of what we're compiling, compiling v8 twice is probably not a big deal. I would rather do that and have the build be clear about what it's trying to do.

I'll poke at something to see what I can work up.
Another blocker of choice 2) would be that mksnapshot also runs on the host and must be 32 bits with the arm simulator to generate the 32 bit snapshot. Since we have to build that anyways, there's no good reason to not have other targets, like v8 shell be built the same way.
Side note: We already have working Android arm bots in gn, right? When building for Android arm on an x64 host, we also need an x86 mksnapshot target running on the host for the snapshot generation. I assume that already works?
(v8 contains some of the slowest-to-build parts of the build, so building it twice wouldn't be so awesome.)
In comment #19, machenbach@ wrote:
> Side note: We already have working Android arm bots in gn, right?

Yes.

> When building for Android arm on an x64 host, we also need an x86 
> mksnapshot target running on the host for the snapshot generation. 
> I assume that already works?

Yes.

Blocking: -432959
As suggested by laforge@ and a conversation w/ the monorail folks, I'm going to try tracking GN-Migration related issues by *just* using the Proj-GN-Migration label, and not using blocking/rollup bugs, so that we can use blocking for just tasks that truly need to be completed before other tasks can make progress.
Blockedon: 625353
I'm going to split off the v8_target_cpu cleanup work into its own bug for clarity, and block this on that.
the v8_target_cpu cleanup is in  bug 625353 . I believe that clang has rolled, so once we get  bug 625353  fixed we should be able to also try again (at least on the non-CrOS builders).
Summary: Flip LKGR builders to GN (was: Some lkgr GN bots are broken)
Status: Started (was: Assigned)
Cc: brettw@chromium.org krasin@chromium.org dpranke@chromium.org aizatsky@chromium.org phajdan.jr@chromium.org
 Issue 542853  has been merged into this issue.
Project Member

Comment 29 by bugdroid1@chromium.org, Jul 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0447aa4b31ac99c1119058ef20761bc3d8cdbd3c

commit 0447aa4b31ac99c1119058ef20761bc3d8cdbd3c
Author: dpranke <dpranke@chromium.org>
Date: Fri Jul 22 17:26:50 2016

Flip all Linux LKGR/clusterfuzz builders to GN.

All of the known bugs have been addressed; let's see what isn't known.
This affects:

  chromium.lkgr:
    - ASAN Debug
    - ASAN Release Media
    - ASAN Release
    - ASAN Debug (32-bit x86 with V8-ARM)
    - ASAN Release (32-bit x86 with V8-ARM)
    - ASAN Release Media (32-bit x86 with V8-ARM)
    - TSAN Debug
    - TSAN Release
    - UBSan Release
    - UBSan vptr Release

With this CL, the only remaining linux builders that use GYP are
the 'Closure Compilation' builders. The LKGR waterfall will still
have a few Win GYP builders that will be flipped in a follow-up CL.

R=inferno@chromium.org, mbarbella@chromium.org
BUG= 621581 

Review-Url: https://codereview.chromium.org/2167963002
Cr-Commit-Position: refs/heads/master@{#407189}

[modify] https://crrev.com/0447aa4b31ac99c1119058ef20761bc3d8cdbd3c/tools/mb/mb_config.pyl

Okay, the change in #29 has cycled in, and everything looks green except for the "ASAN Debug (32-bit x86 with V8-ARM)" builder which mistakenly stayed on GYP and was broken by a GYP-specific change.

I also flipped the CrOS ASAN builder over but had a typo in the GN file for that, so that builder is also red.

I have fixes for both of those issues in the CQ.

There are still Mac and Win builders on GYP though.
(hopefully we'll get them taken care of next week).
Status: Fixed (was: Started)
The linux builders have all been flipped over. The CrOS ASAN builder appears to be broken (compile is hanging); I'll file a separate bug for that.

That leaves the Mac ASAN builders, which I've posted two CLs for, and the Win SyzyzgyASAN builders. We can track flipping those as part of  bug 618468  and  bug 605318 , respectively, so I don't think there's any real advantage to keeping this bug open.

Sign in to add a comment