Reland "Separate check failure message parts more clearly." [chromium/src : main]

0 views

Skip to first unread message

Antonio Alphonse (Gerrit)

unread,

Dec 15, 2025, 2:58:16 PM (19 hours ago) Dec 15

to Chromium LUCI CQ, Daniel Cheng, Titouan Rigoudy, Code Review Nudger, chromium...@chromium.org

Attention needed from Daniel Cheng and Titouan Rigoudy

Antonio Alphonse has uploaded the change for review

Antonio Alphonse would like Titouan Rigoudy, Chromium LUCI CQ and Daniel Cheng to review this change.

Commit message

Reland "Separate check failure message parts more clearly."

This is a reland of commit 7f0eda60d4c991bc9f32f4d77b69537fc6d1bb54

Original change's description:
> Separate check failure message parts more clearly.
>
> This CL encompasses two changes:
>
> 1. Always end check failure messages with a period.
> 2. In fuzzing mode, replace the period with a semicolon.
>
> For example:
>
>   CHECK(false);
>
> Yields:
>
>   Check failed: false.
>
> Or, with additional data streamed in:
>
>   CHECK(false) << "foo";
>
> The output is:
>
>   Check failed: false. foo
>
> In fuzzing mode, however, in a bid to help machines understand where
> to draw the line between the compile-time static part ("false") and
> the runtime-variable part ("foo"), we use a semicolon instead. It
> seems quite difficult to manage to embed a semilicon in the
> compile-time static part. Thus the examples above become:
>
>   Check failed: false;
>   Check failed: false; foo
>
> Change-Id: Ia09851382f0d3207d0720d204cef26e223ade024
> Bug: 443588668
> Cq-Include-Trybots: luci.chromium.try:chromeos-x64-libfuzzer-asan-rel-tests,linux-x64-libfuzzer-asan-dbg-tests,linux-x64-libfuzzer-asan-rel-tests,linux-x64-libfuzzer-msan-rel-tests,linux-x64-libfuzzer-ubsan-rel-tests,linux-x86-libfuzzer-asan-rel-tests,mac-arm64-libfuzzer-asan-rel-tests,win-x64-libfuzzer-asan-rel-tests,linux-x64-centipede-asan-rel-tests
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6917708
> Reviewed-by: Antonio Alphonse <alph...@google.com>
> Commit-Queue: Antonio Alphonse <alph...@google.com>
> Auto-Submit: Titouan Rigoudy <tit...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1558298}

Bug: 443588668

Cq-Include-Trybots: luci.chromium.try:chromeos-x64-libfuzzer-asan-rel-tests,linux-x64-libfuzzer-asan-dbg-tests,linux-x64-libfuzzer-asan-rel-tests,linux-x64-libfuzzer-msan-rel-tests,linux-x64-libfuzzer-ubsan-rel-tests,linux-x86-libfuzzer-asan-rel-tests,mac-arm64-libfuzzer-asan-rel-tests,win-x64-libfuzzer-asan-rel-tests,linux-x64-centipede-asan-rel-tests

Change-Id: I7368a83dd15184cb844dea08a1f50e2cb1110229

Change diff

Change information

Files:

M base/check.cc
M base/check_unittest.cc

Change size: M

Delta: 2 files changed, 73 insertions(+), 39 deletions(-)

Open in Gerrit

Related details

Attention is currently required from:

Daniel Cheng
Titouan Rigoudy

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Antonio Alphonse (Gerrit)

unread,

1:00 AM (9 hours ago) 1:00 AM

to Titouan Rigoudy, Chromium LUCI CQ, Daniel Cheng, Code Review Nudger, chromium...@chromium.org

Attention needed from Titouan Rigoudy

Antonio Alphonse added 2 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Antonio Alphonse . resolved

@titouan! I gave the revert a look the other day to see what might have went wrong. Is this something that we might have missed out on?

File base/check_unittest.cc

Line 731, Patchset 1 (Latest): EXPECT_DUMP_WILL_BE_CHECK("Check failed: a == b (1 vs. 2)",

Antonio Alphonse . unresolved

I'm curious if this is the culprit of the crash in https://luci-milo.appspot.com/ui/inv/build-8695649193655259889/test-results?q=CheckDeathTest.CheckOp . It seems that the message separator isn't appended in these situations, and that we're missing this in those checks. We might want to update these to include the period / separator?