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: windows cgo executables missing some DWARF information #10776

Closed
alexbrainman opened this issue May 11, 2015 · 29 comments
Closed

cmd/link: windows cgo executables missing some DWARF information #10776

alexbrainman opened this issue May 11, 2015 · 29 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Milestone

Comments

@alexbrainman
Copy link
Member

In issue #4069 @minux said:

Ahh, that explains the huge size difference. I will take a look. We should
generate those dwarf sections when doing external linking.

I don't know the details. I am just creating this so we don't forget.

Alex

@alexbrainman alexbrainman added this to the Go1.5 milestone May 11, 2015
@rsc rsc changed the title cmd/ld: windows cgo executables missing some DWARF information cmd/link: windows cgo executables missing some DWARF information Jun 8, 2015
@rsc
Copy link
Contributor

rsc commented Jul 14, 2015

Does PE's .o format have a way to put DWARF information into the object?
What is it?

@alexbrainman
Copy link
Member Author

Does PE's .o format have a way to put DWARF information into the object?
What is it?

I don't know. But we're not talking about some "general" PE's .o format. We're using mingw here, so it is about what mingw requires.

I am certain mingw compiled .o files do contain DWARF information. When we build cgo with hostlink=internal, we get .o files produced by mingw that contain DWARF. I can see C symbols during debugging. But I don't know what the format is.

I tried to see why we don't generate DWARF symbols, and I didn't get very far. I have applied this change:

diff --git a/src/cmd/link/internal/ld/dwarf.go b/src/cmd/link/internal/ld/dwarf.go
index b8fb2e6..d276b29 100644
--- a/src/cmd/link/internal/ld/dwarf.go
+++ b/src/cmd/link/internal/ld/dwarf.go
@@ -2087,6 +2087,8 @@ func writedwarfreloc(s *LSym) int64 {
            i = Thearch.Elfreloc1(r, int64(r.Off))
        } else if HEADTYPE == obj.Hdarwin {
            i = Thearch.Machoreloc1(r, int64(r.Off))
+       } else if HEADTYPE == obj.Hwindows {
+           // TODO: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
        } else {
            i = -1
        }
@@ -2120,7 +2122,7 @@ func Dwarfemitdebugsections() {
        return
    }

