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

Jump to bottom

cmd/pack: produces malformed .a files #21703

Closed
jayconrod opened this issue Aug 30, 2017 · 7 comments
Closed

cmd/pack: produces malformed .a files #21703

jayconrod opened this issue Aug 30, 2017 · 7 comments
Labels
FrozenDueToAge release-blocker
Milestone
Go1.10

Comments

@jayconrod
Copy link
Contributor

What version of Go are you using (go version)?

1.7.6.

Does this issue reproduce with the latest release?

No, but the underlying issue still appears to be present.

I tried to reproduce with Go 1.9 (following the instructions below), but the package export data is different. src/cmd/pack/pack.go has not changed, so unless the export data format has changed in a way that makes this issue impossible, I think the bug still exists.

What operating system and processor architecture are you using (go env)?

linux amd64

What did you do?

This issue was originally reported in bazelbuild/rules_go#769. The compiler panics when building Kubernetes using Bazel and Go 1.7.6. This happens because the .a file produced for a specific target, //vendor:github.com/aws/aws-sdk-go/service/ecr, is malformed. The compiler panics when it reads the malformed .a file when building a dependent package.

I spent this morning looking into how "go build" and Bazel produce .a files. When "go build" invokes the compiler, it passes the -pack option, which produces an .a file directly. The .a file contains two files: __.PKGDEF (export data), and _go_.o (compiled code).

When Bazel invokes the compiler, it does not pass the -pack option, so the compiler produces an .o file that contains the export data and the compiled code together. These sections are separated by a ! on a line by itself. See dumpobj1 in cmd/compile/internal/gc/obj.go. As a separate action, Bazel creates the .a file with cmd/pack. When cmd/pack creates an archive, it looks for a ! line in the first .o file, then extracts the data above that into a __.PKGDEF file. See readPkgdef in pack.go.

The problem seems to be that the export data for this library contains a ! on a line by itself without any escaping or delimitation. I don't know enough about the export data format to know what that means or how it's changed since 1.7.6. cmd/pack interprets this as the end of the export data, so it only extracts about 5K instead of 43K. Consequently, the export data is truncated, and the compiler panics when reading it back in.

Steps to reproduce

This issue can be reproduced by manually running compile and pack commands and observing the output.

  • Clone https://github.com/kubernetes/kubernetes and checkout 473417e.
  • Add vendor/ to GOPATH.
  • go build -x -work github.com/aws/aws-go-sdk/service/ecr
  • Copy the compiled .a file somewhere and extract with ar x ecr.a. Observe that the __.PKGDEF file is about 43K.
  • Modify the compile command printed by "go build" and re-run.
    • Remove -pack
    • Change the output file extension from .a to .o
    • (You'll need to set WORK and cd to the appropriate directory before running the command).
  • Copy the compiled .o file somewhere. Observe that there is more than one \n!\n sequence in the file.
  • Create an archive with go tool pack c ecr.a ecr.o.
  • Extract the archive with ar x ecr.a. Observe that the __.PKGDEF file is only about 5K.

What did you expect to see?

The .o file that gc produces does not store the export data in a format that allows it to be safely extracted. There should either be some escaping or a length of the embedded data. Given that gc can already produce .a files that have lengths like this, it's probably better to prevent gc from emitting export data at all unless it's writing an archive.

Bazel will switch to doing what "go build" does: we'll create an .a file with -pack, then we'll append any additional .o files to it if we have them.
@ianlancetaylor ianlancetaylor changed the title cmd/pack produces malformed .a files cmd/pack: produces malformed .a files Aug 30, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Aug 30, 2017
@mdempsky
Copy link
Member

mdempsky commented Aug 31, 2017 •
edited

I was discussing with @ianlancetaylor a week or two ago to make -pack the default behavior and remove it as a flag. Having two framing formats for package export data makes things a little unnecessarily complex. This bug further suggests to me that would be worthwhile.

I don't think @griesemer or I were aware of that logic in cmd/pack. The object file format delimits the package export data with "$$", so the export data format has an escape sequence to prevent that from occurring. However, no such escape sequences exists to protect against "\n!\n". :/

I can't think of anything specific to 1.8 that would have prevented this issue, but this bug is really easy to reproduce:

$ cat a.go
package a
const X = "\n!\n"

$ cat b.go
package b
import _ "./a"

$ go tool compile a.go
$ go tool pack c a.a a.o

$ go tool compile b.go
b.go:2:8: cannot import "/tmp/aa/a" due to version skew - reinstall package (unexpected object (tag = 23))

@mdempsky mdempsky mentioned this issue Aug 31, 2017
Closed
@griesemer
Copy link
Contributor

@mdempsky Agreed (and yes, I was not aware of that logic). It should be trivial to have an additional escape mechanism (short term fix), but in the long run we should try to avoid the need for escaping in the first place (by identifying the sections in .a and .o files via their positions rather than parsing their content).

@mdempsky
Copy link
Member

I think the easiest fix is in cmd/pack to keep count of the number of "$$" lines, and to only recognize "\n!\n" as ending the export data when we've seen an even count. That would prevent recognizing "\n!\n" within the binary export data, and avoid needing any additional escaping mechanisms.

@gopherbot
Copy link

Change https://golang.org/cl/60773 mentions this issue: cmd/pack: fix export data truncation bug

@mdempsky
Copy link
Member

I just noticed the same search-for-"\n!\n" logic is in cmd/link's ldobj function too.

@mdempsky
Copy link
Member 

$ cat main.go
package main
const X = "\n!\n"
func main() {}

$ go tool compile main.go
$ go tool link main.o
2017/09/26 10:59:11 main.o: invalid file start 9 7f 2 1 1d 3c 61 75

@mdempsky mdempsky reopened this Sep 26, 2017
@gopherbot
Copy link

Change https://golang.org/cl/79135 mentions this issue: cmd/link: fix export data truncation bug

@golang golang locked and limited conversation to collaborators Nov 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge release-blocker
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
5 participants
@mdempsky @jayconrod @ianlancetaylor @griesemer @gopherbot