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

go-fuzz-build fails with Go 1.15 due to uncertain position of comments #294

Closed
oraluben opened this issue Aug 5, 2020 · 40 comments · Fixed by #306
Closed

go-fuzz-build fails with Go 1.15 due to uncertain position of comments #294

oraluben opened this issue Aug 5, 2020 · 40 comments · Fixed by #306

Comments

@oraluben
Copy link
Contributor

oraluben commented Aug 5, 2020

failed case 1:

# yyc @ Yichens-MacBook-Pro in ~/GolandProjects/fuzz-test/fuzz on git:go-ast-bug-demo x [9:58:01] C:1
$ go-fuzz-build
failed to execute go build: exit status 2
# fuzz-test/fuzz
/Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:3: misplaced compiler directive

# yyc @ Yichens-MacBook-Pro in ~/GolandProjects/fuzz-test/fuzz on git:go-ast-bug-demo x [10:07:06] C:1
$ cat fuzzer.go      
package fuzz

//go:noescape
func foo() {}

func Fuzz(input []byte) int {
        foo()
        return 0
}

go-fuzz-build will transform the input file to the following, which is obviously incorrect:

//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:1
package fuzz

//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:1
import

//go:noescape
//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:1
_go_fuzz_dep_ "go-fuzz-dep"

//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:4
func foo()	{ _go_fuzz_dep_.CoverTab[22588]++ }

func Fuzz(input []byte) int {
//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:6
	_go_fuzz_dep_.CoverTab[44810]++
								foo()
								return 0
}

//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:9
var _ = _go_fuzz_dep_.CoverTab

I found this probably a go bug and submitted an issue: golang/go#40546
I've also fixed this import issue with an ugly patch: https://github.com/oraluben/go-fuzz/tree/fix-import, but that does not overcome this issue, see case 2:

failed case 2

# yyc @ Yichens-MacBook-Pro in ~/GolandProjects/fuzz-test/fuzz on git:go-ast-bug-demo x [10:17:46] C:130
$ go-fuzz-build                             
failed to execute go build: exit status 2
# reflect
/Users/yyc/go/go1.15beta1/src/reflect/value.go:1425: misplaced compiler directive

# yyc @ Yichens-MacBook-Pro in ~/GolandProjects/fuzz-test/fuzz on git:go-ast-bug-demo x [10:17:57] C:1
$ cat fuzzer.go                                                               
package fuzz

import "reflect"

func Fuzz(input []byte) int {
        reflect.DeepEqual(1, 1)
        return 0
}

the source: https://github.com/golang/go/blob/master/src/reflect/value.go#L1421-L1451
the transformed code:

//line /Users/yyc/go/go1.15beta1/src/reflect/value.go:1420
				return __gofuzz_v1 !=

//line /Users/yyc/go/go1.15beta1/src/reflect/value.go:1425
				//go:nocheckptr
//line /Users/yyc/go/go1.15beta1/src/reflect/value.go:1420
				__gofuzz_v2
//line /Users/yyc/go/go1.15beta1/src/reflect/value.go:1420
			}() == true
//line /Users/yyc/go/go1.15beta1/src/reflect/value.go:1420
		default:
//line /Users/yyc/go/go1.15beta1/src/reflect/value.go:1420
			_go_fuzz_dep_.CoverTab[7638]++
		}
//line /Users/yyc/go/go1.15beta1/src/reflect/value.go:1421
	}
//line /Users/yyc/go/go1.15beta1/src/reflect/value.go:1421
	_go_fuzz_dep_.CoverTab[54290]++
								panic(&ValueError{"reflect.Value.OverflowUint", v.kind()})
}

you can find //go:nocheckptr was inserted in the middle.
I found it not easy to fix this without fixing the printer's logic, maybe you would have more idea about how to workaround this in go-fuzz-build?

@oraluben
Copy link
Contributor Author

oraluben commented Aug 12, 2020

update 1:
this case didn't fail, which confuse me very much:
case 1a (note it's not a compiler directive, but a regular comment, there's a space there):

