code review 6445085: cmd/6l, cmd/ld, cmd/go: consistent binary for cgo programs

cmd/ld: consistent binary for cgo programs
We use pkg path instead of file name (which contains $WORK) in section symbols names.

[minux1, 2012-08-07 14:18:36]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[minux1, 2012-08-07 14:28:35]

This CL is not complete, please suggest better ways to do the same.

This CL makes building the same (cgo) source code always producing identical
binaries on Linux/amd64.

Two involved problems:
1. debug info. generated by 'gcc -g' will contain path info. this is solved
by -fdebug-prefix-map
suggested by iant (note this flag is only supported in newer version of
gcc, and unfortunately,
gcc 4.2.1 doesn't support it)
2. our ld generates section symbols of the form "filename.a(file.o)", which
contains the
temporary path. I introduced -Y flag to ld to help stripping the $WORK part
from filename.a
(just like the -P flag to pack).

Obviously, problem 2 has at least other solutions.
1. we can simply remove path from section symbol.
2. we can remove section symbol from symbol table of the final binary

what do you think? personally, i don't like the current approach, but each
alternative
has its own downside, which i can't decide.

[albert.strasheim, 2012-08-08 05:52:34]

Hello

Thanks very much for the work so far.

On Tuesday, August 7, 2012 4:28:14 PM UTC+2, minux wrote:
>
> what do you think? personally, i don't like the current approach, but each 
> alternative
> has its own downside, which i can't decide.
>

A few random thoughts. Maybe it helps:

A potential addition/alternative is a modification to the go tool to add an 
option so that it doesn't squirrel away generated code in a random 
directory that gets deleted, but instead puts it in a well-defined place 
like the Makefiles of old.

I get the idea that this will give more consistent debuginfo. When we 
attempted coverage reports with gccgo a few months before, the mix of 
"real" and "temporary" sources lead to a few strange artifacts in the 
reports that you don't see with C.

It would be really nice if Go binaries could interoperate properly with 
ideas like Fedora's debuginfos, which interacts with perf and other tools.

The use case looks something like this:

1. Install foo.rpm
2. Run the foo service.
3. See from top that foo is very busy for some reason
4. Run perf top
5. See that foo is using a lot of CPU, but you can't see exactly what it's 
doing
6. Install foo-debuginfo.rpm
7. Run perf top again.
8. See exactly what foo is doing

perf is really awesome for profiling a whole system running many 
interacting Go processes.

I still want to look into exactly what the RPM build process does with 
debuginfos, because even though you can build in $HOME/rpmbuild/BUILD/..., 
the -debuginfo rpm always puts sources under /usr/src and probably does 
some rewriting of the binaries somewhere so that this works.

Useful reading:

https://fedoraproject.org/wiki/StackTraces

Regards

Albert

[minux1, 2012-08-09 05:41:45]

On Wed, Aug 8, 2012 at 1:52 PM, Albert Strasheim <fullung@gmail.com> wrote:

> On Tuesday, August 7, 2012 4:28:14 PM UTC+2, minux wrote:
>>
>> what do you think? personally, i don't like the current approach, but
>> each alternative
>> has its own downside, which i can't decide.
>>
>
> A few random thoughts. Maybe it helps:
>
> A potential addition/alternative is a modification to the go tool to add
> an option so that it doesn't squirrel away generated code in a random
> directory that gets deleted, but instead puts it in a well-defined place
> like the Makefiles of old.
>
i like this idea, and it's very useful when debugging cgo programs, we just
need a -notmpdir flag
to place the WORK directory in the current directory (but go tool currently
generates a fairly long
path that resemble the real path of source files, this should be changed to
make this feature more
useful).

>
> I get the idea that this will give more consistent debuginfo. When we
> attempted coverage reports with gccgo a few months before, the mix of
> "real" and "temporary" sources lead to a few strange artifacts in the
> reports that you don't see with C.
>
> It would be really nice if Go binaries could interoperate properly with
> ideas like Fedora's debuginfos, which interacts with perf and other tools.
>
maybe we just need a flag for the toolchain (ld) to rewrite paths (like
what -fdebug-prefix-map does for gcc).

[albert.strasheim, 2012-08-09 07:05:37]

Hello

On Thu, Aug 9, 2012 at 7:41 AM, minux <minux.ma@gmail.com> wrote:
> On Wed, Aug 8, 2012 at 1:52 PM, Albert Strasheim <fullung@gmail.com> wrote:
>> On Tuesday, August 7, 2012 4:28:14 PM UTC+2, minux wrote:
>>> what do you think? personally, i don't like the current approach, but
>>> each alternative
>>> has its own downside, which i can't decide.
>> A few random thoughts. Maybe it helps:
>> A potential addition/alternative is a modification to the go tool to add
>> an option so that it doesn't squirrel away generated code in a random
>> directory that gets deleted, but instead puts it in a well-defined place
>> like the Makefiles of old.
> i like this idea, and it's very useful when debugging cgo programs, we just
> need a -notmpdir flag
> to place the WORK directory in the current directory (but go tool currently
> generates a fairly long
> path that resemble the real path of source files, this should be changed to
> make this feature more
> useful).

Sounds like a good plan.

>> I get the idea that this will give more consistent debuginfo. When we
>> attempted coverage reports with gccgo a few months before, the mix of "real"
>> and "temporary" sources lead to a few strange artifacts in the reports that
>> you don't see with C.
>> It would be really nice if Go binaries could interoperate properly with
>> ideas like Fedora's debuginfos, which interacts with perf and other tools.
> maybe we just need a flag for the toolchain (ld) to rewrite paths (like what
> -fdebug-prefix-map does for gcc).

I suspect RPM packaging does some rewriting using elfutils or something.

The script that runs towards the end of the build is this one:

http://rpm.org/gitweb?p=rpm.git;a=blob;f=scripts/find-debuginfo.sh

More interesting reading on work in progress in this area in Fedora:

https://fedoraproject.org/wiki/Releases/FeatureBuildId

Regards

Albert

[minux1, 2012-08-20 16:23:50]

On Tue, Aug 7, 2012 at 10:28 PM, minux <minux.ma@gmail.com> wrote:

> This CL is not complete, please suggest better ways to do the same.
>
> This CL makes building the same (cgo) source code always producing
> identical
> binaries on Linux/amd64.
>
> Two involved problems:
> 1. debug info. generated by 'gcc -g' will contain path info. this is
> solved by -fdebug-prefix-map
> suggested by iant (note this flag is only supported in newer version of
> gcc, and unfortunately,
> gcc 4.2.1 doesn't support it)
> 2. our ld generates section symbols of the form "filename.a(file.o)",
> which contains the
> temporary path. I introduced -Y flag to ld to help stripping the $WORK
> part from filename.a
> (just like the -P flag to pack).
>
> Obviously, problem 2 has at least other solutions.
> 1. we can simply remove path from section symbol.
> 2. we can remove section symbol from symbol table of the final binary
>
> what do you think? personally, i don't like the current approach, but each
> alternative
> has its own downside, which i can't decide.
>
ping? any advices about which route is better?

[rsc, 2012-09-11 01:08:30]

>> Two involved problems:
>> 1. debug info. generated by 'gcc -g' will contain path info. this is
>> solved by -fdebug-prefix-map
>> suggested by iant (note this flag is only supported in newer version of
>> gcc, and unfortunately,
>> gcc 4.2.1 doesn't support it)

Sounds like -fdebug-prefix-map is really our only choice, unless Ian
has another trick up his sleeve.

>> 2. our ld generates section symbols of the form "filename.a(file.o)",
>> which contains the
>> temporary path. I introduced -Y flag to ld to help stripping the $WORK
>> part from filename.a
>> (just like the -P flag to pack).

These seem fine. I don't think there is a -P flag in the linker right
now. Should we use P instead of -Y for consistency with pack?

>> Obviously, problem 2 has at least other solutions.
>> 1. we can simply remove path from section symbol.
>> 2. we can remove section symbol from symbol table of the final binary

I would prefer not to do #1, because I am worried about symbols colliding.
#2 is probably fine but I am unsure about the implications.

There might be a #3: only use the import path, not the full path to
the archive. I think we should know the import path at that point, no?
Then we'd have runtime/cgo(x.o) instead of
/usr/rsc/go/runtime/cgo.a(x.o).

Russ

[iant, 2012-09-11 03:54:09]

On 2012/09/11 01:08:30, rsc wrote:
> >> Two involved problems:
> >> 1. debug info. generated by 'gcc -g' will contain path info. this is
> >> solved by -fdebug-prefix-map
> >> suggested by iant (note this flag is only supported in newer version of
> >> gcc, and unfortunately,
> >> gcc 4.2.1 doesn't support it)
> 
> Sounds like -fdebug-prefix-map is really our only choice, unless Ian
> has another trick up his sleeve.

This is GCC: there is always another trick.  If you set the PWD environment
variable to an absolute path that resolves to the current directory, it will be
used instead of the real path of the current directory.  So one standard trick
to generate the same objects while compiling in a temporary directory on
GNU/Linux is to set PWD=/proc/self/cwd.

The -fdebug-prefix-map option was added in GCC 4.3, by the way.

[minux1, 2012-09-16 18:37:18]

On Tue, Sep 11, 2012 at 9:08 AM, Russ Cox <rsc@golang.org> wrote:
>
>  >> 2. our ld generates section symbols of the form "filename.a(file.o)",
> >> which contains the
> >> temporary path. I introduced -Y flag to ld to help stripping the $WORK
> >> part from filename.a
> >> (just like the -P flag to pack).
>
> These seem fine. I don't think there is a -P flag in the linker right
> now. Should we use P instead of -Y for consistency with pack?
>
-P is only used by 5l
http://tip.golang.org/src/cmd/5l/asm.c#L1118

>
> >> Obviously, problem 2 has at least other solutions.
> >> 1. we can simply remove path from section symbol.
> >> 2. we can remove section symbol from symbol table of the final binary
>
> I would prefer not to do #1, because I am worried about symbols colliding.
> #2 is probably fine but I am unsure about the implications.
>
> There might be a #3: only use the import path, not the full path to
> the archive. I think we should know the import path at that point, no?
> Then we'd have runtime/cgo(x.o) instead of
> /usr/rsc/go/runtime/cgo.a(x.o).
>
this is a good idea, then we don't need to add a new ld option.

[minux1, 2012-09-16 18:42:12]

PTAL.

On Tue, Sep 11, 2012 at 11:54 AM, <iant@golang.org> wrote:

> Sounds like -fdebug-prefix-map is really our only choice, unless Ian
>> has another trick up his sleeve.
>>
>
> This is GCC: there is always another trick.  If you set the PWD
> environment variable to an absolute path that resolves to the current
> directory, it will be used instead of the real path of the current
> directory.  So one standard trick to generate the same objects while
> compiling in a temporary directory on GNU/Linux is to set
> PWD=/proc/self/cwd.
>
Great. This tricks works for 4.2.1, I've tested this CL on Darwin/amd64,
Darwin/386, Linux/amd64 and Linux/386, and finally we can build the
same binary misc/cgo/test on the same machine.

[iant, 2012-09-16 19:26:58]

https://codereview.appspot.com/6445085/diff/12009/src/cmd/go/build.go
File src/cmd/go/build.go (right):

https://codereview.appspot.com/6445085/diff/12009/src/cmd/go/build.go#newcode...
src/cmd/go/build.go:1099: // we can generate object files that doesn't contain
our $WORK
s/doesn't/don't/

https://codereview.appspot.com/6445085/diff/12009/src/cmd/go/build.go#newcode...
src/cmd/go/build.go:1101: os.Setenv("PWD", "/WORK")
Does this work?  GCC will check the PWD environment variable and only use it if
it does in fact name the current directory.  Does the WORK directory exist?

[minux1, 2012-09-16 20:00:31]

On Monday, September 17, 2012, wrote:
>
> https://codereview.appspot.**com/6445085/diff/12009/src/**
>
cmd/go/build.go#newcode1101<https://codereview.appspot.com/6445085/diff/12009/src/cmd/go/build.go#newcode1101>
> src/cmd/go/build.go:1101: os.Setenv("PWD", "/WORK")
> Does this work?  GCC will check the PWD environment variable and only
> use it if it does in fact name the current directory.  Does the WORK
> directory exist?
>
No, it doesn't. And i have found out we can build identical cgo binary even
without this change.
Seems gcc already generate relative path for dwarf info. Is this true?
If it is, we only need to fix the section symbol thing.

[iant, 2012-09-16 20:30:54]

GCC will normally call getcwd to get the current directory, which will normally
give you an absolute path.  This can be overridden by a #line directive.  This
only affects the debug info, so it only matters if you compile with -g.

[minux1, 2012-09-17 13:01:13]

PTAL.

Reverted all gcc related changes, seems unnecessary now.
we cd to $WORK prior to use gcc, and gcc uses relative paths
in debugging infos, so $WORK won't appear in dwarf info.

[rsc, 2012-09-17 20:33:28]

LGTM

"Well that was incredibly easy!" - http://www.youtube.com/watch?v=1yqVD0swvWU

[minux1, 2012-09-19 16:19:52]

*** Submitted as http://code.google.com/p/go/source/detail?r=e24d688078cd ***

cmd/ld: consistent binary for cgo programs
We use pkg path instead of file name (which contains $WORK) in section symbols
names.

R=golang-dev, fullung, rsc, iant
CC=golang-dev
http://codereview.appspot.com/6445085