Wrap external pointers more carefully.

src/api.cc

 // Copyright 2010 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 3248 matching lines @@
                 "v8::Object::SetInternalField()",
                 "Writing internal field out of bounds")) {
     return;
   }
   ENTER_V8;
   i::Handle<i::Object> val = Utils::OpenHandle(*value);
   obj->SetInternalField(index, *val);
 }
 
 
+static bool CanBeEncodedAsSmi(void* ptr) {
+  const intptr_t address = reinterpret_cast<intptr_t>(ptr);
+  const intptr_t mask = (intptr_t(1) << i::kSmiTagSize) - 1;

[Lasse Reichstein, 2011/01/13 08:35:18]

Just use kSmiTagMask.

+  return ((address & mask) == 0) && i::Smi::IsValid(address >> i::kSmiTagSize);

[Lasse Reichstein, 2011/01/13 08:35:18]

Almost no addresses will be valid in 64-bit mode. Only those in the low 31-bit
of the address space will work.

Let's just embed the pointer as a smi-tagged, but non-smi, value, with bits in
the lower half as well.
As long as we only read it again using GetPointerFromInternalField, or in custom
generated code that knows what it's doing, it should be safe.

+}
+
+
+static i::Smi* EncodeAsSmi(void* ptr) {
+  ASSERT(CanBeEncodedAsSmi(ptr));
+  i::Smi* result = i::Smi::FromIntptr(
+      reinterpret_cast<intptr_t>(ptr) >> i::kSmiTagSize);
+  ASSERT_EQ(ptr, i::Internals::GetExternalPointerFromSmi(result));
+  return result;
+}
+
+
 void v8::Object::SetPointerInInternalField(int index, void* value) {
   ENTER_V8;
-  i::Object* as_object = reinterpret_cast<i::Object*>(value);
-  if (as_object->IsSmi()) {
-    Utils::OpenHandle(this)->SetInternalField(index, as_object);
-    return;
+  if (CanBeEncodedAsSmi(value)) {
+    Utils::OpenHandle(this)->SetInternalField(index, EncodeAsSmi(value));
+  } else {
+    HandleScope scope;
+    i::Handle<i::Proxy> proxy =
+        i::Factory::NewProxy(reinterpret_cast<i::Address>(value), i::TENURED);
+    if (!proxy.is_null())
+        Utils::OpenHandle(this)->SetInternalField(index, *proxy);
   }
-  HandleScope scope;
-  i::Handle<i::Proxy> proxy =
-      i::Factory::NewProxy(reinterpret_cast<i::Address>(value), i::TENURED);
-  if (!proxy.is_null())
-      Utils::OpenHandle(this)->SetInternalField(index, *proxy);
+  ASSERT_EQ(value, GetPointerFromInternalField(index));
 }
 
 
 // --- E n v i r o n m e n t ---
 
 bool v8::V8::Initialize() {
   if (i::V8::IsRunning()) return true;
   HandleScope scope;
   if (i::Snapshot::Initialize()) return true;
   return i::V8::Initialize(NULL);
@@ skipping 262 matching lines @@
 static void* ExternalValueImpl(i::Handle<i::Object> obj) {
   return reinterpret_cast<void*>(i::Proxy::cast(*obj)->proxy());
 }
 
 
 Local<Value> v8::External::Wrap(void* data) {
   STATIC_ASSERT(sizeof(data) == sizeof(i::Address));
   LOG_API("External::Wrap");
   EnsureInitialized("v8::External::Wrap()");
   ENTER_V8;
-  i::Object* as_object = reinterpret_cast<i::Object*>(data);
-  if (as_object->IsSmi()) {
-    return Utils::ToLocal(i::Handle<i::Object>(as_object));
-  }
-  return ExternalNewImpl(data);
+
+  v8::Local<v8::Value> result = CanBeEncodedAsSmi(data)
+      ? Utils::ToLocal(i::Handle<i::Object>(EncodeAsSmi(data)))
+      : v8::Local<v8::Value>(ExternalNewImpl(data));
+
+  ASSERT_EQ(data, Unwrap(result));
+  return result;
 }
 
 
 void* v8::Object::SlowGetPointerFromInternalField(int index) {
   i::Handle<i::JSObject> obj = Utils::OpenHandle(this);
   i::Object* value = obj->GetInternalField(index);
   if (value->IsSmi()) {
-    return value;
+    return i::Internals::GetExternalPointerFromSmi(value);
   } else if (value->IsProxy()) {
     return reinterpret_cast<void*>(i::Proxy::cast(value)->proxy());
   } else {
     return NULL;
   }
 }
 
 
 void* v8::External::FullUnwrap(v8::Handle<v8::Value> wrapper) {
   if (IsDeadCheck("v8::External::Unwrap()")) return 0;
   i::Handle<i::Object> obj = Utils::OpenHandle(*wrapper);
   void* result;
   if (obj->IsSmi()) {
-    // The external value was an aligned pointer.
-    result = *obj;
+    result = i::Internals::GetExternalPointerFromSmi(*obj);
   } else if (obj->IsProxy()) {
     result = ExternalValueImpl(obj);
   } else {
     result = NULL;
   }
   ASSERT_EQ(result, QuickUnwrap(wrapper));
   return result;
 }
 
 
@@ skipping 1535 matching lines @@
 
 
 char* HandleScopeImplementer::Iterate(ObjectVisitor* v, char* storage) {
   HandleScopeImplementer* thread_local =
       reinterpret_cast<HandleScopeImplementer*>(storage);
   thread_local->IterateThis(v);
   return storage + ArchiveSpacePerThread();
 }
 
 } }  // namespace v8::internal