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

Enable function sections with windows-gnu #75604

Closed
wants to merge 2 commits into from

Conversation

mati865
Copy link
Contributor

@mati865 mati865 commented Aug 16, 2020

Passed tests locally on x86_64 with LLVM assertions enabled.

@rust-highfive
Copy link
Collaborator

r? @ecstatic-morse

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 16, 2020
With MinGW and specific flags symbol may end up in multiple sections in some cases like:
0000000000000000 p .pdata$_ZN20stable_symbol_names113mono_function17hd4880e88dec8766dE
0000000000000000 t .text$_ZN20stable_symbol_names113mono_function17hd4880e88dec8766dE
0000000000000000 r .xdata$_ZN20stable_symbol_names113mono_function17hd4880e88dec8766dE
0000000000000000 T _ZN20stable_symbol_names113mono_function17hd4880e88dec8766dE

This will cause an error when comparing that output directly with other library
that only imports the symbol since it will appear only once.
To avoid the issue pass "-g" to nm so only the global symbol (the one
with "T" letter) will be listed.
@nagisa
Copy link
Member

nagisa commented Aug 16, 2020

IIRC the reason it was disabled until now was because on Windows there's an egregiously small limit on sections in object files, which then leads to compilation failures. Is windows-gnu not affected by this?

@mati865
Copy link
Contributor Author

mati865 commented Aug 16, 2020

Apparently it was disabled because of the LLVM assertions: #13846
LLVM supports doing it for both MSVC and GNU Windows targets. Short topic that made me try it again: mstorsjo/llvm-mingw#148

I have tried enabling it some time ago but got ui tests crashes.
Just recently I have noticed enabling ASLR or functions_sections had very similar (or even identical) crashes as one of my mistakes when working on #72049. To be exact the crashes happened when statics were imported from other DLLs without dllimport.
If you want I can verify tomorrow if this is unexpected "fallout" from #72049 or something else has fixed it.

@mstorsjo
Copy link

IIRC the reason it was disabled until now was because on Windows there's an egregiously small limit on sections in object files, which then leads to compilation failures. Is windows-gnu not affected by this?

FWIW, the limit on sections in an object file is higher if compiling in "big object" mode. IIRC GCC and/or MSVC require you to pass specific flags for enabling this, while LLVM enables this format automatically as needed.

@ecstatic-morse
Copy link
Contributor

r? @nagisa

@nagisa
Copy link
Member

nagisa commented Aug 21, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Aug 21, 2020

📌 Commit b655027 has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 21, 2020
@bors
Copy link
Contributor

bors commented Aug 21, 2020

⌛ Testing commit b655027 with merge 18dd0795f6aecafa408c6b35e9b28d2798a6ea8a...

@bors
Copy link
Contributor

bors commented Aug 21, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 21, 2020
@mati865
Copy link
Contributor Author

mati865 commented Aug 21, 2020

Appears to be bug fixed in binutils 2.32 I'll look for a workaround.

@mati865
Copy link
Contributor Author

mati865 commented Aug 22, 2020

I can reproduce it locally with the same Binutils version as on CI, Binutils 2.34 are not affected.
I doubt we can raise minimal Binutils version to 2.32 (released 2019-02-02) soon, this will have to wait until the requirement can be bumped or we migrate to LLD for MinGW.

@mstorsjo do you remember if this is easily workaroundable in LLVM?

@mstorsjo
Copy link

I can reproduce it locally with the same Binutils version as on CI, Binutils 2.34 are not affected.
I doubt we can raise minimal Binutils version to 2.32 (released 2019-02-02) soon, this will have to wait until the requirement can be bumped or we migrate to LLD for MinGW.

@mstorsjo do you remember if this is easily workaroundable in LLVM?

What issue in particular are you referring to here?

@mati865
Copy link
Contributor Author

mati865 commented Aug 22, 2020

@mstorsjo I apologize I thought I had included the link. Here is ld.bfd bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=23872
The error:

  Mingw-w64 runtime failure:
    Unknown pseudo relocation protocol version 16777216.

@mstorsjo
Copy link

@mstorsjo I apologize I thought I had included the link. Here is ld.bfd bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=23872
The error:

  Mingw-w64 runtime failure:
    Unknown pseudo relocation protocol version 16777216.

Ah, that one.

I have one potential fix in mind - appending a final object file that contains .rdata contents that is aligned to 4 bytes, included as the last object file in the link command. The object file can be assembled from e.g. this:

        .section .rdata$zzzz
        .align 4
        .long 0

That should take care to make sure the end of the .rdata section is aligned to 4 bytes, avoiding that particular ld bug.

@mati865
Copy link
Contributor Author

mati865 commented Aug 24, 2020

Unfortunately that didn't work. I'll just close it for now.

@mati865 mati865 closed this Aug 24, 2020
@mati865 mati865 deleted the mingw-ffunction-sections branch November 3, 2022 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants