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

Issue 833482 link

Starred by 11 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature


Show other hotlists

Hotlists containing this issue:
chrome-client-infra-backlog


Sign in to add a comment

Setup trybot with DCHECKs disabled

Project Member Reported by tschumann@google.com, Apr 16 2018

Issue description

Currently, all trybots run code with DCHECKs enabled.
This is great as DCHECKs ensure important constrains and assumptions in the code. However, the downside with DCHECKs is, that if they have side-effects, those are not visible when they get disabled.
This happens at times causing breaks of the waterfall (sheriff needs to revert) and creates a hard-to-debug problem for the developer: they have no easy way to reproduce them (unless they know it's a DCHECK related problem).
Having a trybot with DCHECKs enabled will help developers to reproduce the problem early and keep the build green.

My preference would be a trybot targeting the linux platform as that's usually also easiest to reproduce for developers (so they won't start assuming it's just a problem of a specific platform they might not have access to).

Recent examples:
https://bugs.chromium.org/p/chromium/issues/detail?id=832019
https://bugs.chromium.org/p/chromium/issues/detail?id=633613#c11
 
Labels: -Restrict-View-Google
Cc: jbudorick@chromium.org
Components: Infra>Client>Chrome
This is probably lower priority than most of the fires right now, but we should keep it in mind if it did happen to be the cause of a significant number of CI failures.
Status: Available (was: Untriaged)
Added to chrome-client-infra-backlog for tracking.
Hit this just now. CL was reverted due to failing only on Debug builds (important call being made inside a DCHECK)

https://chromium.googlesource.com/chromium/src.git/+/5b4d9b945ac4e215b45df6c82e85d0586578ceae
Owner: chcunningham@chromium.org
Status: Assigned (was: Available)
=> chcunningham@ (chat w/him indicates the underlying fix may already be in place to unblock relanding this trybot).
Owner: ----
I did fix my CL, but I'm not the right owner for the larger infrastructure task that this bug is tracking.
Status: Available (was: Assigned)
Cc: dpranke@chromium.org chiniforooshan@chromium.org
 Issue 740051  has been merged into this issue.
Owner: gbeaty@chromium.org
Status: Started (was: Available)
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 26 2018

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

commit 8d975d16c7bfeb92685574eb6d3de1b073ad909b
Author: Garrett Beaty <gbeaty@chromium.org>
Date: Tue Jun 26 18:19:39 2018

Add an mb builder configuration for no DCHECKs.

DCHECKs are defined as do-nothing operations when the product is
released, but all of the existing trybots enable DCHECKs so that
violations of expectations can be surfaced early. However, if the
expression used as a condition has important side effects, then
disabling DCHECKs changes the meaning of the code and this won't be
discovered until the product is actually released.

Adding a config with DCHECKs disabled will enable creating a trybot to
surface errors around side effects in DCHECK conditions earlier.

Bug: 833482
Change-Id: I687d81735b070fa4ec480072b9372f4ae0f7aa9a
Reviewed-on: https://chromium-review.googlesource.com/1112653
Commit-Queue: Garrett Beaty <gbeaty@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570469}
[modify] https://crrev.com/8d975d16c7bfeb92685574eb6d3de1b073ad909b/tools/mb/mb_config.pyl

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/3095c09874fdeedf8027f742a8cb934196304b5d

commit 3095c09874fdeedf8027f742a8cb934196304b5d
Author: Garrett Beaty <gbeaty@chromium.org>
Date: Tue Jun 26 22:00:10 2018

Add a trybot for Linux with DCHECKs disabled.

DCHECKs are defined as do-nothing operations when the product is
released, but all of the existing trybots enable DCHECKs so that
violations of expectations can be surfaced early. However, if the
expression used as a condition has important side effects, then
disabling DCHECKs changes the meaning of the code and this won't be
discovered until the product is actually released.

Bug: 833482
Change-Id: Ia71fa8e190caeb362ec2c8513ccdad683a2dcc17
Reviewed-on: https://chromium-review.googlesource.com/1114228
Commit-Queue: Garrett Beaty <gbeaty@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>

[modify] https://crrev.com/3095c09874fdeedf8027f742a8cb934196304b5d/tests/masters_recipes_test.py
[modify] https://crrev.com/3095c09874fdeedf8027f742a8cb934196304b5d/scripts/slave/recipe_modules/chromium_tests/trybots.py

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 2

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/config/+/ce188d921228980358ef37c9a1c06c7553e76fcf

commit ce188d921228980358ef37c9a1c06c7553e76fcf
Author: Garrett Beaty <gbeaty@google.com>
Date: Mon Jul 02 20:50:46 2018

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 9

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

commit 713cfb34f070b4297f01f01b3da188d4954e5a3e
Author: Garrett Beaty <gbeaty@chromium.org>
Date: Mon Jul 09 16:34:08 2018

Add linux-dcheck-off-rel builder to buildbucket configuration.

Bug: 833482
Change-Id: I359fd5b64701fd9e960ad380231206e8b268efeb
Reviewed-on: https://chromium-review.googlesource.com/1116245
Commit-Queue: Garrett Beaty <gbeaty@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Ryan Tseng <hinoka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573331}
[modify] https://crrev.com/713cfb34f070b4297f01f01b3da188d4954e5a3e/infra/config/global/cr-buildbucket.cfg

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 20

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/config/+/a400447ad30e88bfdcfc17f04527ef4e4fb11aaf

commit a400447ad30e88bfdcfc17f04527ef4e4fb11aaf
Author: Garrett Beaty <gbeaty@google.com>
Date: Fri Jul 20 19:36:20 2018

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 20

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

commit 2f3569a4f414b420349e6251721c84bdea8fac4e
Author: Garrett Beaty <gbeaty@chromium.org>
Date: Fri Jul 20 21:43:03 2018

Add linux-dcheck-off-rel as an experimental tryjob.

Bug: 833482
Change-Id: Ied9bce9cef4cf2f2a842df578040125b77793a43
Reviewed-on: https://chromium-review.googlesource.com/1145677
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Garrett Beaty <gbeaty@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576997}
[modify] https://crrev.com/2f3569a4f414b420349e6251721c84bdea8fac4e/infra/config/branch/cq.cfg

Can we move this into the CQ now? Another CL got hit. https://chromium-review.googlesource.com/c/chromium/src/+/1298641
Do you have any idea how often we see failures like this? If it's only once every couple months, it's not worth the cost to put this into the CQ.

Sign in to add a comment