Guidance for using assert

[Ziad Youssef]

Hi,

We are wondering if the guidance to use assert for checking invariants still applies. Now that CXX group decided to prefer CHECK over DCHECK (announcement, doc).

[Andrew Grieve]

Hmm, our current guidance is based on DCHECK, and anchor that it points to (#CHECK_DCHECK_and-NOTREACHED) no longer exists, so I think it does at least need to be updated.

Here's the text for reference:

The Chromium build system strips asserts in release builds (via ProGuard) and enables them in debug builds (or when dcheck_always_on=true) (via a build step). You should use asserts in the same scenarios where C++ DCHECK()s make sense. For multi-statement asserts, use org.chromium.build.BuildConfig.ENABLE_ASSERTS to guard your code (similar to #if DCHECK_IS_ON() in C++).

I think the reasons for changing the DCHECK guidance (to strongly prefer CHECK instead) were because of:

1) Ample evidence of DCHECKs that would have prevented security bugs had they been CHECKs.

2) CHECKs became faster (~ a single instructions + the conditional)

I don't think anyone's measured the performance overhead of java asserts, so they might have low enough overhead to enable. I think the security implication is much different in Java though, since it's a memory-safe language.

Here's a proposed new text:

The build system strips asserts in release builds (via R8), enables them in debug builds, and enables them in report-only mode for Canary builds. Use your judgement for when to use asserts vs exceptions. Generally speaking, use asserts to check program invariants (e.g. parameter constraints) and exceptions for unrecoverable error conditions (e.g. OS errors). You should tend to use exceptions more in privacy / security-sensitive code. For multi-statement asserts, use org.chromium.build.BuildConfig.ENABLE_ASSERTSto guard your code (similar to #if DCHECK_IS_ON() in C++).
Do not add checks when the code will crash anyways. E.g.:
```
assert(foo != null);
foo.method(); // This will throw anyways.
```
**DCHECKS vs Java Asserts**
`DCHECK` and `assert` are very similar, but our guidance for them differs: CHECKs are preferred in C++, whereas asserts are preferred in Java. This is because as a memory-safe language, logic bugs in Java are much less likely to be exploitable.

On Thu, Jul 20, 2023 at 9:45 AM 'Ziad Youssef' via java <ja...@chromium.org> wrote:

Hi,

We are wondering if the guidance to use assert for checking invariants still applies. Now that CXX group decided to prefer CHECK over DCHECK (announcement, doc).

--
You received this message because you are subscribed to the Google Groups "java" group.
To unsubscribe from this group and stop receiving emails from it, send an email to java+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/java/CALEAU-FW%3D0TOsMdmmfP%3DnYaSUsfL1vQHNmJo%3D-7hZVjVTmZd1g%40mail.gmail.com.

[Nate Fischer]

I'd be happy to go with Andrew's suggestion. I think it's fine to continue to keep "assert" as the default choice for asserting invariants in Java.

Nate Fischer | Software Engineer | ntf...@google.com

[Tommy Nyquist]

Andrews' suggestion looks helpful to me too.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/java/CACiwn8ZY1iHJS11ZCco%3Dm2vcMREzyCRz1nTOrMGv3kagoQ0%3DFQ%40mail.gmail.com.

[Andrew Grieve]

Put up a review here: https://chromium-review.googlesource.com/c/chromium/src/+/4705109

Will wait until Monday for any further comments.