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

[Alexander Bulekov]

On 201025 1351, Joelle van Dyne wrote:
> > 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?

Maybe memfd_create(2) + mmap? It doesn't require a tmpfs mount, so it should
be affected by noexec.
-Alex

> 
> -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~
>