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

nil pointer dereference panic when computing aliasTypeName #798

Closed
KenFigueiredo opened this issue Sep 27, 2023 · 2 comments · Fixed by #806
Closed

nil pointer dereference panic when computing aliasTypeName #798

KenFigueiredo opened this issue Sep 27, 2023 · 2 comments · Fixed by #806
Labels
bug Something isn't working

Comments

@KenFigueiredo
Copy link

What version of Garble and Go are you using?

$ garble version
mvdan.cc/garble v0.10.1

Build settings:
      -buildmode exe
       -compiler gc
     CGO_ENABLED 1
          GOARCH arm64
            GOOS darwin

$ go version
go version go1.20.8 darwin/arm64

What environment are you running Garble on?

go env Output
$ go env
GO111MODULE="on"
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/me/Library/Caches/go-build"
GOENV="/Users/me/goconf/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/me/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/me/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/me/sdk/go1.20.8"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/me/sdk/go1.20.8/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.8"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/me/go/DataDog/sketches-go/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/mn/2wk41k713rs3c555ftnrx5vc0000gq/T/go-build1407514451=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

When attempting to garble another project, I found one of our DataDog sub-dependencies (specifically: https://github.com/DataDog/sketches-go/tree/master) caused this panic. This can be reproduced by attempting to garble the project directly:

$ git clone https://github.com/DataDog/sketches-go.git
$ cd sketches-go
$ go mod tidy
$ garble -literals -tiny -seed=random build ./ddsketch/encoding   

What did you expect to see?

$ garble -literals -tiny -seed=random build ./ddsketch/encoding 

-seed chosen at random: abcdefg
$

What did you see instead?

garble -literals -tiny -seed=random build ./ddsketch/encoding

-seed chosen at random: abcdefg
# github.com/DataDog/sketches-go/ddsketch/encoding
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x1023f04c0]

goroutine 1 [running]:
go/types.(*Package).Path(...)
        /Users/me/sdk/go1.20.8/src/go/types/package.go:31
main.computePkgCache(0x319ab0f6733c95d0?, 0x1400013bb00, 0x14000075ea0, {0x140001955b0, 0x2, 0x2}, 0x14000075e00)
        /Users/me/go/pkg/mod/mvdan.cc/garble@v0.10.1/main.go:1424 +0x620
main.loadPkgCache(0x1400013bb00, 0x30?, {0x140001955b0, 0x2, 0x2}, 0x0?)
        /Users/me/go/pkg/mod/mvdan.cc/garble@v0.10.1/main.go:1338 +0x208
main.(*transformer).transformCompile(0x140000307e0, {0x14000003720?, 0x1400002c1a4?, 0x30?})
        /Users/me/go/pkg/mod/mvdan.cc/garble@v0.10.1/main.go:948 +0x19c
main.mainErr({0x14000003700, 0x16, 0x16})
        /Users/me/go/pkg/mod/mvdan.cc/garble@v0.10.1/main.go:451 +0x6c4
main.main1()
        /Users/me/go/pkg/mod/mvdan.cc/garble@v0.10.1/main.go:249 +0x21c
main.main()
        /Users/me/go/pkg/mod/mvdan.cc/garble@v0.10.1/main.go:143 +0x1c
exit status 1

Additional Info

I did some digging into the error and narrowed down the nil pointer panic. From the stack trace its here: https://github.com/burrowers/garble/blob/v0.10.1/main.go#L1424

		aliasTypeName := typeName{
			PkgPath: obj.Pkg().Path(),
			Name:    obj.Name(),
		}
I pulled some data on the obj:
obj {
 Name: "byte",
 Pkg: nil,
 IsAlias: true,
 Parent: universe scope 0x14000030240 {
  .  type any = interface{}
  .  builtin append
  .  type bool
  .  type byte
  .  builtin cap
  .  builtin clear
  .  builtin close
  .  type comparable interface{comparable}
  .  builtin complex
  .  type complex128
  .  type complex64
  .  builtin copy
  .  builtin delete
  .  type error interface{Error() string}
  .  const false untyped bool
  .  type float32
  .  type float64
  .  builtin imag
  .  type int
  .  type int16
  .  type int32
  .  type int64
  .  type int8
  .  const iota untyped int
  .  builtin len
  .  builtin make
  .  builtin new
  .  nil
  .  builtin panic
  .  builtin print
  .  builtin println
  .  builtin real
  .  builtin recover
  .  type rune
  .  type string
  .  const true untyped bool
  .  type uint
  .  type uint16
  .  type uint32
  .  type uint64
  .  type uint8
  .  type uintptr
 }
}

From the Datadog library side it looks specifically that these 3 lines are the culprit: https://github.com/DataDog/sketches-go/blob/master/ddsketch/encoding/flag.go#L38-L40

type Flag struct{ byte }
type FlagType struct{ byte } // mask: 0b00000011
type SubFlag struct{ byte }  // mask: 0b11111100

If I edit that file and give those byte values arbitrary keys, garble works as expected. ex:

type Flag struct{ foo byte }
type FlagType struct{ bar byte } // mask: 0b00000011
type SubFlag struct{ baz byte }  // mask: 0b11111100

This however isn't a practical workaround since this library sits as a fairly deep sub-dependency of the Datadog tracer library and won't be easy to patch.

@mvdan
Copy link
Member

mvdan commented Nov 12, 2023

Thanks for the throrough investigation. This does sound like a bug. Happy to accept a patch with an added test case; otherwise I'll look into a fix soon.

@mvdan mvdan added the bug Something isn't working label Nov 12, 2023
@mvdan mvdan changed the title panic: runtime error: invalid memory address or nil pointer dereference nil pointer dereference panic when computing aliasTypeName Nov 12, 2023
mvdan added a commit to mvdan/garble-fork that referenced this issue Nov 14, 2023
TypeName.Pkg is documented as:

    Pkg returns the package to which the object belongs.
    The result is nil for labels and objects in the Universe scope.

When a struct type embeds a builtin alias type, such as byte,
this would lead to a panic since we assumed we could use the Pkg method.

Fixes burrowers#798.
@lu4p lu4p closed this as completed in #806 Nov 15, 2023
lu4p pushed a commit that referenced this issue Nov 15, 2023
TypeName.Pkg is documented as:

    Pkg returns the package to which the object belongs.
    The result is nil for labels and objects in the Universe scope.

When a struct type embeds a builtin alias type, such as byte,
this would lead to a panic since we assumed we could use the Pkg method.

Fixes #798.
@KenFigueiredo
Copy link
Author

Thanks for the quick turn around on this fix! Still was waiting on some internal approval to be able to open a patch w/ test cases 😅 but will validate this on the next version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants