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

runtime, mime/quotedprintable: possible memory corruption #11084

Closed
kostya-sh opened this issue Jun 5, 2015 · 5 comments
Closed

runtime, mime/quotedprintable: possible memory corruption #11084

kostya-sh opened this issue Jun 5, 2015 · 5 comments
Milestone

Comments

@kostya-sh
Copy link
Contributor

Using tip (bbc4351):

The following failure happens fairly consistently (but not every time):

ok      mime/multipart  0.248s
--- FAIL: TestExhaustive-4 (0.88s)
    reader_test.go:202: Got:
        OK: 21576
        invalid bytes after =: 3397
        quotedprintable: invalid hex byte 0x0a: 1400
        quotedprintable: invalid hex byte 0x0d: 2700
        quotedprintable: invalid hex byte 0x20: 2490
        quotedprintable: invalid hex byte 0x3d: 384
        unexpected EOF: 3122
        �p�(�>=e������: 56
        Want:
        OK: 21576
        invalid bytes after =: 3397
        quotedprintable: invalid hex byte 0x0a: 1400
        quotedprintable: invalid hex byte 0x0d: 2700
        quotedprintable: invalid hex byte 0x20: 2490
        quotedprintable: invalid hex byte 0x3d: 440
        unexpected EOF: 3122
FAIL
FAIL    mime/quotedprintable    0.908s
ok      net 2.232s
@ianlancetaylor ianlancetaylor added this to the Go1.5 milestone Jun 5, 2015
@kostya-sh
Copy link
Contributor Author

I am pretty sure (based on git bisect) that the bug was exposed by changing default GOMAXPROCS to NumCPU()

It looks like quotedprintable: invalid hex byte 0x%02x constant string that is used as argument to fmt.Errorf gets overwritten with some random characters.

@bradfitz
Copy link
Contributor

bradfitz commented Jun 5, 2015

The CL where GOMAXPROCS gets set to NumCPU() is unlikely to be the problem. But it might've exposed an existing bug and made it occur more easily.

@kostya-sh
Copy link
Contributor Author

Possibly related (doesn't happen every time):

ok      crypto/subtle   0.018s
# database/sql_test
database/sql/example_test.go:38: break is not in a loop
database/sql/example_test.go:41: label 1 already defined at database/sql/example_test.go:41
database/sql/example_test.go:43: label 2 already defined at database/sql/example_test.go:43
ok      crypto/tls  1.081s
ok      crypto/x509 1.709s
FAIL    database/sql [build failed]
ok      database/sql/driver 0.006s
ok      debug/dwarf 0.012s
ok      debug/elf   0.017s
ok      debug/gosym 1.169s

@aclements
Copy link
Member

@rsc and I are pretty sure we've found the problem. This test creates a map and a map bucket on the stack. When map assignment happens, writes are being done in the runtime map code to a higher stack frame using typedmemmove. This typedmemmove doesn't generate write barriers because the destination is on the stack. But this violates the assumption made by stack barriers that pointer writes to higher stack frames will always have write barriers. If these updated pointers are above the next stack barrier, then the stack rescan won't see them and the garbage collector will collect them.

I'm working on a fix to make typedmemmove detect when its argument is on the stack and remove any stack barriers below (hence forcing the stack re-scan to re-scan what's being updated). The alternative would be to make typedmemmove generate write barriers for stack writes, but this is complicated because we may not have a GC bitmap in this case.

@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Jun 25, 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

5 participants