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/go: linking libgcc into every cgo object can cause multiple definition errors #9510

Closed
ianlancetaylor opened this issue Jan 5, 2015 · 40 comments

Comments

@ianlancetaylor
Copy link
Contributor

issue/a/a.go:

package a

/*
static long long mod(long long a, long long b) { return a % b; }
*/
import "C"

func F(a, b int64) int64 {
    return int64(C.mod(C.longlong(a), C.longlong(b)))
}

issue/b/b.go exactly the same except "package b" instead of "package a".

issue/main.go:

package main

import (
    "fmt"

    "issue/a"
    "issue/b"
)

func main() {
    fmt.Println(a.F(1, 1), b.F(1, 1))
}

GOARCH=386 go build issue

# issue
/var/tmp/go-link-0Vel41/000001.o: In function `__moddi3':
(.text+0x70): multiple definition of `__moddi3'
/var/tmp/go-link-0Vel41/000000.o:(.text+0x70): first defined here
collect2: error: ld returned 1 exit status
/home/iant/go/pkg/tool/linux_amd64/8l: running gcc failed: unsuccessful exit status 0x100

The problem is that for GOARCH=386 both object files require __moddi3. The go command links both cgo objects against libgcc, because of issue #3261. So both object files define __moddi3, and the linker complains.

On systems that support external linking, which I think is everything but Windows, it should no longer be necessary to link libgcc into each cgo object file. Unless, of course, the standard packages that use cgo (net, crypto/x509) use a libgcc function.

@mdempsky
Copy link
Member

mdempsky commented Jan 8, 2015

iant: Did you have a solution in mind for this? I'm thinking something along the lines of:

  1. Have cmd/go/build.go stop passing -lgcc, -lmingw32, etc. to "gcc -Wl,-r" when creating each cgo-using package's _all.o file.
  2. In external linking mode, simply add back -lmingw32, etc. and it should resolve the missing symbols appropriately. (I assume -lgcc isn't needed since gcc should add it automatically?)
  3. In internal linking mode, add an extra "gcc -Wl,-r" phase to collect all the per-package _all.o files into a single "_allall.o" file, and pass -lgcc, -lmingw32, etc. at that time. Then perform internal linking as normal using the one "_allall.o" file instead of every individual _all.o file.

I suspect part 3 could be constrained to only trigger in cases where we'd switch to external linking if it was supported (i.e., "runtime/cgo" imported and a non-internal host object file is present). However, I'm not sure I understand the purpose served by internal linking, so I'm unclear exactly when we'd want/need to omit those extra steps.

@minux
Copy link
Member

minux commented Jan 8, 2015

The problem of your _allall.o approach is that internal linking is no longer
possible without gcc installed (think windows, where mingw is not normally
installed.)

The reason for having internal linking is that people just using standard
cgo packages (os/user, crypto/x509, net, etc.) will not need to have gcc
installed. If we always require gcc, then it's much better to always use
external linking mode for all cgo programs.

@mdempsky
Copy link
Member

mdempsky commented Jan 8, 2015

Ohh, I think I understand now: it's to support the use case where the standard library is pre-compiled on a system where GCC is present, then distributed to systems where it's not? If so, that makes sense. (I was confused how os/user, net, etc. could be compiled in the first place without GCC present.)

@minux
Copy link
Member

minux commented Jan 8, 2015

Yes. We're talking about users of our binary distributions.
The internal linking mode is mostly for them.

@ianlancetaylor
Copy link
Contributor Author

I agree with your items 1 and 2. Note that even -lmingw32 is added automatically. I doubt the go tool will need to add anything special.

Unfortunately as minux notes your item 3 is problematic. For internal linking it would be best if we didn't have to assume that libgcc.a was available at link time. But we also don't want to link it into the .o we build for the package that uses cgo. Unfortunately at the time we build that package we don't know whether we will be doing internal linking or not. We probably don't want to copy libgcc.a around, and we probably don't want to build every cgo package twice. I don't have a simple solution for this.

@minux
Copy link
Member

minux commented Jan 8, 2015

Ian, is there some kind of linker magic that could split the object files
extracted from libgcc.a into a different output object file?

i.e. when linking _all.o, we link libgcc.a as usual, but put all needed
objects from libgcc.a into a separate _gcc.o, not to _all.o.

And we can include _gcc.o as needed when do the final linking, depending
on -linkmode.

The benefit is that, we do this for all cgo packages, so if the user wants,
they can still use internal linking for compatible cgo packages (that don't
use any feature not understand by our 6l.)

@mdempsky
Copy link
Member

mdempsky commented Jan 8, 2015

Wouldn't that just shift the problem of symbol collisions from _all.o to _gcc.o?

@minux
Copy link
Member

minux commented Jan 8, 2015

_gcc.o is only used by our linker, and we can teach the linker to
ignore duplicates. (the go tool ignores files with underscore
prefixes, so any special handling of _gcc.o won't affect users'
code)

External linking will proceed as you've proposed, without using
_gcc.o.

@minux
Copy link
Member

minux commented Jan 8, 2015

Another possibility is that we make the linker label symbols
from libgcc.a as dupok (common symbols?) when linking
_all.o.

@ianlancetaylor
Copy link
Contributor Author

I'm not aware of any linker magic that will put only needed objects into a separate output file. Of course we could do this ourselves.

It is technically possible to mark the symbols from libgcc.a dupok (which in ELF and PE is called comdat--common symbols are also dupok but they are different), but again we would have to do it ourselves. It's non-trivial for ELF.

@mdempsky
Copy link
Member

mdempsky commented Jan 8, 2015

How about this variation to item 3:

3b. In internal linking mode and only if there are cgo-using packages from outside of stdlib, use "gcc -Wl,-r -o _allall.o ... -nostdlib -lgcc ..." to resolve libgcc dependencies.

I looked over the stdlib's cgo dependencies and they seem pretty basic, so I'd be surprised if they end up depending on libgcc. So it's hopefully just non-stdlib cgo objects that should be an issue.

As for Windows, if there are cgo-using packages from outside of stdlib, that seems like a good signal that GCC should be available. Unless we're concerned about supporting Windows users without GCC receiving pre-compiled non-stdlib cgo packages too? (I'm inclined to argue users that want to do that should simply be required to also install GCC.)

@mdempsky
Copy link
Member

mdempsky commented Jan 8, 2015

Or maybe it's overall simpler to implement external linking for Windows, and then we don't need to worry about supporting internal linking with external cgo objects?

@ianlancetaylor
Copy link
Contributor Author

In an ideal world, it would be possible for somebody to distribute binaries for Go packages that use cgo, and for somebody else to install those binaries on a system without a compiler, and use internal linking to build the programs. That is, ideally, internal linking will work for packages other than the standard packages. But perhaps we should make that a future pipedream and focus on getting things working today.

@minux
Copy link
Member

minux commented Jan 9, 2015

Yeah, the ideal goal that Ian just mentioned is worth pursuing.
I always think internal linking should work whenever the packages
used don't need any fancy linker features (i.e. plain C without
constructors, destructors and the like.)

For the idea of splitting objects from libgcc out to a separate object
file, could we feed ld a linker script that links all objects from libgcc
into separate sections, and then use objcopy to split those sections
out?

Of course, we can parse the output of ld -M to find out which symbols
are pulled from libgcc, but that will require we link three times (one with
libgcc to get linker map, and one without to get _all.o, and then once
more to get _gcc.o)

Another idea: use -shared-libgcc when linking cgo's _cgo_main.o,
use -shared-libgcc so that we can discover which parts of libgcc
are being pulled in, and then we can create _gcc.o using those
symbols. This approach needs to link two times (for _all.o and
_gcc.o), but I think that's as few as we can get. I assume all
interesting symbols in libgcc.a are also included in libgcc_s.so,
no?

@ianlancetaylor
Copy link
Contributor Author

I don't see how to do the linker script trick on Darwin.

Here is another idea, kind of like the earlier ideas but it's simpler than I thought. Link the .o with -r without libgcc. Then examine the symbols in the object--we have the packages we need for that. The libgcc symbols are easy to spot: they are undefined and start with "__". We write some archive parsing code--archives are pretty simple and pretty much the same on all platforms. We use that code to read libgcc.a and find the objects that define those symbols (if we can't find the symbol, we ignore it; it must be defined in some other library). We pull those objects out of libgcc.a and store them somewhere in the Go .a file with their name. Now at final link time, when doing internal linking, we gather together all those libgcc objects, pick one for each name, and add them to the final link.

@mdempsky
Copy link
Member

mdempsky commented Jan 9, 2015

Some libgcc.a object files will have dependencies on other objects. E.g., on amd64, _popcountsi2.o (for __popcountti2) depends on _popcount_tab.o (for __popcount_tab). So we'd need to recursively resolve symbols.

