Hello Stéphane,
On 2024-06-19 03:20, Stéphane Glondu wrote:
> While investigating OCaml 5.2.0 build failure on armel, I realized that
> atomic 64-bit reads from mmap-ed memory on armel results in a segfault.
>
> See:
>
> https://github.com/ocaml/ocaml/issues/13234#issuecomment-2176079951
In the reproducer above, try opening the file O_RDWR instead, and mmap
it with PROT_READ | PROT_WRITE. The program does not segfault if you do.
To see why, get the reproducer to segfault under gdb and inspect the
program counter, it will say something like:
(gdb) x/i $pc
=> 0xffff0f78: @ <UNDEFINED> instruction: 0x01a23e96
That instruction is stlexdeq, see [1].
So that's a *store* instruction, which may be a bit confusing in this
context given that in your program you are doing an atomic *read*
really. However, what the atomic read does is a pair of load/store
exclusive to guarantee atomicity of the 64 bit read. The page tables and
underlying storage both need to be writable for that to succeed.
The reason why you need a load/store exclusive pair to guarantee
atomicity of the 64 bit read is that the store exclusive ensures that
both 32 bit values have been read together. Otherwise the store
exclusive would fail and the routine would retry.
>From [2]:
"Store-Release Exclusive Doubleword stores a doubleword from two
registers to memory if the executing PE has exclusive access to the
memory at that address, and returns a status value of 0 if the store was
successful, or of 1 if no store was performed."
[1]
https://sources.debian.org/src/linux/6.7.12-1~bpo12%2B1/arch/arm64/kernel/kuser32.S/#L30
[2]
https://developer.arm.com/documentation/ddi0597/2024-03/Base-Instructions/STLEXD--Store-Release-Exclusive-Doubleword-?lang=en
Index: ocaml-5.2.0/otherlibs/runtime_events/runtime_events_consumer.c
===================================================================
--- ocaml-5.2.0.orig/otherlibs/runtime_events/runtime_events_consumer.c
+++ ocaml-5.2.0/otherlibs/runtime_events/runtime_events_consumer.c
@@ -189,7 +189,7 @@ caml_runtime_events_create_cursor(const
cursor->ring_file_size_bytes = GetFileSize(cursor->ring_file_handle, NULL);
#else
- ring_fd = open(runtime_events_loc, O_RDONLY, 0);
+ ring_fd = open(runtime_events_loc, O_RDWR, 0);
if( ring_fd == -1 ) {
caml_stat_free(cursor);
@@ -210,7 +210,7 @@ caml_runtime_events_create_cursor(const
/* This cast is necessary for compatibility with Illumos' non-POSIX
mmap/munmap */
cursor->metadata = (struct runtime_events_metadata_header *)
- mmap(NULL, cursor->ring_file_size_bytes, PROT_READ,
+ mmap(NULL, cursor->ring_file_size_bytes, PROT_READ | PROT_WRITE,
MAP_SHARED, ring_fd, 0);
if( cursor->metadata == MAP_FAILED ) {
