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

io: add test to verify that ReadAt doesn't affect seek offset (for Plan 9 kernel bug) #14534

Closed
0intro opened this issue Feb 26, 2016 · 7 comments
Closed

io: add test to verify that ReadAt doesn't affect seek offset (for Plan 9 kernel bug) #14534

0intro opened this issue Feb 26, 2016 · 7 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge OS-Plan9
Milestone
Go1.7

Comments

@0intro
Copy link
Member

0intro commented Feb 26, 2016

Running go tool nm on object files doesn't work anymore on Plan 9:

% go tool nm file.o
open file.o: unrecognized object file

This error is returned by rd.parseObject in goobj.Parse.
It reads the "386 deve" string which doesn't match the expected "go objec" string.
It happens because objfile.Open calls debug/elf.NewFile before, which does a ReadAt of 16 bytes. However, the implementation of pread in the Plan 9 kernel contains a bug where it updates the channel offset while it shouldn't. Hence, the first 16 bytes of the files are already read before calling goobj.Parse.

This is related to #11265

This can be fixed by fixing the pread system call in the Plan 9 kernel.

A workaround is to add Seek(0, 0) after calling ReadAt:

--- a/src/cmd/internal/objfile/objfile.go
+++ b/src/cmd/internal/objfile/objfile.go
@@ -50,6 +50,10 @@ func Open(name string) (*File, error) {
        return nil, err
    }
    for _, try := range openers {
+       _, err = r.Seek(0, 0)
+       if err != nil {
+           return nil, err
+       }
        if raw, err := try(r); err == nil {
            return &File{r, raw}, nil
        }
@0intro 0intro self-assigned this Feb 26, 2016
@minux
Copy link
Member

minux commented Feb 26, 2016 via email

@0intro
Copy link
Member Author

0intro commented Feb 26, 2016

Yes, this is a Plan 9 bug. I've opened this issue so people could find it and figure what to do if they encounter this bug.

@bradfitz
Copy link
Contributor

We're not going to change cmd/nm or any of the other million high-level Go programs to work around Plan 9 kernel bugs.

Let's repurpose this bug for several things:

  1. verify we have a test in Go's repo to catch this plan9 bug.
  2. verify Go's builders are running an updated plan9 kernel with this bug fixed.

@bradfitz bradfitz added the Builders x/build issues (builders, bots, dashboards) label Apr 10, 2016
@bradfitz bradfitz changed the title cmd/nm: unrecognized object file on Plan 9 io: add test to verify that ReadAt doesn't affect seek offset (for Plan 9 kernel bug) Apr 10, 2016
@0intro
Copy link
Member Author

0intro commented Apr 10, 2016

Yes, I'll update the builders and write a test once I'll be back in two weeks.

@0intro
Copy link
Member Author

0intro commented Apr 10, 2016

And of course, there was absolutely no intention to change any of the Go tool to work around this bug.

@bradfitz bradfitz added this to the Go1.7 milestone Apr 10, 2016
@gopherbot
Copy link

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

@gopherbot
Copy link

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

gopherbot pushed a commit to golang/build that referenced this issue May 10, 2016
@0intro @bradfitz
env: update Plan 9 image
08b0175 
This new Plan 9 image provides various fixes to the Plan 9 kernel.

The most notable change is a fix to the pread system call, so
pread will not update the channel offset when reading a file.

Updates golang/go#11194.
Updates golang/go#14534.

Change-Id: Ia8f537b3559fbc98a191bda836cd3e0035e16eec
Reviewed-on: https://go-review.googlesource.com/22319
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/build that referenced this issue May 10, 2016
@bradfitz
dashboard: update plan9 image
721b5e3 
Based on https://golang.org/cl/22319 from 0intro.

Updates golang/go#11194
Updates golang/go#14534

Change-Id: I05650d74970f1ca108c520a44a326cbe8229422e
Reviewed-on: https://go-review.googlesource.com/22966
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators May 10, 2017
@rsc rsc unassigned 0intro Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge OS-Plan9
Projects
None yet
Milestone
Go1.7
Development

No branches or pull requests
4 participants
@bradfitz @minux @0intro @gopherbot