commit 63c0c2d9da31d26781f5e77aba6125f0d0988795
author Andra Danciu <andradanciu@google.com> Mon Sep 07 15:50:40 2020 +0000
committer Vladimir Marko <vmarko@google.com> Tue Sep 08 10:56:45 2020 +0000
tree b80d12dee72053edd1feddf115a602ff51a328a2
parent 5c2914dd610e21e9a4a2bbe5c2e28b29f1cd37d6

X86: Fix bug in GenerateVarHandleAccessModeCheck

The condition of taking the slow path for VarHandle access mode check
was reversed.

Solving this bug revealed another problem: in the case of an instance
field, the object might be null. The null check is now done for the
object.

Test: art/test.py --host --32 -r -t 712-varhandle-invocations
Bug: 65872996
Change-Id: I8d047cc0ecba3d05178ea67945afe762c545c27e

compiler/optimizing/intrinsics_x86.cc

diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index b988eb0..53cf350 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -3093,7 +3093,7 @@
 
   // If the access mode is not supported, bail to runtime implementation to handle
   __ testl(Address(varhandle_object, access_modes_bitmask_offset), Immediate(access_mode_bit));
-  __ j(kNotEqual, slow_path->GetEntryLabel());
+  __ j(kZero, slow_path->GetEntryLabel());
 }
 
 static void GenerateVarHandleStaticFieldCheck(Register varhandle_object,
@@ -3111,11 +3111,18 @@
                                        Register temp,
                                        Address type_address,
                                        SlowPathCode* slow_path,
-                                       X86Assembler* assembler) {
+                                       X86Assembler* assembler,
+                                       bool object_can_be_null = true) {
   const uint32_t class_offset = mirror::Object::ClassOffset().Uint32Value();
   const uint32_t super_class_offset = mirror::Class::SuperClassOffset().Uint32Value();
   NearLabel check_type_compatibility, type_matched;
 
+  // If the object is null, there is no need to check the type
+  if (object_can_be_null) {
+    __ testl(object, object);
+    __ j(kZero, &type_matched);
+  }
+
   // Do not unpoison for in-memory comparison.
   // We deliberately avoid the read barrier, letting the slow path handle the false negatives.
   __ movl(temp, Address(object, class_offset));
@@ -3147,12 +3154,17 @@
   __ cmpl(Address(varhandle_object, coordtype1_offset), Immediate(0));
   __ j(kNotEqual, slow_path->GetEntryLabel());
 
+  // Check if the object is null
+  __ testl(object, object);
+  __ j(kZero, slow_path->GetEntryLabel());
+
   // Check the object's class against coordinateType0.
   GenerateSubTypeObjectCheck(object,
                              temp,
                              Address(varhandle_object, coordtype0_offset),
                              slow_path,
-                             assembler);
+                             assembler,
+                             /* object_can_be_null= */ false);
 }
 
 static void GenerateVarHandleCommonChecks(HInvoke *invoke,
@@ -3233,8 +3245,9 @@
     return temp;
   }
 
-  // For instance fields, return the register containing the object
+  // For instance fields, return the register containing the object.
   DCHECK_EQ(expected_coordinates_count, 1u);
+
   return locations->InAt(1).AsRegister<Register>();
 }
 
@@ -3441,6 +3454,7 @@
 
   // Check the varType.primitiveType against the type of the value we're trying to set.
   GenerateVarTypePrimitiveTypeCheck(varhandle_object, temp, value_type, slow_path, assembler);
+
   if (value_type == DataType::Type::kReference) {
     Register value_reg = value.AsRegister<Register>();
     const uint32_t var_type_offset = mirror::VarHandle::VarTypeOffset().Uint32Value();