Fix incorrect string hash value extension during cross-compilation.

Previouly, having a 32-bit Android device and a 64-bit host to compile
boot.oat could lead to an interning table be fulfilled using one hash
function and be worked with using another hash function (which is caused
by sign extension).
Target case is any string with a negative .GetHashCode().

Test: test-art-host-gtest-intern_table_test

Change-Id: I3f417e1ac990ef681f0651160292130e9b3632f0

4 files changed, 33 insertions, 2 deletions

diff --git a/runtime/base/hash_set.h b/runtime/base/hash_set.h
index f24a8625b4..a22efcfe32 100644
--- a/runtime/base/hash_set.h
+++ b/runtime/base/hash_set.h
@@ -672,6 +672,8 @@ class HashSet {
   T* data_;  // Backing storage.
   double min_load_factor_;
   double max_load_factor_;
+
+  ART_FRIEND_TEST(InternTableTest, CrossHash);
 };
 
 template <class T, class EmptyFn, class HashFn, class Pred, class Alloc>
diff --git a/runtime/intern_table.cc b/runtime/intern_table.cc
index 9c05d3c574..3e1914604d 100644
--- a/runtime/intern_table.cc
+++ b/runtime/intern_table.cc
@@ -319,7 +319,9 @@ std::size_t InternTable::StringHashEquals::operator()(const GcRoot<mirror::Strin
   if (kIsDebugBuild) {
     Locks::mutator_lock_->AssertSharedHeld(Thread::Current());
   }
-  return static_cast<size_t>(root.Read<kWithoutReadBarrier>()->GetHashCode());
+  // An additional cast to prevent undesired sign extension.
+  return static_cast<size_t>(
+      static_cast<uint32_t>(root.Read<kWithoutReadBarrier>()->GetHashCode()));
 }
 
 bool InternTable::StringHashEquals::operator()(const GcRoot<mirror::String>& a,
diff --git a/runtime/intern_table.h b/runtime/intern_table.h
index f661d9ffe5..68454fbfd4 100644
--- a/runtime/intern_table.h
+++ b/runtime/intern_table.h
@@ -163,7 +163,11 @@ class InternTable {
         NO_THREAD_SAFETY_ANALYSIS;
 
     // Utf8String can be used for lookup.
-    std::size_t operator()(const Utf8String& key) const { return key.GetHash(); }
+    std::size_t operator()(const Utf8String& key) const {
+      // A cast to prevent undesired sign extension.
+      return static_cast<uint32_t>(key.GetHash());
+    }
+
     bool operator()(const GcRoot<mirror::String>& a, const Utf8String& b) const
         NO_THREAD_SAFETY_ANALYSIS;
   };
@@ -217,6 +221,8 @@ class InternTable {
     // We call AddNewTable when we create the zygote to reduce private dirty pages caused by
     // modifying the zygote intern table. The back of table is modified when strings are interned.
     std::vector<UnorderedSet> tables_;
+
+    ART_FRIEND_TEST(InternTableTest, CrossHash);
   };
 
   // Insert if non null, otherwise return null. Must be called holding the mutator lock.
@@ -276,6 +282,7 @@ class InternTable {
   gc::WeakRootState weak_root_state_ GUARDED_BY(Locks::intern_table_lock_);
 
   friend class Transaction;
+  ART_FRIEND_TEST(InternTableTest, CrossHash);
   DISALLOW_COPY_AND_ASSIGN(InternTable);
 };
 
diff --git a/runtime/intern_table_test.cc b/runtime/intern_table_test.cc
index b91d946095..3991d6550d 100644
--- a/runtime/intern_table_test.cc
+++ b/runtime/intern_table_test.cc
@@ -16,6 +16,7 @@
 
 #include "intern_table.h"
 
+#include "base/hash_set.h"
 #include "common_runtime_test.h"
 #include "mirror/object.h"
 #include "handle_scope-inl.h"
@@ -62,6 +63,25 @@ TEST_F(InternTableTest, Size) {
   EXPECT_EQ(2U, t.Size());
 }
 
+// Check if table indexes match on 64 and 32 bit machines.
+// This is done by ensuring hash values are the same on every machine and limited to 32-bit wide.
+// Otherwise cross compilation can cause a table to be filled on host using one indexing algorithm
+// and later on a device with different sizeof(size_t) can use another indexing algorithm.
+// Thus the table may provide wrong data.
+TEST_F(InternTableTest, CrossHash) {
+  ScopedObjectAccess soa(Thread::Current());
+  InternTable t;
+
+  // A string that has a negative hash value.
+  GcRoot<mirror::String> str(mirror::String::AllocFromModifiedUtf8(soa.Self(), "00000000"));
+
+  MutexLock mu(Thread::Current(), *Locks::intern_table_lock_);
+  for (InternTable::Table::UnorderedSet& table : t.strong_interns_.tables_) {
+    // The negative hash value shall be 32-bit wide on every host.
+    ASSERT_TRUE(IsUint<32>(table.hashfn_(str)));
+  }
+}
+
 class TestPredicate : public IsMarkedVisitor {
  public:
   mirror::Object* IsMarked(mirror::Object* s) OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_) {