Wrap external pointers more carefully.

include/v8.h

Index: include/v8.h
diff --git a/include/v8.h b/include/v8.h
index 3a9827b24d3ba2e361c4ba326446fdd87ebee314..1ceb37913f3bf45689c854ee98ec7bdd681304a9 100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -3353,10 +3353,10 @@ const int kSmiTag = 0;
 const int kSmiTagSize = 1;
 const intptr_t kSmiTagMask = (1 << kSmiTagSize) - 1;
 
-template <size_t ptr_size> struct SmiConstants;
+template <size_t ptr_size> struct SmiTraits;

[Lasse Reichstein, 2011/01/13 12:57:57]

I think I would prefer "SmiTagging", but it's better than SmiConstants.

[antonm, 2011/01/13 15:49:15]

Done.

 
 // Smi constants for 32-bit systems.
-template <> struct SmiConstants<4> {
+template <> struct SmiTraits<4> {
   static const int kSmiShiftSize = 0;
   static const int kSmiValueSize = 31;
   static inline int SmiToInt(internal::Object* value) {
@@ -3364,10 +3364,15 @@ template <> struct SmiConstants<4> {
     // Throw away top 32 bits and shift down (requires >> to be sign extending).
     return static_cast<int>(reinterpret_cast<intptr_t>(value)) >> shift_bits;
   }
+
+  // For 32-bit systems any 2 bytes aligned pointer can be encoded as smi
+  // with a plain reinterpret_cast.
+  static const intptr_t kEncodablePointerMask = 0x1;
+  static const int kPointerToSmiShift = 0;
 };
 
 // Smi constants for 64-bit systems.
-template <> struct SmiConstants<8> {
+template <> struct SmiTraits<8> {
   static const int kSmiShiftSize = 31;
   static const int kSmiValueSize = 32;
   static inline int SmiToInt(internal::Object* value) {
@@ -3375,10 +3380,27 @@ template <> struct SmiConstants<8> {
     // Shift down and throw away top 32 bits.
     return static_cast<int>(reinterpret_cast<intptr_t>(value) >> shift_bits);
   }
+
+  // This is a trade-off: pointer can be encoded if it fits into
+  // 32 bits, so to cover bigger range we need to require better
+  // alignment.  Currently we require 8 bytes alignment (which

[Lasse Reichstein, 2011/01/13 12:57:57]

Drop "better" alignment (the text should be stand-alone, so it should not be
implicitly relative to the 32-bit case).
Just say something like "To maximize the range of pointers that can be encoded
in the available 32 bit, we require them to be 8-byte aligned".

[antonm, 2011/01/13 15:49:15]

Done.

+  // should be the most common case) and hence we have 2 ^ (32 + 3) =
+  // 32G address space covered.  Alas, it might be not enough to
+  // cover stack allocated objects on some platforms.
+  static const int kPointerAlignment = 3;
+
+  static const intptr_t kEncodablePointerMask =
+      intptr_t(0xffffffff) << kPointerAlignment;

[Lasse Reichstein, 2011/01/13 12:57:57]

Shouldn't this be inverted to mask the bits that must be zero?

[antonm, 2011/01/13 15:49:15]

Thanks a lot, of course it should.

+
+  static const int kPointerToSmiShift =
+      kSmiTagSize + kSmiShiftSize - kPointerAlignment;
 };
 
-const int kSmiShiftSize = SmiConstants<kApiPointerSize>::kSmiShiftSize;
-const int kSmiValueSize = SmiConstants<kApiPointerSize>::kSmiValueSize;
+typedef SmiTraits<kApiPointerSize> PlatformSmiTraits;
+const int kSmiShiftSize = PlatformSmiTraits::kSmiShiftSize;
+const int kSmiValueSize = PlatformSmiTraits::kSmiValueSize;
+const intptr_t kEncodablePointerMask = PlatformSmiTraits::kEncodablePointerMask;
+const int kPointerToSmiShift = PlatformSmiTraits::kPointerToSmiShift;
 
 template <size_t ptr_size> struct InternalConstants;
 
@@ -3426,7 +3448,7 @@ class Internals {
   }
 
   static inline int SmiValue(internal::Object* value) {
-    return SmiConstants<kApiPointerSize>::SmiToInt(value);
+    return PlatformSmiTraits::SmiToInt(value);
   }
 
   static inline int GetInstanceType(internal::Object* obj) {
@@ -3435,9 +3457,14 @@ class Internals {
     return ReadField<uint8_t>(map, kMapInstanceTypeOffset);
   }
 
+  static inline void* GetExternalPointerFromSmi(internal::Object* value) {
+    const intptr_t address = reinterpret_cast<intptr_t>(value);
+    return reinterpret_cast<void*>(address >> kPointerToSmiShift);
+  }
+
   static inline void* GetExternalPointer(internal::Object* obj) {
     if (HasSmiTag(obj)) {
-      return obj;
+      return GetExternalPointerFromSmi(obj);
     } else if (GetInstanceType(obj) == kProxyType) {
       return ReadField<void*>(obj, kProxyProxyOffset);
     } else {