driver: don't enable -Bsymbolic on unregisterised targets (Trac #15338)
ClosedPublic

Authored by trofi on Jul 12 2018, 5:42 PM.

Details

Summary

Trac 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.

Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>

Test Plan

test 'ghc-pkg --version | cat' still works

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
trofi created this revision.Jul 12 2018, 5:42 PM

Ok, but we need to document the assumptions carefully here. I believe this is it:

  • -Bsymbolic resolves internal references when the shared library is linked, which is important for performance
  • 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.
  • 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.
  • We work around this for native compiled code by avoiding the generation of R_COPY relocations.

I'm not sure why -Bsymbolic doesn't disable the generation of R_COPY somehow.

Anyway, we can either not do -Bsymbolic or avoid R_COPY. Perhaps it isn't possible to avoid R_COPY if we're compiling unregisterised.

trofi updated this revision to Diff 17311.Jul 13 2018, 4:04 PM

Added 'Note [-Bsymbolic assumptions by GHC]' to describe -Bsymbolic delicacy.

trofi added a comment.Jul 13 2018, 4:13 PM

Ok, but we need to document the assumptions carefully here. I believe this is it:

Agreed. Added 'Note [-Bsymbolic assumptions by GHC]'.

I'm not sure why -Bsymbolic doesn't disable the generation of R_COPY somehow.

The problem here is that -Bsymbolic is used for library linking, R_COPY relocations are generated when final executable is linked (-Bsymbolic is not passed there).

Anyway, we can either not do -Bsymbolic or avoid R_COPY. Perhaps it isn't possible to avoid R_COPY if we're compiling unregisterised.

Yeah. In theory -Bsymbolic can also be used on PIE binaries to avoid R_COPY but PIE does not work everywhere. I think avoiding -Bsymbolic is not too bad.

bgamari edited the summary of this revision. (Show Details)Jul 14 2018, 7:06 PM
In D4959#136709, @trofi wrote:

Anyway, we can either not do -Bsymbolic or avoid R_COPY. Perhaps it isn't possible to avoid R_COPY if we're compiling unregisterised.

Yeah. In theory -Bsymbolic can also be used on PIE binaries to avoid R_COPY but PIE does not work everywhere. I think avoiding -Bsymbolic is not too bad.

You still get COPY relocations with PIE on some architectures, namely those which use PC-relative addressing, which includes x86_64.

jrtc27 accepted this revision.Jul 15 2018, 7:33 AM

From OFTC/#debian-ports:

[13:24:42] <cbmuser> slyfox_: I applied your patch for GHC locally, it seems to work

This change seems logical to me, so it's a LGTM from my point of view.

This revision is now accepted and ready to land.Jul 15 2018, 7:33 AM

Note looks good. Thankyou!

This revision was automatically updated to reflect the committed changes.