Opened 9 years ago

Closed 9 years ago

#4464 closed bug (fixed)

RTS options broken for dynamic libraries

Reported by: rl Owned by:
Priority: normal Milestone: 7.2.1
Component: Runtime System Version: 7.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

There are several problems here. Firstly, GHC doesn't seem to respect -rtsopts when linking dynamic libraries. In DriverPipeline, linkDynLib doesn't care about the flag, only linkBinary does. This causes hs_init to fail if called with some RTS options. This is a blocker for us.

Secondly, here is a snipped from hs_init in RtsStartup.c:

	setFullProgArgv(*argc,*argv);
	setupRtsFlags(argc, *argv, &rts_argc, rts_argv);
	setProgArgv(*argc,*argv);

Of these, setFullProgArgv copies the things pointed to by argc and argv (good), setProgArgv doesn't copy (bad) and setupRtsFlags doesn't seem to, either (it calls setProgName which doesn't copy, at least). We probably should have setupRtsFlags and setProgArgv create its own copy of the args as well and document that the caller doesn't have to hang on to the memory after the call. Alternatively, the docs should make it very clear that the memory must remain allocated until the RTS is shut down.

Finally, hs_init simply aborts if it doesn't like the RTS arguments which is quite unhelpful for dynamic libraries. I took me a day to find out that an application crash was caused by a failing hs_init (because of the -rtsopts problem). I would like to add a check for this to our code but there doesn't seem to be a way to do this. It would be much nicer if hs_init returned a failure/success code, at least for dynamic libraries.

Change History (12)

comment:1 Changed 9 years ago by rl

Component: CompilerRuntime System

comment:2 Changed 9 years ago by rl

In the meantime, I found defaultsHook which seems to allow us to circumvent the problem (and is arguably cleaner than passing flags) but isn't documented anywhere. Is it ok to use this or is it likely to disappear?

comment:3 Changed 9 years ago by simonmar

I doubt that defaultsHook will go away. The preferred method is ghc_rts_opts if possible (see http://www.haskell.org/ghc/docs/6.12.2/html/users_guide/runtime-control.html#rts-hooks), but that doesn't let you change flags conditionally.

comment:4 Changed 9 years ago by rl

Hmm, the docs say:

Owing to the vagaries of DLL linking, these hooks don't work under Windows when the program is built dynamically.

Does this only affect programs or DLLs, too? Presumably, this applies to defaultsHook as well in which case we are back to square one. Although it does seem to work at the moment.

comment:5 Changed 9 years ago by simonmar

Ah, that would apply to a program made from multiple DLLs, not when you're building a single DLL to use as a library. The reason is that defaultsHook relies on the normal linking semantics where an earlier instance of a function replaces a later instance during linking, but DLL linking doesn't respect that (ELF dynamic linking does, however).

comment:6 Changed 9 years ago by igloo

Owner: set to igloo

comment:7 Changed 9 years ago by igloo

Milestone: 7.0.2

I've fixed what I think is the only part that is a blocker/regression for 7.0.1:

Wed Nov 10 14:54:49 GMT 2010  Ian Lynagh <igloo@earth.li>
  * Obey the -rtsopts flag when making shared libraries; part of #4464

Test added: T4464.

comment:8 Changed 9 years ago by igloo

Milestone: 7.0.27.2.1

comment:9 Changed 9 years ago by igloo

Owner: igloo deleted

Why is not copying the arguments bad?

And regarding hs_init aborting, currently we have:

  • barf calls fatalInternalErrorFn which calls abort(), but you can override fatalInternalErrorFn. However, barf then calls stg_exit if fatalInternalErrorFn returns.
  • There are a number of other places that call stg_exit (It might make sense to make a die function similar to barf for these. Perhaps even define the default fatalInternalErrorFn as calling die)

but I'm not really sure what we should do instead. Should we drop the fall-through stg_exit in barf/die, and turn every single barf/stg_exit usage site into

barf(...); return NULL /* or 0 or -1 or whatever makes sense to indicate failure */

? That could presumably also mean a lot of more checking for errors by RTS functions that call other RTS functions (which may transitively barf/die).

comment:10 Changed 9 years ago by simonmar

I think Roman is right, not copying the arguments is bad because the caller doesn't know the lifetime requirements for the memory (it is done this way because historically hs_init was called with the real argv, which is statically allocated so it doesn't matter). In retrospect I should have fixed this in my recent cleanup of RtsFlags, but forgot about this ticket.

If hs_init needs to return an error condition rather than aborting, then we need to define a new API for that, and fix setupRtsFlags. I don't think we need to do this for every case of stg_exit and certainly not for barf: these are all internal error conditions and we have no sensible way to recover.

comment:11 Changed 9 years ago by simonmar

I made hs_init copy the arguments today:

changeset:a6e8418a71b14ef85ee7134be654689b17765f03

comment:12 Changed 9 years ago by simonmar

Resolution: fixed
Status: newclosed

I've created a new ticket for the remaining part of this one: #5219, so I think this can now be closed.

Note: See TracTickets for help on using tickets.