Improve reallocation in alloc_system on Windows

[retep998]

For allocations where the alignment is greater than the alignment guaranteed by HeapAlloc, the implementation will overallocate by align bytes. align_ptr will the get the first address within that allocation satisfying the alignment (other than the base of the allocation) and write the address of the original allocation a pointer size before that aligned address. What the offset is within any given overaligned allocation varies on a per allocation basis, which means the offset of data written to the allocation varies on a per allocation basis.

The old version of reallocate would always call HeapRealloc, which will move the allocation to a new address, which has the side effect of causing the aligned offset within the allocation to possibly change. Since the data itself wasn't moved to the new offset but remained at its old offset, this effectively changed the data in the allocation which is of course quite bad.

The new version does exactly what the unix version does, which is to allocate a brand new buffer, figure out what the aligned address is, and then memcpy over the data from the old aligned address to the new aligned address, which works just fine.

While I was at it, I updated reallocate_inplace to be able to do inplace reallocations of overaligned data. Because inplace reallocation does not change the base address of the allocation, it does not change the aligned offset, therefore the data will safely remain in the same location without issue.

Fixes #42025

[alexcrichton]

I'm not sure I quite understand what the bug is here and how this is fixing it, could you explain in a bit more detail? Would it be possible to add tests for this?

[retep998]

Updated topic with details.

Here's an example that fails with the old implementation which I will turn into a test to ensure the new implementation is correct:

#![feature(attr_literals, repr_align)] #[repr(align(256))] struct Foo(usize); fn main() { let mut foo = vec![Foo(273)]; for i in 0..0x1000 { foo.reserve_exact(i); assert!(foo[0].0 == 273); assert!(foo.as_ptr() as usize & 0xff == 0); foo.shrink_to_fit(); assert!(foo[0].0 == 273); assert!(foo.as_ptr() as usize & 0xff == 0); } }

[alexcrichton]

@bors: r+

Makes sense to me, thanks!

[bors]

📌 Commit 4be66b8 has been approved by alexcrichton

[bors]

⌛ Testing commit 4be66b8 with merge fd0fd18...

[bors]

💔 Test failed - status-travis

[steveklabnik]

@bors: retry

[Mark-Simulacrum]

Spurious failure was #42118. Logging in spreadsheet.

[bors]

☔ The latest upstream changes (presumably #41670) made this pull request unmergeable. Please resolve the merge conflicts.

[eddyb]

@bors r=alexcrichton

[bors]

📌 Commit 42ac311 has been approved by alexcrichton

[gereeter]

Would it be possible to copy that explanation of the change into the main issue body so that it gets immortalized in the commit history?

[bors]

⌛ Testing commit 42ac311 with merge fbb9276...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing fbb9276 to master...