diff options
author | Adin Scannell <ascannell@google.com> | 2020-10-26 11:09:34 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-10-26 11:11:46 -0700 |
commit | 7926a9e28da8852954961af2aea0a280b6bbd210 (patch) | |
tree | d2c87f7108aa207b0c2c87410d12c3f952a56689 | |
parent | e2dce046037c30b585cc62db45d517f59d1a08fc (diff) |
Add nogo configuration.
This splits the nogo rules into a separate configuration yaml file, and
allows for multiple files to be provided.
Because attrs cannot be passed down to aspects, this required that all
findings are propagated up the aspect Provider. This doesn't mean that
any extra work must be done, just that this information must be carried
through the graph, and some additional starlark complexity is required.
PiperOrigin-RevId: 339076357
-rw-r--r-- | BUILD | 7 | ||||
-rw-r--r-- | nogo.yaml | 495 | ||||
-rw-r--r-- | tools/defs.bzl | 12 | ||||
-rw-r--r-- | tools/github/nogo/BUILD | 2 | ||||
-rw-r--r-- | tools/github/nogo/nogo.go | 19 | ||||
-rw-r--r-- | tools/nogo/BUILD | 10 | ||||
-rw-r--r-- | tools/nogo/analyzers.go | 131 | ||||
-rw-r--r-- | tools/nogo/build.go | 16 | ||||
-rw-r--r-- | tools/nogo/check/BUILD | 2 | ||||
-rw-r--r-- | tools/nogo/check/main.go | 92 | ||||
-rw-r--r-- | tools/nogo/config.go | 725 | ||||
-rw-r--r-- | tools/nogo/defs.bzl | 179 | ||||
-rw-r--r-- | tools/nogo/filter/BUILD | 14 | ||||
-rw-r--r-- | tools/nogo/filter/main.go | 131 | ||||
-rw-r--r-- | tools/nogo/findings.go | 63 | ||||
-rwxr-xr-x | tools/nogo/gentest.sh | 48 | ||||
-rw-r--r-- | tools/nogo/matchers.go | 172 | ||||
-rw-r--r-- | tools/nogo/nogo.go | 175 | ||||
-rw-r--r-- | tools/nogo/register.go | 67 | ||||
-rw-r--r-- | tools/nogo/util/BUILD | 9 | ||||
-rw-r--r-- | tools/nogo/util/util.go | 85 | ||||
-rw-r--r-- | website/BUILD | 5 |
22 files changed, 1340 insertions, 1119 deletions
@@ -1,10 +1,17 @@ load("//tools:defs.bzl", "build_test", "gazelle", "go_path") +load("//tools/nogo:defs.bzl", "nogo_config") load("//website:defs.bzl", "doc") package(licenses = ["notice"]) exports_files(["LICENSE"]) +nogo_config( + name = "nogo_config", + srcs = ["nogo.yaml"], + visibility = ["//:sandbox"], +) + doc( name = "contributing", src = "CONTRIBUTING.md", diff --git a/nogo.yaml b/nogo.yaml new file mode 100644 index 000000000..07fd9aa4d --- /dev/null +++ b/nogo.yaml @@ -0,0 +1,495 @@ +groups: + # We define three basic groups: generated (all generated files), + # exteranl (all files outside the repository), and internal (all + # files within the local repository). We can't enforce many style + # checks on generated and external code, so enable those cases + # selectively for analyzers below. + - name: generated + regex: "^(bazel-genfiles|bazel-out|bazel-bin)/" + default: true + - name: external + regex: "^external/" + default: false + - name: internal + regex: ".*" + default: true +global: + generated: + suppress: + # Suppress the basic style checks for + # generated code, but keep the analysis + # that are required for quality & security. + - "should not use ALL_CAPS in Go names" + - "should not use underscores" + - "comment on exported" + - "methods on the same type should have the same receiver name" + - "at least one file in a package" + - "package comment should be of the form" + # Generated code may have dead code paths. + - "identical build constraints" + - "no value of type" + - "is never used" + # go_embed_data rules generate unicode literals. + - "string literal contains the Unicode format character" + - "string literal contains the Unicode control character" + # Some external code will generate protov1 + # implementations. These should be ignored. + - "proto.* is deprecated" + - "xxx_messageInfo_.*" + - "receiver name should be a reflection of its identity" + # Generated gRPC code is not compliant either. + - "error strings should not be capitalized" + - "grpc.Errorf is deprecated" + internal: + suppress: + # We use ALL_CAPS for system definitions, + # which are common enough in the code base + # that we shouldn't annotate exceptions. + # + # Same story for underscores. + - "should not use ALL_CAPS in Go names" + - "should not use underscores in Go names" + exclude: + # A variety of staticcheck and stylecheck + # rules apply here. These should be fixed + # and removed from here, and the global + # rules should be used sparingly. + - pkg/abi/linux/fuse.go:22 + - pkg/abi/linux/fuse.go:25 + - pkg/abi/linux/socket.go:113 + - pkg/abi/linux/tty.go:73 + - pkg/bpf/decoder.go:112 + - pkg/cpuid/cpuid_x86.go:675 + - pkg/eventchannel/event.go:193 + - pkg/eventchannel/event.go:27 + - pkg/eventchannel/event_test.go:22 + - pkg/eventchannel/rate.go:19 + - pkg/gohacks/gohacks_unsafe.go:33 + - pkg/log/json.go:30 + - pkg/log/log.go:359 + - pkg/merkletree/merkletree.go:230 + - pkg/merkletree/merkletree.go:243 + - pkg/merkletree/merkletree.go:249 + - pkg/merkletree/merkletree.go:266 + - pkg/merkletree/merkletree.go:355 + - pkg/merkletree/merkletree.go:369 + - pkg/metric/metric_test.go:20 + - pkg/p9/p9test/client_test.go:687 + - pkg/p9/transport_test.go:196 + - pkg/pool/pool.go:15 + - pkg/refs/refcounter.go:510 + - pkg/refs/refcounter_test.go:169 + - pkg/refs_vfs2/refs.go:16 + - pkg/safemem/block_unsafe.go:89 + - pkg/seccomp/seccomp.go:82 + - pkg/segment/test/set_functions.go:15 + - pkg/sentry/arch/signal.go:166 + - pkg/sentry/arch/signal.go:171 + - pkg/sentry/control/pprof.go:196 + - pkg/sentry/devices/memdev/full.go:58 + - pkg/sentry/devices/memdev/null.go:59 + - pkg/sentry/devices/memdev/random.go:68 + - pkg/sentry/devices/memdev/zero.go:86 + - pkg/sentry/fdimport/fdimport.go:15 + - pkg/sentry/fs/attr.go:257 + - pkg/sentry/fsbridge/fs.go:116 + - pkg/sentry/fsbridge/vfs.go:124 + - pkg/sentry/fsbridge/vfs.go:70 + - pkg/sentry/fs/copy_up.go:365 + - pkg/sentry/fs/copy_up_test.go:65 + - pkg/sentry/fs/dev/net_tun.go:161 + - pkg/sentry/fs/dev/net_tun.go:63 + - pkg/sentry/fs/dev/null.go:97 + - pkg/sentry/fs/dirent_cache.go:64 + - pkg/sentry/fs/file_overlay.go:327 + - pkg/sentry/fs/file_overlay.go:524 + - pkg/sentry/fs/filetest/filetest.go:55 + - pkg/sentry/fs/filetest/filetest.go:60 + - pkg/sentry/fs/fs.go:77 + - pkg/sentry/fs/fsutil/file.go:290 + - pkg/sentry/fs/fsutil/file.go:346 + - pkg/sentry/fs/fsutil/host_file_mapper.go:105 + - pkg/sentry/fs/fsutil/inode_cached.go:676 + - pkg/sentry/fs/fsutil/inode_cached.go:772 + - pkg/sentry/fs/gofer/attr.go:120 + - pkg/sentry/fs/gofer/fifo.go:33 + - pkg/sentry/fs/gofer/inode.go:410 + - pkg/sentry/fsimpl/devpts/devpts.go:110 + - pkg/sentry/fsimpl/devpts/devpts.go:246 + - pkg/sentry/fsimpl/devpts/devpts.go:50 + - pkg/sentry/fsimpl/devpts/master.go:110 + - pkg/sentry/fsimpl/devpts/master.go:55 + - pkg/sentry/fsimpl/devpts/replica.go:113 + - pkg/sentry/fsimpl/devpts/replica.go:57 + - pkg/sentry/fsimpl/devtmpfs/devtmpfs.go:54 + - pkg/sentry/fsimpl/ext/disklayout/superblock_64.go:97 + - pkg/sentry/fsimpl/ext/disklayout/superblock_old.go:92 + - pkg/sentry/fsimpl/ext/disklayout/block_group_32.go:44 + - pkg/sentry/fsimpl/ext/disklayout/inode_new.go:91 + - pkg/sentry/fsimpl/ext/disklayout/inode_old.go:93 + - pkg/sentry/fsimpl/ext/disklayout/superblock_32.go:66 + - pkg/sentry/fsimpl/ext/disklayout/block_group_64.go:53 + - pkg/sentry/fsimpl/eventfd/eventfd.go:268 + - pkg/sentry/fsimpl/ext/directory.go:163 + - pkg/sentry/fsimpl/ext/directory.go:164 + - pkg/sentry/fsimpl/ext/extent_file.go:142 + - pkg/sentry/fsimpl/ext/extent_file.go:143 + - pkg/sentry/fsimpl/ext/ext.go:105 + - pkg/sentry/fsimpl/ext/filesystem.go:287 + - pkg/sentry/fsimpl/ext/regular_file.go:153 + - pkg/sentry/fsimpl/ext/symlink.go:113 + - pkg/sentry/fsimpl/fuse/connection_control.go:194 + - pkg/sentry/fsimpl/fuse/dev.go:387 + - pkg/sentry/fsimpl/fuse/dev_test.go:318 + - pkg/sentry/fsimpl/fuse/fusefs.go:102 + - pkg/sentry/fsimpl/fuse/read_write.go:129 + - pkg/sentry/fsimpl/fuse/request_response.go:71 + - pkg/sentry/fsimpl/gofer/directory.go:135 + - pkg/sentry/fsimpl/gofer/filesystem.go:679 + - pkg/sentry/fsimpl/gofer/gofer.go:1694 + - pkg/sentry/fsimpl/gofer/gofer.go:276 + - pkg/sentry/fsimpl/gofer/regular_file.go:81 + - pkg/sentry/fsimpl/gofer/special_file.go:141 + - pkg/sentry/fsimpl/host/host.go:184 + - pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go:50 + - pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go:90 + - pkg/sentry/fsimpl/kernfs/fd_impl_util.go:273 + - pkg/sentry/fsimpl/kernfs/filesystem.go:247 + - pkg/sentry/fsimpl/kernfs/inode_impl_util.go:320 + - pkg/sentry/fsimpl/kernfs/inode_impl_util.go:497 + - pkg/sentry/fsimpl/kernfs/synthetic_directory.go:52 + - pkg/sentry/fsimpl/overlay/directory.go:119 + - pkg/sentry/fsimpl/overlay/filesystem.go:527 + - pkg/sentry/fsimpl/overlay/non_directory.go:152 + - pkg/sentry/fsimpl/overlay/overlay.go:115 + - pkg/sentry/fsimpl/overlay/overlay.go:719 + - pkg/sentry/fsimpl/pipefs/pipefs.go:74 + - pkg/sentry/fsimpl/proc/filesystem.go:52 + - pkg/sentry/fsimpl/proc/filesystem.go:81 + - pkg/sentry/fsimpl/proc/subtasks.go:126 + - pkg/sentry/fsimpl/proc/subtasks.go:189 + - pkg/sentry/fsimpl/proc/task_fds.go:168 + - pkg/sentry/fsimpl/proc/task_fds.go:228 + - pkg/sentry/fsimpl/proc/task_fds.go:301 + - pkg/sentry/fsimpl/proc/task_fds.go:318 + - pkg/sentry/fsimpl/proc/task_fds.go:67 + - pkg/sentry/fsimpl/proc/task_files.go:112 + - pkg/sentry/fsimpl/proc/task_files.go:158 + - pkg/sentry/fsimpl/proc/task_files.go:259 + - pkg/sentry/fsimpl/proc/task_files.go:285 + - pkg/sentry/fsimpl/proc/task_files.go:305 + - pkg/sentry/fsimpl/proc/task_files.go:384 + - pkg/sentry/fsimpl/proc/task_files.go:403 + - pkg/sentry/fsimpl/proc/task_files.go:428 + - pkg/sentry/fsimpl/proc/task_files.go:691 + - pkg/sentry/fsimpl/proc/task_files.go:770 + - pkg/sentry/fsimpl/proc/task_files.go:797 + - pkg/sentry/fsimpl/proc/task_files.go:828 + - pkg/sentry/fsimpl/proc/task_files.go:879 + - pkg/sentry/fsimpl/proc/task_files.go:910 + - pkg/sentry/fsimpl/proc/task_files.go:961 + - pkg/sentry/fsimpl/proc/task.go:127 + - pkg/sentry/fsimpl/proc/task.go:193 + - pkg/sentry/fsimpl/proc/task_net.go:134 + - pkg/sentry/fsimpl/proc/task_net.go:475 + - pkg/sentry/fsimpl/proc/task_net.go:491 + - pkg/sentry/fsimpl/proc/task_net.go:508 + - pkg/sentry/fsimpl/proc/task_net.go:665 + - pkg/sentry/fsimpl/proc/task_net.go:715 + - pkg/sentry/fsimpl/proc/task_net.go:779 + - pkg/sentry/fsimpl/proc/tasks_files.go:113 + - pkg/sentry/fsimpl/proc/tasks_files.go:388 + - pkg/sentry/fsimpl/proc/tasks.go:232 + - pkg/sentry/fsimpl/proc/tasks_sys.go:145 + - pkg/sentry/fsimpl/proc/tasks_sys.go:181 + - pkg/sentry/fsimpl/proc/tasks_sys.go:239 + - pkg/sentry/fsimpl/proc/tasks_sys.go:291 + - pkg/sentry/fsimpl/proc/tasks_sys.go:375 + - pkg/sentry/fsimpl/signalfd/signalfd.go:124 + - pkg/sentry/fsimpl/signalfd/signalfd.go:15 + - pkg/sentry/fsimpl/signalfd/signalfd.go:126 + - pkg/sentry/fsimpl/sockfs/sockfs.go:36 + - pkg/sentry/fsimpl/sockfs/sockfs.go:79 + - pkg/sentry/fsimpl/sys/kcov.go:49 + - pkg/sentry/fsimpl/sys/kcov.go:99 + - pkg/sentry/fsimpl/sys/sys.go:118 + - pkg/sentry/fsimpl/sys/sys.go:56 + - pkg/sentry/fsimpl/testutil/testutil.go:257 + - pkg/sentry/fsimpl/testutil/testutil.go:260 + - pkg/sentry/fsimpl/timerfd/timerfd.go:87 + - pkg/sentry/fsimpl/tmpfs/directory.go:112 + - pkg/sentry/fsimpl/tmpfs/filesystem.go:195 + - pkg/sentry/fsimpl/tmpfs/regular_file.go:226 + - pkg/sentry/fsimpl/tmpfs/regular_file.go:346 + - pkg/sentry/fsimpl/tmpfs/tmpfs.go:103 + - pkg/sentry/fsimpl/tmpfs/tmpfs.go:733 + - pkg/sentry/fsimpl/verity/filesystem.go:490 + - pkg/sentry/fsimpl/verity/verity.go:156 + - pkg/sentry/fsimpl/verity/verity.go:629 + - pkg/sentry/fsimpl/verity/verity.go:672 + - pkg/sentry/fs/mount.go:162 + - pkg/sentry/fs/mount.go:256 + - pkg/sentry/fs/mount_overlay.go:144 + - pkg/sentry/fs/mounts.go:432 + - pkg/sentry/fs/proc/exec_args.go:104 + - pkg/sentry/fs/proc/exec_args.go:73 + - pkg/sentry/fs/proc/fds.go:269 + - pkg/sentry/fs/proc/loadavg.go:33 + - pkg/sentry/fs/proc/meminfo.go:39 + - pkg/sentry/fs/proc/mounts.go:193 + - pkg/sentry/fs/proc/mounts.go:84 + - pkg/sentry/fs/proc/net.go:125 + - pkg/sentry/fs/proc/proc.go:146 + - pkg/sentry/fs/proc/proc.go:204 + - pkg/sentry/fs/proc/seqfile/seqfile.go:210 + - pkg/sentry/fs/proc/sys.go:146 + - pkg/sentry/fs/proc/sys.go:43 + - pkg/sentry/fs/proc/sys_net.go:113 + - pkg/sentry/fs/proc/sys_net.go:205 + - pkg/sentry/fs/proc/sys_net.go:233 + - pkg/sentry/fs/proc/sys_net.go:307 + - pkg/sentry/fs/proc/sys_net.go:335 + - pkg/sentry/fs/proc/sys_net.go:446 + - pkg/sentry/fs/proc/sys_net.go:456 + - pkg/sentry/fs/proc/sys_net.go:89 + - pkg/sentry/fs/proc/task.go:170 + - pkg/sentry/fs/proc/task.go:322 + - pkg/sentry/fs/proc/task.go:427 + - pkg/sentry/fs/proc/task.go:467 + - pkg/sentry/fs/proc/task.go:500 + - pkg/sentry/fs/proc/task.go:784 + - pkg/sentry/fs/proc/task.go:839 + - pkg/sentry/fs/proc/task.go:920 + - pkg/sentry/fs/proc/uid_gid_map.go:108 + - pkg/sentry/fs/proc/uid_gid_map.go:79 + - pkg/sentry/fs/proc/uptime.go:75 + - pkg/sentry/fs/ramfs/dir.go:447 + - pkg/sentry/fs/tmpfs/inode_file.go:436 + - pkg/sentry/fs/tmpfs/inode_file.go:537 + - pkg/sentry/fs/tty/dir.go:313 + - pkg/sentry/fs/tty/master.go:131 + - pkg/sentry/fs/tty/master.go:91 + - pkg/sentry/fs/tty/replica.go:116 + - pkg/sentry/fs/tty/replica.go:88 + - pkg/sentry/kernel/auth/id_map.go:269 + - pkg/sentry/kernel/fasync/fasync.go:67 + - pkg/sentry/kernel/kcov.go:209 + - pkg/sentry/kernel/kcov.go:223 + - pkg/sentry/kernel/kernel.go:343 + - pkg/sentry/kernel/kernel.go:368 + - pkg/sentry/kernel/pipe/node_test.go:112 + - pkg/sentry/kernel/pipe/node_test.go:119 + - pkg/sentry/kernel/pipe/node_test.go:130 + - pkg/sentry/kernel/pipe/node_test.go:137 + - pkg/sentry/kernel/pipe/node_test.go:149 + - pkg/sentry/kernel/pipe/node_test.go:150 + - pkg/sentry/kernel/pipe/node_test.go:158 + - pkg/sentry/kernel/pipe/node_test.go:174 + - pkg/sentry/kernel/pipe/node_test.go:180 + - pkg/sentry/kernel/pipe/node_test.go:193 + - pkg/sentry/kernel/pipe/node_test.go:202 + - pkg/sentry/kernel/pipe/node_test.go:205 + - pkg/sentry/kernel/pipe/node_test.go:216 + - pkg/sentry/kernel/pipe/node_test.go:219 + - pkg/sentry/kernel/pipe/node_test.go:271 + - pkg/sentry/kernel/pipe/node_test.go:290 + - pkg/sentry/kernel/pipe/pipe_test.go:93 + - pkg/sentry/kernel/pipe/reader_writer.go:65 + - pkg/sentry/kernel/posixtimer.go:157 + - pkg/sentry/kernel/ptrace.go:218 + - pkg/sentry/kernel/semaphore/semaphore.go:323 + - pkg/sentry/kernel/sessions.go:123 + - pkg/sentry/kernel/sessions.go:508 + - pkg/sentry/kernel/signal_handlers.go:57 + - pkg/sentry/kernel/task_context.go:72 + - pkg/sentry/kernel/task_exit.go:67 + - pkg/sentry/kernel/task_sched.go:255 + - pkg/sentry/kernel/task_sched.go:280 + - pkg/sentry/kernel/task_sched.go:323 + - pkg/sentry/kernel/task_stop.go:192 + - pkg/sentry/kernel/thread_group.go:530 + - pkg/sentry/kernel/timekeeper.go:316 + - pkg/sentry/kernel/vdso.go:106 + - pkg/sentry/kernel/vdso.go:118 + - pkg/sentry/memmap/memmap.go:103 + - pkg/sentry/memmap/memmap.go:163 + - pkg/sentry/mm/address_space.go:42 + - pkg/sentry/mm/address_space.go:42 + - pkg/sentry/mm/aio_context.go:208 + - pkg/sentry/mm/aio_context.go:288 + - pkg/sentry/mm/pma.go:683 + - pkg/sentry/mm/special_mappable.go:80 + - pkg/sentry/syscalls/linux/sys_sem.go:62 + - pkg/sentry/syscalls/linux/sys_time.go:189 + - pkg/sentry/usage/cpu.go:42 + - pkg/sentry/vfs/anonfs.go:302 + - pkg/sentry/vfs/anonfs.go:99 + - pkg/sentry/vfs/dentry.go:214 + - pkg/sentry/vfs/epoll.go:168 + - pkg/sentry/vfs/epoll.go:314 + - pkg/sentry/vfs/file_description.go:549 + - pkg/sentry/vfs/file_description_impl_util.go:304 + - pkg/sentry/vfs/file_description_impl_util.go:412 + - pkg/sentry/vfs/filesystem.go:76 + - pkg/sentry/vfs/lock.go:15 + - pkg/sentry/vfs/lock.go:47 + - pkg/sentry/vfs/memxattr/xattr.go:37 + - pkg/sentry/vfs/mount.go:510 + - pkg/sentry/vfs/mount.go:667 + - pkg/sentry/vfs/mount_test.go:106 + - pkg/sentry/vfs/mount_test.go:160 + - pkg/sentry/vfs/mount_test.go:215 + - pkg/sentry/vfs/mount_unsafe.go:153 + - pkg/sentry/vfs/resolving_path.go:228 + - pkg/sentry/vfs/vfs.go:897 + - pkg/shim/runsc/runsc.go:16 + - pkg/shim/runsc/utils.go:16 + - pkg/shim/v1/proc/deleted_state.go:16 + - pkg/shim/v1/proc/exec.go:16 + - pkg/shim/v1/proc/exec_state.go:16 + - pkg/shim/v1/proc/init.go:16 + - pkg/shim/v1/proc/init_state.go:16 + - pkg/shim/v1/proc/io.go:16 + - pkg/shim/v1/proc/process.go:16 + - pkg/shim/v1/proc/types.go:16 + - pkg/shim/v1/proc/utils.go:16 + - pkg/shim/v1/shim/api.go:16 + - pkg/shim/v1/shim/platform.go:16 + - pkg/shim/v1/shim/service.go:16 + - pkg/shim/v1/utils/annotations.go:15 + - pkg/shim/v1/utils/utils.go:15 + - pkg/shim/v1/utils/volumes.go:15 + - pkg/shim/v2/api.go:16 + - pkg/shim/v2/epoll.go:18 + - pkg/shim/v2/options/options.go:15 + - pkg/shim/v2/options/options.go:24 + - pkg/shim/v2/options/options.go:26 + - pkg/shim/v2/runtimeoptions/runtimeoptions.go:16 + - pkg/shim/v2/runtimeoptions/runtimeoptions_cri.go # Generated: exempt all. + - pkg/shim/v2/runtimeoptions/runtimeoptions_test.go:22 + - pkg/shim/v2/service.go:15 + - pkg/shim/v2/service_linux.go:18 + - pkg/state/tests/integer_test.go:23 + - pkg/state/tests/integer_test.go:28 + - pkg/sync/rwmutex_test.go:105 + - pkg/syserr/host_linux.go:35 + - pkg/unet/unet_test.go:634 + - pkg/unet/unet_test.go:662 + - pkg/unet/unet_test.go:703 + - pkg/unet/unet_test.go:98 + - pkg/usermem/addr.go:34 + - pkg/usermem/usermem.go:171 + - pkg/usermem/usermem.go:170 + - runsc/boot/compat.go:22 + - runsc/boot/compat.go:56 + - runsc/boot/loader.go:1115 + - runsc/boot/loader.go:1120 + - runsc/cmd/checkpoint.go:151 + - runsc/config/flags.go:32 + - runsc/container/container.go:641 + - runsc/container/container.go:988 + - runsc/specutils/specutils.go:172 + - runsc/specutils/specutils.go:428 + - runsc/specutils/specutils.go:436 + - runsc/specutils/specutils.go:442 + - runsc/specutils/specutils.go:447 + - runsc/specutils/specutils.go:454 + - test/cmd/test_app/fds.go:171 + - test/iptables/filter_output.go:251 + - test/packetimpact/testbench/connections.go:77 + - tools/bigquery/bigquery.go:106 + - tools/checkescape/test1/test1.go:108 + - tools/checkescape/test1/test1.go:122 + - tools/checkescape/test1/test1.go:137 + - tools/checkescape/test1/test1.go:151 + - tools/checkescape/test1/test1.go:170 + - tools/checkescape/test1/test1.go:39 + - tools/checkescape/test1/test1.go:45 + - tools/checkescape/test1/test1.go:50 + - tools/checkescape/test1/test1.go:64 + - tools/checkescape/test1/test1.go:80 + - tools/checkescape/test1/test1.go:94 + - tools/go_generics/imports.go:51 + - tools/go_generics/imports.go:75 + - tools/go_marshal/gomarshal/generator.go:177 + - tools/go_marshal/gomarshal/generator.go:81 + - tools/go_marshal/gomarshal/generator.go:85 + - tools/go_marshal/test/escape/escape.go:15 + - tools/go_marshal/test/test.go:164 +analyzers: + asmdecl: + external: # Enabled. + assign: + external: + exclude: + - gazelle/walk/walk.go + atomic: + external: # Enabled. + bools: + external: # Enabled. + buildtag: + external: # Enabled. + cgocall: + external: # Enabled. + shadow: # Disable for now. + generated: + exclude: [".*"] + internal: + exclude: [".*"] + composites: # Disable for now. + generated: + exclude: [".*"] + internal: + exclude: [".*"] + errorsas: + external: # Enabled. + httpresponse: + external: # Enabled. + loopclosure: + external: # Enabled. + nilfunc: + external: # Enabled. + nilness: + internal: + exclude: + - pkg/sentry/platform/kvm/kvm_test.go # Intentional. + - tools/bigquery/bigquery.go # False positive. + printf: + external: # Enabled. + shift: + external: # Enabled. + stringintconv: + external: + exclude: + - ".*protobuf/.*.go" # Bad conversions. + - ".*flate/huffman_bit_writer.go" # Bad conversion. + # Runtime internal violations. + - ".*reflect/value.go" + - ".*encoding/xml/xml.go" + - ".*runtime/pprof/internal/profile/proto.go" + - ".*fmt/scan.go" + - ".*go/types/conversions.go" + - ".*golang.org/x/net/dns/dnsmessage/message.go" + tests: + external: # Enabled. + unmarshal: + external: # Enabled. + unreachable: + external: # Enabled. + unsafeptr: + internal: + exclude: + - ".*_test.go" # Exclude tests. + - "pkg/flipcall/.*_unsafe.go" # Special case. + - pkg/gohacks/gohacks_unsafe.go # Special case. + - pkg/sentry/fs/fsutil/host_file_mapper_unsafe.go # Special case. + - pkg/sentry/platform/kvm/bluepill_unsafe.go # Special case. + - pkg/sentry/platform/kvm/machine_unsafe.go # Special case. + - pkg/sentry/platform/ring0/pagetables/allocator_unsafe.go # Special case. + - pkg/sentry/platform/safecopy/safecopy_unsafe.go # Special case. + - pkg/sentry/vfs/mount_unsafe.go # Special case. + - pkg/state/decode_unsafe.go # Special case. + unusedresult: + external: # Enabled. + checkescape: + external: # Enabled. diff --git a/tools/defs.bzl b/tools/defs.bzl index bb291c512..d75e40863 100644 --- a/tools/defs.bzl +++ b/tools/defs.bzl @@ -86,8 +86,10 @@ def go_binary(name, nogo = True, pure = False, static = False, x_defs = None, ** ) nogo_test( name = name + "_nogo", + config = "//:nogo_config", srcs = kwargs.get("srcs", []), - library = ":" + name + "_nogo_library", + deps = [":" + name + "_nogo_library"], + tags = ["nogo"], ) def calculate_sets(srcs): @@ -218,8 +220,10 @@ def go_library(name, srcs, deps = [], imports = [], stateify = True, marshal = F if nogo: nogo_test( name = name + "_nogo", + config = "//:nogo_config", srcs = all_srcs, - library = ":" + name, + deps = [":" + name], + tags = ["nogo"], ) if marshal: @@ -255,8 +259,10 @@ def go_test(name, nogo = True, **kwargs): if nogo: nogo_test( name = name + "_nogo", + config = "//:nogo_config", srcs = kwargs.get("srcs", []), - library = ":" + name, + deps = [":" + name], + tags = ["nogo"], ) def proto_library(name, srcs, deps = None, has_services = 0, **kwargs): diff --git a/tools/github/nogo/BUILD b/tools/github/nogo/BUILD index 0633eaf19..19b7eec4d 100644 --- a/tools/github/nogo/BUILD +++ b/tools/github/nogo/BUILD @@ -10,7 +10,7 @@ go_library( "//tools/github:__subpackages__", ], deps = [ - "//tools/nogo/util", + "//tools/nogo", "@com_github_google_go_github_v28//github:go_default_library", ], ) diff --git a/tools/github/nogo/nogo.go b/tools/github/nogo/nogo.go index b2bc63459..27ab1b8eb 100644 --- a/tools/github/nogo/nogo.go +++ b/tools/github/nogo/nogo.go @@ -24,7 +24,7 @@ import ( "time" "github.com/google/go-github/github" - "gvisor.dev/gvisor/tools/nogo/util" + "gvisor.dev/gvisor/tools/nogo" ) // FindingsPoster is a simple wrapper around the GitHub api. @@ -35,7 +35,7 @@ type FindingsPoster struct { dryRun bool startTime time.Time - findings map[util.Finding]struct{} + findings map[nogo.Finding]struct{} client *github.Client } @@ -47,7 +47,7 @@ func NewFindingsPoster(client *github.Client, owner, repo, commit string, dryRun commit: commit, dryRun: dryRun, startTime: time.Now(), - findings: make(map[util.Finding]struct{}), + findings: make(map[nogo.Finding]struct{}), client: client, } } @@ -63,7 +63,7 @@ func (p *FindingsPoster) Walk(paths []string) error { if !strings.HasSuffix(filename, ".findings") || info.IsDir() { return nil } - findings, err := util.ExtractFindingsFromFile(filename) + findings, err := nogo.ExtractFindingsFromFile(filename) if err != nil { return err } @@ -86,7 +86,7 @@ func (p *FindingsPoster) Post() error { if p.dryRun { for finding, _ := range p.findings { // Pretty print, so that this is useful for debugging. - fmt.Printf("%s: (%s+%d) %s\n", finding.Category, finding.Path, finding.Line, finding.Message) + fmt.Printf("%s: (%s+%d) %s\n", finding.Category, finding.Position.Filename, finding.Position.Line, finding.Message) } return nil } @@ -115,12 +115,13 @@ func (p *FindingsPoster) Post() error { } annotationLevel := "failure" // Always. for finding, _ := range p.findings { + title := string(finding.Category) opts.Output.Annotations = append(opts.Output.Annotations, &github.CheckRunAnnotation{ - Path: &finding.Path, - StartLine: &finding.Line, - EndLine: &finding.Line, + Path: &finding.Position.Filename, + StartLine: &finding.Position.Line, + EndLine: &finding.Position.Line, Message: &finding.Message, - Title: &finding.Category, + Title: &title, AnnotationLevel: &annotationLevel, }) } diff --git a/tools/nogo/BUILD b/tools/nogo/BUILD index 3c6be3339..12b8b597c 100644 --- a/tools/nogo/BUILD +++ b/tools/nogo/BUILD @@ -20,20 +20,14 @@ nogo_stdlib( visibility = ["//visibility:public"], ) -sh_binary( - name = "gentest", - srcs = ["gentest.sh"], - visibility = ["//visibility:public"], -) - go_library( name = "nogo", srcs = [ + "analyzers.go", "build.go", "config.go", - "matchers.go", + "findings.go", "nogo.go", - "register.go", ], nogo = False, visibility = ["//:sandbox"], diff --git a/tools/nogo/analyzers.go b/tools/nogo/analyzers.go new file mode 100644 index 000000000..b919bc2f8 --- /dev/null +++ b/tools/nogo/analyzers.go @@ -0,0 +1,131 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package nogo + +import ( + "encoding/gob" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/asmdecl" + "golang.org/x/tools/go/analysis/passes/assign" + "golang.org/x/tools/go/analysis/passes/atomic" + "golang.org/x/tools/go/analysis/passes/bools" + "golang.org/x/tools/go/analysis/passes/buildtag" + "golang.org/x/tools/go/analysis/passes/cgocall" + "golang.org/x/tools/go/analysis/passes/composite" + "golang.org/x/tools/go/analysis/passes/copylock" + "golang.org/x/tools/go/analysis/passes/errorsas" + "golang.org/x/tools/go/analysis/passes/httpresponse" + "golang.org/x/tools/go/analysis/passes/loopclosure" + "golang.org/x/tools/go/analysis/passes/lostcancel" + "golang.org/x/tools/go/analysis/passes/nilfunc" + "golang.org/x/tools/go/analysis/passes/nilness" + "golang.org/x/tools/go/analysis/passes/printf" + "golang.org/x/tools/go/analysis/passes/shadow" + "golang.org/x/tools/go/analysis/passes/shift" + "golang.org/x/tools/go/analysis/passes/stdmethods" + "golang.org/x/tools/go/analysis/passes/stringintconv" + "golang.org/x/tools/go/analysis/passes/structtag" + "golang.org/x/tools/go/analysis/passes/tests" + "golang.org/x/tools/go/analysis/passes/unmarshal" + "golang.org/x/tools/go/analysis/passes/unreachable" + "golang.org/x/tools/go/analysis/passes/unsafeptr" + "golang.org/x/tools/go/analysis/passes/unusedresult" + "honnef.co/go/tools/staticcheck" + "honnef.co/go/tools/stylecheck" + + "gvisor.dev/gvisor/tools/checkescape" + "gvisor.dev/gvisor/tools/checkunsafe" +) + +// AllAnalyzers is a list of all available analyzers. +var AllAnalyzers = []*analysis.Analyzer{ + asmdecl.Analyzer, + assign.Analyzer, + atomic.Analyzer, + bools.Analyzer, + buildtag.Analyzer, + cgocall.Analyzer, + composite.Analyzer, + copylock.Analyzer, + errorsas.Analyzer, + httpresponse.Analyzer, + loopclosure.Analyzer, + lostcancel.Analyzer, + nilfunc.Analyzer, + nilness.Analyzer, + printf.Analyzer, + shift.Analyzer, + stdmethods.Analyzer, + stringintconv.Analyzer, + shadow.Analyzer, + structtag.Analyzer, + tests.Analyzer, + unmarshal.Analyzer, + unreachable.Analyzer, + unsafeptr.Analyzer, + unusedresult.Analyzer, + checkescape.Analyzer, + checkunsafe.Analyzer, +} + +// EscapeAnalyzers is a list of escape-related analyzers. +var EscapeAnalyzers = []*analysis.Analyzer{ + checkescape.EscapeAnalyzer, +} + +func register(all []*analysis.Analyzer) { + // Register all fact types. + // + // N.B. This needs to be done recursively, because there may be + // analyzers in the Requires list that do not appear explicitly above. + registered := make(map[*analysis.Analyzer]struct{}) + var registerOne func(*analysis.Analyzer) + registerOne = func(a *analysis.Analyzer) { + if _, ok := registered[a]; ok { + return + } + + // Register dependencies. + for _, da := range a.Requires { + registerOne(da) + } + + // Register local facts. + for _, f := range a.FactTypes { + gob.Register(f) + } + + registered[a] = struct{}{} // Done. + } + for _, a := range all { + registerOne(a) + } +} + +func init() { + // Add all staticcheck analyzers. + for _, a := range staticcheck.Analyzers { + AllAnalyzers = append(AllAnalyzers, a) + } + // Add all stylecheck analyzers. + for _, a := range stylecheck.Analyzers { + AllAnalyzers = append(AllAnalyzers, a) + } + + // Register lists. + register(AllAnalyzers) + register(EscapeAnalyzers) +} diff --git a/tools/nogo/build.go b/tools/nogo/build.go index 55d34760f..d173cff1f 100644 --- a/tools/nogo/build.go +++ b/tools/nogo/build.go @@ -20,22 +20,6 @@ import ( "os" ) -var ( - // internalPrefix is the internal path prefix. Note that this is not - // special, as paths should be passed relative to the repository root - // and should not have any special prefix applied. - internalPrefix = fmt.Sprintf("^") - - // internalDefault is applied when no paths are provided. - internalDefault = fmt.Sprintf("%s/.*", notPath("external")) - - // generatedPrefix is a regex for generated files. - generatedPrefix = "^(.*/)?(bazel-genfiles|bazel-out|bazel-bin)/" - - // externalPrefix is external workspace packages. - externalPrefix = "^external/" -) - // findStdPkg needs to find the bundled standard library packages. func findStdPkg(GOOS, GOARCH, path string) (io.ReadCloser, error) { if path == "C" { diff --git a/tools/nogo/check/BUILD b/tools/nogo/check/BUILD index 21ba2c306..e18483a18 100644 --- a/tools/nogo/check/BUILD +++ b/tools/nogo/check/BUILD @@ -2,8 +2,6 @@ load("//tools:defs.bzl", "go_binary") package(licenses = ["notice"]) -# Note that the check binary must be public, since an aspect may be applied -# across lots of different rules in different repositories. go_binary( name = "check", srcs = ["main.go"], diff --git a/tools/nogo/check/main.go b/tools/nogo/check/main.go index 3828edf3a..69bdfe502 100644 --- a/tools/nogo/check/main.go +++ b/tools/nogo/check/main.go @@ -16,9 +16,99 @@ package main import ( + "encoding/json" + "flag" + "fmt" + "io/ioutil" + "log" + "os" + "gvisor.dev/gvisor/tools/nogo" ) +var ( + packageFile = flag.String("package", "", "package configuration file (in JSON format)") + stdlibFile = flag.String("stdlib", "", "stdlib configuration file (in JSON format)") + findingsOutput = flag.String("findings", "", "output file (or stdout, if not specified)") + factsOutput = flag.String("facts", "", "output file for facts (optional)") + escapesOutput = flag.String("escapes", "", "output file for escapes (optional)") +) + +func loadConfig(file string, config interface{}) interface{} { + // Load the configuration. + f, err := os.Open(file) + if err != nil { + log.Fatalf("unable to open configuration %q: %v", file, err) + } + defer f.Close() + dec := json.NewDecoder(f) + dec.DisallowUnknownFields() + if err := dec.Decode(config); err != nil { + log.Fatalf("unable to decode configuration: %v", err) + } + return config +} + func main() { - nogo.Main() + // Parse all flags. + flag.Parse() + + var ( + findings []nogo.Finding + factData []byte + err error + ) + + // Check & load the configuration. + if *packageFile != "" && *stdlibFile != "" { + log.Fatalf("unable to perform stdlib and package analysis; provide only one!") + } + + // Run the configuration. + if *stdlibFile != "" { + // Perform basic analysis. + c := loadConfig(*stdlibFile, new(nogo.StdlibConfig)).(*nogo.StdlibConfig) + findings, factData, err = nogo.CheckStdlib(c, nogo.AllAnalyzers) + + } else if *packageFile != "" { + // Perform basic analysis. + c := loadConfig(*packageFile, new(nogo.PackageConfig)).(*nogo.PackageConfig) + findings, factData, err = nogo.CheckPackage(c, nogo.AllAnalyzers, nil) + + // Do we need to do escape analysis? + if *escapesOutput != "" { + escapes, _, err := nogo.CheckPackage(c, nogo.EscapeAnalyzers, nil) + if err != nil { + log.Fatalf("error performing escape analysis: %v", err) + } + if err := nogo.WriteFindingsToFile(escapes, *escapesOutput); err != nil { + log.Fatalf("error writing escapes to %q: %v", *escapesOutput, err) + } + } + } else { + log.Fatalf("please provide at least one of package or stdlib!") + } + + // Check that analysis was successful. + if err != nil { + log.Fatalf("error performing analysis: %v", err) + } + + // Save facts. + if *factsOutput != "" { + if err := ioutil.WriteFile(*factsOutput, factData, 0644); err != nil { + log.Fatalf("error saving findings to %q: %v", *factsOutput, err) + } + } + + // Write all findings. + if *findingsOutput != "" { + if err := nogo.WriteFindingsToFile(findings, *findingsOutput); err != nil { + log.Fatalf("error writing findings to %q: %v", *findingsOutput, err) + } + } else { + for _, finding := range findings { + fmt.Fprintf(os.Stdout, "%s\n", finding.String()) + } + } } diff --git a/tools/nogo/config.go b/tools/nogo/config.go index b591b1509..2fea5b3e1 100644 --- a/tools/nogo/config.go +++ b/tools/nogo/config.go @@ -15,512 +15,247 @@ package nogo import ( - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/asmdecl" - "golang.org/x/tools/go/analysis/passes/assign" - "golang.org/x/tools/go/analysis/passes/atomic" - "golang.org/x/tools/go/analysis/passes/bools" - "golang.org/x/tools/go/analysis/passes/buildtag" - "golang.org/x/tools/go/analysis/passes/cgocall" - "golang.org/x/tools/go/analysis/passes/composite" - "golang.org/x/tools/go/analysis/passes/copylock" - "golang.org/x/tools/go/analysis/passes/errorsas" - "golang.org/x/tools/go/analysis/passes/httpresponse" - "golang.org/x/tools/go/analysis/passes/loopclosure" - "golang.org/x/tools/go/analysis/passes/lostcancel" - "golang.org/x/tools/go/analysis/passes/nilfunc" - "golang.org/x/tools/go/analysis/passes/nilness" - "golang.org/x/tools/go/analysis/passes/printf" - "golang.org/x/tools/go/analysis/passes/shadow" - "golang.org/x/tools/go/analysis/passes/shift" - "golang.org/x/tools/go/analysis/passes/stdmethods" - "golang.org/x/tools/go/analysis/passes/stringintconv" - "golang.org/x/tools/go/analysis/passes/structtag" - "golang.org/x/tools/go/analysis/passes/tests" - "golang.org/x/tools/go/analysis/passes/unmarshal" - "golang.org/x/tools/go/analysis/passes/unreachable" - "golang.org/x/tools/go/analysis/passes/unsafeptr" - "golang.org/x/tools/go/analysis/passes/unusedresult" - "honnef.co/go/tools/staticcheck" - "honnef.co/go/tools/stylecheck" - - "gvisor.dev/gvisor/tools/checkescape" - "gvisor.dev/gvisor/tools/checkunsafe" + "fmt" + "regexp" ) -var analyzerConfig = map[*analysis.Analyzer]matcher{ - // Standard analyzers. - asmdecl.Analyzer: alwaysMatches(), - assign.Analyzer: externalExcluded( - ".*gazelle/walk/walk.go", // False positive. - ), - atomic.Analyzer: alwaysMatches(), - bools.Analyzer: alwaysMatches(), - buildtag.Analyzer: alwaysMatches(), - cgocall.Analyzer: alwaysMatches(), - composite.Analyzer: and( - disableMatches(), // Disabled for now. - resultExcluded{ - "Object_", - "Range{", - }, - ), - copylock.Analyzer: internalMatches(), // Common external issues (e.g. protos). - errorsas.Analyzer: alwaysMatches(), - httpresponse.Analyzer: alwaysMatches(), - loopclosure.Analyzer: alwaysMatches(), - lostcancel.Analyzer: internalMatches(), // Common external issues. - nilfunc.Analyzer: alwaysMatches(), - nilness.Analyzer: and( - internalMatches(), // Common "tautological checks". - internalExcluded( - "pkg/sentry/platform/kvm/kvm_test.go", // Intentional. - "tools/bigquery/bigquery.go", // False positive. - ), - ), - printf.Analyzer: alwaysMatches(), - shift.Analyzer: alwaysMatches(), - stdmethods.Analyzer: internalMatches(), // Common external issues (e.g. methods named "Write"). - stringintconv.Analyzer: and( - internalExcluded(), - externalExcluded( - ".*protobuf/.*.go", // Bad conversions. - ".*flate/huffman_bit_writer.go", // Bad conversion. +// GroupName is a named group. +type GroupName string + +// AnalyzerName is a named analyzer. +type AnalyzerName string + +// Group represents a named collection of files. +type Group struct { + // Name is the short name for the group. + Name GroupName `yaml:"name"` + + // Regex matches all full paths in the group. + Regex string `yaml:"regex"` + regex *regexp.Regexp `yaml:"-"` + + // Default determines the default group behavior. + // + // If Default is true, all Analyzers are enabled for this + // group. Otherwise, Analyzers must be individually enabled + // by specifying a (possible empty) ItemConfig for the group + // in the AnalyzerConfig. + Default bool `yaml:"default"` +} + +func (g *Group) compile() error { + r, err := regexp.Compile(g.Regex) + if err != nil { + return err + } + g.regex = r + return nil +} + +// ItemConfig is an (Analyzer,Group) configuration. +type ItemConfig struct { + // Exclude are analyzer exclusions. + // + // Exclude is a list of regular expressions. If the corresponding + // Analyzer emits a Finding for which Finding.Position.String() + // matches a regular expression in Exclude, the finding will not + // be reported. + Exclude []string `yaml:"exclude,omitempty"` + exclude []*regexp.Regexp `yaml:"-"` + + // Suppress are analyzer suppressions. + // + // Suppress is a list of regular expressions. If the corresponding + // Analyzer emits a Finding for which Finding.Message matches a regular + // expression in Suppress, the finding will not be reported. + Suppress []string `yaml:"suppress,omitempty"` + suppress []*regexp.Regexp `yaml:"-"` +} + +func compileRegexps(ss []string, rs *[]*regexp.Regexp) error { + *rs = make([]*regexp.Regexp, 0, len(ss)) + for _, s := range ss { + r, err := regexp.Compile(s) + if err != nil { + return err + } + *rs = append(*rs, r) + } + return nil +} + +func (i *ItemConfig) compile() error { + if i == nil { + // This may be nil if nothing is included in the + // item configuration. That's fine, there's nothing + // to compile and nothing to exclude & suppress. + return nil + } + if err := compileRegexps(i.Exclude, &i.exclude); err != nil { + return fmt.Errorf("in exclude: %w", err) + } + if err := compileRegexps(i.Suppress, &i.suppress); err != nil { + return fmt.Errorf("in suppress: %w", err) + } + return nil +} + +func (i *ItemConfig) merge(other *ItemConfig) { + i.Exclude = append(i.Exclude, other.Exclude...) + i.Suppress = append(i.Suppress, other.Suppress...) +} + +func (i *ItemConfig) shouldReport(fullPos, msg string) bool { + if i == nil { + // See above. + return true + } + for _, r := range i.exclude { + if r.MatchString(fullPos) { + return false + } + } + for _, r := range i.suppress { + if r.MatchString(msg) { + return false + } + } + return true +} + +// AnalyzerConfig is the configuration for a single analyzers. +// +// This map is keyed by individual Group names, to allow for different +// configurations depending on what Group the file belongs to. +type AnalyzerConfig map[GroupName]*ItemConfig + +func (a AnalyzerConfig) compile() error { + for name, gc := range a { + if err := gc.compile(); err != nil { + return fmt.Errorf("invalid group %q: %v", name, err) + } + } + return nil +} + +func (a AnalyzerConfig) merge(other AnalyzerConfig) { + // Merge all the groups. + for name, gc := range other { + old, ok := a[name] + if !ok || old == nil { + a[name] = gc // Not configured in a. + continue + } + old.merge(gc) + } +} + +func (a AnalyzerConfig) shouldReport(groupConfig *Group, fullPos, msg string) bool { + gc, ok := a[groupConfig.Name] + if !ok { + return groupConfig.Default + } + + // Note that if a section appears for a particular group + // for a particular analyzer, then it will now be enabled, + // and the group default no longer applies. + return gc.shouldReport(fullPos, msg) +} + +// Config is a nogo configuration. +type Config struct { + // Prefixes defines a set of regular expressions that + // are standard "prefixes", so that files can be grouped + // and specific rules applied to individual groups. + Groups []Group `yaml:"groups"` - // Runtime internal violations. - ".*reflect/value.go", - ".*encoding/xml/xml.go", - ".*runtime/pprof/internal/profile/proto.go", - ".*fmt/scan.go", - ".*go/types/conversions.go", - ".*golang.org/x/net/dns/dnsmessage/message.go", - ), - ), - shadow.Analyzer: disableMatches(), // Disabled for now. - structtag.Analyzer: internalMatches(), // External not subject to rules. - tests.Analyzer: alwaysMatches(), - unmarshal.Analyzer: alwaysMatches(), - unreachable.Analyzer: internalMatches(), - unsafeptr.Analyzer: and( - internalMatches(), - internalExcluded( - ".*_test.go", // Exclude tests. - "pkg/flipcall/.*_unsafe.go", // Special case. - "pkg/gohacks/gohacks_unsafe.go", // Special case. - "pkg/sentry/fs/fsutil/host_file_mapper_unsafe.go", // Special case. - "pkg/sentry/platform/kvm/bluepill_unsafe.go", // Special case. - "pkg/sentry/platform/kvm/machine_unsafe.go", // Special case. - "pkg/sentry/platform/ring0/pagetables/allocator_unsafe.go", // Special case. - "pkg/sentry/platform/safecopy/safecopy_unsafe.go", // Special case. - "pkg/sentry/vfs/mount_unsafe.go", // Special case. - "pkg/sentry/platform/systrap/stub_unsafe.go", // Special case. - "pkg/sentry/platform/systrap/switchto_google_unsafe.go", // Special case. - "pkg/sentry/platform/systrap/sysmsg_thread_unsafe.go", // Special case. - "pkg/state/decode_unsafe.go", // Special case. - ), - ), - unusedresult.Analyzer: alwaysMatches(), + // Global is the global analyzer config. + Global AnalyzerConfig `yaml:"global"` - // Internal analyzers: external packages not subject. - checkescape.Analyzer: internalMatches(), - checkunsafe.Analyzer: internalMatches(), + // Analyzers are individual analyzer configurations. The + // key for each analyzer is the name of the analyzer. The + // value is either a boolean (enable/disable), or a map to + // the groups above. + Analyzers map[AnalyzerName]AnalyzerConfig `yaml:"analyzers"` } -func init() { - staticMatcher := and( - // Only match internal, non-generated files. - internalMatches(), - generatedExcluded(), +// Merge merges two configurations. +func (c *Config) Merge(other *Config) { + // Merge all groups. + for _, g := range other.Groups { + // Is there a matching group? If yes, we just delete + // it. This will preserve the order provided in the + // overriding file, even if it differs. + for i := 0; i < len(c.Groups); i++ { + if g.Name == c.Groups[i].Name { + copy(c.Groups[i:], c.Groups[i+1:]) + c.Groups = c.Groups[:len(c.Groups)-1] + break + } + } + c.Groups = append(c.Groups, g) + } - // We use ALL_CAPS for system definitions, - // which are common enough in the code base - // that we shouldn't annotate exceptions. - // - // Same story for underscores. - resultExcluded([]string{ - "should not use ALL_CAPS in Go names", - "should not use underscores in Go names", - }), + // Merge global configurations. + c.Global.merge(other.Global) - // Exclude existing matches. - internalExcluded( - "pkg/abi/linux/fuse.go:22", - "pkg/abi/linux/fuse.go:25", - "pkg/abi/linux/socket.go:113", - "pkg/abi/linux/tty.go:73", - "pkg/bpf/decoder.go:112", - "pkg/cpuid/cpuid_x86.go:675", - "pkg/eventchannel/event.go:193", - "pkg/eventchannel/event.go:27", - "pkg/eventchannel/event_test.go:22", - "pkg/eventchannel/rate.go:19", - "pkg/gohacks/gohacks_unsafe.go:33", - "pkg/log/json.go:30", - "pkg/log/log.go:359", - "pkg/merkletree/merkletree.go:230", - "pkg/merkletree/merkletree.go:243", - "pkg/merkletree/merkletree.go:249", - "pkg/merkletree/merkletree.go:266", - "pkg/merkletree/merkletree.go:355", - "pkg/merkletree/merkletree.go:369", - "pkg/metric/metric_test.go:20", - "pkg/p9/p9test/client_test.go:687", - "pkg/p9/transport_test.go:196", - "pkg/pool/pool.go:15", - "pkg/refs/refcounter.go:510", - "pkg/refs/refcounter_test.go:169", - "pkg/safemem/block_unsafe.go:89", - "pkg/seccomp/seccomp.go:82", - "pkg/segment/test/set_functions.go:15", - "pkg/sentry/arch/signal.go:166", - "pkg/sentry/arch/signal.go:171", - "pkg/sentry/control/pprof.go:196", - "pkg/sentry/devices/memdev/full.go:58", - "pkg/sentry/devices/memdev/null.go:59", - "pkg/sentry/devices/memdev/random.go:68", - "pkg/sentry/devices/memdev/zero.go:86", - "pkg/sentry/fdimport/fdimport.go:15", - "pkg/sentry/fs/attr.go:257", - "pkg/sentry/fsbridge/fs.go:116", - "pkg/sentry/fsbridge/vfs.go:124", - "pkg/sentry/fsbridge/vfs.go:70", - "pkg/sentry/fs/copy_up.go:365", - "pkg/sentry/fs/copy_up_test.go:65", - "pkg/sentry/fs/dev/net_tun.go:161", - "pkg/sentry/fs/dev/net_tun.go:63", - "pkg/sentry/fs/dev/null.go:97", - "pkg/sentry/fs/dirent_cache.go:64", - "pkg/sentry/fs/file_overlay.go:327", - "pkg/sentry/fs/file_overlay.go:524", - "pkg/sentry/fs/filetest/filetest.go:55", - "pkg/sentry/fs/filetest/filetest.go:60", - "pkg/sentry/fs/fs.go:77", - "pkg/sentry/fs/fsutil/file.go:290", - "pkg/sentry/fs/fsutil/file.go:346", - "pkg/sentry/fs/fsutil/host_file_mapper.go:105", - "pkg/sentry/fs/fsutil/inode_cached.go:676", - "pkg/sentry/fs/fsutil/inode_cached.go:772", - "pkg/sentry/fs/gofer/attr.go:120", - "pkg/sentry/fs/gofer/fifo.go:33", - "pkg/sentry/fs/gofer/inode.go:410", - "pkg/sentry/fsimpl/devpts/devpts.go:110", - "pkg/sentry/fsimpl/devpts/devpts.go:246", - "pkg/sentry/fsimpl/devpts/devpts.go:50", - "pkg/sentry/fsimpl/devpts/master.go:110", - "pkg/sentry/fsimpl/devpts/master.go:55", - "pkg/sentry/fsimpl/devpts/replica.go:113", - "pkg/sentry/fsimpl/devpts/replica.go:57", - "pkg/sentry/fsimpl/devtmpfs/devtmpfs.go:54", - "pkg/sentry/fsimpl/ext/disklayout/superblock_64.go:97", - "pkg/sentry/fsimpl/ext/disklayout/superblock_old.go:92", - "pkg/sentry/fsimpl/ext/disklayout/block_group_32.go:44", - "pkg/sentry/fsimpl/ext/disklayout/inode_new.go:91", - "pkg/sentry/fsimpl/ext/disklayout/inode_old.go:93", - "pkg/sentry/fsimpl/ext/disklayout/superblock_32.go:66", - "pkg/sentry/fsimpl/ext/disklayout/block_group_64.go:53", - "pkg/sentry/fsimpl/eventfd/eventfd.go:268", - "pkg/sentry/fsimpl/ext/directory.go:163", - "pkg/sentry/fsimpl/ext/directory.go:164", - "pkg/sentry/fsimpl/ext/extent_file.go:142", - "pkg/sentry/fsimpl/ext/extent_file.go:143", - "pkg/sentry/fsimpl/ext/ext.go:105", - "pkg/sentry/fsimpl/ext/filesystem.go:287", - "pkg/sentry/fsimpl/ext/regular_file.go:153", - "pkg/sentry/fsimpl/ext/symlink.go:113", - "pkg/sentry/fsimpl/fuse/connection_control.go:194", - "pkg/sentry/fsimpl/fuse/dev.go:387", - "pkg/sentry/fsimpl/fuse/dev_test.go:318", - "pkg/sentry/fsimpl/fuse/fusefs.go:102", - "pkg/sentry/fsimpl/fuse/read_write.go:129", - "pkg/sentry/fsimpl/fuse/request_response.go:71", - "pkg/sentry/fsimpl/gofer/directory.go:135", - "pkg/sentry/fsimpl/gofer/filesystem.go:679", - "pkg/sentry/fsimpl/gofer/gofer.go:1694", - "pkg/sentry/fsimpl/gofer/gofer.go:276", - "pkg/sentry/fsimpl/gofer/regular_file.go:81", - "pkg/sentry/fsimpl/gofer/special_file.go:141", - "pkg/sentry/fsimpl/host/host.go:184", - "pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go:50", - "pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go:90", - "pkg/sentry/fsimpl/kernfs/fd_impl_util.go:273", - "pkg/sentry/fsimpl/kernfs/filesystem.go:247", - "pkg/sentry/fsimpl/kernfs/inode_impl_util.go:320", - "pkg/sentry/fsimpl/kernfs/inode_impl_util.go:497", - "pkg/sentry/fsimpl/kernfs/synthetic_directory.go:52", - "pkg/sentry/fsimpl/overlay/directory.go:119", - "pkg/sentry/fsimpl/overlay/filesystem.go:527", - "pkg/sentry/fsimpl/overlay/non_directory.go:152", - "pkg/sentry/fsimpl/overlay/overlay.go:115", - "pkg/sentry/fsimpl/overlay/overlay.go:719", - "pkg/sentry/fsimpl/pipefs/pipefs.go:74", - "pkg/sentry/fsimpl/proc/filesystem.go:52", - "pkg/sentry/fsimpl/proc/filesystem.go:81", - "pkg/sentry/fsimpl/proc/subtasks.go:126", - "pkg/sentry/fsimpl/proc/subtasks.go:189", - "pkg/sentry/fsimpl/proc/task_fds.go:168", - "pkg/sentry/fsimpl/proc/task_fds.go:228", - "pkg/sentry/fsimpl/proc/task_fds.go:301", - "pkg/sentry/fsimpl/proc/task_fds.go:318", - "pkg/sentry/fsimpl/proc/task_fds.go:67", - "pkg/sentry/fsimpl/proc/task_files.go:112", - "pkg/sentry/fsimpl/proc/task_files.go:158", - "pkg/sentry/fsimpl/proc/task_files.go:259", - "pkg/sentry/fsimpl/proc/task_files.go:285", - "pkg/sentry/fsimpl/proc/task_files.go:305", - "pkg/sentry/fsimpl/proc/task_files.go:384", - "pkg/sentry/fsimpl/proc/task_files.go:403", - "pkg/sentry/fsimpl/proc/task_files.go:428", - "pkg/sentry/fsimpl/proc/task_files.go:691", - "pkg/sentry/fsimpl/proc/task_files.go:770", - "pkg/sentry/fsimpl/proc/task_files.go:797", - "pkg/sentry/fsimpl/proc/task_files.go:828", - "pkg/sentry/fsimpl/proc/task_files.go:879", - "pkg/sentry/fsimpl/proc/task_files.go:910", - "pkg/sentry/fsimpl/proc/task_files.go:961", - "pkg/sentry/fsimpl/proc/task.go:127", - "pkg/sentry/fsimpl/proc/task.go:193", - "pkg/sentry/fsimpl/proc/task_net.go:134", - "pkg/sentry/fsimpl/proc/task_net.go:475", - "pkg/sentry/fsimpl/proc/task_net.go:491", - "pkg/sentry/fsimpl/proc/task_net.go:508", - "pkg/sentry/fsimpl/proc/task_net.go:665", - "pkg/sentry/fsimpl/proc/task_net.go:715", - "pkg/sentry/fsimpl/proc/task_net.go:779", - "pkg/sentry/fsimpl/proc/tasks_files.go:113", - "pkg/sentry/fsimpl/proc/tasks_files.go:388", - "pkg/sentry/fsimpl/proc/tasks.go:232", - "pkg/sentry/fsimpl/proc/tasks_sys.go:145", - "pkg/sentry/fsimpl/proc/tasks_sys.go:181", - "pkg/sentry/fsimpl/proc/tasks_sys.go:239", - "pkg/sentry/fsimpl/proc/tasks_sys.go:291", - "pkg/sentry/fsimpl/proc/tasks_sys.go:375", - "pkg/sentry/fsimpl/signalfd/signalfd.go:124", - "pkg/sentry/fsimpl/signalfd/signalfd.go:15", - "pkg/sentry/fsimpl/signalfd/signalfd.go:126", - "pkg/sentry/fsimpl/sockfs/sockfs.go:36", - "pkg/sentry/fsimpl/sockfs/sockfs.go:79", - "pkg/sentry/fsimpl/sys/kcov.go:49", - "pkg/sentry/fsimpl/sys/kcov.go:99", - "pkg/sentry/fsimpl/sys/sys.go:118", - "pkg/sentry/fsimpl/sys/sys.go:56", - "pkg/sentry/fsimpl/testutil/testutil.go:257", - "pkg/sentry/fsimpl/testutil/testutil.go:260", - "pkg/sentry/fsimpl/timerfd/timerfd.go:87", - "pkg/sentry/fsimpl/tmpfs/directory.go:112", - "pkg/sentry/fsimpl/tmpfs/filesystem.go:195", - "pkg/sentry/fsimpl/tmpfs/regular_file.go:226", - "pkg/sentry/fsimpl/tmpfs/regular_file.go:346", - "pkg/sentry/fsimpl/tmpfs/tmpfs.go:103", - "pkg/sentry/fsimpl/tmpfs/tmpfs.go:733", - "pkg/sentry/fsimpl/verity/filesystem.go:490", - "pkg/sentry/fsimpl/verity/verity.go:156", - "pkg/sentry/fsimpl/verity/verity.go:629", - "pkg/sentry/fsimpl/verity/verity.go:672", - "pkg/sentry/fs/mount.go:162", - "pkg/sentry/fs/mount.go:256", - "pkg/sentry/fs/mount_overlay.go:144", - "pkg/sentry/fs/mounts.go:432", - "pkg/sentry/fs/proc/exec_args.go:104", - "pkg/sentry/fs/proc/exec_args.go:73", - "pkg/sentry/fs/proc/fds.go:269", - "pkg/sentry/fs/proc/loadavg.go:33", - "pkg/sentry/fs/proc/meminfo.go:39", - "pkg/sentry/fs/proc/mounts.go:193", - "pkg/sentry/fs/proc/mounts.go:84", - "pkg/sentry/fs/proc/net.go:125", - "pkg/sentry/fs/proc/proc.go:146", - "pkg/sentry/fs/proc/proc.go:204", - "pkg/sentry/fs/proc/seqfile/seqfile.go:210", - "pkg/sentry/fs/proc/sys.go:146", - "pkg/sentry/fs/proc/sys.go:43", - "pkg/sentry/fs/proc/sys_net.go:113", - "pkg/sentry/fs/proc/sys_net.go:205", - "pkg/sentry/fs/proc/sys_net.go:233", - "pkg/sentry/fs/proc/sys_net.go:307", - "pkg/sentry/fs/proc/sys_net.go:335", - "pkg/sentry/fs/proc/sys_net.go:446", - "pkg/sentry/fs/proc/sys_net.go:456", - "pkg/sentry/fs/proc/sys_net.go:89", - "pkg/sentry/fs/proc/task.go:170", - "pkg/sentry/fs/proc/task.go:322", - "pkg/sentry/fs/proc/task.go:427", - "pkg/sentry/fs/proc/task.go:467", - "pkg/sentry/fs/proc/task.go:500", - "pkg/sentry/fs/proc/task.go:784", - "pkg/sentry/fs/proc/task.go:839", - "pkg/sentry/fs/proc/task.go:920", - "pkg/sentry/fs/proc/uid_gid_map.go:108", - "pkg/sentry/fs/proc/uid_gid_map.go:79", - "pkg/sentry/fs/proc/uptime.go:75", - "pkg/sentry/fs/ramfs/dir.go:447", - "pkg/sentry/fs/tmpfs/inode_file.go:436", - "pkg/sentry/fs/tmpfs/inode_file.go:537", - "pkg/sentry/fs/tty/dir.go:313", - "pkg/sentry/fs/tty/master.go:131", - "pkg/sentry/fs/tty/master.go:91", - "pkg/sentry/fs/tty/replica.go:116", - "pkg/sentry/fs/tty/replica.go:88", - "pkg/sentry/kernel/auth/id_map.go:269", - "pkg/sentry/kernel/fasync/fasync.go:67", - "pkg/sentry/kernel/kcov.go:209", - "pkg/sentry/kernel/kcov.go:223", - "pkg/sentry/kernel/kernel.go:343", - "pkg/sentry/kernel/kernel.go:368", - "pkg/sentry/kernel/pipe/node_test.go:112", - "pkg/sentry/kernel/pipe/node_test.go:119", - "pkg/sentry/kernel/pipe/node_test.go:130", - "pkg/sentry/kernel/pipe/node_test.go:137", - "pkg/sentry/kernel/pipe/node_test.go:149", - "pkg/sentry/kernel/pipe/node_test.go:150", - "pkg/sentry/kernel/pipe/node_test.go:158", - "pkg/sentry/kernel/pipe/node_test.go:174", - "pkg/sentry/kernel/pipe/node_test.go:180", - "pkg/sentry/kernel/pipe/node_test.go:193", - "pkg/sentry/kernel/pipe/node_test.go:202", - "pkg/sentry/kernel/pipe/node_test.go:205", - "pkg/sentry/kernel/pipe/node_test.go:216", - "pkg/sentry/kernel/pipe/node_test.go:219", - "pkg/sentry/kernel/pipe/node_test.go:271", - "pkg/sentry/kernel/pipe/node_test.go:290", - "pkg/sentry/kernel/pipe/pipe_test.go:93", - "pkg/sentry/kernel/pipe/reader_writer.go:65", - "pkg/sentry/kernel/posixtimer.go:157", - "pkg/sentry/kernel/ptrace.go:218", - "pkg/sentry/kernel/semaphore/semaphore.go:323", - "pkg/sentry/kernel/sessions.go:123", - "pkg/sentry/kernel/sessions.go:508", - "pkg/sentry/kernel/signal_handlers.go:57", - "pkg/sentry/kernel/task_context.go:72", - "pkg/sentry/kernel/task_exit.go:67", - "pkg/sentry/kernel/task_sched.go:255", - "pkg/sentry/kernel/task_sched.go:280", - "pkg/sentry/kernel/task_sched.go:323", - "pkg/sentry/kernel/task_stop.go:192", - "pkg/sentry/kernel/thread_group.go:530", - "pkg/sentry/kernel/timekeeper.go:316", - "pkg/sentry/kernel/vdso.go:106", - "pkg/sentry/kernel/vdso.go:118", - "pkg/sentry/memmap/memmap.go:103", - "pkg/sentry/memmap/memmap.go:163", - "pkg/sentry/mm/address_space.go:42", - "pkg/sentry/mm/address_space.go:42", - "pkg/sentry/mm/aio_context.go:208", - "pkg/sentry/mm/aio_context.go:288", - "pkg/sentry/mm/pma.go:683", - "pkg/sentry/mm/special_mappable.go:80", - "pkg/sentry/platform/systrap/subprocess.go:370", - "pkg/sentry/platform/systrap/usertrap/usertrap_amd64.go:124", - "pkg/sentry/syscalls/linux/sys_sem.go:62", - "pkg/sentry/syscalls/linux/sys_time.go:189", - "pkg/sentry/usage/cpu.go:42", - "pkg/sentry/vfs/anonfs.go:302", - "pkg/sentry/vfs/anonfs.go:99", - "pkg/sentry/vfs/dentry.go:214", - "pkg/sentry/vfs/epoll.go:168", - "pkg/sentry/vfs/epoll.go:314", - "pkg/sentry/vfs/file_description.go:549", - "pkg/sentry/vfs/file_description_impl_util.go:304", - "pkg/sentry/vfs/file_description_impl_util.go:412", - "pkg/sentry/vfs/filesystem.go:76", - "pkg/sentry/vfs/lock.go:15", - "pkg/sentry/vfs/lock.go:47", - "pkg/sentry/vfs/memxattr/xattr.go:37", - "pkg/sentry/vfs/mount.go:510", - "pkg/sentry/vfs/mount.go:667", - "pkg/sentry/vfs/mount_test.go:106", - "pkg/sentry/vfs/mount_test.go:160", - "pkg/sentry/vfs/mount_test.go:215", - "pkg/sentry/vfs/mount_unsafe.go:153", - "pkg/sentry/vfs/resolving_path.go:228", - "pkg/sentry/vfs/vfs.go:897", - "pkg/shim/runsc/runsc.go:16", - "pkg/shim/runsc/utils.go:16", - "pkg/shim/v1/proc/deleted_state.go:16", - "pkg/shim/v1/proc/exec.go:16", - "pkg/shim/v1/proc/exec_state.go:16", - "pkg/shim/v1/proc/init.go:16", - "pkg/shim/v1/proc/init_state.go:16", - "pkg/shim/v1/proc/io.go:16", - "pkg/shim/v1/proc/process.go:16", - "pkg/shim/v1/proc/types.go:16", - "pkg/shim/v1/proc/utils.go:16", - "pkg/shim/v1/shim/api.go:16", - "pkg/shim/v1/shim/platform.go:16", - "pkg/shim/v1/shim/service.go:16", - "pkg/shim/v1/utils/annotations.go:15", - "pkg/shim/v1/utils/utils.go:15", - "pkg/shim/v1/utils/volumes.go:15", - "pkg/shim/v2/api.go:16", - "pkg/shim/v2/epoll.go:18", - "pkg/shim/v2/options/options.go:15", - "pkg/shim/v2/options/options.go:24", - "pkg/shim/v2/options/options.go:26", - "pkg/shim/v2/runtimeoptions/runtimeoptions.go:16", - "pkg/shim/v2/runtimeoptions/runtimeoptions_cri.go", // Generated: exempt all. - "pkg/shim/v2/runtimeoptions/runtimeoptions_test.go:22", - "pkg/shim/v2/service.go:15", - "pkg/shim/v2/service_linux.go:18", - "pkg/state/tests/integer_test.go:23", - "pkg/state/tests/integer_test.go:28", - "pkg/sync/rwmutex_test.go:105", - "pkg/syserr/host_linux.go:35", - "pkg/unet/unet_test.go:634", - "pkg/unet/unet_test.go:662", - "pkg/unet/unet_test.go:703", - "pkg/unet/unet_test.go:98", - "pkg/usermem/addr.go:34", - "pkg/usermem/usermem.go:171", - "pkg/usermem/usermem.go:170", - "runsc/boot/compat.go:22", - "runsc/boot/compat.go:56", - "runsc/boot/loader.go:1115", - "runsc/boot/loader.go:1120", - "runsc/cmd/checkpoint.go:151", - "runsc/config/flags.go:32", - "runsc/container/container.go:641", - "runsc/container/container.go:988", - "runsc/specutils/specutils.go:172", - "runsc/specutils/specutils.go:428", - "runsc/specutils/specutils.go:436", - "runsc/specutils/specutils.go:442", - "runsc/specutils/specutils.go:447", - "runsc/specutils/specutils.go:454", - "test/cmd/test_app/fds.go:171", - "test/iptables/filter_output.go:251", - "test/packetimpact/testbench/connections.go:77", - "tools/bigquery/bigquery.go:106", - "tools/checkescape/test1/test1.go:108", - "tools/checkescape/test1/test1.go:122", - "tools/checkescape/test1/test1.go:137", - "tools/checkescape/test1/test1.go:151", - "tools/checkescape/test1/test1.go:170", - "tools/checkescape/test1/test1.go:39", - "tools/checkescape/test1/test1.go:45", - "tools/checkescape/test1/test1.go:50", - "tools/checkescape/test1/test1.go:64", - "tools/checkescape/test1/test1.go:80", - "tools/checkescape/test1/test1.go:94", - "tools/go_generics/imports.go:51", - "tools/go_generics/imports.go:75", - "tools/go_marshal/gomarshal/generator.go:177", - "tools/go_marshal/gomarshal/generator.go:81", - "tools/go_marshal/gomarshal/generator.go:85", - "tools/go_marshal/test/escape/escape.go:15", - "tools/go_marshal/test/test.go:164", - ), - ) + // Merge all analyzer configurations. + for name, ac := range other.Analyzers { + old, ok := c.Analyzers[name] + if !ok { + c.Analyzers[name] = ac // No analyzer in original config. + continue + } + old.merge(ac) + } +} - // Add all staticcheck analyzers; internal only. - for _, a := range staticcheck.Analyzers { - analyzerConfig[a] = staticMatcher +// Compile compiles a configuration to make it useable. +func (c *Config) Compile() error { + for i := 0; i < len(c.Groups); i++ { + if err := c.Groups[i].compile(); err != nil { + return fmt.Errorf("invalid group %q: %w", c.Groups[i].Name, err) + } } - // Add all stylecheck analyzers; internal only. - for _, a := range stylecheck.Analyzers { - analyzerConfig[a] = staticMatcher + if err := c.Global.compile(); err != nil { + return fmt.Errorf("invalid global: %w", err) } + for name, ac := range c.Analyzers { + if err := ac.compile(); err != nil { + return fmt.Errorf("invalid analyzer %q: %w", name, err) + } + } + return nil } -var escapesConfig = map[*analysis.Analyzer]matcher{ - // Informational only: include all packages. - checkescape.EscapeAnalyzer: alwaysMatches(), +// ShouldReport returns true iff the finding should match the Config. +func (c *Config) ShouldReport(finding Finding) bool { + fullPos := finding.Position.String() + + // Find the matching group. + var groupConfig *Group + for i := 0; i < len(c.Groups); i++ { + if c.Groups[i].regex.MatchString(fullPos) { + groupConfig = &c.Groups[i] + break + } + } + + // If there is no group matching this path, then + // we default to accept the finding. + if groupConfig == nil { + return true + } + + // Suppress via global rule? + if !c.Global.shouldReport(groupConfig, fullPos, finding.Message) { + return false + } + + // Try the analyzer config. + ac, ok := c.Analyzers[finding.Category] + if !ok { + return groupConfig.Default + } + return ac.shouldReport(groupConfig, fullPos, finding.Message) } diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl index 543598b52..b3d297308 100644 --- a/tools/nogo/defs.bzl +++ b/tools/nogo/defs.bzl @@ -2,6 +2,28 @@ load("//tools/bazeldefs:go.bzl", "go_context", "go_importpath", "go_rule", "go_test_library") +NogoConfigInfo = provider( + "information about a nogo configuration", + fields = { + "srcs": "the collection of configuration files", + }, +) + +def _nogo_config_impl(ctx): + return [NogoConfigInfo( + srcs = ctx.files.srcs, + )] + +nogo_config = rule( + implementation = _nogo_config_impl, + attrs = { + "srcs": attr.label_list( + doc = "a list of yaml files (schema defined by tool/nogo/config.go).", + allow_files = True, + ), + }, +) + NogoTargetInfo = provider( "information about the Go target", fields = { @@ -20,11 +42,14 @@ nogo_target = go_rule( rule, implementation = _nogo_target_impl, attrs = { - # goarch is the build architecture. This will normally be provided by a - # select statement, but this information is propagated to other rules. - "goarch": attr.string(mandatory = True), - # goos is similarly the build operating system target. - "goos": attr.string(mandatory = True), + "goarch": attr.string( + doc = "the Go build architecture (propagated to other rules).", + mandatory = True, + ), + "goos": attr.string( + doc = "the Go OS target (propagated to other rules).", + mandatory = True, + ), }, ) @@ -81,7 +106,7 @@ NogoStdlibInfo = provider( "information for nogo analysis (standard library facts)", fields = { "facts": "serialized standard library facts", - "findings": "package findings (if relevant)", + "raw_findings": "raw package findings (if relevant)", }, ) @@ -90,7 +115,7 @@ def _nogo_stdlib_impl(ctx): nogo_target_info = ctx.attr._nogo_target[NogoTargetInfo] go_ctx = go_context(ctx, goos = nogo_target_info.goos, goarch = nogo_target_info.goarch) facts = ctx.actions.declare_file(ctx.label.name + ".facts") - findings = ctx.actions.declare_file(ctx.label.name + ".findings") + raw_findings = ctx.actions.declare_file(ctx.label.name + ".raw_findings") config = struct( Srcs = [f.path for f in go_ctx.stdlib_srcs], GOOS = go_ctx.goos, @@ -101,15 +126,15 @@ def _nogo_stdlib_impl(ctx): ctx.actions.write(config_file, config.to_json()) ctx.actions.run( inputs = [config_file] + go_ctx.stdlib_srcs, - outputs = [facts, findings], + outputs = [facts, raw_findings], tools = depset(go_ctx.runfiles.to_list() + ctx.files._nogo_objdump_tool), executable = ctx.files._nogo_check[0], - mnemonic = "GoStandardLibraryAnalysis", + mnemonic = "NogoStandardLibraryAnalysis", progress_message = "Analyzing Go Standard Library", arguments = go_ctx.nogo_args + [ "-objdump_tool=%s" % ctx.files._nogo_objdump_tool[0].path, "-stdlib=%s" % config_file.path, - "-findings=%s" % findings.path, + "-findings=%s" % raw_findings.path, "-facts=%s" % facts.path, ], ) @@ -117,7 +142,7 @@ def _nogo_stdlib_impl(ctx): # Return the stdlib facts as output. return [NogoStdlibInfo( facts = facts, - findings = findings, + raw_findings = raw_findings, )] nogo_stdlib = go_rule( @@ -148,7 +173,8 @@ NogoInfo = provider( "information for nogo analysis", fields = { "facts": "serialized package facts", - "findings": "package findings (if relevant)", + "raw_findings": "raw package findings (if relevant)", + "escapes": "escape-only findings (if relevant)", "importpath": "package import path", "binaries": "package binary files", "srcs": "srcs (for go_test support)", @@ -214,6 +240,7 @@ def _nogo_aspect_impl(target, ctx): # Collect all info from shadow dependencies. fact_map = dict() import_map = dict() + all_raw_findings = [] for dep in deps: # There will be no file attribute set for all transitive dependencies # that are not go_library or go_binary rules, such as a proto rules. @@ -231,6 +258,9 @@ def _nogo_aspect_impl(target, ctx): import_map[info.importpath] = x_files[0] fact_map[info.importpath] = info.facts.path + # Collect all findings; duplicates are resolved at the end. + all_raw_findings.extend(info.raw_findings) + # Ensure the above are available as inputs. inputs.append(info.facts) inputs += info.binaries @@ -244,7 +274,7 @@ def _nogo_aspect_impl(target, ctx): nogo_target_info = ctx.attr._nogo_target[NogoTargetInfo] go_ctx = go_context(ctx, goos = nogo_target_info.goos, goarch = nogo_target_info.goarch) facts = ctx.actions.declare_file(target.label.name + ".facts") - findings = ctx.actions.declare_file(target.label.name + ".findings") + raw_findings = ctx.actions.declare_file(target.label.name + ".raw_findings") escapes = ctx.actions.declare_file(target.label.name + ".escapes") config = struct( ImportPath = importpath, @@ -262,39 +292,39 @@ def _nogo_aspect_impl(target, ctx): inputs.append(config_file) ctx.actions.run( inputs = inputs, - outputs = [facts, findings, escapes], + outputs = [facts, raw_findings, escapes], tools = depset(go_ctx.runfiles.to_list() + ctx.files._nogo_objdump_tool), executable = ctx.files._nogo_check[0], - mnemonic = "GoStaticAnalysis", + mnemonic = "NogoAnalysis", progress_message = "Analyzing %s" % target.label, arguments = go_ctx.nogo_args + [ "-binary=%s" % target_objfile.path, "-objdump_tool=%s" % ctx.files._nogo_objdump_tool[0].path, "-package=%s" % config_file.path, - "-findings=%s" % findings.path, + "-findings=%s" % raw_findings.path, "-facts=%s" % facts.path, "-escapes=%s" % escapes.path, ], ) + # Flatten all findings from all dependencies. + # + # This is done because all the filtering must be done at the + # top-level nogo_test to dynamically apply a configuration. + # This does not actually add any additional work here, but + # will simply propagate the full list of files. + all_raw_findings = [stdlib_info.raw_findings] + depset(all_raw_findings).to_list() + [raw_findings] + # Return the package facts as output. - return [ - NogoInfo( - facts = facts, - findings = findings, - importpath = importpath, - binaries = binaries, - srcs = srcs, - deps = deps, - ), - OutputGroupInfo( - # Expose all findings (should just be a single file). This can be - # used for build analysis of the nogo findings. - nogo_findings = depset([findings]), - # Expose all escape analysis findings (see above). - nogo_escapes = depset([escapes]), - ), - ] + return [NogoInfo( + facts = facts, + raw_findings = all_raw_findings, + escapes = escapes, + importpath = importpath, + binaries = binaries, + srcs = srcs, + deps = deps, + )] nogo_aspect = go_rule( aspect, @@ -327,41 +357,72 @@ nogo_aspect = go_rule( def _nogo_test_impl(ctx): """Check nogo findings.""" - # Build a runner that checks the facts files. - findings = [dep[NogoInfo].findings for dep in ctx.attr.deps] - runner = ctx.actions.declare_file(ctx.label.name) + # Ensure there's a single dependency. + if len(ctx.attr.deps) != 1: + fail("nogo_test requires exactly one dep.") + raw_findings = ctx.attr.deps[0][NogoInfo].raw_findings + escapes = ctx.attr.deps[0][NogoInfo].escapes + + # Build a step that applies the configuration. + config_srcs = ctx.attr.config[NogoConfigInfo].srcs + findings = ctx.actions.declare_file(ctx.label.name + ".findings") ctx.actions.run( - inputs = findings + ctx.files.srcs, - outputs = [runner], - tools = depset(ctx.files._gentest), - executable = ctx.files._gentest[0], - mnemonic = "Gentest", + inputs = raw_findings + ctx.files.srcs + config_srcs, + outputs = [findings], + tools = depset(ctx.files._filter), + executable = ctx.files._filter[0], + mnemonic = "GoStaticAnalysis", progress_message = "Generating %s" % ctx.label, - arguments = [runner.path] + [f.path for f in findings], + arguments = ["-input=%s" % f.path for f in raw_findings] + + ["-config=%s" % f.path for f in config_srcs] + + ["-output=%s" % findings.path], ) + + # Build a runner that checks the filtered facts. + # + # Note that this calls the filter binary without any configuration, so all + # findings will be included. But this is expected, since we've already + # filtered out everything that should not be included. + runner = ctx.actions.declare_file(ctx.label.name) + runner_content = [ + "#!/bin/bash", + "exec %s -input=%s" % (ctx.files._filter[0].short_path, findings.short_path), + "", + ] + ctx.actions.write(runner, "\n".join(runner_content), is_executable = True) + return [DefaultInfo( + # The runner just executes the filter again, on the + # newly generated filtered findings. We still need + # the filter tool as part of our runfiles, however. + runfiles = ctx.runfiles(files = ctx.files._filter + [findings]), executable = runner, + ), OutputGroupInfo( + # Propagate the filtered filters, for consumption by + # build tooling. Note that the build tooling typically + # pays attention to the mnemoic above, so this must be + # what is expected by the tooling. + nogo_findings = depset([findings]), + # Expose all escape analysis findings (see above). + nogo_escapes = depset([escapes]), )] -_nogo_test = rule( +nogo_test = rule( implementation = _nogo_test_impl, attrs = { - # deps should have only a single element. - "deps": attr.label_list(aspects = [nogo_aspect]), - # srcs exist here only to ensure that this target is - # directly affected by changes to the source files. - "srcs": attr.label_list(allow_files = True), - "_gentest": attr.label(default = "//tools/nogo:gentest"), + "config": attr.label( + mandatory = True, + doc = "A rule of kind nogo_config.", + ), + "deps": attr.label_list( + aspects = [nogo_aspect], + doc = "Exactly one Go dependency to be analyzed.", + ), + "srcs": attr.label_list( + allow_files = True, + doc = "Relevant src files. This is ignored except to make the nogo_test directly affected by the files.", + ), + "_filter": attr.label(default = "//tools/nogo/filter:filter"), }, test = True, ) - -def nogo_test(name, srcs, library, **kwargs): - tags = kwargs.pop("tags", []) + ["nogo"] - _nogo_test( - name = name, - srcs = srcs, - deps = [library], - tags = tags, - **kwargs - ) diff --git a/tools/nogo/filter/BUILD b/tools/nogo/filter/BUILD new file mode 100644 index 000000000..e56a783e2 --- /dev/null +++ b/tools/nogo/filter/BUILD @@ -0,0 +1,14 @@ +load("//tools:defs.bzl", "go_binary") + +package(licenses = ["notice"]) + +go_binary( + name = "filter", + srcs = ["main.go"], + nogo = False, + visibility = ["//visibility:public"], + deps = [ + "//tools/nogo", + "@in_gopkg_yaml_v2//:go_default_library", + ], +) diff --git a/tools/nogo/filter/main.go b/tools/nogo/filter/main.go new file mode 100644 index 000000000..9cf41b3b0 --- /dev/null +++ b/tools/nogo/filter/main.go @@ -0,0 +1,131 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Binary check is the nogo entrypoint. +package main + +import ( + "flag" + "fmt" + "io/ioutil" + "log" + "os" + "strings" + + yaml "gopkg.in/yaml.v2" + "gvisor.dev/gvisor/tools/nogo" +) + +type stringList []string + +func (s *stringList) String() string { + return strings.Join(*s, ",") +} + +func (s *stringList) Set(value string) error { + *s = append(*s, value) + return nil +} + +var ( + inputFiles stringList + configFiles stringList + outputFile string + showConfig bool +) + +func init() { + flag.Var(&inputFiles, "input", "findings input files") + flag.StringVar(&outputFile, "output", "", "findings output file") + flag.Var(&configFiles, "config", "findings configuration files") + flag.BoolVar(&showConfig, "show-config", false, "dump configuration only") +} + +func main() { + flag.Parse() + + // Load all available findings. + var findings []nogo.Finding + for _, filename := range inputFiles { + inputFindings, err := nogo.ExtractFindingsFromFile(filename) + if err != nil { + log.Fatalf("unable to extract findings from %s: %v", filename, err) + } + findings = append(findings, inputFindings...) + } + + // Open and merge all configuations. + config := &nogo.Config{ + Global: make(nogo.AnalyzerConfig), + Analyzers: make(map[nogo.AnalyzerName]nogo.AnalyzerConfig), + } + for _, filename := range configFiles { + content, err := ioutil.ReadFile(filename) + if err != nil { + log.Fatalf("unable to read %s: %v", filename, err) + } + var newConfig nogo.Config // For current file. + if err := yaml.Unmarshal(content, &newConfig); err != nil { + log.Fatalf("unable to decode %s: %v", filename, err) + } + config.Merge(&newConfig) + if showConfig { + bytes, err := yaml.Marshal(&newConfig) + if err != nil { + log.Fatalf("error marshalling config: %v", err) + } + mergedBytes, err := yaml.Marshal(config) + if err != nil { + log.Fatalf("error marshalling config: %v", err) + } + fmt.Fprintf(os.Stdout, "Loaded configuration from %s:\n%s\n", filename, string(bytes)) + fmt.Fprintf(os.Stdout, "Merged configuration:\n%s\n", string(mergedBytes)) + } + } + if err := config.Compile(); err != nil { + log.Fatalf("error compiling config: %v", err) + } + if showConfig { + os.Exit(0) + } + + // Filter the findings (and aggregate by group). + filteredFindings := make([]nogo.Finding, 0, len(findings)) + for _, finding := range findings { + if ok := config.ShouldReport(finding); ok { + filteredFindings = append(filteredFindings, finding) + } + } + + // Write the output (if required). + // + // If the outputFile is specified, then we exit here. Otherwise, + // we continue to write to stdout and treat like a test. + if outputFile != "" { + if err := nogo.WriteFindingsToFile(filteredFindings, outputFile); err != nil { + log.Fatalf("unable to write findings: %v", err) + } + return + } + + // Treat the run as a test. + if len(filteredFindings) == 0 { + fmt.Fprintf(os.Stdout, "PASS\n") + os.Exit(0) + } + for _, finding := range filteredFindings { + fmt.Fprintf(os.Stdout, "%s\n", finding.String()) + } + os.Exit(1) +} diff --git a/tools/nogo/findings.go b/tools/nogo/findings.go new file mode 100644 index 000000000..5bd850269 --- /dev/null +++ b/tools/nogo/findings.go @@ -0,0 +1,63 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package nogo + +import ( + "encoding/json" + "fmt" + "go/token" + "io/ioutil" +) + +// Finding is a single finding. +type Finding struct { + Category AnalyzerName + Position token.Position + Message string +} + +// String implements fmt.Stringer.String. +func (f *Finding) String() string { + return fmt.Sprintf("%s: %s: %s", f.Category, f.Position.String(), f.Message) +} + +// WriteFindingsToFile writes findings to a file. +func WriteFindingsToFile(findings []Finding, filename string) error { + content, err := WriteFindingsToBytes(findings) + if err != nil { + return err + } + return ioutil.WriteFile(filename, content, 0644) +} + +// WriteFindingsToBytes serializes findings as bytes. +func WriteFindingsToBytes(findings []Finding) ([]byte, error) { + return json.Marshal(findings) +} + +// ExtractFindingsFromFile loads findings from a file. +func ExtractFindingsFromFile(filename string) ([]Finding, error) { + content, err := ioutil.ReadFile(filename) + if err != nil { + return nil, err + } + return ExtractFindingsFromBytes(content) +} + +// ExtractFindingsFromBytes loads findings from bytes. +func ExtractFindingsFromBytes(content []byte) (findings []Finding, err error) { + err = json.Unmarshal(content, &findings) + return findings, err +} diff --git a/tools/nogo/gentest.sh b/tools/nogo/gentest.sh deleted file mode 100755 index 0a762f9f6..000000000 --- a/tools/nogo/gentest.sh +++ /dev/null @@ -1,48 +0,0 @@ -#!/bin/bash -# Copyright 2019 The gVisor Authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -set -euo pipefail - -if [[ "$#" -lt 2 ]]; then - echo "usage: $0 <output> <findings...>" - exit 2 -fi -declare violations=0 -declare output=$1 -shift - -# Start the script. -echo "#!/bin/sh" > "${output}" - -# Read a list of findings files. -declare filename -declare line -for filename in "$@"; do - if [[ -z "${filename}" ]]; then - continue - fi - while read -r line; do - line="${line@Q}" - violations=$((${violations}+1)); - echo "echo -e '\\033[0;31m${line}\\033[0;31m\\033[0m'" >> "${output}" - done < "${filename}" -done - -# Show violations. -if [[ "${violations}" -eq 0 ]]; then - echo "echo -e '\\033[0;32mPASS\\033[0;31m\\033[0m'" >> "${output}" -else - echo "exit 1" >> "${output}" -fi diff --git a/tools/nogo/matchers.go b/tools/nogo/matchers.go deleted file mode 100644 index b7b73fa27..000000000 --- a/tools/nogo/matchers.go +++ /dev/null @@ -1,172 +0,0 @@ -// Copyright 2019 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package nogo - -import ( - "go/token" - "regexp" - "strings" - - "golang.org/x/tools/go/analysis" -) - -type matcher interface { - ShouldReport(d analysis.Diagnostic, fs *token.FileSet) bool -} - -// pathRegexps filters explicit paths. -type pathRegexps struct { - expr []*regexp.Regexp - - // include, if true, indicates that paths matching any regexp in expr - // match. - // - // If false, paths matching no regexps in expr match. - include bool -} - -// buildRegexps builds a list of regular expressions. -// -// This will panic on error. -func buildRegexps(prefix string, args ...string) []*regexp.Regexp { - result := make([]*regexp.Regexp, 0, len(args)) - for _, arg := range args { - result = append(result, regexp.MustCompile(prefix+arg)) - } - return result -} - -// notPath works around the lack of backtracking. -// -// It is used to construct a regular expression for non-matching components. -func notPath(name string) string { - sb := strings.Builder{} - sb.WriteString("(") - for i := range name { - if i > 0 { - sb.WriteString("|") - } - sb.WriteString(name[:i]) - sb.WriteString("[^") - sb.WriteByte(name[i]) - sb.WriteString("/][^/]*") - } - sb.WriteString(")") - return sb.String() -} - -// ShouldReport implements matcher.ShouldReport. -func (p *pathRegexps) ShouldReport(d analysis.Diagnostic, fs *token.FileSet) bool { - fullPos := fs.Position(d.Pos).String() - for _, path := range p.expr { - if path.MatchString(fullPos) { - return p.include - } - } - return !p.include -} - -// internalExcluded excludes specific internal paths. -func internalExcluded(paths ...string) *pathRegexps { - return &pathRegexps{ - expr: buildRegexps(internalPrefix, paths...), - include: false, - } -} - -// excludedExcluded excludes specific external paths. -func externalExcluded(paths ...string) *pathRegexps { - return &pathRegexps{ - expr: buildRegexps(externalPrefix, paths...), - include: false, - } -} - -// internalMatches returns a path matcher for internal packages. -func internalMatches() *pathRegexps { - return &pathRegexps{ - expr: buildRegexps(internalPrefix, internalDefault), - include: true, - } -} - -// generatedExcluded excludes all generated code. -func generatedExcluded() *pathRegexps { - return &pathRegexps{ - expr: buildRegexps(generatedPrefix, ".*"), - include: false, - } -} - -// resultExcluded excludes explicit message contents. -type resultExcluded []string - -// ShouldReport implements matcher.ShouldReport. -func (r resultExcluded) ShouldReport(d analysis.Diagnostic, _ *token.FileSet) bool { - for _, str := range r { - if strings.Contains(d.Message, str) { - return false - } - } - return true // Not excluded. -} - -// andMatcher is a composite matcher. -type andMatcher struct { - all []matcher -} - -// ShouldReport implements matcher.ShouldReport. -func (a *andMatcher) ShouldReport(d analysis.Diagnostic, fs *token.FileSet) bool { - for _, m := range a.all { - if !m.ShouldReport(d, fs) { - return false - } - } - return true -} - -// and is a syntactic convension for andMatcher. -func and(ms ...matcher) *andMatcher { - return &andMatcher{ - all: ms, - } -} - -// anyMatcher matches everything. -type anyMatcher struct{} - -// ShouldReport implements matcher.ShouldReport. -func (anyMatcher) ShouldReport(analysis.Diagnostic, *token.FileSet) bool { - return true -} - -// alwaysMatches returns an anyMatcher instance. -func alwaysMatches() anyMatcher { - return anyMatcher{} -} - -// neverMatcher will never match. -type neverMatcher struct{} - -// ShouldReport implements matcher.ShouldReport. -func (neverMatcher) ShouldReport(analysis.Diagnostic, *token.FileSet) bool { - return false -} - -// disableMatches returns a neverMatcher instance. -func disableMatches() neverMatcher { - return neverMatcher{} -} diff --git a/tools/nogo/nogo.go b/tools/nogo/nogo.go index e19e3c237..779d4d6d8 100644 --- a/tools/nogo/nogo.go +++ b/tools/nogo/nogo.go @@ -21,7 +21,6 @@ package nogo import ( "encoding/json" "errors" - "flag" "fmt" "go/ast" "go/build" @@ -45,20 +44,20 @@ import ( "gvisor.dev/gvisor/tools/checkescape" ) -// stdlibConfig is serialized as the configuration. +// StdlibConfig is serialized as the configuration. // // This contains everything required for stdlib analysis. -type stdlibConfig struct { +type StdlibConfig struct { Srcs []string GOOS string GOARCH string Tags []string } -// packageConfig is serialized as the configuration. +// PackageConfig is serialized as the configuration. // // This contains everything required for single package analysis. -type packageConfig struct { +type PackageConfig struct { ImportPath string GoFiles []string NonGoFiles []string @@ -84,7 +83,7 @@ type saver func([]byte) error // // This is done because all stdlib data is stored together, and we don't want // to load this data many times over. -func (c *packageConfig) factLoader() (loader, error) { +func (c *PackageConfig) factLoader() (loader, error) { allFacts := make(map[string][]byte) if c.StdlibFacts != "" { data, err := ioutil.ReadFile(c.StdlibFacts) @@ -114,7 +113,7 @@ func (c *packageConfig) factLoader() (loader, error) { // shouldInclude indicates whether the file should be included. // // NOTE: This does only basic parsing of tags. -func (c *packageConfig) shouldInclude(path string) (bool, error) { +func (c *PackageConfig) shouldInclude(path string) (bool, error) { ctx := build.Default ctx.GOOS = c.GOOS ctx.GOARCH = c.GOARCH @@ -128,7 +127,7 @@ func (c *packageConfig) shouldInclude(path string) (bool, error) { // files, and the facts. Note that this importer implementation will always // pass when a given package is not available. type importer struct { - *packageConfig + *PackageConfig fset *token.FileSet cache map[string]*types.Package lastErr error @@ -185,14 +184,14 @@ func (i *importer) Import(path string) (*types.Package, error) { // ErrSkip indicates the package should be skipped. var ErrSkip = errors.New("skipped") -// checkStdlib checks the standard library. +// CheckStdlib checks the standard library. // // This constructs a synthetic package configuration for each library in the -// standard library sources, and call checkPackage repeatedly. +// standard library sources, and call CheckPackage repeatedly. // // Note that not all parts of the source are expected to build. We skip obvious // test files, and cmd files, which should not be dependencies. -func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]string, []byte, error) { +func CheckStdlib(config *StdlibConfig, analyzers []*analysis.Analyzer) (allFindings []Finding, facts []byte, err error) { if len(config.Srcs) == 0 { return nil, nil, nil } @@ -225,7 +224,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str } // Aggregate all files by directory. - packages := make(map[string]*packageConfig) + packages := make(map[string]*PackageConfig) for _, file := range config.Srcs { if !strings.HasPrefix(file, rootSrcPrefix) { // Superflouous file. @@ -243,7 +242,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str } c, ok := packages[pkg] if !ok { - c = &packageConfig{ + c = &PackageConfig{ ImportPath: pkg, GOOS: config.GOOS, GOARCH: config.GOARCH, @@ -262,7 +261,6 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str } // Closure to check a single package. - allFindings := make([]string, 0) stdlibFacts := make(map[string][]byte) stdlibErrs := make(map[string]error) var checkOne func(pkg string) error // Recursive. @@ -301,7 +299,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str }() // Run the analysis. - findings, factData, err := checkPackage(config, ac, checkOne) + findings, factData, err := CheckPackage(config, analyzers, checkOne) if err != nil { // If we can't analyze a package from the standard library, // then we skip it. It will simply not have any findings. @@ -344,7 +342,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str return allFindings, factData, nil } -// checkPackage runs all analyzers. +// CheckPackage runs all given analyzers. // // The implementation was adapted from [1], which was in turn adpated from [2]. // This returns a list of matching analysis issues, or an error if the analysis @@ -352,9 +350,9 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str // // [1] bazelbuid/rules_go/tools/builders/nogo_main.go // [2] golang.org/x/tools/go/checker/internal/checker -func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, importCallback func(string) error) ([]string, []byte, error) { +func CheckPackage(config *PackageConfig, analyzers []*analysis.Analyzer, importCallback func(string) error) (findings []Finding, factData []byte, err error) { imp := &importer{ - packageConfig: config, + PackageConfig: config, fset: token.NewFileSet(), cache: make(map[string]*types.Package), callback: importCallback, @@ -406,7 +404,6 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo // Register fact types and establish dependencies between analyzers. // The visit closure will execute recursively, and populate results // will all required analysis results. - diagnostics := make(map[*analysis.Analyzer][]analysis.Diagnostic) results := make(map[*analysis.Analyzer]interface{}) var visit func(*analysis.Analyzer) error // For recursion. visit = func(a *analysis.Analyzer) error { @@ -421,27 +418,25 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo } } - // Prepare the matcher. - m := ac[a] - report := func(d analysis.Diagnostic) { - if m.ShouldReport(d, imp.fset) { - diagnostics[a] = append(diagnostics[a], d) - } - } - // Run the analysis. factFilter := make(map[reflect.Type]bool) for _, f := range a.FactTypes { factFilter[reflect.TypeOf(f)] = true } p := &analysis.Pass{ - Analyzer: a, - Fset: imp.fset, - Files: syntax, - Pkg: types, - TypesInfo: typesInfo, - ResultOf: results, // All results. - Report: report, + Analyzer: a, + Fset: imp.fset, + Files: syntax, + Pkg: types, + TypesInfo: typesInfo, + ResultOf: results, // All results. + Report: func(d analysis.Diagnostic) { + findings = append(findings, Finding{ + Category: AnalyzerName(a.Name), + Position: imp.fset.Position(d.Pos), + Message: d.Message, + }) + }, ImportPackageFact: facts.ImportPackageFact, ExportPackageFact: facts.ExportPackageFact, ImportObjectFact: facts.ImportObjectFact, @@ -464,7 +459,7 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo } // Visit all analyzers recursively. - for a, _ := range ac { + for _, a := range analyzers { if imp.lastErr == ErrSkip { continue // No local analysis. } @@ -473,114 +468,6 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo } } - // Convert all diagnostics to strings. - findings := make([]string, 0, len(diagnostics)) - for a, ds := range diagnostics { - for _, d := range ds { - // Include the anlyzer name for debugability and configuration. - findings = append(findings, fmt.Sprintf("%s: %s: %s", a.Name, imp.fset.Position(d.Pos), d.Message)) - } - } - // Return all findings. - factData := facts.Encode() - return findings, factData, nil -} - -var ( - packageFile = flag.String("package", "", "package configuration file (in JSON format)") - stdlibFile = flag.String("stdlib", "", "stdlib configuration file (in JSON format)") - findingsOutput = flag.String("findings", "", "output file (or stdout, if not specified)") - factsOutput = flag.String("facts", "", "output file for facts (optional)") - escapesOutput = flag.String("escapes", "", "output file for escapes (optional)") -) - -func loadConfig(file string, config interface{}) interface{} { - // Load the configuration. - f, err := os.Open(file) - if err != nil { - log.Fatalf("unable to open configuration %q: %v", file, err) - } - defer f.Close() - dec := json.NewDecoder(f) - dec.DisallowUnknownFields() - if err := dec.Decode(config); err != nil { - log.Fatalf("unable to decode configuration: %v", err) - } - return config -} - -// Main is the entrypoint; it should be called directly from main. -// -// N.B. This package registers it's own flags. -func Main() { - // Parse all flags. - flag.Parse() - - var ( - findings []string - factData []byte - err error - ) - - // Check the configuration. - if *packageFile != "" && *stdlibFile != "" { - log.Fatalf("unable to perform stdlib and package analysis; provide only one!") - } else if *stdlibFile != "" { - // Perform basic analysis. - c := loadConfig(*stdlibFile, new(stdlibConfig)).(*stdlibConfig) - findings, factData, err = checkStdlib(c, analyzerConfig) - } else if *packageFile != "" { - // Perform basic analysis. - c := loadConfig(*packageFile, new(packageConfig)).(*packageConfig) - findings, factData, err = checkPackage(c, analyzerConfig, nil) - // Do we need to do escape analysis? - if *escapesOutput != "" { - f, err := os.OpenFile(*escapesOutput, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - log.Fatalf("unable to open output %q: %v", *escapesOutput, err) - } - defer f.Close() - escapes, _, err := checkPackage(c, escapesConfig, nil) - if err != nil { - log.Fatalf("error performing escape analysis: %v", err) - } - for _, escape := range escapes { - fmt.Fprintf(f, "%s\n", escape) - } - } - } else { - log.Fatalf("please provide at least one of package or stdlib!") - } - - // Save facts. - if *factsOutput != "" { - if err := ioutil.WriteFile(*factsOutput, factData, 0644); err != nil { - log.Fatalf("error saving findings to %q: %v", *factsOutput, err) - } - } - - // Open the output file. - var w io.Writer = os.Stdout - if *findingsOutput != "" { - f, err := os.OpenFile(*findingsOutput, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - log.Fatalf("unable to open output %q: %v", *findingsOutput, err) - } - defer f.Close() - w = f - } - - // Handle findings & errors. - if err != nil { - log.Fatalf("error checking package: %v", err) - } - if len(findings) == 0 { - return - } - - // Print findings. - for _, finding := range findings { - fmt.Fprintf(w, "%s\n", finding) - } + return findings, facts.Encode(), nil } diff --git a/tools/nogo/register.go b/tools/nogo/register.go deleted file mode 100644 index 34b173937..000000000 --- a/tools/nogo/register.go +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2019 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package nogo - -import ( - "encoding/gob" - "log" - - "golang.org/x/tools/go/analysis" -) - -// analyzers returns all configured analyzers. -func analyzers() (all []*analysis.Analyzer) { - for a, _ := range analyzerConfig { - all = append(all, a) - } - for a, _ := range escapesConfig { - all = append(all, a) - } - return all -} - -func init() { - // Validate basic configuration. - if err := analysis.Validate(analyzers()); err != nil { - log.Fatalf("unable to validate analyzer: %v", err) - } - - // Register all fact types. - // - // N.B. This needs to be done recursively, because there may be - // analyzers in the Requires list that do not appear explicitly above. - registered := make(map[*analysis.Analyzer]struct{}) - var register func(*analysis.Analyzer) - register = func(a *analysis.Analyzer) { - if _, ok := registered[a]; ok { - return - } - - // Regsiter dependencies. - for _, da := range a.Requires { - register(da) - } - - // Register local facts. - for _, f := range a.FactTypes { - gob.Register(f) - } - - registered[a] = struct{}{} // Done. - } - for _, a := range analyzers() { - register(a) - } -} diff --git a/tools/nogo/util/BUILD b/tools/nogo/util/BUILD deleted file mode 100644 index 7ab340b51..000000000 --- a/tools/nogo/util/BUILD +++ /dev/null @@ -1,9 +0,0 @@ -load("//tools:defs.bzl", "go_library") - -package(licenses = ["notice"]) - -go_library( - name = "util", - srcs = ["util.go"], - visibility = ["//visibility:public"], -) diff --git a/tools/nogo/util/util.go b/tools/nogo/util/util.go deleted file mode 100644 index 919fec799..000000000 --- a/tools/nogo/util/util.go +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright 2019 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package util contains nogo-related utilities. -package util - -import ( - "fmt" - "io/ioutil" - "regexp" - "strconv" - "strings" -) - -// findingRegexp is used to parse findings. -var findingRegexp = regexp.MustCompile(`([a-zA-Z0-9_\/\.-]+): (-|([a-zA-Z0-9_\/\.-]+):([0-9]+)(:([0-9]+))?): (.*)`) - -const ( - categoryIndex = 1 - fullPathAndLineIndex = 2 - fullPathIndex = 3 - lineIndex = 4 - messageIndex = 7 -) - -// Finding is a single finding. -type Finding struct { - Category string - Path string - Line int - Message string -} - -// ExtractFindingsFromFile loads findings from a file. -func ExtractFindingsFromFile(filename string) ([]Finding, error) { - content, err := ioutil.ReadFile(filename) - if err != nil { - return nil, err - } - return ExtractFindingsFromBytes(content) -} - -// ExtractFindingsFromBytes loads findings from bytes. -func ExtractFindingsFromBytes(content []byte) (findings []Finding, err error) { - lines := strings.Split(string(content), "\n") - for _, singleLine := range lines { - // Skip blank lines. - singleLine = strings.TrimSpace(singleLine) - if singleLine == "" { - continue - } - m := findingRegexp.FindStringSubmatch(singleLine) - if m == nil { - // We shouldn't see findings like this. - return findings, fmt.Errorf("poorly formated line: %v", singleLine) - } - if m[fullPathAndLineIndex] == "-" { - continue // No source file available. - } - // Cleanup the message. - message := m[messageIndex] - message = strings.Replace(message, " → ", "\n → ", -1) - message = strings.Replace(message, " or ", "\n or ", -1) - // Construct a new annotation. - lineNumber, _ := strconv.ParseUint(m[lineIndex], 10, 32) - findings = append(findings, Finding{ - Category: m[categoryIndex], - Path: m[fullPathIndex], - Line: int(lineNumber), - Message: message, - }) - } - return findings, nil -} diff --git a/website/BUILD b/website/BUILD index 173d30ff0..676c2b701 100644 --- a/website/BUILD +++ b/website/BUILD @@ -11,6 +11,11 @@ docker_image( "EXPOSE 8080/tcp", 'ENTRYPOINT ["/server"]', ], + tags = [ + "local", + "manual", + "nosandbox", + ], ) # files is the full file system of the generated container. |