[clang] [clang][bytecode] Try to load from pointers in `ToLValue` mode (PR #196548)
https://github.com/efriedma-quic commented: Is this really what we want to do here? It seems dubious to try to load from a pointer, then just return the pointer itself if the load fails. https://github.com/llvm/llvm-project/pull/196548 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][bytecode] Try to load from pointers in `ToLValue` mode (PR #196548)
https://github.com/tbaederr updated
https://github.com/llvm/llvm-project/pull/196548
>From b7c7f1870fe889bda7a1b3a22b0647e327545d9b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?=
Date: Fri, 8 May 2026 06:37:44 +0200
Subject: [PATCH] asdf
---
clang/lib/AST/ByteCode/Compiler.cpp | 8 +--
clang/lib/AST/ByteCode/Interp.h | 22 ++
clang/lib/AST/ByteCode/InterpBuiltin.cpp | 2 +-
clang/lib/AST/ByteCode/Opcodes.td | 1 +
.../test/AST/ByteCode/builtin-object-size.cpp | 23 +--
5 files changed, 51 insertions(+), 5 deletions(-)
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::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();
+ 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(Ptr);
+ else
+S.Stk.push(Ptr.deref());
+ return true;
+}
+
template ::T>
bool Store(InterpState &S, CodePtr OpPC) {
const T &Value = S.Stk.pop();
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
[clang] [clang][bytecode] Try to load from pointers in `ToLValue` mode (PR #196548)
llvmorg-github-actions[bot] wrote:
@llvm/pr-subscribers-clang
Author: Timm Baeder (tbaederr)
Changes
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::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();
+ 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(Ptr);
+ else
+S.Stk.push(Ptr.deref());
+ return true;
+}
+
template ::T>
bool Store(InterpState &S, CodePtr OpPC) {
const T &Value = S.Stk.pop();
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 {{no
[clang] [clang][bytecode] Try to load from pointers in `ToLValue` mode (PR #196548)
https://github.com/tbaederr created
https://github.com/llvm/llvm-project/pull/196548
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.
>From b7c7f1870fe889bda7a1b3a22b0647e327545d9b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?=
Date: Fri, 8 May 2026 06:37:44 +0200
Subject: [PATCH] asdf
---
clang/lib/AST/ByteCode/Compiler.cpp | 8 +--
clang/lib/AST/ByteCode/Interp.h | 22 ++
clang/lib/AST/ByteCode/InterpBuiltin.cpp | 2 +-
clang/lib/AST/ByteCode/Opcodes.td | 1 +
.../test/AST/ByteCode/builtin-object-size.cpp | 23 +--
5 files changed, 51 insertions(+), 5 deletions(-)
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::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();
+ 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(Ptr);
+ else
+S.Stk.push(Ptr.deref());
+ return true;
+}
+
template ::T>
bool Store(InterpState &S, CodePtr OpPC) {
const T &Value = S.Stk.pop();
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 expressi
