[Atomics] Handle conversion to SMI in builtin, not code stub.

src/builtins.cc

Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index d054916db0a700e330ea3aac6abf859d0ec4226a..af8b5fc87f46fedc00f7426e18fc3e3209645c6d 100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -5148,7 +5148,8 @@ void Builtins::Generate_StackCheck(MacroAssembler* masm) {
 namespace {
 
 void ValidateSharedTypedArray(compiler::CodeStubAssembler* a,
-                              compiler::Node* tagged, compiler::Node* context) {
+                              compiler::Node* tagged, compiler::Node* context,
+                              compiler::Node** out_instance_type) {
   using namespace compiler;
   CodeStubAssembler::Label is_smi(a), not_smi(a), is_typed_array(a),
       not_typed_array(a), is_shared(a), not_shared(a), is_float_or_clamped(a),
@@ -5199,6 +5200,25 @@ void ValidateSharedTypedArray(compiler::CodeStubAssembler* a,
   a->Return(a->UndefinedConstant());
 
   a->Bind(&not_float_or_clamped);
+  *out_instance_type = elements_instance_type;
+}
+
+void BranchIfSharedTypedArrayIsSigned(
+    compiler::CodeStubAssembler* a, compiler::Node* instance_type,
+    compiler::CodeStubAssembler::Label* is_signed,
+    compiler::CodeStubAssembler::Label* is_unsigned) {
+  STATIC_ASSERT(((FIXED_INT8_ARRAY_TYPE - FIXED_INT8_ARRAY_TYPE) & 1) == 0);
+  STATIC_ASSERT(((FIXED_UINT8_ARRAY_TYPE - FIXED_INT8_ARRAY_TYPE) & 1) == 1);
+  STATIC_ASSERT(((FIXED_INT16_ARRAY_TYPE - FIXED_INT8_ARRAY_TYPE) & 1) == 0);
+  STATIC_ASSERT(((FIXED_UINT16_ARRAY_TYPE - FIXED_INT8_ARRAY_TYPE) & 1) == 1);
+  STATIC_ASSERT(((FIXED_INT32_ARRAY_TYPE - FIXED_INT8_ARRAY_TYPE) & 1) == 0);
+  STATIC_ASSERT(((FIXED_UINT32_ARRAY_TYPE - FIXED_INT8_ARRAY_TYPE) & 1) == 1);
+  a->Branch(a->WordEqual(
+                a->WordAnd(a->Int32Sub(instance_type,
+                                       a->Int32Constant(FIXED_INT8_ARRAY_TYPE)),
+                           a->Int32Constant(1)),
+                a->Int32Constant(0)),
+            is_signed, is_unsigned);
 }
 
 // https://tc39.github.io/ecmascript_sharedmem/shmem.html#Atomics.ValidateAtomicAccess
@@ -5271,7 +5291,9 @@ void Builtins::Generate_AtomicsLoadCheck(compiler::CodeStubAssembler* a) {
   Node* array = a->Parameter(1);
   Node* index = a->Parameter(2);
   Node* context = a->Parameter(3 + 2);
-  ValidateSharedTypedArray(a, array, context);
+  Node* array_instance_type;
+
+  ValidateSharedTypedArray(a, array, context, &array_instance_type);
   Node* index_word = ConvertTaggedAtomicIndexToWord32(a, index, context);
   Node* array_length_word = a->TruncateTaggedToWord32(
       context, a->LoadObjectField(array, JSTypedArray::kLengthOffset));
@@ -5279,8 +5301,18 @@ void Builtins::Generate_AtomicsLoadCheck(compiler::CodeStubAssembler* a) {
 
   Callable atomics_load = CodeFactory::AtomicsLoad(isolate);
   Node* target = a->HeapConstant(atomics_load.code());
-  a->Return(a->CallStub(atomics_load.descriptor(), target, context, array,
-                        index_word));
+  Node* untagged_result = a->CallStub(atomics_load.descriptor(), target,

[Benedikt Meurer, 2016/04/14 04:16:20]

I think this is not safe, because the CallInterfaceDescriptor claims that the
result of the stub call is AnyTagged and therefore we will put this value into
the pointer map, and a potential GC triggered by either ChangeInt32ToTagged or
ChangeUint32ToTagged will blow up because the result looks like a HeapObject*,
but is really just some integer.

The fix should be to change the return type of the AtomicLoadDescriptor to
UntaggedIntegral32 in interface-descriptors.cc.

+                                      context, array, index_word);
+
+  CodeStubAssembler::Label is_signed(a), is_unsigned(a);
+  BranchIfSharedTypedArrayIsSigned(a, array_instance_type, &is_signed,
+                                   &is_unsigned);
+
+  a->Bind(&is_signed);
+  a->Return(a->ChangeInt32ToTagged(untagged_result));
+
+  a->Bind(&is_unsigned);
+  a->Return(a->ChangeUint32ToTagged(untagged_result));
 }
 
 #define DEFINE_BUILTIN_ACCESSOR_C(name, ignore)               \