Bytecode Interpreter: Representing negative pointer offsets #155453
Description
In test/AST/ByteCode/const-eval.c, we have this test:
#define EVAL_EXPR(testno, expr) enum { test##testno = (expr) }; struct check_positive##testno { int a[test##testno]; };
int x;
EVAL_EXPR(48, &x != &x - 1 ? 1 : -1)which uses a negative offset on &x. If we add:
diff --git i/clang/lib/AST/ExprConstant.cpp w/clang/lib/AST/ExprConstant.cpp
index d5c5c827e6fc..9cd6498b56ec 100644
--- i/clang/lib/AST/ExprConstant.cpp
+++ w/clang/lib/AST/ExprConstant.cpp
@@ -14852,6 +14852,10 @@ EvaluateComparisonBinaryOperator(EvalInfo &Info, const BinaryOperator *E,
if (!EvaluatePointer(E->getRHS(), RHSValue, Info) || !LHSOK)
return false;
+
+ llvm::errs() << "LHS: " << LHSValue.toString(Info.Ctx, E->getLHS()->getType()) << '\n';
+ llvm::errs() << "RHS: " << RHSValue.toString(Info.Ctx, E->getRHS()->getType()) << '\n';
+
// Reject differing bases from the normal codepath; we special-case
// comparisons to null.
if (!HasSameBase(LHSValue, RHSValue)) {we can see that the current interpreter correctly saves the negative offset:
LHS: &x
RHS: &x + -1however, we currently save Pointer::Offset as a uint64_t, so we don't get this right.
If we apply
diff --git i/clang/lib/AST/ByteCode/Interp.h w/clang/lib/AST/ByteCode/Interp.h
index 92e60b6b88e6..fc008ab26452 100644
--- i/clang/lib/AST/ByteCode/Interp.h
+++ w/clang/lib/AST/ByteCode/Interp.h
@@ -1079,6 +1079,12 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
const Pointer &RHS = S.Stk.pop<Pointer>();
const Pointer &LHS = S.Stk.pop<Pointer>();
+
+ llvm::errs() << LHS << " / " << RHS << '\n';
+ llvm::errs() << LHS.toDiagnosticString(S.getASTContext()) << " / " << RHS.toDiagnosticString(S.getASTContext()) << '\n';
+
+
+
if (LHS.isZero() && RHS.isZero()) {
S.Stk.push<BoolT>(BoolT::from(Fn(ComparisonCategoryResult::Equal)));
return true;
``` we get the following output:
```console
(Block) 0x7dd20e47b150 {rootptr(0), 0, 8} / (Block) 0x7dd20e47b150 {rootptr(0), 28, 8}
&x / &x + 1That looks almost correct, however, that's just a coincidence. The offset underflows to -1, to which we then add sizeof(InitMapPtr), which happens to be 32. And 32 - 4 = 28. If we change the example to use -7 instead, we get a different result that's clearly wrong:
(Block) 0x7d53cfc7b150 {rootptr(0), 0, 8} / (Block) 0x7d53cfc7b150 {rootptr(0), 4, 8}
&x / &xIn this case, the two pointers also compare equal.
Even if we change Pointer::Offset to a int64_t, we still run into weird errors because that value doesn't just represent the value that was added verbatim but we e.g. multiply it by the element size and/or add the size of the InitMapPtr.