$ cat fuzzer.go      
package fuzz

// go:noescape
func foo() {}

func Fuzz(input []byte) int {
        foo()
        return 0
}

go-fuzz-build generates:

//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:1
package fuzz

//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:1
import _go_fuzz_dep_ "go-fuzz-dep"

// go:noescape
func test()	{ _go_fuzz_dep_.CoverTab[22588]++ }

func Fuzz(input []byte) int {
//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:6
	_go_fuzz_dep_.CoverTab[44810]++
								test()
								return 0
}

//line /Users/yyc/GolandProjects/fuzz-test/fuzz/fuzzer.go:9
var _ = _go_fuzz_dep_.CoverTab

update 1.1
normal comments who aren't doc of a function will be removed by trimComments, that's why this case didn't fail.

@tdewolff
Copy link
Contributor

Same issue, this happens with Go 1.15 and throws a warning for the following files:

/usr/lib/go/src/vendor/golang.org/x/crypto/poly1305/sum_amd64.go:9: misplaced compiler directive
/usr/lib/go/src/crypto/sha256/sha256block_decl.go:9: misplaced compiler directive
/usr/lib/go/src/reflect/value.go:1432: misplaced compiler directive

@degeri
Copy link

degeri commented Aug 19, 2020

can confirm getting similar issues on 1.15

failed to execute go build: exit status 2
# crypto/sha256
/usr/local/go/src/crypto/sha256/sha256block_decl.go:9: misplaced compiler directive


failed to execute go build: exit status 2
# reflect
/usr/local/go/src/reflect/value.go:1432: misplaced compiler directive


@oraluben
Copy link
Contributor Author

I believe #265 has the same root cause.

@oraluben
Copy link
Contributor Author

Assigning a position for all inserted statements/functions should be a useful workaround, I will try this on go-fuzz. POC: https://play.golang.org/p/KNEE2J4FrWt.

@dgryski
Copy link
Contributor

dgryski commented Aug 19, 2020

@dvyukov Can you pin this issue so people see it first?

@dvyukov dvyukov pinned this issue Aug 19, 2020
@dvyukov
Copy link
Owner

dvyukov commented Aug 19, 2020

done

@oraluben oraluben changed the title go-fuzz-build failed due to function with comments go-fuzz-build failed due to uncertain position of comments Aug 20, 2020
oraluben added a commit to oraluben/go-fuzz that referenced this issue Aug 24, 2020
oraluben added a commit to oraluben/go-fuzz that referenced this issue Aug 27, 2020
@karimodm
Copy link

Can confirm this as well; it recently popped up with a new Fuzz function & go 1.15

@c4milo
Copy link

c4milo commented Oct 6, 2020

I'm also running into this

@smijolovic
Copy link

smijolovic commented Oct 12, 2020

Same issue here as well with 1.15.2. Runs just fine on 1.14.9.

