Block UB due to switch on corrupted values [v8/v8 : main]
0 views
Skip to first unread message
Omer Katz (Gerrit)
May 8, 2026, 7:36:30 AMMay 8
to Hannes Payer, android-bu...@system.gserviceaccount.com, Darius Mercadier, Michael Lippautz, v8-s...@luci-project-accounts.iam.gserviceaccount.com, oilpan-r...@chromium.org, pthier...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org, was...@google.com
Attention needed from Darius Mercadier and Michael Lippautz
Omer Katz added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 1:
Darius Mercadier . resolved
Darius MercadierHey, 2 comments:
- Apart from a handful of exceptions, this is CL is mostly just updating DEBUG-only or tracing functions. It's fine of course, but I just wanted to point it out.
- Gemini missed _a bunch_ of examples. Just from the file I currently had opened, the first switch I found was https://source.chromium.org/chromium/chromium/src/+/main:v8/src/compiler/turboshaft/store-store-elimination-reducer-inl.h;l=444;drc=0e7eee33ad88dcbf15116e7f11a8cf8c7b56904f, and this CL didn't update it. From a quick look at the CL, it looks like it only updated functions that _only_ contain a switch. Also fine, but I also wanted to point it out :p
Omer KatzMmmh the Gerrit didn't like my link. I was referring to TryGetRawUint32Constant in store-store-elimination-reducer-inl.h
Omer KatzI don't expect this CL to be exhaustive. It's a bunch of locations that gemini found, but it's not all locations. We could try to run the prompt a few more times and see what it finds. Probably the prompt could also be improved.
I think debug-only still makes sense to update. It's pretty much free and if it means we don't later get a report that we need to downgrade to a bug (and potentially explain why) then I think it's a win.
I ran it a few more times and found a few more instances.
Related details
Attention is currently required from:
- Darius Mercadier
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
May 13, 2026, 10:33:29 AMMay 13
to Hannes Payer, android-bu...@system.gserviceaccount.com, Darius Mercadier, Michael Lippautz, v8-s...@luci-project-accounts.iam.gserviceaccount.com, marja...@chromium.org, oilpan-r...@chromium.org, pthier...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org, was...@google.com
Attention needed from Michael Lippautz
Omer Katz added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
May 15, 2026, 3:32:39 AMMay 15
to Omer Katz, Hannes Payer, android-bu...@system.gserviceaccount.com, Darius Mercadier, v8-s...@luci-project-accounts.iam.gserviceaccount.com, marja...@chromium.org, oilpan-r...@chromium.org, pthier...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org, was...@google.com
Attention needed from Omer Katz
Michael Lippautz voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Omer Katz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
May 15, 2026, 4:06:18 AMMay 15
to Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, Darius Mercadier, v8-s...@luci-project-accounts.iam.gserviceaccount.com, marja...@chromium.org, oilpan-r...@chromium.org, pthier...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org, was...@google.com
Omer Katz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Omer Katz
| Commit-Queue | +2 |
1 comment
Commit Message
Line 16, Patchset 1:
This CL uses Gemini to identify exhaustive switches over enums thatOmer Katz . resolved
Omer KatzFor documentation, the actual prompt was:
```
Find all switch statements over enums that exhaustively list all enum values, each case returns, and there's no default case or `UNREACHABLE()` after the switch. Add an `UNREACHABLE()` after the switch.
```
Acknowledged
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
May 15, 2026, 4:29:49 AMMay 15
to Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, Darius Mercadier, v8-s...@luci-project-accounts.iam.gserviceaccount.com, marja...@chromium.org, oilpan-r...@chromium.org, pthier...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org, was...@google.com
Omer Katz voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
v8-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
May 15, 2026, 4:51:50 AMMay 15
to Omer Katz, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, Darius Mercadier, marja...@chromium.org, oilpan-r...@chromium.org, pthier...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, devtools-...@chromium.org, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org, was...@google.com
v8-s...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Block UB due to switch on corrupted values
When switching over an enum value we already require that the switch
covers all enum values. When that is achieved by exhaustively listing
all values, rather than a default case, a corrupted value can thus not
match any of the cases. If the switch cases are all returning values and
there is no default return after the switch, this can lead to UB and
potentially sandbox escapes.
This CL uses Gemini to identify exhaustive switches over enums that
return but don't have a default catch-all path, and adds UNREACHABLE()
after them to crash in case of unexpected values.
This CL doesn't try to limit the changes to just switches that take
in-sandbox corruptible values or switches that are known to result in
issues. Instead, we take a conservative approach and adjust all found
switches.
Bug: 390617721
Change-Id: I56369a6c384db98e7b7cb679d3fdf166967ab438
Commit-Queue: Omer Katz <omer...@chromium.org>
Reviewed-by: Michael Lippautz <mlip...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107335}
Files:
- M src/codegen/machine-type.h
- M src/common/globals.h
- M src/common/operation.h
- M src/compiler/backend/arm64/instruction-selector-arm64.cc
- M src/compiler/backend/instruction-codes.h
- M src/compiler/backend/instruction-selector.cc
- M src/compiler/backend/instruction.cc
- M src/compiler/backend/ppc/instruction-selector-ppc.cc
- M src/compiler/backend/register-allocator-verifier.cc
- M src/compiler/code-assembler.h
- M src/compiler/globals.h
- M src/compiler/machine-operator.cc
- M src/compiler/turboshaft/dataview-lowering-reducer.h
- M src/compiler/turboshaft/dead-code-elimination-reducer.h
- M src/compiler/turboshaft/graph.cc
- M src/compiler/turboshaft/js-generic-lowering-reducer.h
- M src/compiler/turboshaft/loop-unrolling-reducer.cc
- M src/compiler/turboshaft/operations.cc
- M src/compiler/turboshaft/operations.h
- M src/compiler/turboshaft/representations.cc
- M src/compiler/turboshaft/representations.h
- M src/compiler/turboshaft/store-store-elimination-reducer-inl.h
- M src/compiler/turboshaft/turbolev-graph-builder.cc
- M src/compiler/turboshaft/types.cc
- M src/compiler/turboshaft/types.h
- M src/compiler/turboshaft/wasm-lowering-reducer.h
- M src/compiler/wasm-compiler-definitions.h
- M src/diagnostics/objects-printer.cc
- M src/execution/messages.cc
- M src/heap/cppgc/sweeper.cc
- M src/heap/factory.cc
- M src/heap/gc-tracer.cc
- M src/ic/ic.cc
- M src/maglev/maglev-ir.cc
- M src/objects/lookup.cc
- M src/objects/module.cc
- M src/parsing/parser-base.h
- M src/profiler/profiler-stats.cc
- M src/profiler/symbolizer.cc
- M src/regexp/regexp-bytecodes.cc
- M src/torque/torque.cc
- M src/torque/types.cc
- M src/wasm/canonical-types.cc
- M src/wasm/canonical-types.h
- M src/wasm/turboshaft-graph-interface.cc
- M src/wasm/wasm-debug.h
- M src/wasm/wasm-objects-inl.h
- M src/wasm/wasm-objects.cc
- M src/wasm/wasm-subtyping.cc
- M src/wasm/wasm-tier.h
- M src/wasm/well-known-imports.cc
Change size: M
Delta: 51 files changed, 197 insertions(+), 2 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Michael Lippautz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages