Bug#904055: ghc: Please backport important fix for unregisterised compiler

[John Paul Adrian Glaubitz]

Source: ghc
Version: 8.2.2-4
Severity: normal
Tags: patch upstream
User: debian-68k at lists.debian.org
Usertags: m68k

Hi!

I recently reported a GHC bug upstream which resulted in miscompiled
code when using an unregisterised compiler [1]. Upstream has managed
to track down the issue to GHC producing code that the C compiler
doesn't like. The bug was observed on m68k and sh4 in Debian, but
according to upstream, can affect any other target using an unregisterised
compiler.

The patch can be found in [2], I'm also attaching it. It would be
highly appreciated if the patch could be included in the next upload
so that future uploads of GHC 8.2.x will continue to work on the
affected architectures.

Thanks,
Adrian

> [1] https://ghc.haskell.org/trac/ghc/ticket/15338
> [2] https://git.haskell.org/ghc.git/commitdiff/8ec48990fee9e245bb2fe40dc6f65b61b8612157

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz at debian.org
`. `'   Freie Universitaet Berlin - glaubitz at physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
-------------- next part --------------
Description: driver: skip -Bsymbolic on unregisterised targets
 Trac #15338 is yet another example where -Bsymbolic breaks
 semantics of a C program: global variable duplication happens
 and unsafePerformIO creates two stdout copies.
 .
 When -Bsymbolic is not used both C compiler and linker agree
 on how global variables are handled. In case of sh4 it consists
 on a few assertions:
 .
 1. global variable is exported from shared library
 2. code is referred to this variable via GOT-like mechanism to allow
    interposition
 3. global variable is present .bss section on an executable
    (as an R_*_COPY relocation: symbol contents is copied at executable
    startup time)
 4. and symbol in executable interposes symbol in shared library.
 .
 This way both code in shared library and code in executable refer
 to a copy of global variable in .bss section of an executable.
 .
 Unfortunately -Bsymbolic option breaks assumption [2.] and generates
 direct references to the symbol. This causes mismatch between
 values seen from executable and values seen from shared library code.
 .
 This change disables '-Bsymbolic' for unregisterised targets.

--- ghc-8.2.2.orig/compiler/main/SysTools.hs
+++ ghc-8.2.2/compiler/main/SysTools.hs
@@ -1741,9 +1741,12 @@ linkDynLib dflags0 o_files dep_packages
             -------------------------------------------------------------------
 
             let output_fn = case o_file of { Just s -> s; Nothing -> "a.out"; }
+                unregisterised = platformUnregisterised (targetPlatform dflags)
             let bsymbolicFlag = -- we need symbolic linking to resolve
-                                -- non-PIC intra-package-relocations
-                                ["-Wl,-Bsymbolic"]
+                                -- non-PIC intra-package-relocations for
+                                -- performance (where symbolic linking works)
+                                -- See Note [-Bsymbolic assumptions by GHC]
+                                ["-Wl,-Bsymbolic" | not unregisterised]
 
             runLink dflags (
                     map Option verbFlags
@@ -1800,3 +1803,27 @@ getFrameworkOpts dflags platform
     -- reverse because they're added in reverse order from the cmd line:
     framework_opts = concat [ ["-framework", fw]
                             | fw <- reverse frameworks ]
+
+{-
+Note [-Bsymbolic assumptions by GHC]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+GHC has a few assumptions about interaction of relocations in NCG and linker:
+
+1. -Bsymbolic resolves internal references when the shared library is linked,
+   which is important for performance.
+2. When there is a reference to data in a shared library from the main program,
+   the runtime linker relocates the data object into the main program using an
+   R_*_COPY relocation.
+3. If we used -Bsymbolic, then this results in multiple copies of the data
+   object, because some references have already been resolved to point to the
+   original instance. This is bad!
+
+We work around [3.] for native compiled code by avoiding the generation of
+R_*_COPY relocations.
+
+Unregisterised compiler can't evade R_*_COPY relocations easily thus we disable
+-Bsymbolic linking there.
+
+See related Trac tickets: #4210, #15338
+-}