Skip to content

Fix int64_t overflow in signed varint decoder - #30

Merged
tbybell merged 2 commits into
gtkwave:mainfrom
Rodrigodd:upstream/svarint-overflow
Oct 18, 2026
Merged

tbybell merged 2 commits into
gtkwave:mainfrom
Rodrigodd:upstream/svarint-overflow

Conversation

@Rodrigodd

Copy link
Copy Markdown
Contributor

Problem: fstGetSVarint64() accumulates the varint in an int64_t, so a long enough varint shifts past the width of the type, which is undefined behaviour and is reported by UBSan.

Solution: accumulate in a uint64_t, dropping the bits that do not fit.

Problem: fstGetSVarint64() accumulates the varint in an int64_t, so a
long enough varint shifts past the width of the type, which is undefined
behaviour and is reported by UBSan.

Solution: accumulate in a uint64_t, dropping the bits that do not fit.
@Rodrigodd

Rodrigodd commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor Author

CI failed on Windows, but it was in a pre-existing test. I think it is unrelated to the changes here and is probably due to some race/timing issue.

An AI review pointed out that tmpfile_open on Windows uses GetTempFileName with uUnique set to 0, which derives the filename from the current system time. This means that multiple tests running in parallel are very likely to cause problems.

I will create another branch with a fix for that. Rerunning the CI here should make it pass.

Edit: actually GetTempFileName guarantees that the filename is unique while the file exists, but that function opens the file with unlink_fopen which deletes and recreate the file non-atomically, creating a gap where another process could open the same file.

@tbybell

tbybell commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

OK, #30 passes checks now, thanks.

@tbybell
tbybell merged commit 17f6a5d into gtkwave:main Oct 18, 2026
3 checks passed
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