New issue
Advanced search Search tips

Issue 913943 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK failure in HAS_SMI_TAG(ptr) in smi.h

Project Member Reported by ClusterFuzz, Dec 11

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5134236073590784

Fuzzer: lokihardt_jshitter
Job Type: linux_asan_d8_v8_arm_dbg
Platform Id: linux

Crash Type: DCHECK failure
Crash Address: 
Crash State:
  HAS_SMI_TAG(ptr) in smi.h
  V8_Dcheck
  void v8::internal::AddToDictionaryTemplate<v8::internal::NameDictionary, v8::int
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_v8_arm_dbg&range=57251:57252

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5134236073590784

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Dec 11

Labels: Test-Predator-Auto-Owner
Owner: jkummerow@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/6d706ae3a0153cf0272760132b775ae06ef13b1a ([ubsan] Port Smi to the new design).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Project Member

Comment 2 by sheriffbot@chromium.org, Dec 11

Labels: Pri-1
Labels: Security_Impact-Head M-72
Cc: jkummerow@chromium.org
Labels: -Type-Bug-Security -Restrict-View-SecurityTeam -Security_Impact-Head -Security_Severity-High Type-Bug
Owner: ishell@chromium.org
Ooohh, exciting! No security impact though, I think.

The newly introduced IsSmi() check in Smi::cast is flushing out an existing bug: in AddToDictionaryTemplate() in literal-objects.cc, we have the following code structure in the case entry != kNotFound, value_kind == kData:

  if (existing_value->IsAccessorPair()) {
    ...
  } else {
    int existing_value_index = Smi::ToInt(existing_value);
    ...
  }

However, in the "else" branch, existing_value is an AccessorInfo.

I'm not sure what the right fix is, i.e. how AccessorInfos should be handled there. It *might* be as easy as s/Smi::ToInt/GetExistingValueIndex/ like elsewhere in that function, but I don't know whether that leads to the correct behavior. FWIW, in non-SLOW_DCHECK builds, Chrome currently refuses to overwrite "length", whereas Firefox allows it.

For convenience, here's the repro:

  class Foo extends Function {
    static ['length']() { return 42; }
  };

Assigning to Igor who wrote all that code, so hopefully has some context.

Sign in to add a comment