Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Posix: Move magic DSO registration code to rt.dso druntime module #3850

Merged
merged 6 commits into from
Jan 13, 2022

Conversation

kinke
Copy link
Member

@kinke kinke commented Oct 15, 2021

Addressing #3786.

@JohanEngelen
Copy link
Member

Very nice @kinke

@kinke kinke marked this pull request as ready for review October 16, 2021 22:01
@kinke
Copy link
Member Author

kinke commented Oct 17, 2021

I can't reproduce the MSan failures

912: Failed Tests (2):
912:   LDC :: sanitizers/msan_noerror.d
912:   LDC :: sanitizers/msan_uninitialized.d

from the single failing LLVM 9 GH Actions job, with -DBUILD_SHARED_LIBS=ON -DRT_SUPPORT_SANITIZERS=ON - here on Ubuntu 20.04 with LLVM 12.0.0. Maybe caused by a difference in CRT ctor execution order, just a wild guess.

@JohanEngelen
Copy link
Member

Maybe caused by a difference in CRT ctor execution order, just a wild guess.

Could it be fixed by changing link order ?

@kinke kinke changed the title Generalize rt.dso_windows to rt.dso, for Posix too Posix: Move magic DSO registration code to rt.dso druntime module Oct 23, 2021
@kinke
Copy link
Member Author

kinke commented Oct 24, 2021

Doesn't look like it, putting it as first object file in the linker cmdline hasn't changed anything. Some more tests have shown that those 2 MSan tests on Ubuntu 18.04 fail with shared default libs - at least with LLVM 8 & 9 for GH Actions; LLVM 11 & 12 work with the same image (and shared libs). So I guess this has to do with the MSan version.

Further tests in #3858 with lld and bfd linkers show that this does seem to fix #3786, the LTO lit-tests with lld v12.0.1 now succeed. There's apparently one regression with bfd though, runnable/test14901.d failing for the GH Actions LLVM 11 job with BUILD_SHARED_LIBS=ON (which is green in #3848 alone). All other jobs seem unaffected, i.e., #3849 is the only other problem.

@kinke
Copy link
Member Author

kinke commented Oct 24, 2021

On Linux, all 3 linkers are CI-tested now:

  • Azure & Travis: lld (from LDC-LLVM)
  • GH Actions: gold
  • Cirrus & Circle: bfd (incl. 1 Ubuntu 16.04 job)

@kinke
Copy link
Member Author

kinke commented Oct 27, 2021

This also fixes a general issue with LLD v13 - with current master, a hello-world fails to link (but works with LLD v10, and according to CI, with v12 too):

ld: error: undefined hidden symbol: __start___minfo
>>> referenced by hello.d
>>>               hello.o:(ldc.register_dso)

ld: error: undefined hidden symbol: __stop___minfo
>>> referenced by hello.d
>>>               hello.o:(ldc.register_dso)

Not sure why it works with this, but it does.

Instead, use the object file generated by compiling shared druntime
directly. By using the `@hidden` UDA in that module,
`-fvisibility=public` isn't a problem anymore.

We also end up with a 32-bit `ldc_rt.dso.o` for MULTILIB=ON this way.
This should help on Windows if those tools aren't available/in PATH.
Mainly, avoid running the MSan lit-tests with shared default libs,
which apparently regressed with the rt.dso introduction with older
LLVM/MSan versions - for GH Actions, at least with LLVM v8 and v9;
v11 and v12 do work.
@kinke kinke enabled auto-merge January 13, 2022 16:20
@kinke kinke merged commit 37cbf7b into ldc-developers:master Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants