Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

integer overflow in std.sort.block #18252

Closed
dylan-conway opened this issue Dec 11, 2023 · 2 comments
Closed

integer overflow in std.sort.block #18252

dylan-conway opened this issue Dec 11, 2023 · 2 comments
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.

Comments

@dylan-conway
Copy link

Zig Version

0.12.0-dev.1808+69195d0cd

Steps to Reproduce and Observed Behavior

repro:
block-sort-bug.zig.zip

Found in code using std.sort.block to sort a list of 1600 semver prerelease versions. To minify the repro, the order of true/false returned from lessThan is saved to an array and returned in the same order.

original code and data:
https://github.com/oven-sh/bun/blob/800fb12906c4b9e485c6d5a6611c9e470a2183f4/src/install/npm.zig#L1660
gatsby-manifest.json

Expected Behavior

Block sort should not integer overflow. sort.heap, sort.insertion, and sort.pdq do not have any problems with the original list of semver versions or the order of true/false in the minified repro.

@dylan-conway dylan-conway added the bug Observed behavior contradicts documented or intended behavior label Dec 11, 2023
@Vexu Vexu added the standard library This issue involves writing Zig code for the standard library. label Dec 11, 2023
@Vexu Vexu added this to the 0.13.0 milestone Dec 11, 2023
@paperclover
Copy link
Contributor

some discussion came up today about this, and it seems that our sort function doesn't always follow strict weak ordering (where if lessThan(a, b) and lessThan(b, c), then lessThan(a, c) is guaranteed). i might be misunderstanding it.

actually, i just read the doc comment on this function

NOTE: The algorithm only works when the comparison is less-than or greater-than.
(See #8289)

It seems this is related, but that issue was closed with a newly added assertion for debug mode that guards against that.

I wonder if we can add an assertion for this case, but not sure.

@dylan-conway
Copy link
Author

With this bug fix std.sort.block no longer integer overflows 0cf405e32149f36bf85ec8fbd75020de5324d919. Like dave said, seems our lessThan function was inconsistent, potentially causing undefined behavior in wikisort

@Vexu Vexu removed this from the 0.13.0 milestone Dec 12, 2023
@Vexu Vexu closed this as not planned Won't fix, can't repro, duplicate, stale Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

3 participants