Skip to content

Optimize testee memory allocation #215

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 12, 2019
Merged

Optimize testee memory allocation #215

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4cd0d59
go-fuzz: add reuseable buffers for testee communication
josharian Feb 17, 2019
5536980
go-fuzz: use a re-usable buffer for testee communication
josharian Mar 5, 2019
77a1f72
go-fuzz: don't drop data when Read also returns an error
josharian Mar 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 27 additions & 14 deletions go-fuzz/testee.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package main
import (
"encoding/binary"
"fmt"
"io"
"io/ioutil"
"log"
"os"
Expand All @@ -28,6 +29,8 @@ type Testee struct {
inPipe *os.File
outPipe *os.File
stdoutPipe *os.File
datalen [8]byte // reusable encoded data length
resbuf [24]byte // reusable results buffer
execs int
startTime int64
outputC chan []byte
Expand All @@ -46,7 +49,8 @@ type TestBinary struct {
inputRegion []byte
sonarRegion []byte

testee *Testee
testee *Testee
testeeBuffer []byte // reusable buffer for collecting testee output

stats *Stats
}
Expand All @@ -58,6 +62,10 @@ func init() {
}
}

// testeeBufferSize is how much output a test binary can emit
// before we start to overwrite old output.
const testeeBufferSize = 1 << 20

func newTestBinary(fileName string, periodicCheck func(), stats *Stats) *TestBinary {
comm, err := ioutil.TempFile("", "go-fuzz-comm")
if err != nil {
Expand All @@ -75,6 +83,7 @@ func newTestBinary(fileName string, periodicCheck func(), stats *Stats) *TestBin
inputRegion: mem[CoverSize : CoverSize+SonarRegionSize],
sonarRegion: mem[CoverSize+SonarRegionSize:],
stats: stats,
testeeBuffer: make([]byte, testeeBufferSize),
}
}

Expand All @@ -99,7 +108,7 @@ func (bin *TestBinary) test(data []byte) (res int, ns uint64, cover, sonar, outp
bin.stats.execs++
if bin.testee == nil {
bin.stats.restarts++
bin.testee = newTestee(bin.fileName, bin.comm, bin.coverRegion, bin.inputRegion, bin.sonarRegion)
bin.testee = newTestee(bin.fileName, bin.comm, bin.coverRegion, bin.inputRegion, bin.sonarRegion, bin.testeeBuffer)
}
var retry bool
res, ns, cover, sonar, crashed, hanged, retry = bin.testee.test(data)
Expand All @@ -121,7 +130,7 @@ func (bin *TestBinary) test(data []byte) (res int, ns uint64, cover, sonar, outp
}
}

func newTestee(bin string, comm *Mapping, coverRegion, inputRegion, sonarRegion []byte) *Testee {
func newTestee(bin string, comm *Mapping, coverRegion, inputRegion, sonarRegion []byte, buffer []byte) *Testee {
retry:
rIn, wIn, err := os.Pipe()
if err != nil {
Expand Down Expand Up @@ -181,25 +190,24 @@ retry:
// deadlock we periodically read out stdout.
// This goroutine also collects crash output.
ticker := time.NewTicker(time.Second)
const N = 1 << 20
data := make([]byte, N)
data := buffer
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eeee
this buffer is returned from TestBinary.test and then leaked to crasherQueue
we can't just do this
we can do something like this, but we can reuse the buffer only on periodic testee restart, on crashes we can either handoff the buffer to caller (assuming crashes are not so frequent, or if they are perf does not matter) or make a copy of the used part of the buffer and then reuse the original buffer

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize, I don't follow. A few lines down, around line 215, I see:

		trimmed := make([]byte, filled)
		copy(trimmed, data)
		t.outputC <- trimmed

So it seems like data/buffer doesn't leak. What am I missing?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right!
I assumed we send the buffer to the caller as-is. I guess I did it to not pin the large buffer... perhaps...

filled := 0
for {
select {
case <-ticker.C:
case <-t.downC:
}
n, err := t.stdoutPipe.Read(data[filled:])
if err != nil {
break
}
if *flagV >= 3 {
log.Printf("testee: %v\n", string(data[filled:filled+n]))
}
filled += n
if filled > N/4*3 {
copy(data, data[N/2:filled])
filled -= N / 2
if filled > testeeBufferSize/4*3 {
copy(data, data[testeeBufferSize/2:filled])
filled -= testeeBufferSize / 2
}
if err != nil {
break
}
}
ticker.Stop()
Expand Down Expand Up @@ -258,7 +266,8 @@ func (t *Testee) test(data []byte) (res int, ns uint64, cover, sonar []byte, cra

copy(t.inputRegion[:], data)
atomic.StoreInt64(&t.startTime, time.Now().UnixNano())
if err := binary.Write(t.outPipe, binary.LittleEndian, uint64(len(data))); err != nil {
binary.LittleEndian.PutUint64(t.datalen[:], uint64(len(data)))
if _, err := t.outPipe.Write(t.datalen[:]); err != nil {
if *flagV >= 1 {
log.Printf("write to testee failed: %v", err)
}
Expand All @@ -272,8 +281,12 @@ func (t *Testee) test(data []byte) (res int, ns uint64, cover, sonar []byte, cra
Ns uint64
Sonar uint64
}
var r Reply
err := binary.Read(t.inPipe, binary.LittleEndian, &r)
_, err := io.ReadFull(t.inPipe, t.resbuf[:])
r := Reply{
Res: binary.LittleEndian.Uint64(t.resbuf[:]),
Ns: binary.LittleEndian.Uint64(t.resbuf[8:]),
Sonar: binary.LittleEndian.Uint64(t.resbuf[16:]),
}
hanged = atomic.LoadInt64(&t.startTime) == -1
atomic.StoreInt64(&t.startTime, 0)
if err != nil || hanged {
Expand Down