Skip to content

Commit 6048d96

Browse files
l46kokcopybara-github
authored andcommitted
Tighten the counterexample domain for double/int and parameterized unknown
PiperOrigin-RevId: 957339001
1 parent eb6fc20 commit 6048d96

28 files changed

Lines changed: 3041 additions & 268 deletions

BUILD.bazel

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ java_library(
9595
],
9696
)
9797

98+
java_library(
99+
name = "java_jline",
100+
exports = [
101+
"@maven//:org_jline_jline_reader",
102+
"@maven//:org_jline_jline_terminal",
103+
],
104+
)
105+
98106
default_java_toolchain(
99107
name = "repository_default_toolchain",
100108
configuration = DEFAULT_TOOLCHAIN_CONFIGURATION,

MODULE.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ maven.install(
9595
"info.picocli:picocli:4.7.7",
9696
"org.antlr:antlr4-runtime:4.13.2",
9797
"org.freemarker:freemarker:2.3.34",
98+
"org.jline:jline-reader:3.26.1",
99+
"org.jline:jline-terminal:3.26.1",
98100
"org.jspecify:jspecify:1.0.0",
99101
"org.threeten:threeten-extra:1.8.0",
100102
"org.yaml:snakeyaml:2.5",

verifier/README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,3 +433,7 @@ What this means for verification:
433433
default unless you have a specific need and bounded inputs.
434434

435435
---
436+
437+
## Tools & CLI
438+
439+
For command-line verification and interactive execution, see the [CLI Tool documentation](tools/README.md).

verifier/src/main/java/dev/cel/verifier/CelAstAlphaHasher.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424
import dev.cel.common.ast.CelConstant;
2525
import dev.cel.common.ast.CelExpr;
2626
import java.util.ArrayList;
27+
import java.util.HashMap;
2728
import java.util.List;
29+
import java.util.Map;
2830
import org.jspecify.annotations.Nullable;
2931

3032
/**
@@ -83,16 +85,11 @@ private static void hashAst(CelExpr expr, @Nullable Scope scope, HasherContext c
8385
context.hasher.putByte((byte) 0); // 0 = bound
8486
context.hasher.putInt(bIdx);
8587
} else {
86-
int fIdx = -1;
87-
for (int i = 0; i < context.freeVars.size(); i++) {
88-
if (context.freeVars.get(i).ident().name().equals(name)) {
89-
fIdx = i;
90-
break;
91-
}
92-
}
93-
if (fIdx == -1) {
88+
Integer fIdx = context.freeVarIndices.get(name);
89+
if (fIdx == null) {
9490
context.freeVars.add(expr);
9591
fIdx = context.freeVars.size() - 1;
92+
context.freeVarIndices.put(name, fIdx);
9693
}
9794
context.hasher.putByte((byte) 1); // 1 = free
9895
context.hasher.putInt(fIdx);
@@ -125,9 +122,11 @@ private static void hashAst(CelExpr expr, @Nullable Scope scope, HasherContext c
125122
}
126123
break;
127124
case STRUCT:
125+
context.hasher.putInt(expr.struct().messageName().length());
128126
context.hasher.putString(expr.struct().messageName(), UTF_8);
129127
context.hasher.putInt(expr.struct().entries().size());
130128
for (CelExpr.CelStruct.Entry entry : expr.struct().entries()) {
129+
context.hasher.putInt(entry.fieldKey().length());
131130
context.hasher.putString(entry.fieldKey(), UTF_8);
132131
context.hasher.putBoolean(entry.optionalEntry());
133132
hashAst(entry.value(), scope, context);
@@ -210,6 +209,7 @@ private static void hashConstant(CelConstant constant, HasherContext context) {
210209

211210
private static final class HasherContext {
212211
final Hasher hasher;
212+
final Map<String, Integer> freeVarIndices = new HashMap<>();
213213
final List<CelExpr> freeVars = new ArrayList<>();
214214

215215
HasherContext(HashFunction hashFunction) {

verifier/src/main/java/dev/cel/verifier/CelAstToZ3Translator.java

Lines changed: 77 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -741,8 +741,10 @@ private TranslatedValue translateCall(CelExpr expr, CelAbstractSyntaxTree ast) {
741741
typeConstraints.add(ctx.mkNot(typeSystem.isUnknown(callRes)));
742742
typeConstraints.add(ctx.mkNot(typeSystem.isError(callRes)));
743743

744+
boolean isDynamic = ast.getTypeOrThrow(exprId).equals(SimpleType.DYN);
745+
BoolExpr isApprox = ctx.mkBool(!isDynamic);
744746
return TranslatedValue.propagateStrict(
745-
ctx, typeSystem, callRes, Optional.of(expr), ctx.mkTrue(), args);
747+
ctx, typeSystem, callRes, Optional.of(expr), isApprox, args);
746748
});
747749
}
748750

@@ -875,10 +877,6 @@ private TranslatedValue translateDynamicComprehension(
875877
ArrayExpr mapPresence =
876878
isMap ? (ArrayExpr) typeSystem.getMapPresence(typeSystem.getMapRef(iterRange)) : null;
877879

878-
if (isMap) {
879-
applyBoundedMapBijection(mapPresence, seq, lengthExpr);
880-
}
881-
882880
BoolExpr isTruncated = ctx.mkGt(lengthExpr, ctx.mkInt(comprehensionUnrollLimit));
883881
truncationConditions.add(isTruncated);
884882

@@ -891,14 +889,15 @@ private TranslatedValue translateDynamicComprehension(
891889
}
892890
}
893891

894-
private void applyBoundedMapBijection(
892+
private BoolExpr getBoundedMapBijection(
895893
ArrayExpr mapPresence, SeqExpr<?> seq, ArithExpr lengthExpr) {
894+
List<BoolExpr> constraints = new ArrayList<>();
896895
for (int i = 0; i < comprehensionUnrollLimit; i++) {
897896
for (int j = i + 1; j < comprehensionUnrollLimit; j++) {
898897
BoolExpr validPair = ctx.mkLt(ctx.mkInt(j), lengthExpr);
899898
BoolExpr notEqual =
900899
ctx.mkNot(ctx.mkEq(ctx.mkNth(seq, ctx.mkInt(i)), ctx.mkNth(seq, ctx.mkInt(j))));
901-
typeConstraints.add(ctx.mkImplies(validPair, notEqual));
900+
constraints.add(ctx.mkImplies(validPair, notEqual));
902901
}
903902
}
904903

@@ -913,7 +912,8 @@ private void applyBoundedMapBijection(
913912
ctx.mkStore(seqMap, ctx.mkNth(seq, ctx.mkInt(i)), ctx.mkTrue()),
914913
seqMap);
915914
}
916-
typeConstraints.add(ctx.mkImplies(isNotTruncated, ctx.mkEq(mapPresence, seqMap)));
915+
constraints.add(ctx.mkImplies(isNotTruncated, ctx.mkEq(mapPresence, seqMap)));
916+
return CelZ3TypeSystem.mkAndFlattened(ctx, constraints);
917917
}
918918

919919
private TranslatedValue[] evaluateLoopCondAndStep(
@@ -1245,9 +1245,10 @@ private BoolExpr createTypeConstraintForType(Expr<?> val, CelType type) {
12451245
}
12461246
Expr<?> optRef = typeSystem.getOptionalRef(val);
12471247
BoolExpr hasValue = typeSystem.optHasValue(optRef);
1248-
BoolExpr valConstraint =
1249-
createTypeConstraintForType(typeSystem.getOptionalValue(optRef), paramType);
1250-
return ctx.mkAnd(isOpt, ctx.mkImplies(hasValue, valConstraint));
1248+
Expr<?> optVal = typeSystem.getOptionalValue(optRef);
1249+
BoolExpr optValNotError = ctx.mkNot(typeSystem.isError(optVal));
1250+
BoolExpr valConstraint = createTypeConstraintForType(optVal, paramType);
1251+
return ctx.mkAnd(isOpt, ctx.mkImplies(hasValue, ctx.mkAnd(optValNotError, valConstraint)));
12511252
}
12521253
if (type.equals(SimpleType.BOOL)) {
12531254
return (BoolExpr) ctx.mkApp(typeSystem.boolCons().getTesterDecl(), val);
@@ -1287,15 +1288,13 @@ private BoolExpr createTypeConstraintForType(Expr<?> val, CelType type) {
12871288
}
12881289

12891290
if (type instanceof ListType) {
1290-
// Lists are explicitly bounded (sequence theory). We're safe in using for-all quantifiers
1291-
// here.
1291+
// Constrain list elements using bounded unrolling up to comprehensionUnrollLimit rather
1292+
// than Z3 forall quantifiers to prevent MBQI quantifier instantiation loops.
1293+
// Assert: isList(val) ∧ for all unrolled 0 <= i < length: ¬isError(seq[i]) ∧
1294+
// typeConstraint(seq[i])
12921295
BoolExpr isList = typeSystem.isList(val);
12931296
CelType elemType = ((ListType) type).elemType();
1294-
if (elemType.equals(SimpleType.DYN)) {
1295-
return isList;
1296-
}
12971297

1298-
// isList(val) ∧ ∀i. (0 <= i < length) ⇒ elemType(seq[i])
12991298
Expr<?> listRef = typeSystem.getListRef(val);
13001299
SeqExpr seq = typeSystem.getSeq(listRef);
13011300
Expr length = ctx.mkLength(seq);
@@ -1305,20 +1304,70 @@ private BoolExpr createTypeConstraintForType(Expr<?> val, CelType type) {
13051304
for (int i = 0; i < comprehensionUnrollLimit; i++) {
13061305
IntExpr idx = ctx.mkInt(i);
13071306
Expr elem = ctx.mkNth(seq, idx);
1308-
BoolExpr elemConstraint = createTypeConstraintForType(elem, elemType);
13091307
BoolExpr validIndex = ctx.mkLt(idx, length);
1310-
boundsAndTypes.add(ctx.mkImplies(validIndex, elemConstraint));
1311-
BoolExpr outOfBounds = ctx.mkGe(idx, length);
1312-
boundsAndTypes.add(ctx.mkImplies(outOfBounds, ctx.mkEq(elem, typeSystem.mkUnknown())));
1308+
// Assert ¬isError(elem) as a domain invariant so Z3 never synthesizes an Error element in
1309+
// list(dyn). For concrete types, this is already implied by createTypeConstraintForType.
1310+
boundsAndTypes.add(ctx.mkImplies(validIndex, ctx.mkNot(typeSystem.isError(elem))));
1311+
// Short-circuit DYN element types to prevent generating redundant validIndex ⇒ TRUE
1312+
// clauses.
1313+
if (!elemType.equals(SimpleType.DYN)) {
1314+
BoolExpr elemConstraint = createTypeConstraintForType(elem, elemType);
1315+
boundsAndTypes.add(ctx.mkImplies(validIndex, elemConstraint));
1316+
}
13131317
}
13141318

13151319
return CelZ3TypeSystem.mkAndFlattened(ctx, boundsAndTypes);
13161320
}
13171321
if (type instanceof MapType) {
1318-
// Do NOT emit a for-all quantifier over map keys here.
1319-
// Doing so forces MBQI into an infinite loop. Structural equivalence of dynamic keys is
1320-
// naturally constrained by the primitive key assertions in getStructuralEquality().
1321-
return typeSystem.isMap(val);
1322+
// Do NOT emit a for-all quantifier over map keys or values here.
1323+
// Doing so forces MBQI into an infinite loop. Instead, constrain keys and values using
1324+
// bounded unrolling over the key sequence up to comprehensionUnrollLimit.
1325+
// Assert: isMap(val) ∧ for all unrolled 0 <= i < length: isPrimitiveKey(key) ∧ ¬isError(key)
1326+
// ∧ (presence(key) ⇒ ¬isError(val) ∧ typeConstraint(val))
1327+
BoolExpr isMap = typeSystem.isMap(val);
1328+
MapType mapType = (MapType) type;
1329+
CelType keyType = mapType.keyType();
1330+
CelType valType = mapType.valueType();
1331+
1332+
Expr<?> mapRef = typeSystem.getMapRef(val);
1333+
SeqExpr seq = typeSystem.getMapKeys(mapRef);
1334+
Expr length = ctx.mkLength(seq);
1335+
ArrayExpr mapValues = (ArrayExpr) typeSystem.getMapValues(mapRef);
1336+
ArrayExpr mapPresence = (ArrayExpr) typeSystem.getMapPresence(mapRef);
1337+
1338+
List<BoolExpr> boundsAndTypes = new ArrayList<>();
1339+
boundsAndTypes.add(isMap);
1340+
boundsAndTypes.add(getBoundedMapBijection(mapPresence, seq, (ArithExpr) length));
1341+
1342+
for (int i = 0; i < comprehensionUnrollLimit; i++) {
1343+
IntExpr idx = ctx.mkInt(i);
1344+
Expr key = ctx.mkNth(seq, idx);
1345+
BoolExpr validIndex = ctx.mkLt(idx, length);
1346+
1347+
BoolExpr isKeyPrim = typeSystem.isPrimitiveKey(key);
1348+
BoolExpr keyNotError = ctx.mkNot(typeSystem.isError(key));
1349+
// Assert isKeyPrim ∧ ¬isError(key) so Z3 never synthesizes a non-primitive or Error key in
1350+
// map(dyn, ...). For concrete map types, this is already implied by keyType constraints.
1351+
boundsAndTypes.add(ctx.mkImplies(validIndex, ctx.mkAnd(isKeyPrim, keyNotError)));
1352+
// Short-circuit DYN key types to prevent generating redundant validIndex ⇒ TRUE clauses.
1353+
if (!keyType.equals(SimpleType.DYN)) {
1354+
boundsAndTypes.add(ctx.mkImplies(validIndex, createTypeConstraintForType(key, keyType)));
1355+
}
1356+
1357+
BoolExpr presence = (BoolExpr) ctx.mkSelect(mapPresence, key);
1358+
BoolExpr validEntry = ctx.mkAnd(validIndex, presence);
1359+
1360+
Expr mapVal = ctx.mkSelect(mapValues, key);
1361+
BoolExpr valNotError = ctx.mkNot(typeSystem.isError(mapVal));
1362+
boundsAndTypes.add(ctx.mkImplies(validEntry, valNotError));
1363+
// Short-circuit DYN value types to prevent generating redundant validEntry ⇒ TRUE clauses.
1364+
if (!valType.equals(SimpleType.DYN)) {
1365+
boundsAndTypes.add(
1366+
ctx.mkImplies(validEntry, createTypeConstraintForType(mapVal, valType)));
1367+
}
1368+
}
1369+
1370+
return CelZ3TypeSystem.mkAndFlattened(ctx, boundsAndTypes);
13221371
}
13231372
if (type.kind() == CelKind.STRUCT) {
13241373
return ctx.mkAnd(
@@ -1371,6 +1420,9 @@ private Optional<Object> toCacheKey(CelExpr expr) {
13711420
case CONSTANT:
13721421
return Optional.of(expr.constant());
13731422
case LIST:
1423+
if (!expr.list().optionalIndices().isEmpty()) {
1424+
return Optional.empty(); // Safely skip caching lists with optional elements
1425+
}
13741426
ImmutableList.Builder<Object> builder = ImmutableList.builder();
13751427
for (CelExpr elem : expr.list().elements()) {
13761428
Optional<Object> elemKey = toCacheKey(elem);

verifier/src/main/java/dev/cel/verifier/CelVerifierZ3Impl.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,8 +303,10 @@ CelVerificationResult verifyImplication(
303303
/* isCounterexample= */ true));
304304
case TRUNCATED:
305305
return CelVerificationResult.inconclusive(
306-
String.format("Inconclusive: %s holds within the current loop unroll limit, but"
307-
+ " may be violated for larger collections.", subjectName.toLowerCase(Locale.US)));
306+
String.format(
307+
"Inconclusive: %s holds within the current loop unroll limit, but"
308+
+ " may be violated for larger collections.",
309+
subjectName.toLowerCase(Locale.US)));
308310
case NO_MATCH:
309311
return CelVerificationResult.verified();
310312
case SOLVER_UNKNOWN:

0 commit comments

Comments
 (0)