Skip to content

Conversation

folkertdev
Copy link
Collaborator

@folkertdev folkertdev commented Apr 1, 2025

Using an incorrect alignment is UB (on some platforms, e.g. miri caught it on windows, but not on linux)

@folkertdev folkertdev requested a review from bjorn3 April 1, 2025 08:24
Copy link

codecov bot commented Apr 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
zlib-rs/src/allocate.rs 97.92% <100.00%> (+0.19%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@folkertdev folkertdev force-pushed the zeroed-allocation-alignment branch from 95b27fd to 69659f5 Compare April 1, 2025 08:38
@@ -251,7 +270,11 @@ impl Allocator<'_> {
fn allocate_layout_zeroed(&self, layout: Layout) -> *mut c_void {
#[cfg(feature = "rust-allocator")]
if self.zalloc == Allocator::RUST.zalloc {
return unsafe { std::alloc::System.alloc_zeroed(layout).cast() };
let ptr = unsafe { zalloc_rust_calloc(self.opaque, layout.size() as _, 1) };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the input alignment now ignored entirely?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm, I'll add some asserts then. There is little we can do about it, but we know that our allocations never have alignments higher than 64.

@folkertdev folkertdev force-pushed the zeroed-allocation-alignment branch from cedab6d to be1026a Compare April 1, 2025 10:22
it's allocator is quite different, so let's test that
@folkertdev folkertdev force-pushed the zeroed-allocation-alignment branch from be1026a to 3c9cbcd Compare April 1, 2025 10:26
@folkertdev folkertdev merged commit 639b4cc into main Apr 1, 2025
21 checks passed
@folkertdev folkertdev deleted the zeroed-allocation-alignment branch April 1, 2025 10:29
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