[clang] [clang][bytecode] Diagnose comparisons with literals (PR #106734)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 30 07:06:32 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Timm Baeder (tbaederr)
<details>
<summary>Changes</summary>
This requires adding a new opcode for PointerToBoolean casts, since we otherwise emit too many diagnostics. But that fixes an older problem when casting weak pointers to bool.
---
Full diff: https://github.com/llvm/llvm-project/pull/106734.diff
7 Files Affected:
- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+1-6)
- (modified) clang/lib/AST/ByteCode/Interp.h (+43-18)
- (modified) clang/lib/AST/ByteCode/Opcodes.td (+5)
- (modified) clang/lib/AST/ByteCode/Pointer.cpp (+11)
- (modified) clang/lib/AST/ByteCode/Pointer.h (+4)
- (modified) clang/test/AST/ByteCode/builtin-functions.cpp (+6)
- (modified) clang/test/AST/ByteCode/weak.cpp (-6)
``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 9bd77edb0a550f..b4bf12ff6c757e 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -505,14 +505,9 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
case CK_MemberPointerToBoolean: {
PrimType PtrT = classifyPrim(SubExpr->getType());
- // Just emit p != nullptr for this.
if (!this->visit(SubExpr))
return false;
-
- if (!this->emitNull(PtrT, nullptr, CE))
- return false;
-
- return this->emitNE(PtrT, CE);
+ return this->emitIsNonNull(PtrT, CE);
}
case CK_IntegralComplexToBoolean:
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f8af51b71101d2..53048968bdcd3b 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -986,24 +986,7 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
}
}
- if (!Pointer::hasSameBase(LHS, RHS)) {
- if (LHS.isOnePastEnd() && !RHS.isOnePastEnd() && !RHS.isZero() &&
- RHS.getOffset() == 0) {
- const SourceInfo &Loc = S.Current->getSource(OpPC);
- S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end)
- << LHS.toDiagnosticString(S.getASTContext());
- return false;
- } else if (RHS.isOnePastEnd() && !LHS.isOnePastEnd() && !LHS.isZero() &&
- LHS.getOffset() == 0) {
- const SourceInfo &Loc = S.Current->getSource(OpPC);
- S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end)
- << RHS.toDiagnosticString(S.getASTContext());
- return false;
- }
-
- S.Stk.push<BoolT>(BoolT::from(Fn(ComparisonCategoryResult::Unordered)));
- return true;
- } else {
+ if (Pointer::hasSameBase(LHS, RHS)) {
unsigned VL = LHS.getByteOffset();
unsigned VR = RHS.getByteOffset();
@@ -1019,6 +1002,34 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR))));
return true;
}
+ // Otherwise we need to do a bunch of extra checks before returning Unordered.
+ if (LHS.isOnePastEnd() && !RHS.isOnePastEnd() && !RHS.isZero() &&
+ RHS.getOffset() == 0) {
+ const SourceInfo &Loc = S.Current->getSource(OpPC);
+ S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end)
+ << LHS.toDiagnosticString(S.getASTContext());
+ return false;
+ } else if (RHS.isOnePastEnd() && !LHS.isOnePastEnd() && !LHS.isZero() &&
+ LHS.getOffset() == 0) {
+ const SourceInfo &Loc = S.Current->getSource(OpPC);
+ S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end)
+ << RHS.toDiagnosticString(S.getASTContext());
+ return false;
+ }
+
+ // Reject comparisons to literals.
+ for (const auto &P : {LHS, RHS}) {
+ if (P.isZero())
+ continue;
+ if (P.pointsToLiteral()) {
+ const SourceInfo &Loc = S.Current->getSource(OpPC);
+ S.FFDiag(Loc, diag::note_constexpr_literal_comparison);
+ return false;
+ }
+ }
+
+ S.Stk.push<BoolT>(BoolT::from(Fn(ComparisonCategoryResult::Unordered)));
+ return true;
}
template <>
@@ -2291,6 +2302,20 @@ inline bool Null(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
return true;
}
+template <PrimType Name, class T = typename PrimConv<Name>::T>
+inline bool IsNonNull(InterpState &S, CodePtr OpPC) {
+ if constexpr (std::is_same_v<T, Pointer>) {
+ const Pointer &P = S.Stk.pop<T>();
+ if (P.isWeak())
+ return false;
+ S.Stk.push<Boolean>(Boolean::from(!P.isZero()));
+ return true;
+ }
+
+ S.Stk.push<Boolean>(Boolean::from(!S.Stk.pop<T>().isZero()));
+ return true;
+}
+
//===----------------------------------------------------------------------===//
// This, ImplicitThis
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 7374a441c8bb19..f286c71a129d1d 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -597,6 +597,11 @@ def Comp: Opcode {
let HasGroup = 1;
}
+def IsNonNull : Opcode {
+ let Types = [PtrTypeClass];
+ let HasGroup = 1;
+}
+
//===----------------------------------------------------------------------===//
// Cast, CastFP.
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 5b9e83764cfa50..9eaf0db45c7451 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -463,6 +463,17 @@ bool Pointer::hasSameArray(const Pointer &A, const Pointer &B) {
A.getFieldDesc()->IsArray;
}
+bool Pointer::pointsToLiteral() const {
+ if (isZero() || !isBlockPointer())
+ return false;
+
+ const Expr *E = block()->getDescriptor()->asExpr();
+ if (block()->isDynamic())
+ return false;
+
+ return E && !isa<MaterializeTemporaryExpr, StringLiteral>(E);
+}
+
std::optional<APValue> Pointer::toRValue(const Context &Ctx,
QualType ResultType) const {
const ASTContext &ASTCtx = Ctx.getASTContext();
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index ef90e6e0dd7bd1..d05d8e9bc1f388 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -692,6 +692,10 @@ class Pointer {
/// Checks if both given pointers point to the same block.
static bool pointToSameBlock(const Pointer &A, const Pointer &B);
+ /// Whether this points to a block that's been created for a "literal lvalue",
+ /// i.e. a non-MaterializeTemporaryExpr Expr.
+ bool pointsToLiteral() const;
+
/// Prints the pointer.
void print(llvm::raw_ostream &OS) const;
diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp
index 1cff2228cd7a97..e215597579224c 100644
--- a/clang/test/AST/ByteCode/builtin-functions.cpp
+++ b/clang/test/AST/ByteCode/builtin-functions.cpp
@@ -951,3 +951,9 @@ namespace shufflevector {
}
#endif
+
+namespace FunctionStart {
+ void a(void) {}
+ static_assert(__builtin_function_start(a) == a, ""); // both-error {{not an integral constant expression}} \
+ // both-note {{comparison of addresses of literals has unspecified value}}
+}
diff --git a/clang/test/AST/ByteCode/weak.cpp b/clang/test/AST/ByteCode/weak.cpp
index d4aac3ff764dde..0322241beef83b 100644
--- a/clang/test/AST/ByteCode/weak.cpp
+++ b/clang/test/AST/ByteCode/weak.cpp
@@ -1,14 +1,8 @@
// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter -verify=expected,both %s
// RUN: %clang_cc1 -std=c++20 -verify=ref,both %s
-
-
-
-/// FIXME: The new interpreter also emits the "address of weak declaration" note in the pointer-to-bool case.
-
[[gnu::weak]] extern int a;
int ha[(bool)&a]; // both-warning {{variable length arrays in C++ are a Clang extension}} \
- // expected-note {{comparison against address of weak declaration}} \
// both-error {{variable length array declaration not allowed at file scope}}
int ha2[&a == nullptr]; // both-warning {{variable length arrays in C++ are a Clang extension}} \
// both-note {{comparison against address of weak declaration '&a' can only be performed at runtime}} \
``````````
</details>
https://github.com/llvm/llvm-project/pull/106734
More information about the cfe-commits
mailing list