Tested with prometheus -
go-fuzz-build -libfuzzer -x -func ${FUZZ_FUNCTIONS[i]} -o ${TARGETS[i]}.a ./promql
clang-9 -fsanitize=fuzzer ${TARGETS[i]}.a -o ${TARGETS[i]}

  • GO111MODULE=off
  • TARGETS=("promql-parse-metric" "promql-parse-open-metric" "promql-parse-metric-selector" "promql-parse-expr")
  • FUZZ_FUNCTIONS=("FuzzParseMetric" "FuzzParseOpenMetric" "FuzzParseMetricSelector" "FuzzParseExpr")
  • (( i=0 ))
  • (( i<4 ))
  • pwd
  • export GOPATH=/home/rpmbuild/nimbus8/prometheus-build/prometheus-chaasm-2.21.0/src/github.com/prometheus/prometheus/../../../..:/usr/share/gocode
  • GOPATH=/home/rpmbuild/nimbus8/prometheus-build/prometheus-chaasm-2.21.0/src/github.com/prometheus/prometheus/../../../..:/usr/share/gocode
  • go-fuzz-build -libfuzzer -x -func FuzzParseMetric -o promql-parse-metric.a ./promql
  • clang-9 -fsanitize=fuzzer promql-parse-metric.a -o promql-parse-metric
  • (( ++i ))
  • (( i<4 ))
  • pwd
  • export GOPATH=/home/rpmbuild/nimbus8/prometheus-build/prometheus-chaasm-2.21.0/src/github.com/prometheus/prometheus/../../../..:/usr/share/gocode
  • GOPATH=/home/rpmbuild/nimbus8/prometheus-build/prometheus-chaasm-2.21.0/src/github.com/prometheus/prometheus/../../../..:/usr/share/gocode
  • go-fuzz-build -libfuzzer -x -func FuzzParseOpenMetric -o promql-parse-open-metric.a ./promql
  • clang-9 -fsanitize=fuzzer promql-parse-open-metric.a -o promql-parse-open-metric
  • (( ++i ))
  • (( i<4 ))
  • pwd
  • export GOPATH=/home/rpmbuild/nimbus8/prometheus-build/prometheus-chaasm-2.21.0/src/github.com/prometheus/prometheus/../../../..:/usr/share/gocode
  • GOPATH=/home/rpmbuild/nimbus8/prometheus-build/prometheus-chaasm-2.21.0/src/github.com/prometheus/prometheus/../../../..:/usr/share/gocode
  • go-fuzz-build -libfuzzer -x -func FuzzParseMetricSelector -o promql-parse-metric-selector.a ./promql
  • clang-9 -fsanitize=fuzzer promql-parse-metric-selector.a -o promql-parse-metric-selector
  • (( ++i ))
  • (( i<4 ))
  • pwd
  • export GOPATH=/home/rpmbuild/nimbus8/prometheus-build/prometheus-chaasm-2.21.0/src/github.com/prometheus/prometheus/../../../..:/usr/share/gocode
  • GOPATH=/home/rpmbuild/nimbus8/prometheus-build/prometheus-chaasm-2.21.0/src/github.com/prometheus/prometheus/../../../..:/usr/share/gocode
  • go-fuzz-build -libfuzzer -x -func FuzzParseExpr -o promql-parse-expr.a ./promql
  • clang-9 -fsanitize=fuzzer promql-parse-expr.a -o promql-parse-expr
  • (( ++i ))
  • (( i<4 ))

With 1.15.2 - failed to execute go build: exit status 2

cd /home/rpmbuild/nimbus8/prometheus-build/tmp/go-fuzz-build886823602/goroot/src/vendor/golang.org/x/crypto/poly1305
/home/rpmbuild/nimbus8/prometheus-build/tmp/go-fuzz-build886823602/goroot/pkg/tool/linux_amd64/compile -o $WORK/b103/pkg.a -trimpath "$WORK/b103=>;/home/rpmbuild/nimbus8/prometheus-build/tmp/go-fuzz-build886823602/goroot/src/vendor/golang.org/x/crypto/poly1305=>vendor/golang.org/x/crypto/poly1305" -shared -p vendor/golang.org/x/crypto/poly1305 -std -installsuffix shared -buildid 33iG-gyaedDveO6r-jm3/33iG-gyaedDveO6r-jm3 -goversion go1.15.2 -symabis $WORK/b103/symabis -D "" -importcfg $WORK/b103/importcfg -pack -asmhdr $WORK/b103/go_asm.h ./bits_go1.13.go ./poly1305.go ./sum_amd64.go ./sum_generic.go

vendor/golang.org/x/crypto/poly1305
/usr/local/go15/src/vendor/golang.org/x/crypto/poly1305/sum_amd64.go:9: misplaced compiler directive
crypto/sha256
/usr/local/go15/src/crypto/sha256/sha256block_decl.go:9: misplaced compiler directive"

@holykol
Copy link

holykol commented Oct 25, 2020

My quick fix until the issue is resolved:

sudo sed -i "s/go:nocheckptr/ go:nocheckptr/" /usr/lib/go/src/reflect/value.go

