New issue
Advanced search Search tips

Issue 894237 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug
Build-Toolchain



Sign in to add a comment

Chrome OS amd64-generic-ubsan builder is failing to build chromeos-kernel-4_14

Project Member Reported by manojgupta@chromium.org, Oct 10

Issue description

https://cros-goldeneye.corp.google.com/chromeos/legoland/builderHistory?buildConfig=amd64-generic-ubsan&buildBranch=master

https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?buildbucketId=8933065378933543424

Root cause is compilation fail when building kernel 4.14:
chromeos-kernel-4_14-4.14.74-r533: drivers/gpu/drm/i915/intel_engine_cs.o: In function `intel_engine_init_execlist':
chromeos-kernel-4_14-4.14.74-r533: /build/amd64-generic/var/cache/portage/sys-kernel/chromeos-kernel-4_14/../../../../../tmp/portage/sys-kernel/chromeos-kernel-4_14-4.14.74-r533/work/chromeos-kernel-4_14-4.14.74/drivers/gpu/drm/i915/intel_engine_cs.c:411: undefined reference to `__compiletime_assert_411'

Suspect CL:
https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1262160

Repro steps:

$ ./setup_board --board=amd64-generic --profile=ubsan
$ emerge-amd64-generic chromeos-kernel-4_14
 
Well removing the BUILD_BUG_ON_POWER_OF_2 line where this is happening fixes it. And right above that line we assign the variable that we're testing to see if it's a power of two when you add 1 to it. I guess the compiler really just can't optimize any of this stuff when UBSAN is enabled and so then the BUILD_BUG_ON fails. Best plan is to send a patch to the mailing list to see if they're happy with just removing this and adding a comment indicating it should be a power of two, or moving this to a runtime check.
I sent a patch to the list

http://lkml.kernel.org/r/20181015203410.155997-1-swboyd@chromium.org

and it was rejected in favor of another patch to make it a runtime check:

https://marc.info/?l=freedesktop-intel-gfx&m=153969300911406

so we can pull this one in when it lands in a git tree
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 19

Labels: merge-merged-chromeos-4.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e6257ccf82e571f6fb995cbcb19abc6eee7423de

commit e6257ccf82e571f6fb995cbcb19abc6eee7423de
Author: Jani Nikula <jani.nikula@intel.com>
Date: Fri Oct 19 02:01:20 2018

FROMGIT: drm/i915: Ensure intel_engine_init_execlist() builds with Clang

Clang build with UBSAN enabled leads to the following build error:

drivers/gpu/drm/i915/intel_engine_cs.o: In function `intel_engine_init_execlist':
drivers/gpu/drm/i915/intel_engine_cs.c:411: undefined reference to `__compiletime_assert_411'

Again, for this to work the code would first need to be inlined and then
constant folded, which doesn't work for Clang because semantic analysis
happens before optimization/inlining.

Use GEM_BUG_ON() instead of BUILD_BUG_ON().

v2: Use is_power_of_2() from log2.h (Chris)

