Skip to content

Commit

Permalink
[vm] Fix Instance::IdentityHashCode for strings
Browse files Browse the repository at this point in the history
This method should return the same value
as `String::Hash` to not be misaligned
with how `identityHashCode()` is implemented.

On 64bit platforms we also use the same
location in the object header to store
identity hash and string hash so these two
can't be misaligned.

Fixes #56749

TEST=vm/cc/String,vm/dart/regress_56749
[email protected]

Change-Id: I221cc8ad85f7ffb7f7d2bcdd0f4341c3ecf25663
Fixed: 56749
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/385860
Commit-Queue: Slava Egorov <[email protected]>
Reviewed-by: Ryan Macnak <[email protected]>
  • Loading branch information
mraleph authored and Commit Queue committed Sep 18, 2024
1 parent bf36861 commit bb59b5c
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 0 deletions.
19 changes: 19 additions & 0 deletions runtime/tests/vm/dart/regress_56749_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import "package:expect/expect.dart";

void main() {
String a = "Hello, world!";
String b = a.substring(0, 5) + a.substring(5);

// `Closure::ComputeHash` will invoke `Instance::IdentityHashCode` on
// the receiver (b). Previously this function did not handle String
// specially and used a random hash code instead of the expected
// content-based hash code.
print(b.toString.hashCode);

Expect.equals(a.hashCode, b.hashCode);
Expect.equals(a, b);
}
3 changes: 3 additions & 0 deletions runtime/vm/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20727,6 +20727,9 @@ ObjectPtr Instance::HashCode() const {
// Keep in sync with AsmIntrinsifier::Object_getHash.
IntegerPtr Instance::IdentityHashCode(Thread* thread) const {
if (IsInteger()) return Integer::Cast(*this).ptr();
if (IsString()) {
return Smi::New(String::Cast(*this).Hash());
}

#if defined(HASH_IN_OBJECT_HEADER)
intptr_t hash = Object::GetCachedHash(ptr());
Expand Down
11 changes: 11 additions & 0 deletions runtime/vm/object_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,17 @@ ISOLATE_UNIT_TEST_CASE(String) {
EXPECT_EQ(0x7FFF, str16.CharAt(1));
EXPECT_EQ(0xFFFF, str16.CharAt(2));
}

// Check that String's identity hash and hashCode are the same.
{
for (auto str : {"Hello", "Hell\xC3\x98"}) {
const String& s = String::Handle(String::New(str));
const String& s2 = String::Handle(String::New(str));
EXPECT_EQ(s.Hash(),
static_cast<uword>(
Integer::Handle(s2.IdentityHashCode(thread)).Value()));
}
}
}

ISOLATE_UNIT_TEST_CASE(StringFormat) {
Expand Down

0 comments on commit bb59b5c

Please sign in to comment.