Darwin/ARM64: Fix throwing of synchronous memory exceptions #36625

Keno · 2020-07-12T00:38:54Z

Needs decoding of the ARM64 ESR register. Additionally, the Aarch64 ABI requires 16 byte aligned stack
pointers on function entry, so we can't push the return pointer there. Whether we need to construct a
stack frame instead will depend on the unwinder (which doesn't work yet), but I don't believe so,
since the ABI mandates a frame pointer.

Keno · 2020-07-12T00:40:24Z

Oh, also added a test for this code path, since I don't think we had one.

yuyichao · 2020-07-12T00:42:54Z

src/signals-mach.c

+#ifdef _CPU_X86_64_
    // push (null) $RIP onto the stack
    rsp -= sizeof(void*);
    *(void**)rsp = NULL;


The corresponding aarch64 operation is setting lr to zero.

julia/src/signals-unix.c

Line 135 in a23a4ff

ctx->uc_mcontext.regs[29] = 0; // Clear link register (x29)

And is

julia/src/signals-unix.c

Line 162 in a23a4ff

ucontext64_t *ctx = (ucontext64_t*)_ctx;

used?

Right, I was wondering if we should set lr to NULL, but I haven't looked at the unwinder yet. I guess it can't really hurt.

Yeah it shoudn't hurt. And I think at least in one case I've seen it helps the unwinder (either ours or gdb) terminates the stack trace. (on linux...)

yuyichao · 2020-07-12T00:48:44Z

test/misc.jl

+# Test that read fault on a prot-none region does not incorrectly give
+# ReadOnlyMemoryEror, but rather crashes the program
+const MAP_ANONYMOUS_PRIVATE = Sys.isbsd() ? 0x1002 : 0x22
+let script = :(let ptr = Ptr{Cint}(ccall(:jl_mmap, Ptr{Cvoid},


Do we actually guarantee this everywhere else?

Yes, I think we have this logic on every platform.

It throws readonlymmoryerror on linux for example. I don't really see why it's that important to differentiate reading PROT_NONE from writing PROT_READ. The linux one only check for permission violation and would tread violation of read/write/execution the same.

I imaging what you could do is to unmap the page before issueing the unsafe_load. Assuming the code is compiled it should make sure you are loading from a hole in the mapping and that should be a crash.

I imaging what you could do is to unmap the page before issueing the unsafe_load. Assuming the code is compiled it should make sure you are loading from a hole in the mapping and that should be a crash.

That doesn't exercise this code path. We explicitly check that the mapping exists first.

I think we may just want to fix the linux behavior here to bring it in line with mac/windows. I don't see a good reason to throw ReadOnlyMemoryError here.

... Sure, that's fine. It's technically breaking but it was an error before anyway...

Or it's a bug fix and brings linux in line with the other platforms ;).

"Breaking" for ppl (like me) who hardly ever use julia anywhere other than linux... Not that I've ever cought any read only memory error when reading things anyway.............

I do agree the name of the exception is very suggestive of what it should do...

yuyichao

Either the test or the implementation needs to be updated.

yuyichao · 2020-07-12T00:55:10Z

test/misc.jl

+# Test that read fault on a prot-none region does not incorrectly give
+# ReadOnlyMemoryEror, but rather crashes the program
+const MAP_ANONYMOUS_PRIVATE = Sys.isbsd() ? 0x1002 : 0x22
+let script = :(let ptr = Ptr{Cint}(ccall(:jl_mmap, Ptr{Cvoid},


It throws readonlymmoryerror on linux for example. I don't really see why it's that important to differentiate reading PROT_NONE from writing PROT_READ. The linux one only check for permission violation and would tread violation of read/write/execution the same.

This replaces #36625. Essentially the question is what should reading from PROT_NONE memory do. At the moment we throw ReadOnlyMemoryError on Linux and pass through the segfault on Windows/Mac. It's not super clear what the right answer is, though it would be nice to be consistent. After some discussion, it seemed that not throwing ReadOnlyMemoryError on Linux was probably the better consistency direction for two reasons: 1. Getting a ReadOnlyMemoryError is confusing if the failing operation is a read. We could of course rename the error, but that would be a bigger breaking change. 2. The purpose of this exception is to protect people who accidentally try to write to mmap'ed memory that they don't have write permissions to. It was never intended to become a general memory access error exception, so it seems prudent to restrict it as much as possible. To make this happen, there is some platform specific code here to read the memory error register out of the signal context. Decoding this register is shared code between the operating systems, but the register itself is in an OS-specifc location in the signal frame. I have implemented the retrieval code for all our CI'ed platforms. Other users will get an appropriate `#warning` (similar to other code in this file) and can fill in the necessary code at their leisure. A new test in test/misc.jl will indicate whether or not the check is correct.

This replaces JuliaLang#36625. Essentially the question is what should reading from PROT_NONE memory do. At the moment we throw ReadOnlyMemoryError on Linux and pass through the segfault on Windows/Mac. It's not super clear what the right answer is, though it would be nice to be consistent. After some discussion, it seemed that not throwing ReadOnlyMemoryError on Linux was probably the better consistency direction for two reasons: 1. Getting a ReadOnlyMemoryError is confusing if the failing operation is a read. We could of course rename the error, but that would be a bigger breaking change. 2. The purpose of this exception is to protect people who accidentally try to write to mmap'ed memory that they don't have write permissions to. It was never intended to become a general memory access error exception, so it seems prudent to restrict it as much as possible. To make this happen, there is some platform specific code here to read the memory error register out of the signal context. Decoding this register is shared code between the operating systems, but the register itself is in an OS-specifc location in the signal frame. I have implemented the retrieval code for all our CI'ed platforms. Other users will get an appropriate `#warning` (similar to other code in this file) and can fill in the necessary code at their leisure. A new test in test/misc.jl will indicate whether or not the check is correct.

Keno requested a review from yuyichao July 12, 2020 00:38

Keno mentioned this pull request Jul 12, 2020

Darwin/ARM64 tracking issue #36617

Closed

31 tasks

Darwin/ARM64: Fix throwing of synchronous memory exceptions

4e07953

Keno force-pushed the kf/applearm64segv branch from fd59faf to 4e07953 Compare July 12, 2020 00:40

yuyichao reviewed Jul 12, 2020

View reviewed changes

yuyichao requested changes Jul 12, 2020

View reviewed changes

Keno mentioned this pull request Feb 24, 2021

Make ReadOnlyError on PROT_NONE consistent #39803

Merged

Keno closed this Feb 24, 2021

DilumAluthge deleted the kf/applearm64segv branch March 25, 2021 21:57

Uh oh!

Darwin/ARM64: Fix throwing of synchronous memory exceptions #36625

Darwin/ARM64: Fix throwing of synchronous memory exceptions #36625

Uh oh!

Conversation

Keno commented Jul 12, 2020

Uh oh!

Keno commented Jul 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuyichao Jul 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Keno Jul 12, 2020 • edited by yuyichao Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuyichao Jul 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuyichao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yuyichao Jul 12, 2020 •

edited

Loading

Keno Jul 12, 2020 •

edited by yuyichao

Loading

yuyichao Jul 12, 2020 •

edited

Loading