Skip to contentSkip to conversation

rust: alloc: satisfy POSIX alignment requirement

PATCH Applied Tamir Duberstein 3 participants 4 messages Feb 13, 2025 – Mar 5, 2025

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v7 1/1 rust: alloc: satisfy POSIX alignment requirement

ISO C's aligned_alloc is partially implementation-defined; on some systems it inherits stricter requirements from POSIX's posix_memalign.

This causes the call added in commit dd09538fb409 ("rust: alloc:
implement Cmalloc in module allocator_test") to fail on macOS because it doesn't meet the requirements of posix_memalign.

Adjust the call to meet the POSIX requirement and add a comment. This fixes failures in make rusttest on macOS.

Fixes dd09538fb409 ("rust: alloc: implement `Cmalloc` in module allocator_test")

---
Changes in v7:
- Remove errant second pad_to_align call. (Gary Guo)
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- Replace incorrect use of build_error with map_err. build_error seems not to work correctly on the host. (Danilo Krummrich)
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Remove errant newline in commit message. (Miguel Ojeda)
- Use more succinct expression. (Gary Guo)
- Drop and then add Danilo's Acked-by again.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- Revert to aligned_alloc and correct rationale. (Miguel Ojeda)
- Apply Danilo's Acked-by from v2.
- Rebase on v6.14-rc2.
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Replace aligned_alloc with posix_memalign for portability.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Shorten some variable names. (Danilo Krummrich)
- Replace shadowing alignment variable with a second call to
Layout::align. (Danilo Krummrich)
- Link to v1: https://lore.kernel.org/r/[email protected]

---
 rust/kernel/alloc/allocator_test.rs | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
rust/kernel/alloc/allocator_test.rs +18 -0
index e3240d16040b..c37d4c0c64e9 100644
@@ -62,6 +62,24 @@ unsafe fn realloc(
             ));
         }
 
+        // ISO C (ISO/IEC 9899:2011) defines `aligned_alloc`:
+        //
+        // > The value of alignment shall be a valid alignment supported by the implementation
+        // [...].
+        //
+        // As an example of the "supported by the implementation" requirement, POSIX.1-2001 (IEEE
+        // 1003.1-2001) defines `posix_memalign`:
+        //
+        // > The value of alignment shall be a power of two multiple of sizeof (void *).
+        //
+        // and POSIX-based implementations of `aligned_alloc` inherit this requirement. At the time
+        // of writing, this is known to be the case on macOS (but not in glibc).
+        //
+        // Satisfy the stricter requirement to avoid spurious test failures on some platforms.
+        let min_align = core::mem::size_of::<*const crate::ffi::c_void>();
+        let layout = layout.align_to(min_align).map_err(|_| AllocError)?;
+        let layout = layout.pad_to_align();
+
         // SAFETY: Returns either NULL or a pointer to a memory allocation that satisfies or
         // exceeds the given size and alignment requirements.
         let dst = unsafe { libc_aligned_alloc(layout.align(), layout.size()) } as *mut u8;

---
base-commit: 8a5aae7dbbfb612509c8a2f112f7e0f79029ed45
change-id: 20250201-aligned-alloc-b52cb2353c82

Best regards,

Signature · 1 line
Tamir Duberstein <[email protected]>
reviewer Reviewed-by on v7 1/1
#1 lore raw
Quoted 7 lines · On Thu, 13 Feb 2025 06:34:18 -0500 Tamir Duberstein <[email protected]> wrote:

ISO C's aligned_alloc is partially implementation-defined; on some systems it inherits stricter requirements from POSIX's posix_memalign.

This causes the call added in commit dd09538fb409 ("rust: alloc:
implement Cmalloc in module allocator_test") to fail on macOS because it doesn't meet the requirements of posix_memalign.

Quoted 6 lines

Adjust the call to meet the POSIX requirement and add a comment. This fixes failures in make rusttest on macOS.

Fixes dd09538fb409 ("rust: alloc: implement `Cmalloc` in module allocator_test")
Quoted 70 lines

---
Changes in v7:
- Remove errant second pad_to_align call. (Gary Guo)
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- Replace incorrect use of build_error with map_err. build_error seems not to work correctly on the host. (Danilo Krummrich)
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Remove errant newline in commit message. (Miguel Ojeda)
- Use more succinct expression. (Gary Guo)
- Drop and then add Danilo's Acked-by again.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- Revert to aligned_alloc and correct rationale. (Miguel Ojeda)
- Apply Danilo's Acked-by from v2.
- Rebase on v6.14-rc2.
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Replace aligned_alloc with posix_memalign for portability.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Shorten some variable names. (Danilo Krummrich)
- Replace shadowing alignment variable with a second call to
Layout::align. (Danilo Krummrich)
- Link to v1: https://lore.kernel.org/r/[email protected]

---
 rust/kernel/alloc/allocator_test.rs | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
rust/kernel/alloc/allocator_test.rs +18 -1
index e3240d16040b..c37d4c0c64e9 100644
@@ -62,6 +62,24 @@ unsafe fn realloc(
             ));
         }
 
+        // ISO C (ISO/IEC 9899:2011) defines `aligned_alloc`:
+        //
+        // > The value of alignment shall be a valid alignment supported by the implementation
+        // [...].
+        //
+        // As an example of the "supported by the implementation" requirement, POSIX.1-2001 (IEEE
+        // 1003.1-2001) defines `posix_memalign`:
+        //
+        // > The value of alignment shall be a power of two multiple of sizeof (void *).
+        //
+        // and POSIX-based implementations of `aligned_alloc` inherit this requirement. At the time
+        // of writing, this is known to be the case on macOS (but not in glibc).
+        //
+        // Satisfy the stricter requirement to avoid spurious test failures on some platforms.
+        let min_align = core::mem::size_of::<*const crate::ffi::c_void>();
+        let layout = layout.align_to(min_align).map_err(|_| AllocError)?;
+        let layout = layout.pad_to_align();
+
         // SAFETY: Returns either NULL or a pointer to a memory allocation that satisfies or
         // exceeds the given size and alignment requirements.
         let dst = unsafe { libc_aligned_alloc(layout.align(), layout.size()) } as *mut u8;
 
---

base-commit: 8a5aae7dbbfb612509c8a2f112f7e0f79029ed45
change-id: 20250201-aligned-alloc-b52cb2353c82

Best regards,

#2 lore raw
Quoted 11 lines · Gary Guo
Quoted 7 lines · On Thu, 13 Feb 2025 06:34:18 -0500 Tamir Duberstein <[email protected]> wrote:

ISO C's aligned_alloc is partially implementation-defined; on some systems it inherits stricter requirements from POSIX's posix_memalign.

This causes the call added in commit dd09538fb409 ("rust: alloc:
implement Cmalloc in module allocator_test") to fail on macOS because it doesn't meet the requirements of posix_memalign.

Quoted 8 lines
Quoted 6 lines

Adjust the call to meet the POSIX requirement and add a comment. This fixes failures in make rusttest on macOS.

Fixes dd09538fb409 ("rust: alloc: implement `Cmalloc` in module allocator_test")

Thanks for the reviews! Gentle ping: are more reviews required?

Applied
#3 lore raw
In reply to Tamir Duberstein
Quoted 8 lines · Tamir Duberstein

ISO C's aligned_alloc is partially implementation-defined; on some systems it inherits stricter requirements from POSIX's posix_memalign.

This causes the call added in commit dd09538fb409 ("rust: alloc:
implement Cmalloc in module allocator_test") to fail on macOS because it doesn't meet the requirements of posix_memalign.

Quoted 6 lines

Adjust the call to meet the POSIX requirement and add a comment. This fixes failures in make rusttest on macOS.

Fixes dd09538fb409 ("rust: alloc: implement `Cmalloc` in module allocator_test")

Applied to rust-fixes -- thanks everyone!

[ Added Cc: stable. - Miguel ]

Cheers,
Miguel