[clang] [clang][Interp] Do r-to-l conversion immediately when returning (PR #80662)

Timm Baeder via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 5 07:58:56 PST 2024


https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/80662

>From 91c130565aca5e915140aacf3ffca5c59b689523 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 5 Feb 2024 10:29:45 +0100
Subject: [PATCH] [clang][Interp] Do r-to-l conversion immediately when
 returning

First, we need to register local constant variables in C, so we get
the same diagnostic behavior as the current interpeter.

Second, when returning an LValue (as a Pointer), which we eventually
convert to an RValue, we need to do the conversion immediately when
saving the Pointer in the EvaluationResult. Otherwise, we will
possibly deallocate the data before doing the conversion (which will
look at the Block*).
---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp |  3 +--
 clang/lib/AST/Interp/Context.cpp         | 16 ++++------------
 clang/lib/AST/Interp/EvalEmitter.cpp     | 20 ++++++++++++++++++--
 clang/lib/AST/Interp/EvalEmitter.h       |  5 ++++-
 clang/lib/AST/Interp/EvaluationResult.h  |  2 +-
 clang/test/AST/Interp/c.c                | 13 +++++++++++++
 clang/test/Sema/warn-char-subscripts.c   |  1 +
 7 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 74a5284ff812c..7d9df217a4aab 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -3009,8 +3009,7 @@ bool ByteCodeExprGen<Emitter>::VisitDeclRefExpr(const DeclRefExpr *E) {
   // This happens in C.
   if (!Ctx.getLangOpts().CPlusPlus) {
     if (const auto *VD = dyn_cast<VarDecl>(D);
-        VD && VD->hasGlobalStorage() && VD->getAnyInitializer() &&
-        VD->getType().isConstQualified()) {
+        VD && VD->getAnyInitializer() && VD->getType().isConstQualified()) {
       if (!this->visitVarDecl(VD))
         return false;
       // Retry.
diff --git a/clang/lib/AST/Interp/Context.cpp b/clang/lib/AST/Interp/Context.cpp
index 5f5a6622f10f3..061f4e1f35779 100644
--- a/clang/lib/AST/Interp/Context.cpp
+++ b/clang/lib/AST/Interp/Context.cpp
@@ -44,7 +44,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
   assert(Stk.empty());
   ByteCodeExprGen<EvalEmitter> C(*this, *P, Parent, Stk, Result);
 
-  auto Res = C.interpretExpr(E);
+  auto Res = C.interpretExpr(E, /*ConvertResultToRValue=*/E->isGLValue());
 
   if (Res.isInvalid()) {
     Stk.clear();
@@ -58,16 +58,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
   Stk.clear();
 #endif
 
-  // Implicit lvalue-to-rvalue conversion.
-  if (E->isGLValue()) {
-    std::optional<APValue> RValueResult = Res.toRValue();
-    if (!RValueResult) {
-      return false;
-    }
-    Result = *RValueResult;
-  } else {
-    Result = Res.toAPValue();
-  }
+  Result = Res.toAPValue();
 
   return true;
 }
@@ -120,7 +111,8 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
         !Res.checkFullyInitialized(C.getState()))
       return false;
 
-    // lvalue-to-rvalue conversion.
+    // lvalue-to-rvalue conversion. We do this manually here so we can
+    // examine the result above before converting and returning it.
     std::optional<APValue> RValueResult = Res.toRValue();
     if (!RValueResult)
       return false;
diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp
index a60f893de8bda..5a36b46b74543 100644
--- a/clang/lib/AST/Interp/EvalEmitter.cpp
+++ b/clang/lib/AST/Interp/EvalEmitter.cpp
@@ -33,7 +33,9 @@ EvalEmitter::~EvalEmitter() {
   }
 }
 
-EvaluationResult EvalEmitter::interpretExpr(const Expr *E) {
+EvaluationResult EvalEmitter::interpretExpr(const Expr *E,
+                                            bool ConvertResultToRValue) {
+  this->ConvertResultToRValue = ConvertResultToRValue;
   EvalResult.setSource(E);
 
   if (!this->visitExpr(E))
@@ -119,12 +121,26 @@ template <PrimType OpType> bool EvalEmitter::emitRet(const SourceInfo &Info) {
 template <> bool EvalEmitter::emitRet<PT_Ptr>(const SourceInfo &Info) {
   if (!isActive())
     return true;
-  EvalResult.setPointer(S.Stk.pop<Pointer>());
+
+  const Pointer &Ptr = S.Stk.pop<Pointer>();
+  // Implicitly convert lvalue to rvalue, if requested.
+  if (ConvertResultToRValue) {
+    if (std::optional<APValue> V = Ptr.toRValue(Ctx)) {
+      EvalResult.setValue(*V);
+    } else {
+      return false;
+    }
+  } else {
+    EvalResult.setPointer(Ptr);
+  }
+
   return true;
 }
 template <> bool EvalEmitter::emitRet<PT_FnPtr>(const SourceInfo &Info) {
   if (!isActive())
     return true;
+  // Function pointers are always lvalues to us and cannot be converted
+  // to rvalues, so don't do any conversion here.
   EvalResult.setFunctionPointer(S.Stk.pop<FunctionPointer>());
   return true;
 }
diff --git a/clang/lib/AST/Interp/EvalEmitter.h b/clang/lib/AST/Interp/EvalEmitter.h
index deb2ebc4e61fa..8159e489f168e 100644
--- a/clang/lib/AST/Interp/EvalEmitter.h
+++ b/clang/lib/AST/Interp/EvalEmitter.h
@@ -34,7 +34,8 @@ class EvalEmitter : public SourceMapper {
   using AddrTy = uintptr_t;
   using Local = Scope::Local;
 
-  EvaluationResult interpretExpr(const Expr *E);
+  EvaluationResult interpretExpr(const Expr *E,
+                                 bool ConvertResultToRValue = false);
   EvaluationResult interpretDecl(const VarDecl *VD);
 
   InterpState &getState() { return S; }
@@ -86,6 +87,8 @@ class EvalEmitter : public SourceMapper {
   InterpState S;
   /// Location to write the result to.
   EvaluationResult EvalResult;
+  /// Whether the result should be converted to an RValue.
+  bool ConvertResultToRValue = false;
 
   /// Temporaries which require storage.
   llvm::DenseMap<unsigned, std::unique_ptr<char[]>> Locals;
diff --git a/clang/lib/AST/Interp/EvaluationResult.h b/clang/lib/AST/Interp/EvaluationResult.h
index 2b9fc16f1a0ab..52a6c011e39e1 100644
--- a/clang/lib/AST/Interp/EvaluationResult.h
+++ b/clang/lib/AST/Interp/EvaluationResult.h
@@ -56,8 +56,8 @@ class EvaluationResult final {
   void setSource(DeclTy D) { Source = D; }
 
   void setValue(const APValue &V) {
+    // V could still be an LValue.
     assert(empty());
-    assert(!V.isLValue());
     Value = std::move(V);
     Kind = RValue;
   }
diff --git a/clang/test/AST/Interp/c.c b/clang/test/AST/Interp/c.c
index 53ce62fa1452e..0ecd4eec8390c 100644
--- a/clang/test/AST/Interp/c.c
+++ b/clang/test/AST/Interp/c.c
@@ -125,3 +125,16 @@ _Static_assert(sizeof(name2) == 0, ""); // expected-error {{failed}} \
                                         // expected-note {{evaluates to}} \
                                         // pedantic-expected-error {{failed}} \
                                         // pedantic-expected-note {{evaluates to}}
+
+void t14(void) {
+  int array[256] = { 0 }; // expected-note {{array 'array' declared here}} \
+                          // pedantic-expected-note {{array 'array' declared here}} \
+                          // ref-note {{array 'array' declared here}} \
+                          // pedantic-ref-note {{array 'array' declared here}}
+  const char b = -1;
+  int val = array[b]; // expected-warning {{array index -1 is before the beginning of the array}} \
+                      // pedantic-expected-warning {{array index -1 is before the beginning of the array}} \
+                      // ref-warning {{array index -1 is before the beginning of the array}} \
+                      // pedantic-ref-warning {{array index -1 is before the beginning of the array}}
+
+}
diff --git a/clang/test/Sema/warn-char-subscripts.c b/clang/test/Sema/warn-char-subscripts.c
index 0a012f68feae0..c2f7a3731d72c 100644
--- a/clang/test/Sema/warn-char-subscripts.c
+++ b/clang/test/Sema/warn-char-subscripts.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -Wchar-subscripts -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wchar-subscripts -fsyntax-only -verify %s -fexperimental-new-constant-interpreter
 
 void t1(void) {
   int array[1] = { 0 };



More information about the cfe-commits mailing list