Don't barf on failures in the RTS linker
The RTS linker currently calls barf() when it fails. This is a problem because:
- It appears that there is a bug in GHC, when there is no bug.
- Failures to load code really should be recoverable.
According to a TODO in the code, the culprit is resource deallocation, which is very difficult due to the code being written in C and having complicated control flow. There are a few solutions:
- Port the RTS linker to C++ and use RAII for resource management. Failures would be handled (internally to the linker) by throwing a C++ exception. This is actually my favorite, but might not be popular with the GHC devs.
- Build a huge context struct containing all needed resources and free it before returning. Signal errors with
longjmp(). - Try to find each and every place where resources need to be free, and free them by hand. Signal errors with return codes. This seems too error-prone.
Trac metadata
| Trac field | Value |
|---|---|
| Version | 8.0.1 |
| Type | Bug |
| TypeOfFailure | OtherFailure |
| Priority | normal |
| Resolution | Unresolved |
| Component | Runtime System (Linker) |
| Test case | |
| Differential revisions | |
| BlockedBy | |
| Related | |
| Blocking | |
| CC | |
| Operating system | |
| Architecture |
Activity
changed milestone to %8.2.1
added RTS Tbug Trac import linking labels
I would be the first to agree that the RTS Linker code is nowhere near as nice as it should be be. This code has been put together via accretion over a decade or more. It has had over a dozen people work on it and all of those people know C but would prefer to write Haskell. Another problem is that code has to support at least 6 CPU architecture about 5 different Unix variants and Windows.
I personally think porting the linker to C++ is a really bad idea. The linker is already under-resourced (in terms of people working on it) and using C++ instead of C would make it significantly more difficult for newcomers to work on it.
As for your other two suggestions I have no strong feelings for or against, but would happy to have anyone work on the linker to help improve it.
One last thought, a system using or based on talloc (https://talloc.samba.org/talloc/doc/html/index.html) may help.
I already fixed a bunch of these in 5300099e, but there are definitely more. I think you're talking about
loadArchive, right? That's where I ran out of energy, because it needs a lot of refactoring.C++ would make it easy, but I don't think it's worth going to C++ just for this one bit of code. I would refactor it carefully, splitting up the function into smaller pieces and building a decent test suite.
Something that might help here is gcc's cleanup attribute: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html which gives you some of the benefits of RAII in plain C.
Going to C++ even for a small bit of the codebase would be a big step because we would have to use g++ as the linker and we get dependencies on C++ standard libs etc.
I have a patch. It doesn't solve the problem completely, but it solves the problem in
loadArchive(). Failures in that function now cause an error return and a message to be written viaerrorBelch(). This isn't great, but it is better than the current situation.Currently running
./validate --slowto test.Trac metadata
Trac field Value Differential revisions - → D2570 mentioned in commit 488a9ed3
mentioned in commit aa10c67e
mentioned in commit 83d69dca
mentioned in commit c766d53f
changed milestone to %8.4.1
changed milestone to %8.6.1
changed milestone to %8.8.1
changed milestone to %8.10.1
added Pnormal label
