Ticket #18 (closed patch: fixed)

Opened 6 years ago

Last modified 5 years ago

Jit code buffers need to be allocated via mmap()

Reported by: Infinoid Owned by: Infinoid
Priority: normal Milestone:
Component: none Version:
Severity: none Keywords: JIT patch
Cc: Language:
Patch status: applied Platform:

Description

Currently on linux x86, jit code buffers are allocated from the heap. On certain platforms and under certain conditions, the heap is not executable. This results in a segfault immediately upon attempting to execute some JIT code. We are currently able to reproduce this on feather3, when executing tene's build from within a screen session (don't ask).

Parrot does call mprotect() in an attempt to set the right permissions on the buffer. However, the mprotect manpage on my linux box says that POSIX mprotect behavior is undefined when called on a non-mmaped buffer. So, it looks like we need to switch mem_alloc_executable()/mem_free_executable()/mem_realloc_executable() over to using mmap.

Attachments

01_exec_memory_alloc.diff Download (4.0 KB) - added by santtu 5 years ago.
Updated Infinoid's 01 patch to work in HEAD (r37916)
04_jitcode-pmc.diff Download (5.9 KB) - added by santtu 5 years ago.
Another approach to calling mem_free_executable with correct values (see Inifinoid's 02 patch): use custom and only-for-JIT PMC (JitCode) instead of generic ManagedStruct
01_free-executable-takes-size-argument.patch.txt Download (4.9 KB) - added by Infinoid 5 years ago.
Updated patch from Santtu++, thanks!
02_managedstruct-free_func-attr.patch.txt Download (2.4 KB) - added by Infinoid 5 years ago.
Implement custom free and clone callback functions (and opaque private pointer arguments).
03_mmap-jit.patch.txt Download (10.0 KB) - added by Infinoid 5 years ago.
Minor update to Makefile interdependencies (this fixes builds on non-jit architectures)
summary.patch.txt Download (15.3 KB) - added by Infinoid 5 years ago.
Summary patch (contains 01, 02, 03) for easier testing.

Change History

Changed 6 years ago by Infinoid

I've attached patches to do this. Three patches implement the functionality in stages (API change, ManagedStruct change, and then finally the JIT change), the fourth patch is for debugging/testing it.

It doesn't quite work for me. It runs fine under gdb, and the debugging statements output the right values, but then it segfaults when calling the free-function pointer (apparently trying to call an address other than the one it just printed to stderr).

If we can get that part working, I believe it is the correct fix for the issue on feather3.

Changed 6 years ago by coke

  • keywords JIT added
  • summary changed from [JIT] Jit code buffers need to be allocated via mmap() to Jit code buffers need to be allocated via mmap()

Changed 6 years ago by Infinoid

Small followup: the old "don't run it in a screen session" trick is no longer making things work, so polyglotbot is currently configuring parrot with "--jitcapable=0". This seems to be an effective workaround.

Changed 6 years ago by Infinoid

  • keywords patch added
  • patch set to new

Changed 6 years ago by santtu

As a side comment and further rationale on using mmap: selinux in fedora particularly tends to disallow changing heap or stack to executable (using mprotect), the only more-or-less guaranteed working way is to use mmap (with MAP_SHARED -- even MAP_PRIVATE will fail if allow_execmod=0).

Changed 5 years ago by santtu

Updated Infinoid's 01 patch to work in HEAD (r37916)

Changed 5 years ago by santtu

Another approach to calling mem_free_executable with correct values (see Inifinoid's 02 patch): use custom and only-for-JIT PMC (JitCode) instead of generic ManagedStruct

Changed 5 years ago by santtu

I added two patches, first one is just update on earlier 01* patch. The second one takes alternative approach to 02*.

The 02 patch is needed, because if mmap/munmap is used, the munmap *must* be called with correct size value. Thus parrot needs to keep the actual JIT memory block allocation size somewhere around. ManagedStruct already has size parameter (int struct value), but it for JIT code it needed to be passed to mem_free_executable. Thus the 02 patch added custom free routine.

I chose instead to not modify existing ManagedStruct, which probably has other uses too and is overloaded with many accessors (via extending UnmanagedStruct). The JIT code basically just needs to be GCable and accessible from parrot C core -- not really PIR code at all so any accessors etc. are unused. Thus the JitCode PMC is very simple -- it just holds memory pointer and its size, and calls mem_free_executable when GCed.

Changed 5 years ago by Infinoid

  • owner set to Infinoid

Changed 5 years ago by Infinoid

Updated patch from Santtu++, thanks!

Changed 5 years ago by Infinoid

Implement custom free and clone callback functions (and opaque private pointer arguments).

Changed 5 years ago by Infinoid

This latest version of the patch set is what fell out of this morning's design discussion on #parrot. It preserves existing behavior as much as possible, and allows feature reuse as much as possible.

It could use some testing on selinux/x86 and on win32.

Changed 5 years ago by Infinoid

Minor update to Makefile interdependencies (this fixes builds on non-jit architectures)

Changed 5 years ago by Infinoid

Summary patch (contains 01, 02, 03) for easier testing.

Changed 5 years ago by Infinoid

  • status changed from new to closed
  • resolution set to fixed
  • type changed from bug to patch
  • patch changed from new to applied

I've applied my newest versions of these patches as r37991, r37992, and r37993, with additional fixes in r37994. Thanks very much to Santtu++ for patches and guidance.

This should get basic jit working. Santtu has some ideas for how to get jit working in even more restricted selinux configurations, but I believe we should make a new ticket for that.

(If the checkins I mentioned above broke anything, please reopen this ticket.)

Changed 5 years ago by Infinoid

Minor bump, TT #581 appears to be the same issue, again on Fedora. The requester complains that selinux in restricted mode doesn't work, but permissive mode does work.

Note: See TracTickets for help on using tickets.