EXPERIMENTAL sign-extend addresses in CompactRingBuffer

[vitalybuka]

Is this still needed?

[ro]

I have no idea. However, I've stopped LLVM development at least for a time, so I'm not going to pursue this at the moment.

[eugenis]

This is an alternative to D91617, it's not currently needed anywhere outside solaris/sparcv9. Let's shelve it for now.

[ro]

I've now tested this patch (together with the revised D91615) on sparcv9-sun-solaris2.11, amd64-pc-solaris2.11, and x86_64-pc-linux-gnu. In combination, they fix the CompactRingBuffer.int64 failure discussed in D91617.

[ro]

@eugenis: With D91615 now in, could you please commit this patch? It's the remaining piece required to make CompactRingBuffer.int64 work on Solaris/sparcv9.

Thanks.

[eugenis]

rebase + better description

compiler-rt/lib/sanitizer_common/sanitizer_ring_buffer.h

 // See a comment in hwasan/hwasan_thread_list.h for the motivation behind this.
 #if SANITIZER_WORDSIZE == 64
 template <class T>
 class CompactRingBuffer {
   // Top byte of long_ stores the buffer size in pages.
   // Lower bytes store the address of the next buffer element.
   static constexpr int kPageSizeBits = 12;
   static constexpr int kSizeShift = 56;
+  static constexpr int kSizeBits = 64 - kSizeShift;
   static constexpr uptr kNextMask = (1ULL << kSizeShift) - 1;
 
   uptr GetStorageSize() const { return (long_ >> kSizeShift) << kPageSizeBits; }
 
+  static uptr SignExtend(uptr x) { return ((sptr)x) << kSizeBits >> kSizeBits; }
+
   void Init(void *storage, uptr size) {
     CHECK_EQ(sizeof(CompactRingBuffer<T>), sizeof(void *));
     CHECK(IsPowerOfTwo(size));
     CHECK_GE(size, 1 << kPageSizeBits);
     CHECK_LE(size, 128 << kPageSizeBits);
     CHECK_EQ(size % 4096, 0);
     CHECK_EQ(size % sizeof(T), 0);
-    CHECK_EQ((uptr)storage % (size * 2), 0);
-    long_ = (uptr)storage | ((size >> kPageSizeBits) << kSizeShift);
+    uptr st = (uptr)storage;
+    CHECK_EQ(st % (size * 2), 0);
+    CHECK_EQ(st, SignExtend(st & kNextMask));
+    long_ = (st & kNextMask) | ((size >> kPageSizeBits) << kSizeShift);
   }
 
   void SetNext(const T *next) {
-    long_ = (long_ & ~kNextMask) | (uptr)next;
+    long_ = (long_ & ~kNextMask) | ((uptr)next & kNextMask);
   }
 
  public:
   CompactRingBuffer(void *storage, uptr size) {
     Init(storage, size);
   }
 
   // A copy constructor of sorts.
   CompactRingBuffer(const CompactRingBuffer &other, void *storage) {
     uptr size = other.GetStorageSize();
     internal_memcpy(storage, other.StartOfStorage(), size);
     Init(storage, size);
     uptr Idx = other.Next() - (const T *)other.StartOfStorage();
     SetNext((const T *)storage + Idx);
   }
 
-  T *Next() const { return (T *)(long_ & kNextMask); }
+  T *Next() const { return (T *)(SignExtend(long_ & kNextMask)); }
 
   void *StartOfStorage() const {
     return (void *)((uptr)Next() & ~(GetStorageSize() - 1));
   }
 
   void *EndOfStorage() const {
     return (void *)((uptr)StartOfStorage() + GetStorageSize());
   }