-   if Linkmode == LinkExternal {
+   if Linkmode == LinkExternal && HEADTYPE != obj.Hwindows {
        if !Iself && HEADTYPE != obj.Hdarwin {
            return
        }

to restore writing of DWARF sections for when Linkmode == LinkExternal. And I can see Go DWARF info is stored in the exe now:

C:\dev\src\foo>type main.go
package main

import "C"

var alexval = "alex"

func alexfunc(s string) string {
        return s
}

func main() {
        println(alexfunc(alexval))
}

C:\dev\src\foo>go build

C:\dev\src\foo>objdump --dwarf foo.exe | find "alex"
objdump: Warning: Corrupt unit length (0x0) found in section .debug_info
objdump: Warning: Only DWARF 2 and 3 aranges are currently supported.
objdump: Warning: Only DWARF 2 and 3 pubnames are currently supported
    2292d       main.alexval
objdump: Warning: Bogus end-of-siblings marker detected at offset 11a6 in .debug_info section
objdump: Warning: Bogus end-of-siblings marker detected at offset 11a7 in .debug_info section
objdump: Warning: DIE at offset 11a8 refers to abbreviation number 67 which does not exist
objdump: Warning: Only DWARF version 2, 3 and 4 line info is currently supported.
objdump: Warning: Unable to load/parse the .debug_info section, so cannot interpret the .debug_loc section.

C:\dev\src\foo>

But DWARF info looks corrupted (see objdump warnings). And gdb complains about it too:

C:\dev\src\foo>gdb foo.exe
GNU gdb (GDB) 7.6.1
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "mingw32".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from C:\dev\src\foo\foo.exe...Dwarf Error: wrong version in compilation unit header (is 0, should be 2, 3, or 4) [in module C:\dev\src\foo\foo.exe]
(no debugging symbols found)...done.
(gdb)

I tried reading DWARF info with our debug/dwarf package (similar to TestDWARF in debug/pe), and it does not complain about anything.

I don't have any great ideas about this. Perhaps next step would be to understand why gdb and objdump complain. So I need to download their source and build them.

Do you have anything better?

Alex

@ianlancetaylor
Copy link
Contributor

DWARF goes into PE exactly as for ELF: in sections with special names that the debugger looks for. The debug/pe package can even find the DWARF info.

@alexbrainman To make this work for external linking, you really can't skip the generation of the external reloc.

I'm going to move this to unplanned. It would be great if somebody wants to work on it, but I think it's too late for 1.5.

@alexbrainman
Copy link
Member Author

My latest investigations are here https://go-review.googlesource.com/#/c/13571

Alex

@alexbrainman
Copy link
Member Author

Adding Go1.6 label. I believe this was working in go1.4, but is broken now. In the hope that others will help ..

Alex

@rsc rsc modified the milestones: Unplanned, Go1.6 Dec 5, 2015
@rsc
Copy link
Contributor

rsc commented Dec 5, 2015

Looks like help did not arrive.

@clns
Copy link

clns commented May 19, 2016

Looks like this didn't land in Go 1.6... Any hopes for Go 1.7?

@alexbrainman
Copy link
Member Author

I didn't have time to work on this.

Alex

@bradfitz bradfitz modified the milestones: Go1.8, Unplanned May 19, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 7, 2016
@rsc rsc modified the milestones: Go1.9Early, Go1.8 Oct 25, 2016
@alexbrainman
Copy link
Member Author

I've just experienced this issue via delve on both Windows and Max OSX with both Go 1.8 RC and 1.7.5

I am not sure about Max OSX, but yes it is currently broken on Windows.

setting -ldflags="-linkmode internal" seems to do the trick and fix the issue for it.

Correct.

When using -linkmode external the issue seems to happen again.

Correct.

As I've yet to find the information on what these two linking modes are doing

I do not know of any documentation, but https://golang.org/pkg/cmd/link/. But (the way I understand it), the -linkmode flag specifies if cmd/link links final executable itself (internal), or calling external linker - gcc (external). This current issue (issue #10776) is that when -linkmode=external is used, final executable does not includes DWARF information - it means it cannot be debugged properly. Also -linkmode=external setting is default - if you don't pass -linkmode flag, then it will be set to external on Windows. So any Windows cgo executable have no debug information by default.

I hope it helps.

Alex

@ianlancetaylor
Copy link
Contributor

@alexbrainman We can pick internal or external linking mode until after all the input files have been seen. But from a quick look, I don't think the three values you mention are actually used in any way until after the link mode is set anyhow. That is, they are currently set in Peinit, but I think they could be set later, specifically, in Asmbpe.

@alexbrainman
Copy link
Member Author

I don't think the three values you mention are actually used in any way until after the link mode is set anyhow. That is, they are currently set in Peinit, but I think they could be set later, specifically, in Asmbpe.

Peinit is called from archinit: https://github.com/golang/go/blob/master/src/cmd/link/internal/amd64/obj.go#L145-L156
PESECTALIGN (and other variables) is used there in archinit to set ld.FlagRound. If I move code that sets PESECTALIGN value from Peinit and into Asmbpe, the ld.FlagRound value (and others) will be wrong. Isn't it?

I can, probably, add code in Asmbpe that sets HEADR, FlagTextAddr, FlagRound as well as PESECTHEADR, PESECTALIGN and PEFILEALIGN, but I am not sure what other global variables or state did I miss. Sounds too easy to break in the future.

I was hoping we could move call to archinit after loadlib or something. Or maybe split archinit into 2 parts. Or maybe call archinit twice - before loadlib call and after.

Thank you.

Alex

@ianlancetaylor
Copy link
Contributor

Calling archinit twice is not going to be a good idea, and it won't fix your problem anyhow since you won't be updating FlagRound. Though I have to say that whole notion of setting FlagRound seems quite broken.

Anyhow, FlagRound too is most likely only used by Asmbpe and later, and same for the other flags.

This kind of code is definitely easy to break. That is the nature of our current linker: we set values in one place and use them in another, and it's hard to see the connections between the two and it's hard to be certain that the place where we set the value always runs before the place where we use the value. Perhaps we need to add a set-once, use-many type that gives an error on a second set or on a set after a use.

@alexbrainman
Copy link
Member Author

Perhaps we need to add a set-once, use-many type that gives an error on a second set or on a set after a use.

I had a quick look and I think ld.HEADR, ld.FlagTextAddr, ld.FlagDataAddr and ld.FlagRound can be made private, they are set in archinit and then used in ld package only. Might make this whole thing look less scary.

Alex

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Feb 16, 2017
For #10776.

Change-Id: I7931558257c1f6b895e4d44b46d320a54de0d677
Reviewed-on: https://go-review.googlesource.com/36973
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
gopherbot pushed a commit that referenced this issue Feb 16, 2017
This is what gcc does when it generates object files.
And it is easier to count everything, when it starts from 0.
Make go linker do the same.

gcc also does not output IMAGE_OPTIONAL_HEADER or
PE64_IMAGE_OPTIONAL_HEADER for object files.
Perhaps we should do the same, but not in this CL.

For #10776.

Change-Id: I9789c337648623b6cfaa7d18d1ac9cef32e180dc
Reviewed-on: https://go-review.googlesource.com/36974
Reviewed-by: Ian Lance Taylor <iant@golang.org>
gopherbot pushed a commit that referenced this issue Feb 16, 2017
For #10776.

Change-Id: Id64a7e35c7cdcd9be16cbe3358402fa379090e36
Reviewed-on: https://go-review.googlesource.com/36975
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Feb 21, 2017
This is what gcc does when it generates object files.
And pecoff.doc says: "for simplicity, compilers should
 set this to zero". It is easier to count everything,
when it starts from 0. Make go linker do the same.

For #10776.

Change-Id: Iffa4b3ad86160624ed34adf1c6ba13feba34c658
Reviewed-on: https://go-review.googlesource.com/36976
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Feb 21, 2017
Introduce a slice that keeps long pe section names as we add them.
It will be used later to output pe symbol table and dwarf relocations.

For #10776.

Change-Id: I02f808a456393659db2354031baf1d4f9e0b2d61
Reviewed-on: https://go-review.googlesource.com/36977
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Feb 21, 2017
dwarf relocations refer to dwarf section symbols, so dwarf
section symbols must be present in pe symbol table before we
write dwarf relocations.

.ctors pe section already refer to .text symbol.

Write all pe section name symbols into symbol table, so we
can use them whenever we need them.

This CL also simplified some code.

For #10776.

Change-Id: I9b8c680ea75904af90c797a06bbb1f4df19e34b6
Reviewed-on: https://go-review.googlesource.com/36978
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Feb 21, 2017
The symbols get in a way when using external linker. They are
not associated with a section. And linker fails when
generating relocations for them.

__image_base__ and _image_base__ have been added long time ago.
I do not think they are needed anymore. If I delete them, all
tests still PASS. I tried going back to the commit that added
them to see if I can reproduce original error, but I cannot
build it. I don't have hg version of go repo, and my gcc is
complaining about cc source code.

I wasted too much time with this, so I decided to leave them only
for internal linker. That is what they were originally added for.

For #10776.

Change-Id: Ibb72b04f3864947c782f964a7badc69f4b074e25
Reviewed-on: https://go-review.googlesource.com/36979
Reviewed-by: Ian Lance Taylor <iant@golang.org>
gopherbot pushed a commit that referenced this issue Feb 21, 2017
dwarf writing code assumes that dwarf sections follow
.data and .bss, not .ctors. Make pe section writing code
match that assumption.

For #10776.

Change-Id: I128c3ad125f7d0db19e922f165704a054b2af7ba
Reviewed-on: https://go-review.googlesource.com/36980
Reviewed-by: Ian Lance Taylor <iant@golang.org>
gopherbot pushed a commit that referenced this issue Feb 21, 2017
No functional changes.

For #10776.

Change-Id: If9a5ef832af116c5802b06a38e0c050d7363f2d5
Reviewed-on: https://go-review.googlesource.com/36981
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
gopherbot pushed a commit that referenced this issue Mar 1, 2017
For #10776.

Change-Id: I11dd441d8e5d6316889ffa8418df8b58c57c677d
Reviewed-on: https://go-review.googlesource.com/36982
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Mar 1, 2018
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. OS-Windows
Projects
None yet
Development

No branches or pull requests

10 participants