GN: unknown warning option '-Wno-maybe-uninitialized' |
||||||||||||||||
Issue descriptionWe are seeing warnings such as the following when building v8: [14184/23376] CXX v8_snapshot/obj/v8/v8_base/mark-compact.o warning: unknown warning option '-Wno-maybe-uninitialized'; did you mean '-Wno-uninitialized'? [-Wunknown-warning-option] 1 warning generated. These are not fatal but will generate noise in the build logs.
,
Jun 21 2016
I saw this recently. It's non fatal. I will check to see if it is still happening.
,
Jun 21 2016
This is not fixed.
,
Jun 21 2016
ok, but I dont see these on daisy. what board are you using?
,
Jun 21 2016
amd64-generic
,
Jun 21 2016
Also arm-generic and x86-generic. Note: I am using the prebuilts, i.e. not using the --chroot arg, but I did rerun 'gn gen out_$SDK_BOARD/Release --args="$GN_ARGS"'
,
Jun 21 2016
Hmm. 'daisy' doesn't build for me with tot at all, looks like an afdo issue. I will file a separate bug.
,
Jun 24 2016
looked at this problem. it comes from host compiles with clang. Most of them from host compiles into v8_snapshot. For ChromeOS simple chrome workflow, we are using GCC for target compiles and the chrome bundled clang for host compiles. I found some places in GN where this option is added for GCC compiles. https://cs.chromium.org/search/?q=Wno-maybe-uninitialized&sq=package:chromium&type=cs It seems that GN is getting confused because we use GCC for target and clang for host. On GYP it seems to me this option was only added for target for GCC on target compiles. Maybe that is the easiest fix on GN? Assigning to dpranke for a fix.
,
Jun 24 2016
I thought we were turning off clang for the host toolchain? It looks like the v8_snapshot toolchain isn't looking at cros_host_is_clang (or any is_clang), so, yeah, it would use clang. That's an easy enough fix. Is this showing up on a builder somewhere?
,
Jun 24 2016
Actually, there is already a flag for this. Are we setting cros_v8_snapshot_is_clang=false in the $GN_ARGS ?
,
Jun 24 2016
Yes, now that we are using GN in the SimpleChromeWorkflow stage of the PFQ builders: https://uberchromegw.corp.google.com/i/chromeos/builders/amd64-generic-chromium-pfq/builds/8791/steps/SimpleChromeWorkflow/logs/stdio
,
Jun 24 2016
@stevenjb - unfortunately, that doesn't appear to actually log the $GN_ARGS ?
,
Jun 24 2016
never mind, I just can't read. Yup, add cros_v8_snapshot_is_clang=false. Also, it looks like some of the cros_ args are getting set twice?
,
Jun 24 2016
sigh. still can't read; ignore the part about args getting set twice. Okay, I thought we didn't want to use clang (or the chromium toolchains) anywhere, i.e., the whole point was to use gcc across the board so that you only had to worry about one toolchain? Should these things be switched to gcc? I agree it's possibly we're incorrectly adding -Wno-maybe-uninitialized; I'll look at that. Feel free to re-assign to me once you clarify if clang is intentional or not.
,
Jun 24 2016
yes, clang for host compiles in intentional. Unfortunately the chromeos host compiler only works within the chroot (some library issues). So, we have to use the clang from chromium and we cannot use clang from the host. For target compiles we use the compiler from ChromeOS. I am not sure why this option is even added. v8_snapshot is not passing -Wall, and my understanding was that -Wall did not imply -Wmaybe-unitialized. Anyway, there must be a reason for this. As I said, maybe the simplest fix will be to just only add this options for target compilations?
,
Jun 24 2016
Okay, I'll work on it, then.
,
Jul 1 2016
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.
,
Jul 17 2016
,
Jul 18 2016
Okay, I finally got a chance to look at this. We should be setting -Wall when building the v8 snapshot, because it's chromium code (and, so, -Wall is on by default). -Wno-maybe-uninitialized gets set when we think this is a gcc build (i.e., is_clang=false) and when the v8_snapshot flags aren't overridding that. So, as I said in comment #13, we just need to add `cros_v8_snapshot_is_clang=false` to the list of args that are getting set in the environment. @llozano - can you do this? I looked at https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild#321 which seems to be where you would add that flag, but I don't understand how that keys off of host vs. target?
,
Jul 19 2016
yes, I will work on this. Remember that these are not fatal so I dont think this needs to be a P1.
,
Jul 21 2016
whoops, you should set cros_v8_snapshot_is_clang=true when using clang on the host toolchains, of course, since the v8_snapshot runs on the host.
,
Jul 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/6da947426f1479bd148fcec6543eb5baac1ea401 commit 6da947426f1479bd148fcec6543eb5baac1ea401 Author: Luis Lozano <llozano@chromium.org> Date: Fri Jul 22 00:43:41 2016 GN: Fix spurious compiler warning in Simple Chrome. BUG= chromium:618346 TEST=verify by hand we don't get the warning anymore. Change-Id: Iecf5f68d611b293c8366c5159b886cfa414a66ac Reviewed-on: https://chromium-review.googlesource.com/362500 Commit-Ready: Luis Lozano <llozano@chromium.org> Tested-by: Luis Lozano <llozano@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> [modify] https://crrev.com/6da947426f1479bd148fcec6543eb5baac1ea401/cli/cros/cros_chrome_sdk.py
,
Jul 24 2016
I think this is fixed now.
,
Aug 29 2016
,
Oct 7 2016
,
Oct 10 2016
,
Nov 19 2016
,
Jan 21 2017
,
Mar 4 2017
,
Mar 17 2017
|
||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||
Comment 1 by dpranke@chromium.org
, Jun 21 2016