Repository navigation
fix(rst): a section's text no longer reads an unwritten byte - #2583
Merged
Merged
Conversation
rst_body cut a reST section's text at 500 bytes by setting a stop value past the end, then backing off from out[RST_BODY_MAX] while that byte looked like a UTF-8 continuation byte. That byte was never written, so where the text ended (499 or 500 bytes) depended on what the arena memory held, and when a multibyte character did not fit, bytes never written could end up in the text. Only whole characters are written now, and the text ends at the last one written. The same django tree indexed twice gave doc_link_candidates rows that differed in about 250 of 11,300 (through the section text's TF-IDF); with this change two runs are identical, and so are the 6,594 sections. The new test pins the cut at the boundary: a word that fills the 500 bytes, a two-byte character that would cross them, a word cut at a character. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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.
rst_bodycuts a reST section's text at 500 bytes. It did so by setting a stop value one past the end and then backing off fromout[RST_BODY_MAX]while that byte looked like a UTF-8 continuation byte. That byte was never written, which had two effects:Fix: only whole characters are written, and the text ends at the last one written.
Measured
doc_link_candidatesrows that differed in about 250 of 11,300. The section text feeds the candidates' TF-IDF scores, so a different cut changes them. With this change, two runs give identical rows and identical text for all 6,594 sections.Test.
rst_section_text_cutpins the cut at the boundary for three cases: a word that ends exactly at byte 500, a two-byte character that would cross it, and a word cut at a character. The words are spread over short paragraphs on purpose.docker compose -f test-infrastructure/docker-compose.yml run --rm test-msan doc_links_rst, local arm64): with the oldrst_body, the test aborts withMemorySanitizer: stack-overflow … nested bug in the same thread. That happened three runs out of three, always at the same pc. With the fix, the suite passes: 9 passed, nothing reported.Possibly related. The stack overflow appears only when the code reads the unwritten byte, so it looks like MSan failing while it reports that read. The five suites
scripts/msan.shexcludes for "stack-overflow under instrumentation" may hide real uninitialized reads the same way. This PR doesn't change those exclusions.