Wrap external pointers more carefully.

include/v8.h

 // Copyright 2007-2009 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 3335 matching lines @@
 // Tag information for HeapObject.
 const int kHeapObjectTag = 1;
 const int kHeapObjectTagSize = 2;
 const intptr_t kHeapObjectTagMask = (1 << kHeapObjectTagSize) - 1;
 
 // Tag information for Smi.
 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) {
     int shift_bits = kSmiTagSize + kSmiShiftSize;
     // 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) {
     int shift_bits = kSmiTagSize + kSmiShiftSize;
     // 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;
 
 // Internal constants for 32-bit systems.
 template <> struct InternalConstants<4> {
   static const int kStringResourceOffset = 3 * kApiPointerSize;
 };
 
 // Internal constants for 64-bit systems.
 template <> struct InternalConstants<8> {
@@ skipping 27 matching lines @@
   static inline bool HasHeapObjectTag(internal::Object* value) {
     return ((reinterpret_cast<intptr_t>(value) & kHeapObjectTagMask) ==
             kHeapObjectTag);
   }
 
   static inline bool HasSmiTag(internal::Object* value) {
     return ((reinterpret_cast<intptr_t>(value) & kSmiTagMask) == kSmiTag);
   }
 
   static inline int SmiValue(internal::Object* value) {
-    return SmiConstants<kApiPointerSize>::SmiToInt(value);
+    return PlatformSmiTraits::SmiToInt(value);
   }
 
   static inline int GetInstanceType(internal::Object* obj) {
     typedef internal::Object O;
     O* map = ReadField<O*>(obj, kHeapObjectMapOffset);
     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 {
       return NULL;
     }
   }
 
   static inline bool IsExternalTwoByteString(int instance_type) {
     int representation = (instance_type & kFullStringRepresentationMask);
     return representation == kExternalTwoByteRepresentationTag;
@@ skipping 346 matching lines @@
 
 
 }  // namespace v8
 
 
 #undef V8EXPORT
 #undef TYPE_CHECK
 
 
 #endif  // V8_H_