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

cmd/link: using lld linker fails on Windows #39326

Closed
Keithcat1 opened this issue May 30, 2020 · 21 comments
Closed

cmd/link: using lld linker fails on Windows #39326

Keithcat1 opened this issue May 30, 2020 · 21 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@Keithcat1
Copy link

after the following commands:
set CGO_LDFLAGS=-fuse-ld=lld
go build

I get this error:

command-line-arguments

C:\go\pkg\tool\windows_amd64\link.exe: running gcc failed: exit status 1
lld: error: unknown argument: -T
collect2.exe: error: ld returned 1 exit status
The argument which is causing this is :
-Wl,-T,C:\Users\Keith\AppData\Local\Temp\go-link-100899651\fix_debug_gdb_scripts.ld
I found a way to remove this one argument, but the resulting executable crashes when I start it.

@ianlancetaylor ianlancetaylor changed the title LLD linker doesn't work on windows cmd/link: using lld linker fails on Windows May 30, 2020
@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label May 30, 2020
@ianlancetaylor ianlancetaylor added this to the Go1.16 milestone May 30, 2020
@thanm
Copy link
Contributor

thanm commented Dec 11, 2020

I took a look at the most resource source code for LLD; The "-T" or "--script" option is supported only for ELF targets.

With that said, the reason that we introduced the use of the linker script in the first place was to work around a bug in ld.bfd (https://sourceware.org/bugzilla/show_bug.cgi?id=21459), so one would expect that we would not need the same workaround for lld.

The main issue then becomes figuring out which linker is being used when doing external linking on windows, which should be doable. I will see about sending a CL.

@thanm thanm self-assigned this Dec 11, 2020
@thanm
Copy link
Contributor

thanm commented Dec 11, 2020

Interesting. Now looking at the script we're using:

SECTIONS
{
  .debug_gdb_scripts BLOCK(__section_alignment__) (NOLOAD) :
  {
    *(.debug_gdb_scripts)
  }
}
INSERT AFTER .debug_types;

Note the final line -- this refers to the ".debug_types" section, which as far as I know is not even generated by most compilers (clang doesn't emit it as far as I know, and gcc emits it only if you specifically as for it with a command line option).

Maybe we don't need the linker script after all?

@ianlancetaylor
Copy link
Contributor

It will insert the fragment after where .debug_types in the default linker script, and that does exist, whether or not there is actually an input section named .debug_types.

That said, I do in fact see this in the default COFF/PE linker script:

  /* DWARF 4.  */
  .debug_types ${RELOCATING+BLOCK(__section_alignment__)} ${RELOCATING+(NOLOAD)} :
  {
    *(.debug_types${RELOCATING+ .gnu.linkonce.wt.*})
  }
  .zdebug_types ${RELOCATING+BLOCK(__section_alignment__)} ${RELOCATING+(NOLOAD)} :
  {
    *(.zdebug_types${RELOCATING+ .gnu.linkonce.wt.*})
  }

  /* For Go and Rust.  */
  .debug_gdb_scripts ${RELOCATING+BLOCK(__section_alignment__)} ${RELOCATING+(NOLOAD)} :
  {
    *(.debug_gdb_scripts)
  }
  .zdebug_gdb_scripts ${RELOCATING+BLOCK(__section_alignment__)} ${RELOCATING+(NOLOAD)} :
  {
    *(.zdebug_gdb_scripts)
  }

The .debug_gdb_scripts section was added May 15, 2017, and is in GNU binutils 2.29 and later. So it may be that we can simply drop the -T option, and tell people running old binutils versions to add it themselves via --extldflags.

@thanm
Copy link
Contributor

thanm commented Dec 11, 2020

Thanks @ianlancetaylor , that makes sense.

A lot of our gomotes still seem to be running older versions of binutils, so I can definitely still see the problem if remove the workaround.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/278932 mentions this issue: cmd/link: avoid use of -T when linking with lld

@thanm
Copy link
Contributor

thanm commented Dec 17, 2020

I've sent a CL that should take care of the primary issue. @Keithcat1 if you are able to build Go from source, perhaps you can try it out once the CL goes in.

It would also be helpful to know exactly which version of lld you are testing against.

Verifying the CL turned out to be a bit more work than I had thought; the C compilers on our gomotes are pretty old (do not support -fuse-ld=lld). I tested using a toolchain from https://musl.cc/x86_64-w64-mingw32-native.zip, in combination with a copy of LLD extracted from https://github.com/llvm/llvm-project/releases/download/llvmorg-11.0.0/LLVM-11.0.0-win64.exe.

@Keithcat1
Copy link
Author

At the time when I posted this issue, I was using LLD 10.0 and GCC 9 or GCC 10 as well. I was using MSYS64 to get GCC and LLD.

@thanm
Copy link
Contributor

thanm commented Dec 17, 2020

Thanks @Keithcat1 . Do you want me to hold the bug open, or can it be closed once the change is in?

@Keithcat1
Copy link
Author

I haven't been able to verify yet that this works. Thanks for helping with this though.

gopherbot pushed a commit that referenced this issue Dec 18, 2020
When doing external linking on Windows, auto-detect the linker flavor
(bfd vs gold vs lld) and when linking with "lld", avoid the use of
"-T" (linker script), since this option is not supported by lld.
[Note: the Go linker currently employs -T to ensure proper placement
of the .debug_gdb_scripts section, to work around issues in older
versions of binutils; LLD recognizes this section and does place it
properly].

Updates #39326.

Change-Id: I3ea79cdceef2316bf86eccdb60188ac3655264ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/278932
Trust: Than McIntosh <[email protected]>
Run-TryBot: Than McIntosh <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Jeremy Faller <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
@Keithcat1
Copy link
Author

I can't get it to work.
I tried "set CGO_LDFLAGS=-fuse-ld=lld and I got that same old error message:
lld: error: unknown argument: -T
I'm using:
go version devel +2de7866470 Fri Dec 18 18:25:14 2020 +0000 windows/386

@thanm
Copy link
Contributor

thanm commented Dec 21, 2020

Hmm. That is puzzling. Perhaps the auto-detection scheme that I put in is not working?

On the system where you are seeing the failure, what output do you see when you run the command

$CC -fuse-ld=lld -Wl,--version

Thanks.

@Keithcat1
Copy link
Author

The -T argument is properly removed when I do:
go build --ldflags=--extldflags=-fuse-ld=lld
My binary is linking successfully as well.
But actually running it causes windows to say that it's not responding.
There doesn't seem to be any panic text or anything, it just crashes.
I had the idea of running the binary under LLDB, and LLDB informed me that there was an exception on amd64.s:123
Did it work on your end?

@thanm
Copy link
Contributor

thanm commented Dec 21, 2020

I see something similar on my windows machine. I diff'd the section header dumps for the two binaries (ld.lld and ld.bfd) and there are a lot of differences-- hard to know exactly what the problem might be.

asm64.s:123 doesn't sound like it is in the Go runtime (at least I can't find a file named asm64.s anywhere in the sources, except as testdata).

@dmitshur
Copy link
Contributor

@thanm Is there something here that needs to be done for Go 1.16 release, or should this issue be moved to a different milestone?

@thanm
Copy link
Contributor

thanm commented Feb 10, 2021

I'll move it to the next milestone -- there is still work to do here, but it won't happen for 1.16.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/291630 mentions this issue: cmd/link: set SizeOfRawData filed in COFF files for .bss section

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/291630 mentions this issue: cmd/link: set SizeOfRawData field in COFF files for .bss section

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/291632 mentions this issue: cmd/link: do not pass -Bsymbolic for PE DLLs

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/291633 mentions this issue: cmd/link: set .ctors COFF section to writable and aligned to 8 bytes

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/291633 mentions this issue: cmd/link: set .ctors COFF section to writable and aligned

gopherbot pushed a commit that referenced this issue Feb 23, 2021
This is only a valid option on ELF. Binutils accepts it, but LLVM
rejects it, so for Windows, it's best to just omit it.

Updates #44250.
Updates #39326.
Updates #38755.
Updates #36439.
Updates #43800.

Change-Id: Iffd2345d757f23dd737e63bd464cd412527077c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/291632
Trust: Jason A. Donenfeld <[email protected]>
Run-TryBot: Jason A. Donenfeld <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
gopherbot pushed a commit that referenced this issue Feb 23, 2021
Without setting these flags, LLVM's LLD ignores the .ctors section when
merging objects.

Updates #44250.
Updates #39326.
Updates #38755.
Updates #36439.
Updates #43800.

Change-Id: I8766104508f7acd832088a590ee7d68afa0d6065
Reviewed-on: https://go-review.googlesource.com/c/go/+/291633
Trust: Jason A. Donenfeld <[email protected]>
Run-TryBot: Jason A. Donenfeld <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
@Keithcat1
Copy link
Author

Debug info seems to be broken when trying to debug executables created by LLVM Mingw.

@rsc rsc unassigned thanm Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants