Skip to content

Commit 7719de5

Browse files
l46kokcopybara-github
authored andcommitted
Add support for optional field pruning in verifier
PiperOrigin-RevId: 951556122
1 parent c11a0f5 commit 7719de5

5 files changed

Lines changed: 219 additions & 60 deletions

File tree

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

Lines changed: 14 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,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) {
90+
fIdx = context.freeVars.size();
9491
context.freeVars.add(expr);
95-
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);
@@ -121,6 +118,10 @@ private static void hashAst(CelExpr expr, @Nullable Scope scope, HasherContext c
121118
for (CelExpr elem : expr.list().elements()) {
122119
hashAst(elem, scope, context);
123120
}
121+
context.hasher.putInt(expr.list().optionalIndices().size());
122+
for (int optIndex : expr.list().optionalIndices()) {
123+
context.hasher.putInt(optIndex);
124+
}
124125
break;
125126
case STRUCT:
126127
context.hasher.putString(expr.struct().messageName(), UTF_8);
@@ -208,10 +209,13 @@ private static void hashConstant(CelConstant constant, HasherContext context) {
208209

209210
private static final class HasherContext {
210211
final Hasher hasher;
211-
final List<CelExpr> freeVars = new ArrayList<>();
212+
final List<CelExpr> freeVars;
213+
final Map<String, Integer> freeVarIndices;
212214

213215
HasherContext(HashFunction hashFunction) {
214216
this.hasher = hashFunction.newHasher();
217+
this.freeVars = new ArrayList<>();
218+
this.freeVarIndices = new HashMap<>();
215219
}
216220
}
217221

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

Lines changed: 141 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.util.ArrayList;
4949
import java.util.Arrays;
5050
import java.util.HashMap;
51+
import java.util.HashSet;
5152
import java.util.LinkedHashSet;
5253
import java.util.List;
5354
import java.util.Map;
@@ -88,7 +89,11 @@ final class CelAstToZ3Translator {
8889
private final ImmutableSet<String> unknownIdentifiers;
8990
private final int comprehensionUnrollLimit;
9091
private final CelTypeProvider typeProvider;
91-
private final List<BoolExpr> truncationConditions;
92+
private final Set<BoolExpr> truncationConditions;
93+
private final Set<SeqExpr<?>> boundedMapBijections;
94+
private final Set<Expr<?>> appliedBijections;
95+
private final Map<Long, TranslatedValue> pureExprCache;
96+
private final Set<String> activeLoopVars;
9297
private final Map<String, Expr<?>> emptyMessageCache;
9398
private final Map<Object, Expr<?>> listLiteralCache;
9499
private Expr<?> emptyListCache;
@@ -102,7 +107,7 @@ Set<BoolExpr> getTypeConstraints() {
102107
}
103108

104109
BoolExpr hasTruncation() {
105-
return CelZ3TypeSystem.mkOrFlattened(ctx, truncationConditions);
110+
return CelZ3TypeSystem.mkOrFlattened(ctx, new ArrayList<>(truncationConditions));
106111
}
107112

108113
CelZ3TypeSystem getTypeSystem() {
@@ -264,12 +269,14 @@ private TranslatedValue translateIdent(CelExpr celExpr, CelAbstractSyntaxTree as
264269

265270
return TranslatedValue.create(
266271
v,
267-
CelExpr.newBuilder().setIdent(ident).setId(exprId).build(),
272+
Optional.of(CelExpr.newBuilder().setIdent(ident).setId(exprId).build()),
268273
typeSystem,
269-
/* isApproximate= */ ctx.mkFalse());
274+
/* isApproximate= */ ctx.mkFalse(),
275+
/* isLoopInvariant= */ true);
270276
});
271277

272-
return TranslatedValue.create(tv.z3Expr(), celExpr, typeSystem, tv.isApproximate());
278+
boolean isLoopInvariant = !activeLoopVars.contains(name) && tv.isLoopInvariant();
279+
return TranslatedValue.create(tv.z3Expr(), Optional.of(celExpr), typeSystem, tv.isApproximate(), isLoopInvariant);
273280
}
274281

275282
private TranslatedValue translateList(CelExpr celExpr, CelAbstractSyntaxTree ast) {
@@ -284,11 +291,23 @@ private TranslatedValue translateList(CelExpr celExpr, CelAbstractSyntaxTree ast
284291
// check to a trivial identity check (e.g., `list_ref_0 == list_ref_0`).
285292
if (listRef == null) {
286293
SeqExpr seq = ctx.mkEmptySeq(ctx.mkSeqSort(typeSystem.celValueSort()));
287-
for (CelExpr element : createList.elements()) {
294+
ImmutableSet<Integer> optionalIndices = ImmutableSet.copyOf(createList.optionalIndices());
295+
for (int i = 0; i < createList.elements().size(); i++) {
296+
CelExpr element = createList.elements().get(i);
288297
TranslatedValue elem = translateExpr(element, ast);
289298
elementsTv.add(elem);
290299

291-
seq = typeSystem.mkConcatSafe(seq, ctx.mkUnit(elem.z3Expr()));
300+
boolean isOptional = optionalIndices.contains(i);
301+
OptionalUnwrap opt = unwrapOptional(elem.z3Expr(), isOptional);
302+
SeqExpr optSeq =
303+
isOptional
304+
? (SeqExpr)
305+
ctx.mkITE(
306+
opt.isPresent,
307+
ctx.mkUnit(opt.effectiveValue),
308+
ctx.mkEmptySeq(ctx.mkSeqSort(typeSystem.celValueSort())))
309+
: ctx.mkUnit(opt.effectiveValue);
310+
seq = typeSystem.mkConcatSafe(seq, optSeq);
292311
}
293312
listRef = typeSystem.mkListRefConst(LIST_REF_PREFIX);
294313
typeConstraints.add(ctx.mkEq(typeSystem.getSeq(listRef), seq));
@@ -297,7 +316,9 @@ private TranslatedValue translateList(CelExpr celExpr, CelAbstractSyntaxTree ast
297316
}
298317

299318
Expr<?> result = typeSystem.wrapList(listRef);
300-
return TranslatedValue.propagateStrict(ctx, typeSystem, result, celExpr, elementsTv);
319+
BoolExpr baseTaint = ctx.mkFalse();
320+
return TranslatedValue.propagateStrict(
321+
ctx, typeSystem, result, Optional.of(celExpr), baseTaint, elementsTv);
301322
}
302323

303324
private TranslatedValue translateMap(CelExpr celExpr, CelAbstractSyntaxTree ast) {
@@ -318,20 +339,31 @@ private TranslatedValue translateMap(CelExpr celExpr, CelAbstractSyntaxTree ast)
318339
Expr<?> value = valueTv.z3Expr();
319340
elementsTv.add(valueTv);
320341

321-
BoolExpr keyAlreadyPresent = (BoolExpr) ctx.mkSelect(mapPresence, key);
322-
keysSeq =
323-
ctx.mkITE(keyAlreadyPresent, keysSeq, typeSystem.mkConcatSafe(keysSeq, ctx.mkUnit(key)));
342+
OptionalUnwrap opt = unwrapOptional(value, entryAst.optionalEntry());
324343

325-
mapValues = ctx.mkStore(mapValues, key, value);
326-
mapPresence = ctx.mkStore(mapPresence, key, ctx.mkTrue());
344+
BoolExpr keyAlreadyPresent = (BoolExpr) ctx.mkSelect(mapPresence, key);
345+
BoolExpr shouldAddKey = ctx.mkAnd(opt.isPresent, ctx.mkNot(keyAlreadyPresent));
346+
347+
SeqExpr keyOptSeq =
348+
(SeqExpr)
349+
ctx.mkITE(
350+
shouldAddKey,
351+
ctx.mkUnit(key),
352+
ctx.mkEmptySeq(ctx.mkSeqSort(typeSystem.celValueSort())));
353+
354+
keysSeq = typeSystem.mkConcatSafe(keysSeq, keyOptSeq);
355+
mapValues = storeIf(opt.isPresent, mapValues, key, opt.effectiveValue);
356+
mapPresence = storeIf(opt.isPresent, mapPresence, key, ctx.mkTrue());
327357
}
328358

329359
typeConstraints.add(ctx.mkEq(typeSystem.getMapValues(mapRef), mapValues));
330360
typeConstraints.add(ctx.mkEq(typeSystem.getMapPresence(mapRef), mapPresence));
331361
typeConstraints.add(ctx.mkEq(typeSystem.getMapKeys(mapRef), keysSeq));
332362

333363
Expr<?> result = typeSystem.wrapMap(mapRef);
334-
return TranslatedValue.propagateStrict(ctx, typeSystem, result, celExpr, elementsTv);
364+
BoolExpr baseTaint = ctx.mkFalse();
365+
return TranslatedValue.propagateStrict(
366+
ctx, typeSystem, result, Optional.of(celExpr), baseTaint, elementsTv);
335367
}
336368

337369
private TranslatedValue translateStruct(CelExpr celExpr, CelAbstractSyntaxTree ast) {
@@ -379,15 +411,15 @@ private TranslatedValue translateStruct(CelExpr celExpr, CelAbstractSyntaxTree a
379411
// (`msg1 == msg2`) to work without using quantifiers (which avoids MBQI loops).
380412
// Because proto3 singular primitives do not have field presence, we also skip setting
381413
// `msgPresence`.
414+
OptionalUnwrap opt = unwrapOptional(value, entryAst.optionalEntry());
415+
382416
BoolExpr shouldBypass =
383-
fieldType.kind().isPrimitive() ? ctx.mkEq(value, defaultVal) : ctx.mkFalse();
417+
fieldType.kind().isPrimitive() ? ctx.mkEq(opt.effectiveValue, defaultVal) : ctx.mkFalse();
384418

385-
msgValues =
386-
(ArrayExpr) ctx.mkITE(shouldBypass, msgValues, ctx.mkStore(msgValues, key, value));
419+
BoolExpr shouldStore = ctx.mkAnd(opt.isPresent, ctx.mkNot(shouldBypass));
387420

388-
msgPresence =
389-
(ArrayExpr)
390-
ctx.mkITE(shouldBypass, msgPresence, ctx.mkStore(msgPresence, key, ctx.mkTrue()));
421+
msgValues = storeIf(shouldStore, msgValues, key, opt.effectiveValue);
422+
msgPresence = storeIf(shouldStore, msgPresence, key, ctx.mkTrue());
391423
}
392424

393425
typeConstraints.add(
@@ -396,7 +428,9 @@ private TranslatedValue translateStruct(CelExpr celExpr, CelAbstractSyntaxTree a
396428
typeConstraints.add(ctx.mkEq(typeSystem.getMsgPresence(msgRef), msgPresence));
397429

398430
Expr<?> result = typeSystem.wrapMessage(msgRef);
399-
return TranslatedValue.propagateStrict(ctx, typeSystem, result, celExpr, elementsTv);
431+
BoolExpr baseTaint = ctx.mkFalse();
432+
return TranslatedValue.propagateStrict(
433+
ctx, typeSystem, result, Optional.of(celExpr), baseTaint, elementsTv);
400434
}
401435

402436
private Expr<?> getDefaultValueForType(CelType type) {
@@ -472,6 +506,28 @@ private Expr<?> getDefaultValueForType(CelType type) {
472506
return typeSystem.mkUnknown();
473507
}
474508

509+
private static final class OptionalUnwrap {
510+
final BoolExpr isPresent;
511+
final Expr<?> effectiveValue;
512+
513+
OptionalUnwrap(BoolExpr isPresent, Expr<?> effectiveValue) {
514+
this.isPresent = isPresent;
515+
this.effectiveValue = effectiveValue;
516+
}
517+
}
518+
519+
private OptionalUnwrap unwrapOptional(Expr<?> value, boolean isOptional) {
520+
if (!isOptional) {
521+
return new OptionalUnwrap(ctx.mkTrue(), value);
522+
}
523+
Expr<?> optRef = typeSystem.getOptionalRef(value);
524+
return new OptionalUnwrap(typeSystem.optHasValue(optRef), typeSystem.getOptionalValue(optRef));
525+
}
526+
527+
private ArrayExpr storeIf(BoolExpr condition, ArrayExpr array, Expr<?> key, Expr<?> value) {
528+
return (ArrayExpr) ctx.mkITE(condition, ctx.mkStore(array, key, value), array);
529+
}
530+
475531
private static final class FieldAccess {
476532
final Expr<?> presence;
477533
final Expr<?> value;
@@ -657,24 +713,53 @@ private TranslatedValue translateComprehension(CelExpr celExpr, CelAbstractSynta
657713
// For statically known list/map literals, unroll them exactly.
658714
if (iterRangeExpr.exprKind().getKind() == ExprKind.Kind.LIST) {
659715
ImmutableList<CelExpr> elements = iterRangeExpr.list().elements();
716+
ImmutableList<Integer> optionalIndices = iterRangeExpr.list().optionalIndices();
660717
for (int i = 0; i < elements.size(); i++) {
661718
TranslatedValue valueTv = translateExpr(elements.get(i), ast);
662-
Expr<?> value = valueTv.z3Expr();
663-
taints.add(valueTv.isApproximate());
664-
iterationElements.add(new IterationElement(typeSystem.mkInt(i), value));
665-
allRangeElems.add(value);
719+
boolean isOptional = optionalIndices.contains(i);
720+
OptionalUnwrap opt = unwrapOptional(valueTv.z3Expr(), isOptional);
721+
if (isOptional && ctx.mkFalse().equals(opt.isPresent)) {
722+
continue;
723+
}
724+
taints.add(
725+
isOptional
726+
? CelZ3TypeSystem.mkAndFlattened(ctx, opt.isPresent, valueTv.isApproximate())
727+
: valueTv.isApproximate());
728+
iterationElements.add(
729+
new IterationElement(
730+
typeSystem.mkInt(i),
731+
opt.effectiveValue,
732+
isOptional ? Optional.of(opt.isPresent) : Optional.empty()));
733+
allRangeElems.add(
734+
isOptional
735+
? ctx.mkITE(opt.isPresent, opt.effectiveValue, typeSystem.mkInt(0))
736+
: opt.effectiveValue);
666737
}
667738
} else if (iterRangeExpr.exprKind().getKind() == ExprKind.Kind.MAP) {
668739
for (CelExpr.CelMap.Entry entry : iterRangeExpr.map().entries()) {
669740
TranslatedValue keyTv = translateExpr(entry.key(), ast);
670741
Expr<?> key = keyTv.z3Expr();
671742
taints.add(keyTv.isApproximate());
672743
TranslatedValue valueTv = translateExpr(entry.value(), ast);
673-
Expr<?> value = valueTv.z3Expr();
674-
taints.add(valueTv.isApproximate());
675-
iterationElements.add(new IterationElement(key, value));
744+
boolean isOptional = entry.optionalEntry();
745+
OptionalUnwrap opt = unwrapOptional(valueTv.z3Expr(), isOptional);
746+
if (isOptional && ctx.mkFalse().equals(opt.isPresent)) {
747+
continue;
748+
}
749+
taints.add(
750+
isOptional
751+
? CelZ3TypeSystem.mkAndFlattened(ctx, opt.isPresent, valueTv.isApproximate())
752+
: valueTv.isApproximate());
753+
iterationElements.add(
754+
new IterationElement(
755+
key,
756+
opt.effectiveValue,
757+
isOptional ? Optional.of(opt.isPresent) : Optional.empty()));
676758
allRangeElems.add(key);
677-
allRangeElems.add(value);
759+
allRangeElems.add(
760+
isOptional
761+
? ctx.mkITE(opt.isPresent, opt.effectiveValue, typeSystem.mkInt(0))
762+
: opt.effectiveValue);
678763
}
679764
} else {
680765
return translateDynamicComprehension(celExpr, ast);
@@ -693,13 +778,23 @@ private TranslatedValue translateComprehension(CelExpr celExpr, CelAbstractSynta
693778
comp, ast, iterElem.keyOrIndex, iterElem.value, currentAccu, isMap, isTwoVar);
694779
Expr<?> condition = condAndStep[0].z3Expr();
695780
Expr<?> step = condAndStep[1].z3Expr();
696-
taints.add(condAndStep[1].isApproximate());
781+
if (iterElem.hasValue.isPresent()) {
782+
taints.add(
783+
CelZ3TypeSystem.mkAndFlattened(
784+
ctx, iterElem.hasValue.get(), condAndStep[1].isApproximate()));
785+
} else {
786+
taints.add(condAndStep[1].isApproximate());
787+
}
697788

698789
Expr<?> stepVal = ctx.mkITE((BoolExpr) typeSystem.unwrapBool(condition), step, currentAccu);
699790
Expr<?> typeErrorOrStep =
700791
typeSystem.withRuntimeError(stepVal, ctx.mkNot(typeSystem.isBool(condition)));
701792

702-
accu = typeSystem.propagateErrorAndUnknown(typeErrorOrStep, condition);
793+
Expr<?> updatedAccu = typeSystem.propagateErrorAndUnknown(typeErrorOrStep, condition);
794+
accu =
795+
iterElem.hasValue.isPresent()
796+
? ctx.mkITE(iterElem.hasValue.get(), updatedAccu, currentAccu)
797+
: updatedAccu;
703798
}
704799

705800
TranslatedValue resultTv =
@@ -773,6 +868,9 @@ private TranslatedValue translateDynamicComprehension(
773868

774869
private void applyBoundedMapBijection(
775870
ArrayExpr mapPresence, SeqExpr<?> seq, ArithExpr lengthExpr) {
871+
if (!boundedMapBijections.add(seq)) {
872+
return;
873+
}
776874
for (int i = 0; i < comprehensionUnrollLimit; i++) {
777875
for (int j = i + 1; j < comprehensionUnrollLimit; j++) {
778876
BoolExpr validPair = ctx.mkLt(ctx.mkInt(j), lengthExpr);
@@ -1223,16 +1321,26 @@ private BoolExpr createTypeConstraintForType(Expr<?> val, CelType type) {
12231321
this.emptyMessageCache = new HashMap<>();
12241322
this.listLiteralCache = new HashMap<>();
12251323
this.typeProvider = typeProvider;
1226-
this.truncationConditions = new ArrayList<>();
1324+
this.truncationConditions = new LinkedHashSet<>();
1325+
this.boundedMapBijections = new LinkedHashSet<>();
1326+
this.appliedBijections = new HashSet<>();
1327+
this.pureExprCache = new HashMap<>();
1328+
this.activeLoopVars = new LinkedHashSet<>();
12271329
}
12281330

12291331
private static class IterationElement {
12301332
final Expr<?> keyOrIndex;
12311333
final Expr<?> value;
1334+
final Optional<BoolExpr> hasValue;
12321335

12331336
IterationElement(Expr<?> keyOrIndex, Expr<?> value) {
1337+
this(keyOrIndex, value, Optional.empty());
1338+
}
1339+
1340+
IterationElement(Expr<?> keyOrIndex, Expr<?> value, Optional<BoolExpr> hasValue) {
12341341
this.keyOrIndex = keyOrIndex;
12351342
this.value = value;
1343+
this.hasValue = hasValue;
12361344
}
12371345
}
12381346

@@ -1249,6 +1357,7 @@ private Optional<Object> toCacheKey(CelExpr expr) {
12491357
}
12501358
builder.add(elemKey.get());
12511359
}
1360+
builder.add(expr.list().optionalIndices());
12521361
return Optional.of(builder.build());
12531362
default:
12541363
return Optional.empty();

0 commit comments

Comments
 (0)