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.

Sign up for GitHub

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

fix: 32bit build fails #390

Merged
merged 2 commits into from
Apr 30, 2024
Merged

fix: 32bit build fails #390

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f200cac
fix: 32bit build fails
zeroshade Apr 30, 2024
07476d4
split value for 32-bit vs 64-bit
zeroshade Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const (
defaultMaxByteSliceSize = 1_048_576 // 1 MiB
// Max allocation size for an array due to the limit in number of bits in a heap address:
// https://github.com/golang/go/blob/7f76c00fc5678fa782708ba8fece63750cb89d03/src/runtime/malloc.go#L183
maxAllocSize = int(1 << 48)
maxAllocSize = int64(1 << 48)
)

// DefaultConfig is the default API.
Expand Down Expand Up @@ -279,8 +279,8 @@ func (c *frozenConfig) getMaxByteSliceSize() int {

func (c *frozenConfig) getMaxSliceAllocSize() int {
size := c.config.MaxSliceAllocSize
if size > maxAllocSize || size <= 0 {
return maxAllocSize
if int64(size) > maxAllocSize || size <= 0 {
return int(maxAllocSize)
Copy link
Member

Choose a reason for hiding this comment

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

Would this not overflow on a 32bit system? Perhaps the entire function needs to return an int64?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, if I change that to be an int64 we end up with an issue in codec_array.go because sliceType.UnsafeGrow accepts an int and not an int64. Which means we'd just be pushing the overflow to that spot instead.

Maybe we need to change the maxAllocSize to a different value for 32-bit systems?

Copy link
Member

Choose a reason for hiding this comment

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

That seems reasonable. I am a little surprised the actual value is not different between these systems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated this with my most recent commit. Let me know what you think

}
return size
}
Loading