@Dentrax
Copy link

Dentrax commented Oct 26, 2020

@dvyukov

Any updates about this? Getting a similar error output:

failed to execute go build: exit status 2
# reflect
/usr/local/Cellar/go/1.15.2/libexec/src/reflect/value.go:1432: misplaced compiler directive

@dvyukov
Copy link
Owner

dvyukov commented Oct 26, 2020

No updates.

@dvyukov dvyukov changed the title go-fuzz-build failed due to uncertain position of comments go-fuzz-build fails with Go 1.15 due to uncertain position of comments Oct 27, 2020
dvyukov added a commit to dvyukov/syzkaller that referenced this issue Oct 27, 2020
Use syz-old-env because it contains Go 1.14.
syz-env contains Go 1.15 and go-fuzz is broken with Go 1.15:
dvyukov/go-fuzz#294
dvyukov added a commit to google/syzkaller that referenced this issue Oct 27, 2020
Use syz-old-env because it contains Go 1.14.
syz-env contains Go 1.15 and go-fuzz is broken with Go 1.15:
dvyukov/go-fuzz#294
@dolmen
Copy link

dolmen commented Nov 13, 2020

Could you put "misplaced compiler directive" in the title of this issue? I think that's how people will find it.

@josharian
Copy link
Collaborator

Will you try #307 out? Thanks.

@josharian josharian reopened this Nov 14, 2020
@gsora
Copy link

gsora commented Nov 14, 2020

Nope, doesn't work, same error is still there.

@josharian
Copy link
Collaborator

I’m confused, then. After that PR I can’t reproduce. When you say “the same error”, are you talking about crypto/sha256?

@gsora
Copy link

gsora commented Nov 15, 2020

Yes, I'm talking about the crypto/sha256 error.

@josharian
Copy link
Collaborator

@degeri can you also still reproduce using that PR?

@degeri
Copy link

degeri commented Nov 15, 2020

Hi @josharian . First of all thanks for all the work.

I am using 307 and still getting

failed to execute go build: exit status 2
# crypto/sha256
/usr/local/go/src/crypto/sha256/sha256block_decl.go:9: misplaced compiler directive

failed to execute go build: exit status 2
# crypto/sha256
/usr/local/go/src/crypto/sha256/sha256block_decl.go:9: misplaced compiler directive

https://github.com/degeri/dcrd-continuous-fuzz#there-is-also-a-dockerless-version if my repo if you want to have a look.

@josharian
Copy link
Collaborator

I tried, and I can't reproduce using your dockerless shell script and Go 1.15.5. Can you re-install go-fuzz-build from that PR branch and try again? I am really at a loss as to what the difference could be.

@degeri
Copy link

degeri commented Nov 15, 2020

Yup I am quiet sure since I deleted my bins and built on your PR.

image

@josharian
Copy link
Collaborator

Can you add a line after

c.writeFile(tmp, buf.Bytes())
that also prints buf.Bytes to stdout and re-run and provide the instrumented file contents of sha256block_decl.go? Thanks.

@danp
Copy link

danp commented Nov 15, 2020

Was seeing if I could reproduce the error @degeri was still running into. At first I thought I was but then I noticed the second commit on #307 isn't actually on the branch so I had to work a bit harder to get it (with git cherry-pick). Once I did that, the test case I had using crypto/sha256 started working. Here's the output for sha256block_decl.go:

// Copyright 2013 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// +build 386 amd64 s390x ppc64le

//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:7
package sha256

//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:7
import (
//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:7
	_go_fuzz_dep_ "go-fuzz-dep"
//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:7
)

//go:noescape

func block(dig *digest, p []byte)

//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:11
var _ = _go_fuzz_dep_.CoverTab

// Copyright 2013 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// +build 386 amd64 s390x ppc64le

//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:7
package sha256

//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:7
import (
//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:7
	_go_fuzz_dep_ "go-fuzz-dep"
//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:7
)

//go:noescape

func block(dig *digest, p []byte)

