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

src/builtins.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/builtins.h"
 
 #include "src/api.h"
 #include "src/api-arguments.h"
 #include "src/api-natives.h"
 #include "src/base/once.h"
@@ skipping 5130 matching lines @@
 }
 
 
 void Builtins::Generate_StackCheck(MacroAssembler* masm) {
   masm->TailCallRuntime(Runtime::kStackGuard);
 }
 
 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),
       not_float_or_clamped(a), invalid(a);
 
   // Fail if it is not a heap object.
   a->Branch(a->WordIsSmi(tagged), &is_smi, &not_smi);
   a->Bind(&is_smi);
   a->Goto(&invalid);
 
@@ skipping 30 matching lines @@
             &not_float_or_clamped, &is_float_or_clamped);
   a->Bind(&is_float_or_clamped);
   a->Goto(&invalid);
 
   a->Bind(&invalid);
   a->CallRuntime(Runtime::kThrowNotIntegerSharedTypedArrayError, context,
                  tagged);
   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
 compiler::Node* ConvertTaggedAtomicIndexToWord32(compiler::CodeStubAssembler* a,
                                                  compiler::Node* tagged,
                                                  compiler::Node* context) {
   using namespace compiler;
   CodeStubAssembler::Variable var_result(a, MachineRepresentation::kWord32);
 
   Callable to_number = CodeFactory::ToNumber(a->isolate());
@@ skipping 52 matching lines @@
 }
 
 }  // anonymous namespace
 
 void Builtins::Generate_AtomicsLoadCheck(compiler::CodeStubAssembler* a) {
   using namespace compiler;
   Isolate* isolate = a->isolate();
   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));
   ValidateAtomicIndex(a, index_word, array_length_word, context);
 
   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)               \
 Handle<Code> Builtins::name() {                               \
   Code** code_address =                                       \
       reinterpret_cast<Code**>(builtin_address(k##name));     \
   return Handle<Code>(code_address);                          \
 }
 #define DEFINE_BUILTIN_ACCESSOR_A(name, kind, state, extra) \
 Handle<Code> Builtins::name() {                             \
@@ skipping 17 matching lines @@
 BUILTIN_LIST_T(DEFINE_BUILTIN_ACCESSOR_T)
 BUILTIN_LIST_H(DEFINE_BUILTIN_ACCESSOR_H)
 BUILTIN_LIST_DEBUG_A(DEFINE_BUILTIN_ACCESSOR_A)
 #undef DEFINE_BUILTIN_ACCESSOR_C
 #undef DEFINE_BUILTIN_ACCESSOR_A
 #undef DEFINE_BUILTIN_ACCESSOR_T
 #undef DEFINE_BUILTIN_ACCESSOR_H
 
 }  // namespace internal
 }  // namespace v8