Skip to content

Validate RV32I prebuilt count metadata - #14

Merged
jserv merged 1 commit into
sysprog21:mainfrom
thc1006:rv32i-count-metadata
Aug 3, 2026
Merged

Validate RV32I prebuilt count metadata#14
jserv merged 1 commit into
sysprog21:mainfrom
thc1006:rv32i-count-metadata

Conversation

@thc1006

@thc1006 thc1006 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

rv32i-prebuilt.sh treats count.txt as optional and strips all non-digit characters before comparing it. As a result, a release without the file can pass, and a value such as 1x2 is interpreted as 12.

Require count.txt to contain exactly one positive decimal integer. Return 77 when the metadata is missing, malformed, or does not match the number of conformance ELF files. Add offline cases covering each condition.

make verify-prebuilt passes.
make check-all passes.


Summary by cubic

Enforces strict validation of count.txt in RV32I prebuilts to prevent truncated or miscounted suites.
rv32i-prebuilt.sh now requires exactly one positive decimal via rv32i-count.sh and compares as strings; missing, malformed, or mismatched counts exit 77, offline metadata cases and live smoke use the same helper, and the full pass-case runs only when count.txt matches the ELFs.

Written for commit 2d9a7d0. Summary will update on new commits.

Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

@thc1006
thc1006 force-pushed the rv32i-count-metadata branch from 44864fd to 82c888f Compare August 3, 2026 07:02

@jserv jserv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the metadata parser, the guard ordering, the stubs, and the portability. The change is correct and well covered. The strict parser resolves every fixture plus the un-fixtured edges (empty, trailing newline versus a real second line, CRLF, whitespace, huge decimal), requiring count.txt does not regress a legitimate release (a demos-only release skips at the conformance == 0 guard first), and the set -e command-substitution idiom is portable in dash. The switch from -ne to a string compare also avoids the old arithmetic error on a malformed count.

meta_case leading-zero-count '01\n'
meta_case junk-count '1x\n'
meta_case multiline-count '1\n\n'
meta_case mismatched-count '2\n'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thorough fixture set. The parser also correctly rejects a few inputs not pinned here: CRLF (1\r\n), space padded ( 1\n or 1 \n), and it accepts an arbitrarily large decimal as a string. Fixtures for those would guard the behavior against a future parser change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank u Prof. for pointing out the remaining parser edges. I add fixtures for CRLF and leading/trailing whitespace, along with a direct parser check that preserves an arbitrarily large decimal verbatim. The direct check distinguish a valid-but-mismatched large count from a malformed count, since both would otherwise make the end-to-end consumer return 77.

Comment thread tests/rv32i-prebuilt-test.sh Outdated
@thc1006
thc1006 force-pushed the rv32i-count-metadata branch from 82c888f to c1a1a98 Compare August 3, 2026 07:38
rv32i-prebuilt.sh treats count.txt as optional and removes all
non-digit characters before comparing it. A release without the file
can pass, and a value such as 1x2 is read as 12.

Require one positive decimal count and return 77 when the metadata is
missing, malformed, or does not match the number of conformance ELF
files. Put that rule in rv32i-count.sh so the script and its contract
test cannot drift on it, add offline cases for these inputs, and gate
the full pass-case on a count.txt matching the elfs beside it, so a
partial build tree skips instead of reddening the gate.

make verify-prebuilt passes.
make check-all passes.

Signed-off-by: thc1006 <hctsai1006@cs.nctu.edu.tw>
@thc1006
thc1006 force-pushed the rv32i-count-metadata branch from c1a1a98 to 2d9a7d0 Compare August 3, 2026 07:58
@jserv
jserv merged commit b222d8a into sysprog21:main Aug 3, 2026
6 checks passed
@jserv

jserv commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Thank @thc1006 for contributing!

@thc1006
thc1006 deleted the rv32i-count-metadata branch August 3, 2026 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants