Repository navigation
check_decode: add Tree's map fields so the map-depth rows test the decoder - #54
Merged
Merged
Conversation
The corpus's Tree gained map<string, Tree> branches and
map<string, string> tags (protowire#449). check_decode's PB leg decodes
into a hand-written struct, so it skipped both as unknown and the
pb/deep-map-101 and pb/deep-scalar-map-101 rows failed for that reason
alone. With the fields added, every row passes: the decoders already
count a map's { and every PB map entry.
The Tree map holds unique_ptr so its value type is complete at the
member's declaration, as the standard requires.
Advances trendvidia/protowire#449
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Without this change, two new corpus rows fail for this port, and only because its test harness ignores the fields they use. The C++ decoders needed no fix: they already count a map's
{and every PB map entry towardMaxNestingDepth, on the fast path too.Advances trendvidia/protowire#449, with the corpus rows in protowire PR #465.
Changes
check_decode's hand-writtenTreegainsbranchesandtags. Its PB leg decodes into that struct, so it was skipping both fields as unknown. PXF needed nothing, because it compiles the corpus.protoat run time.branchesholdsunique_ptr<Tree>, which keeps the map's value type complete insideTree.Evidence
All gate runs:
CPP_DIR=<this worktree> SKIP_PORTS=go,ts,java,rust,swift,dart,csharp,python bash scripts/cross_security_check.sh, from thecorpus/map-depth-449branch.pb/deep-map-101.binpbandpb/deep-scalar-map-101.binpbasFAIL_VERDICT. All four PXF map rows passed.Ran 46 (port, corpus) pairs across 1 ports.andAll ports conform. The other 7 of the 53 rows skip C++ in the manifest. Run by hand, each-100row exits 0 and each-101row exits 1 withnesting depth exceeds MaxNestingDepth=100.Descend()/Ascend()inDecodeMapInline(the fast path) and the map-entry check inpb.h. The gate then failedpxf/deep-nesting-maps-101.pxf,pxf/deep-nesting-scalar-map-101.pxfandpb/deep-scalar-map-101.binpb.pb/deep-map-101still passed, because a message value is counted on its own. Both changes were reverted.ctest --test-dir build --output-on-failure:100% tests passed out of 298.No decoder line changed, so no unit test is added. These bounds are pinned by the corpus rows, which do not run in this repo's CI.