nil pointer dereference panic when computing aliasTypeName

KenFigueiredo commented 1 year ago

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 commented 10 months ago

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.

KenFigueiredo commented 10 months ago

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.

burrowers / garble