Installation and other issues

MikkelHJuul commented 3 years ago

So many problems:

normal git clone will put the project in the folder 'gcatch' you restrict it to be cased 'GCatch' in installation, there is no real reason for this

suggestion: remove the install scripts, installation is really simple to install z3:

cd tools/z3
python scripts/mk_make.py
cd build
make
make install

install GCatch requires z3 (WITH SOURCES) (which z3 or /usr/[local/]bin/z3 is not enough; the packaged binaries don't save the headers etc. -- you would simply get an error compiling)

cd cmd/GCatch
go install

please expose the project in some other way, fx

FROM golang as builder
RUN go get github.com/system-pclub/GCatch
WORKDIR /go/src/github.com/system-pclub/GCatch/GCatch
RUN python tools/z3/scripts/mk_make.py
WORKDIR tools/z3/build
RUN make && make install
WORKDIR /go/src/github.com/system-pclub/GCatch/GCatch/cmd/GCatch
RUN go install

FROM golang //alpine requires static linking  << expose this docker image
COPY --from=builder /go/bin/GCatch /GCatch 
//if there are external dependencies left outside of the binary, also copy those
ENTRYPOINT ["/GCatch"]

btw when this is all done the script still fails with the error message (even when it is blatantly false)

The target project is not in a GOPATH, because its path doesn't contain "/src/"

I would advise this project to change how it works, to at least scan the current directory without dependencies, so that you can scan your own project without relying on scanning the full dependency graph. also there must be another way to figure the dependent project's if you wan't to scan those as well. GOPATH is not even standard any longer (move to reading the go.mod file -- you could find other versions of the projects in GOPATH -- there must be core or helper-libraries for this)

You really designed yourself into a corner with your application relying on packages inside your GOPATH, and default working with dependencies as well as the specific filetree.

check the CLI of a project like scc (sloc, cloc and counter) essentially your tool should be able to be run like this:

docker run -v /some/dir/:/some/other/dir/in/container -w /some/other/dir/in/container system-pclub/GCatch

and give some readable feedback including possible warnings and/or error codes based on findings. then maybe add a -recurse-dependencies if you want to do a dependency scan. and you could scan a project using something like GCatch -remote github.com/some/project@tag (or make the -include tag scan another project as well as the current directory filetree)

I'm just sad that running this code is virtually impossible.

thejerf commented 3 years ago

I am not associated with the GCatch project. I just want to let the team know that I'm actually excited you've been engaging with us at all here in GitHub and I am excited to work with you to get this engineered up to where it can be used by "real people". I often am saddened when academia produces nifty, useful things that are then essentially abandoned because they can't be used. (I'm sure there's another paper to be written about "2 Years of GCatch Use in the Wild", if you get my drift. :) )

I'm sure the reason why they're using GOPATH is that this project has been in progress since before Go modules were officially the way to go.

GCatch team, I've been somewhat interested in digging in and turning this into a conventional and standard Go project, using current go modules, but one thing thing that I've been unsure about is the forks that @MikkelHJuul talks about here. Can you give us an idea about whether or not you've actually made significant changes to these included libraries, or whether or not these are essentially just "vendored" with no changes? If you can answer that, that will give us a lot more confidence to create PRs to bring these programs up to go module spec and resolve the issues @MikkelHJuul is pointing out here.

BurtonQin commented 3 years ago

@thejerf The code under the directory tools IS modified. I think it is hard and unnecessary to update them.

BurtonQin commented 3 years ago

Thx. @MikkelHJuul I've just created a to-do list so that we can handle them one by one.

[x] Separate z3 installation from the installation script.
[x] Use a better way to detect if z3 is installed from the source before installing GCatch
[ ] Improve the docker script
[ ] ~Scan go.mod instead of relying on GOPATH (Hard)~
[ ] ~Toggle the scan of the dependencies. (We have the code)~
[ ] Improve the usage command

MikkelHJuul commented 3 years ago

[ ] Scan go.mod instead of relying on GOPATH (Hard)

There must be other tools out there that does this? Or focus on the go.sum in stead

#!/bin/bash
while read -r line; do
    GCatch -repo $line > report/$line
done < go.sum

There is a reason why other tools don't Scan dependencies like GCatch does

bash can do so much of the work for us

Then you can focus on making a great tool and not bugfixing your dependencies.

BurtonQin commented 3 years ago

@thejerf It is not trivial to migrate GCatch from GOPATH to go.mod

When I typed the command "go mod init && go mod tidy" under "GCatch/GCatch", I found that "cmd/guru/serial" and "imports" do not exist under "GCatch/GCatch/tools".

GCatch/tools/internal/lsp/cmd/definition.go：15 
         guru "github.com/system-pclub/GCatch/GCatch/tools/cmd/guru/serial"

GCatch/tools/internal/lsp/source/format.go:16
        "github.com/system-pclub/GCatch/GCatch/tools/imports"

@lzhfromustc They seem harmless when building with GOPATH but buggy for go.mod

Even after I added the missing source code from "golang/tools" and successfully built&installed GCatch using go.mod (https://github.com/BurtonQin/GCatch/tree/build-with-go-mod), GCatch still reported many building errors when scanning the demo buggy grpc without turning off GO111MODULE

I reckon the underlying analysis framework of GCatch is too old to support go module. Even if I can make GCatch BUILD with go module, it still requires a lot of work to make GCatch WORK on projects with go module on. So Task 4 is not just a job of shells. @MikkelHJuul For Task 5, we can use "-r" to toggle the CHECKING of dependencies. But dependencies are still compiled since the framework requires a whole program SSA. https://github.com/system-pclub/GCatch/blob/01c0e0281d7042d27d43e2bbdb5e63f8360b3cdc/GCatch/cmd/GCatch/main.go#L144

Skarlso commented 3 years ago

That is super sad! :( I find this project to be wonderful and the paper is insightful. I would like to see this tool incorporated as a linter into everything I touch.

Is there any way I could assist?

thejerf commented 3 years ago

I wasn't expecting it to be trivial to migrate to go modules. I was expecting it to be the sort of thing software engineers can plow through without having to understand the underlying code per se. I expected both problems building it, and that the underlying code that gets the source code for processing will hae to be changed too. It's a perfect sort of engineering/academic partnership project.

CarlosRosuero commented 1 year ago

Hi all.

Has there been any progress on this? I'm struggling to run even the provided example:

carlos@LAPTOP-K6R30EOS:~/go/src/github.com/system-pclub/GCatch/GCatch$ ./run.sh
Make sure you have run installZ3.sh and then install.sh
Step 1: setting GOPATH of the checked grpc
GOPATH is set to /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/testdata/grpc-buggy

Description of flags of GCatch:
Required Flag: -path=Full path of the application to be checked
Required Flag: -include=Relative path (what's after /src/) of the application to be checked
Required Flag: -checker=The checkers you want to run, divided by ":".    Default value:BMOC
Optional Flag: -r    Whether all children packages should also be checked recursively
Optional Flag: -compile-error    Whether compilation errors should be printed, if there are any
Optional Flag: -vendor=Packages that will be ignored, divided by ":".    Default value:vendor

Step 2: running GCatch on a buggy version of grpc in testdata
Note: all bugs reported below should be real BMOC bugs
GO111MODULE=off /home/carlos/go/bin/GCatch -path=/home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/testdata/grpc-buggy/src/google.golang.org/grpc -include=google.golang.org/grpc -checker=BMOC -r
panic: no concrete method: func (*sync/atomic.Pointer[string]).CompareAndSwap(old *string, new *string) (swapped bool)

goroutine 3142 [running]:
github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Program).declaredFunc(0xc00f42d680, 0xc0142d3a40)        
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/methods.go:124 +0xf9
github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Program).addMethod(0x7d4bf8?, 0xc0103173c0, 0xc0142d1950)
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/methods.go:86 +0x156
github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Program).needMethods(0xc00f42d680, {0x7d4bf8?, 0xc001681930?}, 0x0)
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/methods.go:173 +0x645
github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Program).needMethods(0xc00f42d680, {0x7d4c70?, 0xc0024aae10?}, 0x1)
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/methods.go:228 +0x55e
github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Program).needMethods(0xc00f42d680, {0x7d4bd0?, 0xc0009a8c40?}, 0x0)
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/methods.go:221 +0x40c
github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Program).needMethodsOf(0xc00f42d680, {0x7d4bd0?, 0xc0009a8c40?})
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/methods.go:145 +0x70
github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Package).build(0xc00fcd1380)
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/builder.go:2281 +0x111
sync.(*Once).doSlow(0xc000297fb8?, 0x654b5e?)
        /usr/local/go/src/sync/once.go:74 +0xc2
