Checked Span: Crash on nullptr bucket [chromium/src : main]

0 views

Skip to first unread message

Stephen Nusko (Gerrit)

unread,

1:25 AM (10 hours ago) 1:25 AM

to Kalvin Lee, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, Kentaro Hara, Chromium LUCI CQ, bartek...@chromium.org, lize...@chromium.org, lizeb...@chromium.org, wfh+...@chromium.org

Attention needed from Kalvin Lee

Stephen Nusko voted and added 1 comment

Votes added by Stephen Nusko

Code-Review

1 comment

File base/allocator/partition_allocator/src/partition_alloc/bounds_checks.cc

Line 33, Patchset 1 (Latest):


  // This branch handles a mystery whose root cause is still unknown.
  //
  // See also: https://crrev.com/c/7953898
  if (slot_start.value() == 0) [[unlikely]] {
    PA_DEBUG_DATA_ON_STACK("orig", orig_address);
    PA_DEBUG_DATA_ON_STACK("test", test_address);
    PA_DEBUG_DATA_ON_STACK("pool", pool);
    PA_IMMEDIATE_CRASH();
  }

Stephen Nusko . unresolved

Should we instead make this be handled in SlotAddressAndSize or easier to do this here?

I ask because this slot_start.value() == 0 is only needed because bucket can be a nullptr. Otherwise I don't believe it ever returns 0? So we could even remove this branch entirely and base it purely on the bucket being nullptr?

Open in Gerrit

Related details

Attention is currently required from:

Kalvin Lee

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Kalvin Lee (Gerrit)

unread,

8:28 AM (3 hours ago) 8:28 AM

to Stephen Nusko, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, Kentaro Hara, Chromium LUCI CQ, bartek...@chromium.org, lize...@chromium.org, lizeb...@chromium.org, wfh+...@chromium.org

Attention needed from Stephen Nusko

Kalvin Lee added 1 comment

File base/allocator/partition_allocator/src/partition_alloc/bounds_checks.cc

Line 33, Patchset 1:


  // This branch handles a mystery whose root cause is still unknown.
  //
  // See also: https://crrev.com/c/7953898
  if (slot_start.value() == 0) [[unlikely]] {
    PA_DEBUG_DATA_ON_STACK("orig", orig_address);
    PA_DEBUG_DATA_ON_STACK("test", test_address);
    PA_DEBUG_DATA_ON_STACK("pool", pool);
    PA_IMMEDIATE_CRASH();
  }

Stephen Nusko . resolved

Should we instead make this be handled in SlotAddressAndSize or easier to do this here?
I ask because this slot_start.value() == 0 is only needed because bucket can be a nullptr. Otherwise I don't believe it ever returns 0? So we could even remove this branch entirely and base it purely on the bucket being nullptr?

Kalvin Lee

Thanks, fixed in patchset 2.