//line /Users/dan/Projects/go/project/go/src/crypto/sha256/sha256block_decl.go:11
var _ = _go_fuzz_dep_.CoverTab

I found it a bit tricky to ensure I was using the right version of go-fuzz-build due to modules. Ended up having to add a go.mod to my go-fuzz clone and had a go.mod looking like this in my test project:

module github.com/danp/fuzz

go 1.16

replace github.com/dvyukov/go-fuzz => ../../dvyukov/go-fuzz

require (
	github.com/dvyukov/go-fuzz v0.0.0-20201114070042-d9fd8180255a // indirect
	github.com/elazarl/go-bindata-assetfs v1.0.1 // indirect
	github.com/stephens2424/writerset v1.0.2 // indirect
	golang.org/x/tools v0.0.0-20201114224030-61ea331ec02b // indirect
)

@josharian
Copy link
Collaborator

Thanks, @danp. Based on that, I’ve merged the PR. Hopefully it’ll be easier to test now.

@degeri
Copy link

degeri commented Nov 15, 2020

@joeshaw should I test on master now ?

@josharian
Copy link
Collaborator

Yes please

@degeri
Copy link

degeri commented Nov 15, 2020

works now 🎉
Double checked on a fresh docker

degeri added a commit to degeri/dcrd-continuous-fuzz that referenced this issue Nov 15, 2020
dvyukov/go-fuzz#294

has been fixed hence we can update to 1.15.5
@dolmen
Copy link

dolmen commented Nov 16, 2020

Thank you @dvyukov for this project, and thank you everyone involved in this fix!

@MartinPetkov
Copy link

I'm still seeing this issue with Go 1.17. I did the following:

  1. Create a fuzz.go in a project I'm working on, which uses github.com/OneOfOne/xxhash@v1.2.8
  2. Run it in docker to ensure the version:
    docker run -it -v $(pwd):/src:ro golang:1.17 /bin/bash
    
  3. Install go-fuzz-build:
    go get -u github.com/dvyukov/go-fuzz/go-fuzz@latest  \
           github.com/dvyukov/go-fuzz/go-fuzz-build@latest
    
  4. Install go-fuzz-dep in the repo
    cd /src
    go get -u github.com/dvyukov/go-fuzz/go-fuzz-dep@latest
    
  5. Try building it, which gives an error:
    cd /src/internal/fuzz/target
    go-fuzz-build
    
    failed to execute go build: exit status 2
    # github.com/OneOfOne/xxhash
    /usr/local/google/home/mpetkov/go/pkg/mod/github.com/!one!of!one/xxhash@v1.2.8/xxhash_unsafe.go:106: misplaced compiler directive
    

If I'm reading this thread correctly, this should no longer be an issue with the newest version of go-fuzz-build. Is there something else I should do to get it to work?

@MartinPetkov
Copy link

For anyone finding this, I actually took a look into open-policy-agent/opa#3243 and it seems the following bypasses the error I see:

go-fuzz-build -preserve github.com/OneOfOne/xxhash

@fuzzah
Copy link

fuzzah commented Feb 2, 2024

go 1.21.6, had a similar problem with crypto/internal/bigmod, fixed in the same way:

go-fuzz-build -preserve crypto/internal/bigmod

For anyone facing this: just make sure the module you're preserving (excluding from go-fuzz instrumentation) is not the module you want to fuzz :)

josharian added a commit that referenced this issue Feb 2, 2024
This fixes yet more manifestations of #294.
josharian added a commit that referenced this issue Feb 2, 2024
And re-instrument reflect. Sheesh.

#294. Again.
@josharian
Copy link
Collaborator

Sent #353 which will fix more such issues.

For others finding this thread with their own new packages that don't build: Please provide a short standalone reproducer, not just a package path (or worse, just an error message with a line number). Thank you.

josharian added a commit that referenced this issue Feb 3, 2024
This fixes yet more manifestations of #294.
josharian added a commit that referenced this issue Feb 3, 2024
And re-instrument reflect. Sheesh.

#294. Again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet