Skip to content

Commit 21c9700

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

34 files changed

Lines changed: 3049 additions & 187 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/BUILD.bazel

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@ java_library(
4141
exports = ["//verifier/src/main/java/dev/cel/verifier:verifier_factory"],
4242
)
4343

44+
java_library(
45+
name = "numeric_bounds",
46+
compatible_with = [],
47+
visibility = [":verifier_internal"],
48+
exports = ["//verifier/src/main/java/dev/cel/verifier:numeric_bounds"],
49+
)
50+
4451
java_library(
4552
name = "type_system",
4653
compatible_with = [],

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/BUILD.bazel

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,27 @@ java_library(
9191
],
9292
)
9393

94+
java_library(
95+
name = "numeric_bounds",
96+
srcs = ["CelNumericBounds.java"],
97+
compatible_with = [],
98+
tags = [
99+
],
100+
deps = [
101+
"//:auto_value",
102+
"//common/annotations",
103+
"@maven//:com_google_guava_guava",
104+
],
105+
)
106+
94107
java_library(
95108
name = "type_system",
96109
srcs = ["CelZ3TypeSystem.java"],
97110
compatible_with = [],
98111
tags = [
99112
],
100113
deps = [
114+
":numeric_bounds",
101115
"//common/internal:proto_time_utils",
102116
"@maven//:com_google_errorprone_error_prone_annotations",
103117
"@maven//:com_google_guava_guava",
@@ -121,6 +135,7 @@ java_library(
121135
tags = [
122136
],
123137
deps = [
138+
":numeric_bounds",
124139
":type_system",
125140
":verifier",
126141
"//:auto_value",

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

Lines changed: 6 additions & 10 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,29 +85,22 @@ 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);
9996
}
10097
break;
10198
case SELECT:
10299
hashAst(expr.select().operand(), scope, context);
103-
context.hasher.putInt(expr.select().field().length());
104100
context.hasher.putString(expr.select().field(), UTF_8);
105101
context.hasher.putBoolean(expr.select().testOnly());
106102
break;
107103
case CALL:
108-
context.hasher.putInt(expr.call().function().length());
109104
context.hasher.putString(expr.call().function(), UTF_8);
110105
context.hasher.putBoolean(expr.call().target().isPresent());
111106
if (expr.call().target().isPresent()) {
@@ -210,6 +205,7 @@ private static void hashConstant(CelConstant constant, HasherContext context) {
210205

211206
private static final class HasherContext {
212207
final Hasher hasher;
208+
final Map<String, Integer> freeVarIndices = new HashMap<>();
213209
final List<CelExpr> freeVars = new ArrayList<>();
214210

215211
HasherContext(HashFunction hashFunction) {

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

Lines changed: 73 additions & 28 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(
@@ -1230,7 +1228,7 @@ private BoolExpr createTypeConstraint(Expr<?> val, long exprId, CelAbstractSynta
12301228
.orElseThrow(
12311229
() -> new IllegalArgumentException("Type not found for expr ID: " + exprId));
12321230
BoolExpr typeConstraint = createTypeConstraintForType(val, type);
1233-
return ctx.mkOr(typeSystem.isError(val), typeSystem.isUnknown(val), typeConstraint);
1231+
return ctx.mkOr(typeSystem.isErrorOrUnknown(val), typeConstraint);
12341232
}
12351233

12361234
private BoolExpr createTypeConstraintForType(Expr<?> val, CelType type) {
@@ -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);
@@ -1258,15 +1257,15 @@ private BoolExpr createTypeConstraintForType(Expr<?> val, CelType type) {
12581257
Expr<?> unwrapped = ctx.mkApp(typeSystem.intCons().getAccessorDecls()[0], val);
12591258
return ctx.mkAnd(
12601259
ctx.mkApp(typeSystem.intCons().getTesterDecl(), val),
1261-
ctx.mkGe((ArithExpr) unwrapped, ctx.mkInt(CelZ3TypeSystem.MIN_INT64)),
1262-
ctx.mkLe((ArithExpr) unwrapped, ctx.mkInt(CelZ3TypeSystem.MAX_INT64)));
1260+
ctx.mkGe((ArithExpr) unwrapped, ctx.mkInt(CelNumericBounds.MIN_INT64)),
1261+
ctx.mkLe((ArithExpr) unwrapped, ctx.mkInt(CelNumericBounds.MAX_INT64)));
12631262
}
12641263
if (type.equals(SimpleType.UINT)) {
12651264
Expr<?> unwrapped = ctx.mkApp(typeSystem.uintCons().getAccessorDecls()[0], val);
12661265
return ctx.mkAnd(
12671266
ctx.mkApp(typeSystem.uintCons().getTesterDecl(), val),
12681267
ctx.mkGe((ArithExpr) unwrapped, ctx.mkInt(0)),
1269-
ctx.mkLe((ArithExpr) unwrapped, ctx.mkInt(CelZ3TypeSystem.MAX_UINT64)));
1268+
ctx.mkLe((ArithExpr) unwrapped, ctx.mkInt(CelNumericBounds.MAX_UINT64)));
12701269
}
12711270
if (type.equals(SimpleType.DOUBLE)) {
12721271
return (BoolExpr) ctx.mkApp(typeSystem.doubleCons().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,62 @@ 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);
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+
BoolExpr elemConstraint = createTypeConstraintForType(elem, elemType);
13121312
boundsAndTypes.add(ctx.mkImplies(validIndex, elemConstraint));
1313-
BoolExpr outOfBounds = ctx.mkGe(idx, length);
1314-
boundsAndTypes.add(ctx.mkImplies(outOfBounds, ctx.mkEq(elem, typeSystem.mkUnknown())));
13151313
}
13161314

13171315
return CelZ3TypeSystem.mkAndFlattened(ctx, boundsAndTypes);
13181316
}
13191317
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);
1318+
// Do NOT emit a for-all quantifier over map keys or values here.
1319+
// Doing so forces MBQI into an infinite loop. Instead, constrain keys and values using
1320+
// bounded unrolling over the key sequence up to comprehensionUnrollLimit.
1321+
// Assert: isMap(val) ∧ for all unrolled 0 <= i < length: isPrimitiveKey(key) ∧ ¬isError(key)
1322+
// ∧ (presence(key) ⇒ ¬isError(val) ∧ typeConstraint(val))
1323+
BoolExpr isMap = typeSystem.isMap(val);
1324+
MapType mapType = (MapType) type;
1325+
CelType keyType = mapType.keyType();
1326+
CelType valType = mapType.valueType();
1327+
1328+
Expr<?> mapRef = typeSystem.getMapRef(val);
1329+
SeqExpr seq = typeSystem.getMapKeys(mapRef);
1330+
Expr length = ctx.mkLength(seq);
1331+
ArrayExpr mapValues = (ArrayExpr) typeSystem.getMapValues(mapRef);
1332+
ArrayExpr mapPresence = (ArrayExpr) typeSystem.getMapPresence(mapRef);
1333+
1334+
List<BoolExpr> boundsAndTypes = new ArrayList<>();
1335+
boundsAndTypes.add(isMap);
1336+
boundsAndTypes.add(getBoundedMapBijection(mapPresence, seq, (ArithExpr) length));
1337+
1338+
for (int i = 0; i < comprehensionUnrollLimit; i++) {
1339+
IntExpr idx = ctx.mkInt(i);
1340+
Expr key = ctx.mkNth(seq, idx);
1341+
BoolExpr validIndex = ctx.mkLt(idx, length);
1342+
1343+
BoolExpr isKeyPrim = typeSystem.isPrimitiveKey(key);
1344+
BoolExpr keyNotError = ctx.mkNot(typeSystem.isError(key));
1345+
// Assert isKeyPrim ∧ ¬isError(key) so Z3 never synthesizes a non-primitive or Error key in
1346+
// map(dyn, ...). For concrete map types, this is already implied by keyType constraints.
1347+
boundsAndTypes.add(ctx.mkImplies(validIndex, ctx.mkAnd(isKeyPrim, keyNotError)));
1348+
boundsAndTypes.add(ctx.mkImplies(validIndex, createTypeConstraintForType(key, keyType)));
1349+
1350+
BoolExpr presence = (BoolExpr) ctx.mkSelect(mapPresence, key);
1351+
BoolExpr validEntry = ctx.mkAnd(validIndex, presence);
1352+
1353+
Expr mapVal = ctx.mkSelect(mapValues, key);
1354+
BoolExpr valNotError =
1355+
unknownIdentifiers.isEmpty()
1356+
? ctx.mkNot(typeSystem.isErrorOrUnknown(mapVal))
1357+
: ctx.mkNot(typeSystem.isError(mapVal));
1358+
boundsAndTypes.add(ctx.mkImplies(validEntry, valNotError));
1359+
boundsAndTypes.add(ctx.mkImplies(validEntry, createTypeConstraintForType(mapVal, valType)));
1360+
}
1361+
1362+
return CelZ3TypeSystem.mkAndFlattened(ctx, boundsAndTypes);
13241363
}
13251364
if (type.kind() == CelKind.STRUCT) {
13261365
return ctx.mkAnd(
@@ -1373,6 +1412,12 @@ private Optional<Object> toCacheKey(CelExpr expr) {
13731412
case CONSTANT:
13741413
return Optional.of(expr.constant());
13751414
case LIST:
1415+
if (!expr.list().optionalIndices().isEmpty()) {
1416+
// Do not cache lists with optional elements. Optional elements conditionally alter
1417+
// sequence length and presence via ITE branches at runtime; caching would collide
1418+
// [1, 2] with [?1, 2] and freeze conditional evaluations to a static reference.
1419+
return Optional.empty();
1420+
}
13761421
ImmutableList.Builder<Object> builder = ImmutableList.builder();
13771422
for (CelExpr elem : expr.list().elements()) {
13781423
Optional<Object> elemKey = toCacheKey(elem);

0 commit comments

Comments
 (0)