Re: [PATCH v2 6/9] tcg: implement mirror mapped JIT for iOS

[Joelle van Dyne]

> Finally, I'd like to have this implemented on Linux as well, or I'm afraid the
> feature will bit-rot.  This can be trivially done by either (1)
> MREMAP_DONTUNMAP or (2) mapping from posix shared memory instead of MAP_ANON 
> so
> that you can map the same memory twice.  Thus virtually all of the ifdefs
> should go away.
Just spent an hour trying to implement this for Linux and running into issues.

1) Doesn't work because MREMAP_DONTUNMAP does in fact remove the entry
from the page table. According to the man pages "After completion, any
access to the range specified by old_address and old_size will result
in a page fault." Seems like the feature is designed around memory
locking, not mirror mapping.

2) I tried doing shm_open() and mmap() but you can't PROT_EXEC on shm
(see https://stackoverflow.com/questions/25275777/shared-executable-memory
)

I think it may be possible to map a file on an executable partition,
but I can foresee countless issues there including some security
issues. Anyone have any other ideas?

-j

On Mon, Oct 19, 2020 at 5:20 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 10/19/20 3:39 PM, Joelle van Dyne wrote:
> >> Explicit cast may not be needed here so this could be a macro if caling it
> >> differently helps or why don't you just use tcg_mirror_prr_rw directly
> >> everywhere?
> >
> > There are quite a bit of code that depends on tcg_insn_unit * type such as
> >
> > *tcg_code_ptr_rw(s, code_ptr) = insn;
> >
> > and
> >
> > (tcg_code_ptr_rw(s, p))[i] = NOP;
> >
> > I think it's cleaner to not have to manually cast in every one of 30+
> > instances of this. In v1, I used a macro but was told to use an inline
> > function instead.
>
> Yep.
>
> >> Is that !defined or are you missing an implementation and #else here?
> > No, `flush_dcache_range` is only needed when mirror mapped (after
> > writing to the RW mirror). Now there is no iOS compatible compiler for
> > any other arch than x86 and ARM. However, in the slim chance that
> > Apple decides to change arch again in the future and moves to RISC-V
> > or something, then we get a nice compiler error.
>
> *shrug* As opposed to the nice compiler error you get for a missing function
> declaration?
>
> That said, I think __builtin___clear_cache() may be the target-independent
> runtime function that you need.  Both GCC and LLVM support this, and I'd be
> surprised if that doesn't carry through to iOS.
>
> >> Maybe this patch could be split up some more, making the RW offset
> >> handling and cache management separate patches even if they don't work
> >> separately just to make it easier to review.
> >
> > I can probably do that for v3 but imo most of the LOC here is because
> > the same change has to be done to every TCG target. No matter how you
> > split up the patches, it will look like a lot of changes.
>
> It occurs to me that the majority of the code changes in patches 5 and 6 are
> due to your choice that code_gen_buffer points to the RX copy and not the RW 
> copy.
>
> Swap the two, and instead have an inline function that produces the executable
> pointer from the rw pointer, and suddenly there are very much fewer changes
> required.
>
> For the most part, tcg/$cpu/ generates pc-relative code, so it need not
> consider the absolute address.  There are a few exceptions including,
> obviously, 32-bit x86.  But the number of places that occurs is small.
>
> There's the assignment to tb->tc.ptr of course, and
> tcg_ctx.code_gen_prologue/epilogue.
>
> In any case, each of these changes (generic, per tcg backend) can occur before
> you finally add a non-zero displacement that actually separates the RX and RW
> mappings.
>
> Finally, I'd like to have this implemented on Linux as well, or I'm afraid the
> feature will bit-rot.  This can be trivially done by either (1)
> MREMAP_DONTUNMAP or (2) mapping from posix shared memory instead of MAP_ANON 
> so
> that you can map the same memory twice.  Thus virtually all of the ifdefs
> should go away.
>
>
> r~