align return value of sbrk()

[dcstes]

Possible fix for issue mmap() sqUnixSpurMemory patch #665

David Stes

[eliotmiranda]

Hi David, can you try a simpler version of the patch? That is just add

hint = roundUpToPage(hint);

before hint is used. So that could go on the line after hint = sbrk(0);

[cstes]

I believe that I tried that simpler patch and that worked, meaning the loop did not end with an intialAlloc failed message, but my PR (pull reauest) also works and it also means that the loop does not end with an initialAlloc failed. I have to look at the code in sqAllocateMemorySegmentOfSizeAboveAllocatedSizeInto() and at the code for using sbrk() in the case of the stack interpteter.

[cstes]

There is a call to sbrk() in sqAllocateMemory() but there is another call to sbrk() in allocateJITMemory(). So there are multiple calls to sbrk() in platforms/unix/vm/sqUnixSpurMemory.c. The pull request #672 adds an alignment calculation to the return value of sbrk() in allocateJITMemory() so to mirror the calculation in sqAllocateMemory(). The OpenGroup manpage on sbrk() says that it is unspecified whether the pointer returned by sbrk() is aligned suitably for any purpose (see https://pubs.opengroup.org/onlinepubs/7908799/xsh/brk.html).

[eliotmiranda]

We should abstract it into a function which names its purpose, eg lowestPageAlignedAddressForSegmentAllocation.I’ll do this today.,,,^..^,,, (phone)On Jan 27, 2024, at 6:10 AM, David Stes @.***> wrote: There is a call to sbrk() in sqAllocateMemory() but there is another call to sbrk() in allocateJITMemory(). So there are multiple calls to sbrk() in platforms/unix/vm/sqUnixSpurMemory.c. The pull request #672 adds an alignment calculation to the return value of sbrk() in allocateJITMemory() so to mirror the calculation in sqAllocateMemory(). The OpenGroup manpage on sbrk() says that it is unspecified whether the pointer returned by sbrk() is aligned suitably for any purpose (see https://pubs.opengroup.org/onlinepubs/7908799/xsh/brk.html).

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

[eliotmiranda]

On Jan 28, 2024, at 11:01 AM, Eliot Miranda @.> wrote:We should abstract it into a function which names its purpose, eg lowestPageAlignedAddressForSegmentAllocation.I’ll do this today.I’ve tried this and something strange happened on my Mac; changing the hint appears to have changed the address of the initial allocation to something much higher, which causes the existing code to fail to allocate (the validation loop in the main segment allocator).  So this is taking longer than anticipated. ,,,^..^,,, (phone)On Jan 27, 2024, at 6:10 AM, David Stes @.> wrote: There is a call to sbrk() in sqAllocateMemory() but there is another call to sbrk() in allocateJITMemory(). So there are multiple calls to sbrk() in platforms/unix/vm/sqUnixSpurMemory.c. The pull request #672 adds an alignment calculation to the return value of sbrk() in allocateJITMemory() so to mirror the calculation in sqAllocateMemory(). The OpenGroup manpage on sbrk() says that it is unspecified whether the pointer returned by sbrk() is aligned suitably for any purpose (see https://pubs.opengroup.org/onlinepubs/7908799/xsh/brk.html).

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

[cstes]

Hi Eliot, Thanks for looking at this issue and pull request. Also the fact that I post it on github does not mean you have to immediately work on it. But I wanted to remind you on this issue so that hopefully you find a good solution. I appreciate your work on OpenSmalltalk and your knowledge on the Smalltalk environment will be of great help here. Working on such a large and complex piece of software and looking at the feedback can be a lot of work and I only want to give some feedback, not increase your (pesumably high) workload ... Based on the feedback from David Lewis I understand thatthe pull request modification works on Linux. Because the specificatoin of sbrk() in the SUS (Single UNIX Specification) is probably applicable to all sbrk() implementations on all UNIX based operating systems, I think it is likely to work on all UNIX based platforms. Regards

[dcstes]

solved by new code lowestPageAligendAddressForMMap() macro in Spur memory management for Unix