sync.(*Once).Do(...)
        /usr/local/go/src/sync/once.go:65
github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Package).Build(...)
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/builder.go:2269
github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Program).Build.func1(0x0?)
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/builder.go:2253 +0x4c
created by github.com/system-pclub/GCatch/GCatch/tools/go/ssa.(*Program).Build
        /home/carlos/go/src/github.com/system-pclub/GCatch/GCatch/tools/go/ssa/builder.go:2252 +0x19c

As well as the beta functionality:

carlos@LAPTOP-K6R30EOS:~$ GCatch -mod -mod-abs-path /playground/kubernetes -mod-module-path k8s.io/kube
rnetes/staging/src/k8s.io/apimachinery/pkg/util/wait -compile-error
-: cannot query module due to -mod=vendor
        (Go version in go.mod is at least 1.14 and vendor directory exists.)
Failed to build the whole program. The entrance package or its dependencies have error. type_err
Exit. You are using the -mod flag, which only supports building and checking the specified program (and its dependencies)

Time of main(): seconds 0.535229652

Or building the provided Dockerfile:

carlos@LAPTOP-K6R30EOS:~/go/src/github.com/system-pclub/GCatch/GCatch$ docker build - < Dockerfile
[+] Building 1.3s (11/12)
 => [internal] load build definition from Dockerfile                                             0.0s
 => => transferring dockerfile: 524B                                                             0.0s
 => [internal] load .dockerignore                                                                0.0s
 => => transferring context: 2B                                                                  0.0s
 => [internal] load metadata for docker.io/library/golang:1.16.4                                 0.9s
 => [1/8] FROM docker.io/library/golang:1.16.4@sha256:8a106c4b4005efb43c0ba4bb5763b84742c7e222b  0.0s
 => [internal] load build context                                                                0.0s
 => => transferring context: 2B                                                                  0.0s
 => CACHED [2/8] RUN git clone https://github.com/Z3Prover/z3 /repos/z3                          0.0s
 => CACHED [3/8] WORKDIR /repos/z3                                                               0.0s
 => CACHED [4/8] RUN python scripts/mk_make.py   && cd build   && make   && make install         0.0s
 => CACHED [5/8] COPY . /go/src/github.com/system-pclub/GCatch/GCatch                            0.0s
 => CACHED [6/8] RUN GO111MODULE=off go get golang.org/x/xerrors && GO111MODULE=off go get gola  0.0s
 => ERROR [7/8] RUN /go/src/github.com/system-pclub/GCatch/GCatch/install.sh                     0.4s
------
 > [7/8] RUN /go/src/github.com/system-pclub/GCatch/GCatch/install.sh:
#0 0.373 /bin/sh: 1: /go/src/github.com/system-pclub/GCatch/GCatch/install.sh: not found
------
Dockerfile:17
--------------------
  15 |     && GO111MODULE=off go get golang.org/x/sys/execabs
  16 |
  17 | >>> RUN /go/src/github.com/system-pclub/GCatch/GCatch/install.sh
  18 |
  19 |     WORKDIR /playground
--------------------
ERROR: failed to solve: process "/bin/sh -c /go/src/github.com/system-pclub/GCatch/GCatch/install.sh" did not complete successfully: exit code: 127

All in all, I'm at a loss as to how to make it work. I would be happy to work on the code to get it to work, but I would need some pointers.

Appreciate any help!

CarlosRosuero commented 1 year ago

After some tinkering, I have managed to make it work on go.mod modules, by adding BuildFlags: []string{"-mod=mod"} to the following line: https://github.com/system-pclub/GCatch/blob/f0710b464a03c125aa57a1631882c3b331a3b881/GCatch/ssabuild/build.go#L61

Leading to the following output on a sample-like input:

carlos@LAPTOP-K6R30EOS:~$ GCatch -mod -mod-abs-path /playground/kubernetes/staging/src/k8s.io/metrics -mod-module-path k8s.io/metrics -compile-error
Successfully built whole program. Now running checkers
Exit. You are using the -mod flag, which only supports building and checking the specified program (and its dependencies)

Time of main(): seconds 0.289908609

Unfortunately, I have not managed to produce an example where GCatch detects a bug, partly because it seems to fail on any function involving generics, e.g:

carlos@LAPTOP-K6R30EOS:~/goat$ GCatch -mod -mod-abs-path /playground/kubernetes/staging/src/k8s.io/sample-apiserver -mod-module-path k8s.io/sample-apiserver -compile-error
panic: no concrete method: func (*sync/atomic.Pointer[string]).CompareAndSwap(old *string, new *string) (swapped bool)

CarlosRosuero commented 1 year ago

Indeed, the problem arises here: https://github.com/system-pclub/GCatch/blob/f0710b464a03c125aa57a1631882c3b331a3b881/GCatch/tools/go/ssa/source.go#L188-L193 The function receives another function obj, whose Pkg() is in prog.packages, but which is not in pkg.values. In one example, obj was func (*sync/atomic.Pointer[any]).CompareAndSwap(old *any, new *any) (swapped bool), and pkg.values contained close relatives, most notably func (*sync/atomic.Pointer[T]).CompareAndSwap(old *T, new *T) (swapped bool).

I believe a potential cause for this is the version of the ssa package used cannot deal with generics, which were introduced later. Will keep looking.

CarlosRosuero commented 1 year ago

Indeed, migrating to golang.org/x/tools/go/ssa (and necessarily golang.org/x/tools/go/packages) solved it. I am now dealing with the following:

Error when building callgraph with nil Queries: internal error in pointer analysis: cannot flatten unsupported type *types.TypeParam (please report this bug)

which is thrown here: https://github.com/system-pclub/GCatch/blob/f0710b464a03c125aa57a1631882c3b331a3b881/GCatch/tools/go/mypointer/util.go#L173

The documentation for the official golang.org/x/tools/go/pointer package reads:

Type parameters are not directly supported by the analysis. Calls to generic functions will be left as if they had empty bodies. Users of the package are expected to use the ssa.InstantiateGenerics builder mode when building code that uses or depends on code containing generics.

and gives the following example:

prog := ssautil.CreateProgram(iprog, ssa.InstantiateGenerics)

However, I have added ssa.InstantiateGenerics to both places I have found it is needed, to no avail: https://github.com/system-pclub/GCatch/blob/f0710b464a03c125aa57a1631882c3b331a3b881/GCatch/ssabuild/build.go#L86 https://github.com/system-pclub/GCatch/blob/f0710b464a03c125aa57a1631882c3b331a3b881/GCatch/tools/go/analysis/passes/buildssa/buildssa.go#L51-L53

I have also looked through all uses of ssa.BuilderMode and all of its possible values, also to no avail.

BurtonQin commented 1 year ago

Thank you for your interest in the project. The project was built a few years ago, and at that time Go didn't support generics. I appreciate your suggestion and will definitely explore the possibility of keeping the project up-to-date. However, I must admit that it won't be an easy task and may require a significant amount of effort.

CarlosRosuero commented 1 year ago

Hi @BurtonQin. Thanks for your reply! This is a very interesting project indeed. I am optimistic about getting it to work, especially given that the underlying packages (golang.org/x/tools/go/pointer specifically) seem to be able to deal with generics (with some help).

My current guess is that a different ssa.BuilderMode is hardcoded somewhere else in the code, but I have been unable to find it.

On another note, it would be very useful to know if any changes were made to the ssa or packages packages, as I had to revert to the official implementations. Apologies if these are documented somewhere.

BurtonQin commented 1 year ago

The original coder of the core code is lzhfromustc. As far as I know, his current email address is zil060@ucsd.edu. You can try to contact him for more details about the changes. The changes are briefed in tools/README.md and ssabuild/build.go

BurtonQin commented 1 year ago

I adapt the latest version of golang.org/x/tools/pointer to GCatch, which is a drop-in replacement of the mypointer package in current GCatch: The code is in https://github.com/BurtonQin/mypointer

CarlosRosuero commented 1 year ago

Thanks for that! I've replaced the package with that (and made a few other needed modifications) but I'm still getting the same error. Is there something else I need to do?

CarlosRosuero commented 1 year ago

I added your version of mypointer as a submodule to my repo. I have had to make a couple of changes:

Changing the package name to mypointer
Importing the GCatch versions of typeparams and callgraph

Is there any chance I could get push permissions to https://github.com/BurtonQin/mypointer? Ditto for GCatch, but I understand that's a bigger commitment.

BurtonQin commented 1 year ago

You can create a fork and make a PR to BurtonQin/mypointer. As for GCatch, can you push your changes to BurtonQin/GCatch?

CarlosRosuero commented 1 year ago

I don't have permission to push to BurtonQin/GCatch. Should that be another PR?

BurtonQin commented 1 year ago

I just sent you an invitation email.

BurtonQin commented 1 year ago

@CarlosRosuero I have upgraded the dependencies of GCatch is in https://github.com/BurtonQin/GCatch/tree/generic/GCatch, which should have support the detection of generic functions.

system-pclub / GCatch

Installation and other issues #23