[clang] [clang][bytecode] Try to load from pointers in `ToLValue` mode (PR #196548)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

We never did the load in the first place, causing us to only look at the `const char *p` parameter when evaluating the `__builtin_object_size` call. The current interpreter however _tries_ to do the load and only uses the subexpr pointer if the load fails. Add a `LoadPopL` op to do exactly that.

---
Full diff: https://github.com/llvm/llvm-project/pull/196548.diff


5 Files Affected:

- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+6-2) 
- (modified) clang/lib/AST/ByteCode/Interp.h (+22) 
- (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+1-1) 
- (modified) clang/lib/AST/ByteCode/Opcodes.td (+1) 
- (modified) clang/test/AST/ByteCode/builtin-object-size.cpp (+21-2) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 11c5478a5c748..1ed2a7066c37e 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -218,8 +218,12 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *E) {
 
   switch (E->getCastKind()) {
   case CK_LValueToRValue: {
-    if (ToLValue && E->getType()->isPointerType())
-      return this->delegate(SubExpr);
+    if (ToLValue && E->getType()->isPointerType()) {
+      assert(!DiscardResult);
+      if (!this->visit(SubExpr))
+        return false;
+      return this->emitLoadPopL(E);
+    }
 
     if (SubExpr->getType().isVolatileQualified())
       return this->emitInvalidCast(CastKind::Volatile, /*Fatal=*/true, E);
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 4a550fdd63bfb..f0e0099d06a6f 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2132,6 +2132,28 @@ bool LoadPop(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
+/// Like LoadPop above, but if any of the checks fail, we simply
+/// push the Pointer itself.
+inline bool LoadPopL(InterpState &S, CodePtr OpPC) {
+  const Pointer &Ptr = S.Stk.pop<Pointer>();
+  auto P = S.getEvalStatus().Diag;
+  S.getEvalStatus().Diag = nullptr;
+  bool Failed = false;
+  if (!CheckLoad(S, OpPC, Ptr))
+    Failed = true;
+  if (!Ptr.isBlockPointer())
+    Failed = true;
+  if (!Ptr.canDeref(PT_Ptr))
+    Failed = true;
+  S.getEvalStatus().Diag = P;
+
+  if (Failed)
+    S.Stk.push<Pointer>(Ptr);
+  else
+    S.Stk.push<Pointer>(Ptr.deref<Pointer>());
+  return true;
+}
+
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool Store(InterpState &S, CodePtr OpPC) {
   const T &Value = S.Stk.pop<T>();
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index 77ea83605cc16..7964f02ecabbf 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -2438,7 +2438,7 @@ UnsignedOrNone evaluateBuiltinObjectSize(const ASTContext &ASTCtx,
   bool ReportMinimum = (Kind & 2u);
   if (!UseFieldDesc || DetermineForCompleteObject) {
     // Can't read beyond the pointer decl desc.
-    if (!ReportMinimum && DeclDesc->getType()->isPointerType())
+    if (!ReportMinimum && DeclDesc->getDataType(ASTCtx)->isPointerType())
       return std::nullopt;
 
     if (InvalidBase)
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 3fb25a5fa0884..a5c486460fdc0 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -515,6 +515,7 @@ class LoadOpcode : Opcode {
 def Load : LoadOpcode {}
 // [Pointer] -> [Value]
 def LoadPop : LoadOpcode {}
+def LoadPopL : Opcode {}
 
 class StoreOpcode : Opcode {
   let Types = [AllTypeClass];
diff --git a/clang/test/AST/ByteCode/builtin-object-size.cpp b/clang/test/AST/ByteCode/builtin-object-size.cpp
index e4433ea700ccb..ebd98284c9972 100644
--- a/clang/test/AST/ByteCode/builtin-object-size.cpp
+++ b/clang/test/AST/ByteCode/builtin-object-size.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=both,expected %s
 // RUN: %clang_cc1                                         -verify=both,ref      %s
-
-// ref-no-diagnostics
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=both,expected %s -std=c++2c -Winvalid-constexpr
+// RUN: %clang_cc1                                         -verify=both,ref      %s -std=c++2c -Winvalid-constexpr
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -50,8 +50,27 @@ namespace InvalidBase {
   constexpr size_t bos_name = __builtin_object_size(invalid_base().name, 1); // expected-error {{must be initialized by a constant expression}} \
                                                                              // expected-note {{non-constexpr function 'invalid_base'}}
 
+#if __cplusplus < 202002L
   struct T { ~T(); };
   T invalid_base_2();
   constexpr size_t bos_dtor = __builtin_object_size(&(T&)(T&&)invalid_base_2(), 0); // expected-error {{must be initialized by a constant expression}} \
                                                                                     // expected-note {{non-literal type 'T'}}
+#endif
+}
+
+constexpr int stringLength(const char *p) { // ref-error {{never produces a constant expression}}
+  return __builtin_dynamic_object_size(p, 0); // ref-note 2{{subexpression not valid in a constant expression}}
+}
+static_assert(stringLength("hello") == 6); // ref-error {{not an integral constant expression}} \
+                                           // ref-note {{in call to}}
+#if __cplusplus >= 202002L
+
+constexpr int allocation(unsigned n) {
+  const char * ptr = new char[n];
+  int res = stringLength(ptr);
+  delete[] ptr;
+  return res;
 }
+static_assert(allocation(1) == 1);
+static_assert(allocation(14) == 14);
+#endif

``````````

</details>


https://github.com/llvm/llvm-project/pull/196548