[RTS] Add loadNativeObj and unloadNativeObj
Changes PlannedPublic

Authored by simonmar on Dec 12 2017, 12:07 PM.

Details

Summary

This adds two functions:

  • loadNativeObj
  • unloadNativeObj

and implements them for Linux.

They are useful if you want to load a shared object with Haskell code
using the system linker and have GHC call dlclose() after the
code is no longer referenced from the heap.

Using the system linker allows you to load the shared object
above outside the lowmem region. It also loads the DWARF sections
in a way that perf understands.

dl_iterate_phdr is what makes this implementation Linux specific.

Test Plan

manual testing

niteria created this revision.Dec 12 2017, 12:07 PM

I'm happy to take naming suggestions and suggestions about where to put the code.

I've tried to put it in linker/Elf.c, but that required to expand the scope of dl_mutex.

niteria updated this revision to Diff 14924.Dec 12 2017, 12:36 PM

Avoid:

rts\Linker.c:2003:6: error:
328      error: function might be candidate for attribute 'noreturn' [-Werror=suggest-attribute=noreturn]
329      void freeNativeCode (NativeCode *nc)
330           ^~~~~~~~~~~~~~
331      |
332 2003 | void freeNativeCode (NativeCode *nc)
333      |      ^
niteria updated this revision to Diff 14925.Dec 12 2017, 4:06 PM

fix 2 bugs

Harbormaster failed remote builds in B18872: Diff 14925!
niteria updated this revision to Diff 14927.Dec 12 2017, 4:37 PM

I think I diffed out from the wrong branch

niteria updated this revision to Diff 14928.Dec 12 2017, 7:13 PM

handle stablePtrs

niteria planned changes to this revision.Dec 12 2017, 7:20 PM
niteria added inline comments.
rts/Linker.c
1885

Oh no, this is a deadlock when called from loadNativeObj_ELF

niteria updated this revision to Diff 14929.Dec 12 2017, 7:33 PM

fix deadlock

niteria marked an inline comment as done.Dec 13 2017, 11:09 AM
niteria updated this revision to Diff 14937.Dec 14 2017, 8:28 AM
  • add a test
  • deal with newCAF coming from the shared obj
Phyx added a comment.Dec 14 2017, 9:46 AM

Hi @nitera,

I haven't had a chance to take a detailed look at this yet (hoping this weekend) but can you give me a high level overview of how this differs conceptually from the current so loading code in addDLL?

Would we be able to drop one? Say can the addDLL implementation be swapped out for the his new approach? For ghci it would be handy to be able to unload the dlls that are created every time a bit earlier.

Also maybe a public haskell interface for this is In order? I know there's another ticket somewhere about being able to supply additional libraries to be used during template haskell compilation. And this would help that implementation a lot of think.

Overall this is great, we just need to work on the way to handle keepCAFs/retain_cafs.

includes/rts/Linker.h
84

Suggest

typedef void *NativeObjHandle;
rts/Linker.c
1846–1859

probably shouldn't paste LGPL code into this file

rts/sm/Storage.c
431–449

We need to find a better way to do this, but anyway shouldn't this depend on whether the linker was initialised with retain_cafs = true or not?

@Phyx The long-term plan would be to drop addDLL(), yes.

niteria added inline comments.Dec 15 2017, 9:16 AM
rts/sm/Storage.c
431–449

If retain_cafs = false then this check is unnecessary because we don't retain any cafs either way.

simonmar added inline comments.Dec 15 2017, 11:19 AM
rts/sm/Storage.c
431–449

I mean if retain_cafs == true then the check is wrong, and things also go wrong if we had keepCAFs==false && retain_cafs==true. Not that we ever use that combination, but I believe it currently does what it should do with the runtime linker.

Phyx added a comment.Dec 19 2017, 3:15 PM

Overall looks good to me, just some minor comments.

rts/Linker.c
1967

use a standard bool.

2005

need to use these arguments to fix the werror failures. simple (void)path; etc should do.

rts/LinkerInternals.h
226

please use bool and (true/false instead of 1.0). We're trying to use more c99 types to make the parameters clearer.

niteria planned changes to this revision.Dec 20 2017, 6:21 AM

There's a couple of things I need to do here.
It's unlikely I'm going to find time before the holidays.

TODO:

  • make newCAF work without range checks, the current plan is a shared-object-local variable
  • make it build on other platforms, Phyx provided valuable hints
  • make the handle type opaque - should help if we implement such an API on Windows or OSX
  • remove GPL code from the comments
  • provide Haskell API wrappers: loadNativeObj, unloadNativeObj, lookupNativeObj

TODON'T:

  • no one seems to be unhappy with the names, so I will keep them as is
  • I will keep it in Linker.c if no-one objects
  • migrating users of addDLL can happen as a follow-up

Hey @niteria, do you plan to finish this or shall we take it over?

@simonmar: I'm sorry for dropping the ball on this. Feel free to finish it, I don't know when I will get to it.

simonmar added inline comments.Jul 10 2018, 11:15 AM
rts/CheckUnload.c
322–323

looks like this is a duplicate message (just confused us while debugging)

simonmar commandeered this revision.Oct 1 2018, 2:47 AM
simonmar edited reviewers, added: niteria; removed: simonmar.

imcommandeeringthisdiff