test: make buffer sizes 32bit-aware in test-internal-util-construct-sab#61026
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nodejs-github-bot added needs-ci 
Renegade334 added dont-land-on-v20.x 
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #61026 +/- ## ========================================== - Coverage 88.55% 88.53% -0.02% ========================================== Files 703 703 Lines 208522 208522 Branches 40219 40205 -14 ========================================== - Hits 184647 184609 -38 - Misses 15905 15919 +14 - Partials 7970 7994 +24 🚀 New features to boost your workflow:
|
| // Specifically test the following cases: | ||
| // - out-of-range allocation requests should not crash the process | ||
| // - no int64 overflow | ||
| for(constlengthof[-1,kMaxLength+1,2**64]){ |
There was a problem hiding this comment.
Don't we want to keep Number.MAX_SAFE_INTEGER + 1 test case?
There was a problem hiding this comment.
The intention behind Number.MAX_SAFE_INTEGER was that this is the maximum array buffer size on 64-bit builds, so that particular test value lives on as kMaxLength + 1.
There was a problem hiding this comment.
IMO it's still interesting to test with an "unsafe" integer
There was a problem hiding this comment.
That's kinda covered by 2^64, but can add this if preferred
There was a problem hiding this comment.
If it had to be one or the other, I would pick Number.MAX_SAFE_INTEGER + 1 over 2 ** 64, that seems more interesting / intent revelling to me. I don't feel strongly either way, so non-blocking comment
There was a problem hiding this comment.
The significance of 2^64 is that the size is passed via an int64_t intermediate, so has potential to overflow if not correctly clamped. It's a little academic, I can't imagine any of our internals will be allocating terabytes of shared memory 😀
aduh95 added author ready 
github-actionsbot removed the request-ci 
nodejs-github-bot commented Dec 19, 2025
nodejs-github-bot commented Dec 20, 2025
aduh95 added the commit-queue nodejs-github-bot removed the commit-queue nodejs-github-bot merged commit ed6ec96 into nodejs:mainUh oh!
There was an error while loading. Please reload this page.
nodejs-github-bot commented Dec 20, 2025
Landed in ed6ec96 |
PR-URL: nodejs#61026Fixes: nodejs#61025 Refs: nodejs#60497 Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #61026Fixes: #61025 Refs: #60497 Reviewed-By: Antoine du Hamel <[email protected]>
3 participants
Fixes: #61025
Refs: #60497
The range checks for bound SharedArrayBuffer construction weren't compatible with the maximum array buffer size on 32-bit architectures.