Fix singlefile on OSX ARM64 #68845
Fix singlefile on OSX ARM64 #68845
Conversation
|
Tagging subscribers to this area: @agocke, @vitek-karas, @VSadov Issue Detailsnull
|
| prevSectionEnd = (char*)loadedHeader + headerSize; // the first "section" for our purposes is the header | ||
| void* prevSectionEndAligned; | ||
| // the first "section" for our purposes is the header | ||
| prevSectionEndAligned = ALIGN_UP((char*)loadedHeader + headerSize, GetVirtualPageSize()); |
There was a problem hiding this comment.
The does not change how we map. I just renamed prevSectionEnd and made it always aligned. I think it is more readable.
| // know the actual page size on the target system. | ||
| // We may need one page gap of unused VA space before the next section starts. | ||
| // We will assume the page size is <= RVAAlign | ||
| sectionStartRva += RVAAlign; |
There was a problem hiding this comment.
I think I understand the layout mechanism more now. The way we construct sectionStartRva guarantees that sections will not overlap - that is if the file starts form offset 0.
That is not the case in a singlefile bundle. In the bundle everything has an additional file-wide offset when we map.
On ARM64 the file from a bundle will start at an offset that is 4K aligned. On systems with 4K page size (i.e. default Ubuntu) such aligned offset will be as good as starting form 0. Thus we do not see asserts on Linux.
On OSX ARM64 the page size is 16K, so we may need to "spill" a section past the end of the target VA region and that may cause VA regions to overlap, which trigger a failure in the loader and an assert.
Same could happen on unusually configured Linuxes with larger page sizes.
I think we can assume that 64K is enough to cover any reasonable page size and leaving 64K unused VA gap between sections will ensure that we will not overlap.
| // This is only necessary for R2R assemblies, but we do it for all assemblies for simplicity. | ||
| // See https://github.com/dotnet/runtime/issues/41832. |
There was a problem hiding this comment.
Once you get past the assert, you may still see crashes. I think they were because of this. We align files in the bundle on 4K for Linux ARM64, to make sure that ADRP assembly instructions are stable without performing relocations.
I think the same applies to OSX as well as it is specific to instruction set. The fix could be older than Apple Silicon and therefore covered Linux only.
There was a problem hiding this comment.
Is it possible that on osx-arm64 or unusually configured linux-arm64 systems, it is not 4K page aligned but something like 16K? In which case, can we use GetVirtualPageSize() instead of hardcoded 4K here?
There was a problem hiding this comment.
This is in a compile-time tool. It cold be a completely different platform/os.
We do not want arbitrary big alignment too, since it will increase the total size of the single file app.
There was a problem hiding this comment.
Also this is because of ADRP instruction, which operates with 4k offsets regardless of OS page.
|
It would be nice to run Installer OSX ARM64 tests with this, but I think they are disabled in CI and broken. When I tried running Installer tests locally they failed before even running - the restore scripts failed. |
|
Thanks!! |
* Same alignment in the bundle on OSX as on Linux * Extra VA gap between section in casse we run from sf bundle * Rename prevSectionEnd -> prevSectionEndAligned and make it aligned. * Suppress assert for now on OSX
ghost
locked as resolved and limited conversation to collaborators
Fixes: #67062 , #68654