[clang][Interp] Fix dereferencing arrays with no offset applied

[tbaeder]

Summary

There is a difference between a Pointer and a "Pointer to the first element of an array".

[aaron.ballman]

There is a difference between a Pointer and a "Pointer to the first element of an array".

I'm pretty confused because this statement is false per the language standard (http://eel.is/c++draft/expr.sub#2). Basically, array subscripting works through pointer arithmetic, so &array[0] and array (decayed to a pointer) have the same value. Why do we need to offset to get to the first element in the interpreter?

[tbaeder]

In D137082#3899032, @aaron.ballman wrote:

There is a difference between a Pointer and a "Pointer to the first element of an array".

I'm pretty confused because this statement is false per the language standard (http://eel.is/c++draft/expr.sub#2). Basically, array subscripting works through pointer arithmetic, so &array[0] and array (decayed to a pointer) have the same value. Why do we need to offset to get to the first element in the interpreter?

That's just an implementation detail in the Pointer class. For primitive arrays, we need the sizeof(InitMap*) applied, which happens via atIndex(). Otherwise, deref() will look at the first few bytes of the InitMap* pointer.

I've added some documentation about this in https://reviews.llvm.org/D135750 (and the MetadataSize added to Descriptor there could be used to clean this up I think).

[aaron.ballman]

clang/lib/AST/Interp/Interp.h

966–967 ↗

(On Diff #472007)

So why don't we need to do this dance for Store/StorePop?

[tbaeder]

clang/lib/AST/Interp/Interp.h

966–967 ↗

(On Diff #472007)

We do, I just realized.

It might make more sense to do this in Pointer::deref() directly, so 'deref()`'ing an array always returns the first element.

[aaron.ballman]

LGTM! I think this is a more clear approach than the previous one.

clang/lib/AST/Interp/Pointer.h

if (Offset != Base) {
       if (getFieldDesc()->ElemDesc)
         Adjust = sizeof(InlineDescriptor);
       else
         Adjust = sizeof(InitMap *);
     }
     return Offset - Base - Adjust;
   }
 
+  /// Whether this array refers to an array, but not
+  /// to the first element.
+  bool isArrayRoot() const { return inArray() && Offset == Base; }
+
   /// Checks if the innermost field is an array.
   bool inArray() const { return getFieldDesc()->IsArray; }
   /// Checks if the structure is a primitive array.
   bool inPrimitiveArray() const { return getFieldDesc()->isPrimitiveArray(); }
   /// Checks if the structure is an array of unknown size.
   bool isUnknownSizeArray() const {
     return getFieldDesc()->isUnknownSizeArray();
   }
public:
   }
 
   /// Checks if the pointer is an out-of-bounds element pointer.
   bool isElementPastEnd() const { return Offset == PastEndMark; }
 
   /// Dereferences the pointer, if it's live.
   template <typename T> T &deref() const {
     assert(isLive() && "Invalid pointer");
+    if (isArrayRoot())
+      return *reinterpret_cast<T *>(Pointee->rawData() + Base +
+                                    sizeof(InitMap *));
+
     return *reinterpret_cast<T *>(Pointee->rawData() + Offset);
   }
 
   /// Dereferences a primitive element.
   template <typename T> T &elem(unsigned I) const {
     return reinterpret_cast<T *>(Pointee->rawData())[I];
   }
 

clang/test/AST/Interp/arrays.cpp

 static_assert(getElement(1) == 4, "");
 static_assert(getElement(5) == 36, "");
 
 constexpr int data[] = {5, 4, 3, 2, 1};
 constexpr int getElement(const int *Arr, int index) {
   return *(Arr + index);
 }
 
+constexpr int derefPtr(const int *d) {
+  return *d;
+}
+static_assert(derefPtr(data) == 5, "");
+
+constexpr int storePtr() {
+  int b[] = {1,2,3,4};
+  int *c = b;
+
+  *c = 4;
+  return *c;
+}
+static_assert(storePtr() == 4, "");
+
+
 static_assert(getElement(data, 1) == 4, "");
 static_assert(getElement(data, 4) == 1, "");
 
 constexpr int getElementFromEnd(const int *Arr, int size, int index) {
   return *(Arr + size - index - 1);
 }
 static_assert(getElementFromEnd(data, 5, 0) == 1, "");
 static_assert(getElementFromEnd(data, 5, 4) == 5, "");