References: http://mid.mail-archive.com/20181015203410.155997-1-swboyd@chromium.org
Reported-by: Stephen Boyd <swboyd@chromium.org>
Cc: Stephen Boyd <swboyd@chromium.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181016122938.18757-2-jani.nikula@intel.com
(cherry picked from commit 410ed5731a6566498a3aa904420aa2e49ba0ba90
 git://anongit.freedesktop.org/git/drm-intel drm-intel-next-queued)

BUG= chromium:894237 
TEST=Build amd64-generic with ubsan use flag

Change-Id: I0a1973f817cb5e5e0fe55a1d1231cb40a34054ad
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1286074
Commit-Ready: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/e6257ccf82e571f6fb995cbcb19abc6eee7423de/drivers/gpu/drm/i915/intel_engine_cs.c

Status: Fixed (was: Assigned)
Fixed then?
Components: Tools>ChromeOS-Toolchain
Thanks, the build issue is fixed but now started failing in build_image since kernel image is too big now. I am not sure if the size issue is easy to resolve or we should rename/mask ubsan USE flag in kernel to resolve this issue.

Should I open a new bug or will this bug suffice? 

https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?buildbucketId=8932238679429187392

INFO    : Kernel partition image emitted: /mnt/host/source/src/build/images/amd64-generic/R72-11173.0.2018_10_19_0619-a1/hd_vmlinuz.image
INFO    : Root filesystem hash emitted: /mnt/host/source/src/build/images/amd64-generic/R72-11173.0.2018_10_19_0619-a1/rootfs.hash
INFO    : Kernel image A   size is 26217984 bytes.
INFO    : Kernel image B   size is 26217984 bytes.
INFO    : Kernel partition A size is 16777216 bytes.
ERROR   : Fri Oct 19 06:24:10 PDT 2018
ERROR   :  PGID  PPID   PID     ELAPSED     TIME %CPU COMMAND
ERROR   : Arguments of 9982:  '/mnt/host/source/src/build/images/amd64-generic/R72-11173.0.2018_10_19_0619-a1' 'chromiumos_base_image.bin' '--adjust_part='
ERROR   : Backtrace:  (most recent call is last)
ERROR   :  cros_make_image_bootable:443:main(), called: make_image_bootable '/mnt/host/source/src/build/images/amd64-generic/R72-11173.0.2018_10_19_0619-a1/chromiumos_base_image.bin' 
ERROR   :  cros_make_image_bootable:318:make_image_bootable(), called: check_kernel_size '26217984' '2' 'A' 
ERROR   :  cros_make_image_bootable:177:check_kernel_size(), called: die 'Kernel image won't fit in partition A!' 

let's disable the flag for kernels. if someone wants to get it working, they can figure it out without breaking the builder 

along those lines, there's no reason I can think of for generic boards to not have big kernel partitions. assuming it's 16mb now, bump it up to 64mb.
Status: Assigned (was: Fixed)
Seems OK to set it to assigned for now.  Sounds like the quick-and-dirty fix is suggested which is to rename the kernel use flag to "kernel_ubsan" so you can turn on the rest of the ubsan on without the kernel's until the kernel's one is working.

---

You might be able to get things smaller by using a similar technique to 1274168.  Ideally someone would separate out the "xz" fragment and then it could be used by ubsan and debug.

---

I have no objections to the generic boards having a bigger kernel partition assuming it doesn't break any testers / builders.
 Issue 897811  has been merged into this issue.
Cc: malaykeshav@chromium.org amoylan@chromium.org
Ok I'm going to throw in a change to make the flag kernel_ubsan instead of ubsan per vapier's comment and then look into fixing the size issue next week. How do I make the kernel partition 64mb? I tried modifying the disk layout file but it didn't work and failed with some weird exception I didn't understand. I'll have to go read that code to figure out what to do.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/ce090134a47fd4be12f9a2fe2c11428ee294425c

commit ce090134a47fd4be12f9a2fe2c11428ee294425c
Author: Stephen Boyd <swboyd@chromium.org>
Date: Wed Oct 24 02:22:01 2018

cros-kernel2: Rename 'usban' to 'kernel_ubsan'

The ubsan builder can't fit the kernel image into the 16MB partition
that we have by default. Until that is fixed, let's rename this flag to
kernel_ubsan so the ubsan builder is happy. In the meantime, we can
increase the size and put this back to ubsan when it's known to be
working.

BUG= chromium:894237 
TEST=Build amd64-ubsan image and see kernel fits into partition

Change-Id: I51541e5aa6fefeece5a5e7744775cc5a81b09e9e
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1296034
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/ce090134a47fd4be12f9a2fe2c11428ee294425c/eclass/cros-kernel2.eclass

Status: Verified (was: Assigned)
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/440fb92e713c08da3f14832f0ea84c5042e7984d

commit 440fb92e713c08da3f14832f0ea84c5042e7984d
Author: Stephen Boyd <swboyd@chromium.org>
Date: Wed Oct 31 15:17:37 2018

cros-kernel2: Add xz use flag for compression choice and use it

The kernel is too big sometimes and so we'd like to shrink the size down
with xz. Add a use flag for this purpose and reuse it in the 'debug' use
flag for simplification. This way debugging kernel images don't get too
big on x86/amd64 images and we can use this to shrink 'ubsan' built
images too.

BUG= chromium:894237 
TEST=Build kernel image and see if it fits into 16MB
Signed-off-by: Stephen Boyd <swboyd@chromium.org>

Change-Id: I996fa649e48a8529ab48366db9b3fb600b62152c
Reviewed-on: https://chromium-review.googlesource.com/1291650
Commit-Ready: Stephen Boyd <swboyd@chromium.org>
Tested-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Yu Zhao <yuzhao@chromium.org>

[modify] https://crrev.com/440fb92e713c08da3f14832f0ea84c5042e7984d/eclass/cros-kernel2.eclass

> along those lines, there's no reason I can think of for generic boards to not have big kernel partitions. assuming it's 16mb now, bump it up to 64mb.

I've tried this, and I've found that I need to make the EFI partition (partition 12) larger too. Otherwise, we can't fit the vmlinuz onto that partition. Should that be 2 times the size of the kernel partition?
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/5f6e53196296a2ca4cbfd64ab7080d1f5dc9ff2b

commit 5f6e53196296a2ca4cbfd64ab7080d1f5dc9ff2b
Author: Stephen Boyd <swboyd@chromium.org>
Date: Fri Nov 09 15:19:36 2018

amd64-generic: Expand kernel partition to 64 MB

We're running out of disk space for the kernel image when we enable all
the zillions of debug options that we have. For example, an
amd64-generic kernel with the 'debug' and 'ubsan' use flags enabled is
26 MB. It's suggested that we increase the size to 64 MB to be safe for
the foreseeable future. Let's do that, and also increase the EFI
partition to be twice the size of the kernel partition because we copy
the vmlinuz file there on this architecture.

BUG= chromium:894237 
TEST=Build image and see partitions 2, 4 and 12 are the size intended

Change-Id: I63b4afd23243cc1777fdb318e5f57ca86e71d9c6
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1318213
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/5f6e53196296a2ca4cbfd64ab7080d1f5dc9ff2b/overlay-amd64-generic/scripts/disk_layout.json

Project Member

Comment 16 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/13a3fd6ddd01a560e34c0f40e91d6d76e7e715ae

commit 13a3fd6ddd01a560e34c0f40e91d6d76e7e715ae
Author: Stephen Boyd <swboyd@chromium.org>
Date: Fri Nov 09 15:19:50 2018

ubsan: Compress kernel image as much as possible

Let's use the XZ compression method for the kernel when we're building
an ubsan image. Ubsan in the kernel makes the vmlinuz quite large so
compressing it with a slower compression method allows it to fit into
a smaller partition size while sacrificing some decompression time, but
this is all debug anyway so it seems to be a worthwhile tradeoff.

BUG= chromium:894237 
TEST=Build ubsan profile and see kernel compressed with xz

Change-Id: I3304f7f85488bba84864d6bd92bd31443c54a1c5
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1318403
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/13a3fd6ddd01a560e34c0f40e91d6d76e7e715ae/profiles/ubsan/make.defaults

Project Member

Comment 17 by bugdroid1@chromium.org, Nov 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/0e5b13b30b7bbf2b9716e9f50ef835ed2f099c35

commit 0e5b13b30b7bbf2b9716e9f50ef835ed2f099c35
Author: Stephen Boyd <swboyd@chromium.org>
Date: Thu Nov 15 10:18:26 2018

cros-kernel2: Rename 'kernel_ubsan' to 'ubsan'

Now that we've increased the kernel partition size we can rename this to
just 'ubsan'. Boards with small kernel partitions will still fail when
enabling ubsan, but that's just how it is going to be.

BUG= chromium:894237 
TEST=Build ubsan amd64 profile with ubsan use flag and see kernel fit
into partition

Change-Id: Ia0c630b687292c3bfda8c9096213514631604316
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1319387
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/0e5b13b30b7bbf2b9716e9f50ef835ed2f099c35/eclass/cros-kernel2.eclass

Sign in to add a comment