Also, not all "__" prefixed symbols are necessarily from libgcc. E.g., __tls_get_addr is from libc. Hm, I also even see "isinfd{32,64,128}" defined in my libgcc.a.

I'm curious if maybe after linking _all.o without -lgcc, we could write a second stub.o file that merely contains relocations for all of the unresolved maybe-libgcc symbols we find in _all.o, and then use "gcc -o _libgcc.o stub.o -Wl,-r" to let ld handle recursively figuring out which objects we need? We could then post-process _libgcc.o to mark all the symbols as comdat or teach cmd/ld to treat it that way?

@minux
Copy link
Member

minux commented Jan 9, 2015

yes, that sounds like a plan. we don't even need to parse libgcc.a.

first link _all.o without -lgcc, and then just write a dummy C file that
pulls in all the undefined symbols from _all.o,
gcc -o _gcc.o -Wl,-r dummy.c -lgcc
should give us what we want.

We don't even need to create a C file to pull in the symbols, we
can create a empty ELF object .o (for example, by as -o empty.o /dev/null),
and then use
ld -r -o dummy.o empty.o -u NEEDED_SYMBOL -u NEEDED_SYMBOL2 ...
Then
ld -r -o _gcc.o dummy.o gcc -print-libgcc-file-name

In this way, we don't waste any memory while still pulling in every
needed symbols from libgcc.

NOTE: Although all three commands work on Darwin, the final output file
_gcc.o contains no symbols, so for darwin, we still need to create a C file
to pull in all the symbols.

@ianlancetaylor
Copy link
Contributor Author

I thought about the ld -r -lgcc approach, but it doesn't work. You wind up with a single object. In the final link, you will have a collection of single objects. It could be the case that you wind up with two objects, one of which defines symbols A and B, and one of which defines symbols A and C. You can't link those objects together, because you will get a multiple definition error on A. You can't omit either one. So you are stuck.

If it were easy to modify a file to mark a symbol as comdat we could do that, but it is not. In ELF symbols are not comdat, sections are. You have to create a section group that refers to other sections, and mark that entire group as comdat. Once the object has been linked together, it's too late.

You're right about libgcc objects referring to other libgcc objects. That is a bit of a pain.

It doesn't matter if you see __ symbols that aren't in libgcc. You can ignore those, as they must be coming from somewhere else.

You're right about isinfd32/isinfd64/isinfd128. I have no idea what those are doing in libgcc. They are part of the binary decimal floating point support. I've asked about them on the gcc mailing list. I suspect it's a mistake.

@mdempsky
Copy link
Member

mdempsky commented Jan 9, 2015

Yeah, I was thinking about that later too. Too bad ld doesn't have a link-time analog to -f{function,data}-sections, or a mode like -Wl,-r but that outputs a .a archive instead of a .o object.

Alright, I think I'm sold on your Jan 8, 2015, 9:41PM PST suggestion as the least hacky solution. (Is there really no easy to reference ID for github issue comments??)

@cespare
Copy link
Contributor

cespare commented Jan 9, 2015

@mdempsky Sure there is:

#9510 (comment)

(click on on the "5 hours ago" -- or whatever -- timestamp).

@cespare
Copy link
Contributor

cespare commented Jan 9, 2015

Well, that's the automatically linkified version. The actual link is

https://github.com/golang/go/issues/9510#issuecomment-69295721

@minux
Copy link
Member

minux commented Jan 9, 2015

OK, we need to parse the linker generated map file to get the name object
files linked.

-Map=outputfile on GNU ld, -mapfile=outputfile on Darwin.

The output format is similar, we can just look for the pattern
libgcc.a(object.o)
and ignore any duplicates.

// on darwin

Object files:

[ 0] linker synthesized
[ 1] a.o
[ 2]
/usr/llvm-gcc-4.2/bin/../lib/gcc/i686-apple-darwin11/4.2.1/x86_64/libgcc.a(darwin-64.o)

// on Linux
Archive member included because of file (symbol)

/usr/local/lib/gcc/x86_64-unknown-linux-gnu/5.0.0/libgcc.a(_ashldi3.o)
a.o (__ashlti3)

And then we need to either extract these files using our ar parser or just
ask use "ar x"
command.

@minux
Copy link
Member

minux commented Mar 26, 2015

I still think the ld -r -lgcc approach could work. It will only
be used in internal linking, and we can modify our linker
to ignore multiple definitions from object files coming from
libgcc.

Yes, this will waste space in the final executable, but the
resulting executable should still work and the implementation
complexity is significantly less than other approaches (we
don't need to extract object files from libgcc.a and don't need
to handle inter-dependencies between those object files.

(Internal linking already produces larger executable than
external linker. for example, linking misc/cgo/test on
windows/amd64 with external linking produces a binary 1MB
smaller than the one produced with internal linking. Now that
all platforms support external linking, internal linking of cgo
programs is really just for compiling programs using just the
standard packages.)

@karalabe
Copy link
Contributor

Just hit this error while cross compiling CGO enabled stuff to Android. I'm not sure if this helps other people or not, but I thought I'd post a very ugly, hacky, etc workaround that however did solve the issue I hit with the multiple definitions (notably, it was some NDK stack unwinding code duplication).

The temporary workaround that I've found to fix this issue is to request the external linker to allow multiple definitions. Maybe this is not the correct solution in the general case, but for my specific scenario I'm rebuilding things from scratch, so all code referencing the same symbol is sure to use the same version.

-extldflags=-Wl,--allow-multiple-definition

If this is something very very bad, please enlighten me :)

@rsc
Copy link
Contributor

rsc commented Nov 4, 2015

@ianlancetaylor, what do you think of -extldflags=-Wl,--allow-multiple-definition by default?

@ianlancetaylor
Copy link
Contributor Author

I think --allow-multiple-definitions is pretty ugly, but, more importantly, I think it would not work on Windows or Darwin. I skimmed the docs for those linkers and I didn't see any corresponding option.

I have a different idea that may be simpler. This is only an issue for internal linking. Let's have cmd/dist run ld -nostdlib -Wl,-r -Wl,--whole-archive -lgcc to get libgcc as a single object. Let's store that somewhere under pkg, and link against it when using internal linking.

@ianlancetaylor
Copy link
Contributor Author

Hmmm, libgcc is surprisingly large. Linking it all together into every tool built with internal linking may not be the best idea.

@weitzj
Copy link

weitzj commented Nov 5, 2015

Will this also benefit me, if I crosscompile on OSX for Android?

Right now I have to use:

gomobile bind -target=android -ldflags "-extldflags=-Wl,--allow-multiple-definition"

@ianlancetaylor
Copy link
Contributor Author

@weitzj Yes, if we fix this issue, you won't have to pass --allow-multiple-definition.

@gopherbot
Copy link

CL https://golang.org/cl/16741 mentions this issue.

nalind added a commit to nalind/docker that referenced this issue Dec 2, 2015
Assume that the linker can make sense of us passing in the -z,muldefs
option to tell it to ignore symbol-multiply-defined errors triggered by
golang/go#9510.  We should be able to stop
doing this once we move to Go 1.6.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com> (github: nalind)
nalind added a commit to nalind/docker that referenced this issue Jan 4, 2016
Pull in changes from upstream PR#18197:

Assume that the linker can make sense of us passing in the -z,muldefs
option to tell it to ignore symbol-multiply-defined errors triggered by
golang/go#9510.  We should be able to stop
doing this once we move to Go 1.6.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com> (github: nalind)
nalind added a commit to nalind/docker that referenced this issue Jan 4, 2016
Pull in changes from upstream PR#18197:

Assume that the linker can make sense of us passing in the -z,muldefs
option to tell it to ignore symbol-multiply-defined errors triggered by
golang/go#9510.  We should be able to stop
doing this once we move to Go 1.6.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com> (github: nalind)
rhatdan pushed a commit to rhatdan/moby1 that referenced this issue Jan 22, 2016
Pull in changes from upstream PR#18197:

Assume that the linker can make sense of us passing in the -z,muldefs
option to tell it to ignore symbol-multiply-defined errors triggered by
golang/go#9510.  We should be able to stop
doing this once we move to Go 1.6.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com> (github: nalind)
rhatdan pushed a commit to projectatomic/docker that referenced this issue Jan 26, 2016
Pull in changes from upstream PR#18197:

Assume that the linker can make sense of us passing in the -z,muldefs
option to tell it to ignore symbol-multiply-defined errors triggered by
golang/go#9510.  We should be able to stop
doing this once we move to Go 1.6.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com> (github: nalind)
@golang golang locked and limited conversation to collaborators Nov 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants