Skip to content

Commit ed9860b

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

29 files changed

Lines changed: 2936 additions & 170 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: 75 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ 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.getType(exprId).map(SimpleType.DYN::equals).orElse(true);
744+
boolean isDynamic = ast.getTypeOrThrow(exprId).equals(SimpleType.DYN);
745745
BoolExpr isApprox = ctx.mkBool(!isDynamic);
746746
return TranslatedValue.propagateStrict(
747747
ctx, typeSystem, callRes, Optional.of(expr), isApprox, args);
@@ -877,10 +877,6 @@ private TranslatedValue translateDynamicComprehension(
877877
ArrayExpr mapPresence =
878878
isMap ? (ArrayExpr) typeSystem.getMapPresence(typeSystem.getMapRef(iterRange)) : null;
879879

880-
if (isMap) {
881-
applyBoundedMapBijection(mapPresence, seq, lengthExpr);
882-
}
883-
884880
BoolExpr isTruncated = ctx.mkGt(lengthExpr, ctx.mkInt(comprehensionUnrollLimit));
885881
truncationConditions.add(isTruncated);
886882

@@ -893,14 +889,15 @@ private TranslatedValue translateDynamicComprehension(
893889
}
894890
}
895891

896-
private void applyBoundedMapBijection(
892+
private BoolExpr getBoundedMapBijection(
897893
ArrayExpr mapPresence, SeqExpr<?> seq, ArithExpr lengthExpr) {
894+
List<BoolExpr> constraints = new ArrayList<>();
898895
for (int i = 0; i < comprehensionUnrollLimit; i++) {
899896
for (int j = i + 1; j < comprehensionUnrollLimit; j++) {
900897
BoolExpr validPair = ctx.mkLt(ctx.mkInt(j), lengthExpr);
901898
BoolExpr notEqual =
902899
ctx.mkNot(ctx.mkEq(ctx.mkNth(seq, ctx.mkInt(i)), ctx.mkNth(seq, ctx.mkInt(j))));
903-
typeConstraints.add(ctx.mkImplies(validPair, notEqual));
900+
constraints.add(ctx.mkImplies(validPair, notEqual));
904901
}
905902
}
906903

@@ -915,7 +912,8 @@ private void applyBoundedMapBijection(
915912
ctx.mkStore(seqMap, ctx.mkNth(seq, ctx.mkInt(i)), ctx.mkTrue()),
916913
seqMap);
917914
}
918-
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);
919917
}
920918

921919
private TranslatedValue[] evaluateLoopCondAndStep(
@@ -1247,9 +1245,10 @@ private BoolExpr createTypeConstraintForType(Expr<?> val, CelType type) {
12471245
}
12481246
Expr<?> optRef = typeSystem.getOptionalRef(val);
12491247
BoolExpr hasValue = typeSystem.optHasValue(optRef);
1250-
BoolExpr valConstraint =
1251-
createTypeConstraintForType(typeSystem.getOptionalValue(optRef), paramType);
1252-
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)));
12531252
}
12541253
if (type.equals(SimpleType.BOOL)) {
12551254
return (BoolExpr) ctx.mkApp(typeSystem.boolCons().getTesterDecl(), val);
@@ -1289,15 +1288,13 @@ private BoolExpr createTypeConstraintForType(Expr<?> val, CelType type) {
12891288
}
12901289

12911290
if (type instanceof ListType) {
1292-
// Lists are explicitly bounded (sequence theory). We're safe in using for-all quantifiers
1293-
// 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])
12941295
BoolExpr isList = typeSystem.isList(val);
12951296
CelType elemType = ((ListType) type).elemType();
1296-
if (elemType.equals(SimpleType.DYN)) {
1297-
return isList;
1298-
}
12991297

1300-
// isList(val) ∧ ∀i. (0 <= i < length) ⇒ elemType(seq[i])
13011298
Expr<?> listRef = typeSystem.getListRef(val);
13021299
SeqExpr seq = typeSystem.getSeq(listRef);
13031300
Expr length = ctx.mkLength(seq);
@@ -1307,20 +1304,70 @@ private BoolExpr createTypeConstraintForType(Expr<?> val, CelType type) {
13071304
for (int i = 0; i < comprehensionUnrollLimit; i++) {
13081305
IntExpr idx = ctx.mkInt(i);
13091306
Expr elem = ctx.mkNth(seq, idx);
1310-
BoolExpr elemConstraint = createTypeConstraintForType(elem, elemType);
13111307
BoolExpr validIndex = ctx.mkLt(idx, length);
1312-
boundsAndTypes.add(ctx.mkImplies(validIndex, elemConstraint));
1313-
BoolExpr outOfBounds = ctx.mkGe(idx, length);
1314-
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+
}
13151317
}
13161318

13171319
return CelZ3TypeSystem.mkAndFlattened(ctx, boundsAndTypes);
13181320
}
13191321
if (type instanceof MapType) {
1320-
// Do NOT emit a for-all quantifier over map keys here.
1321-
// Doing so forces MBQI into an infinite loop. Structural equivalence of dynamic keys is
1322-
// naturally constrained by the primitive key assertions in getStructuralEquality().
1323-
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);
13241371
}
13251372
if (type.kind() == CelKind.STRUCT) {
13261373
return ctx.mkAnd(
@@ -1373,6 +1420,9 @@ private Optional<Object> toCacheKey(CelExpr expr) {
13731420
case CONSTANT:
13741421
return Optional.of(expr.constant());
13751422
case LIST:
1423+
if (!expr.list().optionalIndices().isEmpty()) {
1424+
return Optional.empty(); // Safely skip caching lists with optional elements
1425+
}
13761426
ImmutableList.Builder<Object> builder = ImmutableList.builder();
13771427
for (CelExpr elem : expr.list().elements()) {
13781428
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)