| Age | Commit message (Collapse) | Author |
|---|
|
PiperOrigin-RevId: 390346783
|
|
PiperOrigin-RevId: 390318725
|
|
Co-authored-by: Andrei Vagin <avagin@google.com>
PiperOrigin-RevId: 390232925
|
|
This CL introduces a 'checklinkname' analyzer, which provides rudimentary
type-checking that verifies that function signatures on the local and remote
sides of //go:linkname directives match expected values.
If the Go standard library changes the definitions of any of these function,
checklinkname will flag the change as a finding, providing an error informing
the gVisor team to adapt to the upstream changes. This allows us to eliminate
the majority of gVisor's forward-looking negative build tags, as we can catch
mismatches in testing [1].
The remaining forward-looking negative build tags are covering shared struct
definitions, which I hope to add to checklinkname in a future CL.
[1] Of course, semantics/requirements can change without the signature
changing, so we still must be careful, but this covers the common case.
PiperOrigin-RevId: 387873847
|
|
Build constraints are now inferred from go:build directives rather than +build
directives. +build directives are still emitted in generated files as required
in Go 1.16 and earlier.
Note that go/build/constraint was added in Go 1.16, so gVisor now requires Go
1.16.
PiperOrigin-RevId: 387240779
|
|
Currently behavior of config groups with `default: false` is buggy. The
intention is that adding an empty suppression section for that group to a
specific analyzer config should enable reporting for that analyzer. i.e.,
```
groups:
- name: foo
regex: "^foo/"
default: false
global:
...
analyzers:
asmdecl:
foo: # Enabled.
```
This should enable the foo group only for asmdecl. Unfortunately, today the
actual behavior depends on the contents of the `global:` section. If `global:`
contains an entry for foo, then it will work as described. If `global:` does
_not_ contain an entry for foo, then the group default (disabled) always
applies and the individual analyzer options have no effect.
The cause of this is confusion in `AnalyzerConfig.shouldReport`, which doesn't
distinguish between explicit suppression via a global suppression/exclude and
simply having no configuration at all. Make this more explicit, so that the no
configuration case can continue to per-analyzer configuration before falling
back to the group default.
The last test case in the added test fails without this change.
This re-enables several opted-in analyzers for external dependencies, which
have gained a few more false positives to suppress.
PiperOrigin-RevId: 386904725
|
|
#6322 tried to update Go to 1.16, but existing nodes fail to upgrade due to the
presence of old Go [1]. Specifically when trying to add Go to `/usr/bin`:
```
ln: failed to create symbolic link '/usr/bin/go': File exists
```
Also:
- Removing `golang-go` also removes apt installs of `gcc` and `pkg-config`, so
those are installed explicitly.
- Add `-c` to wget, which will prevent re-downloading Go for each run.
- Disable GO111MODULE when building cri-tools and containerd, since we're using
pre-module versions of each.
1 - https://buildkite.com/gvisor/pipeline/builds/7285#3593244c-e411-472d-804a-9c7fbbd24762
PiperOrigin-RevId: 386106881
|
|
PiperOrigin-RevId: 385894869
|
|
PiperOrigin-RevId: 385200993
|
|
We're currently on 1.13, which can cause build issues with code targeting later
versions.
PiperOrigin-RevId: 385029528
|
|
Update the following from syserror to the linuxerr equivalent:
EEXIST
EFAULT
ENOTDIR
ENOTTY
EOPNOTSUPP
ERANGE
ESRCH
PiperOrigin-RevId: 384329869
|
|
PiperOrigin-RevId: 383472507
|
|
This change makes the checklocks analyzer considerable more powerful, adding:
* The ability to traverse complex structures, e.g. to have multiple nested
fields as part of the annotation.
* The ability to resolve simple anonymous functions and closures, and perform
lock analysis across these invocations. This does not apply to closures that
are passed elsewhere, since it is not possible to know the context in which
they might be invoked.
* The ability to annotate return values in addition to receivers and other
parameters, with the same complex structures noted above.
* Ignoring locking semantics for "fresh" objects, i.e. objects that are
allocated in the local frame (typically a new-style function).
* Sanity checking of locking state across block transitions and returns, to
ensure that no unexpected locks are held.
Note that initially, most of these findings are excluded by a comprehensive
nogo.yaml. The findings that are included are fundamental lock violations.
The changes here should be relatively low risk, minor refactorings to either
include necessary annotations to simplify the code structure (in general
removing closures in favor of methods) so that the analyzer can be easily
track the lock state.
This change additional includes two changes to nogo itself:
* Sanity checking of all types to ensure that the binary and ast-derived
types have a consistent objectpath, to prevent the bug above from occurring
silently (and causing much confusion). This also requires a trick in
order to ensure that serialized facts are consumable downstream. This can
be removed with https://go-review.googlesource.com/c/tools/+/331789 merged.
* A minor refactoring to isolation the objdump settings in its own package.
This was originally used to implement the sanity check above, but this
information is now being passed another way. The minor refactor is preserved
however, since it cleans up the code slightly and is minimal risk.
PiperOrigin-RevId: 382613300
|
|
This is a good Go convention that we should follow.
PiperOrigin-RevId: 378538679
|
|
PiperOrigin-RevId: 377611852
|
|
Fixes #6084
PiperOrigin-RevId: 376293659
|
|
Ignore calls to atomic functions in case there is no analysis information.
It is unclear why this has broken in some cases, perhaps these functions
have been replaced by intrinsics as an optimization?
PiperOrigin-RevId: 374682441
|
|
Fix TODO comments referring to incorrect issue numbers. Also fix the link in
issue reviver comments to include the right url fragment.
PiperOrigin-RevId: 373491821
|
|
PiperOrigin-RevId: 373271579
|
|
Fixes invocation of the Github issue reviver by including the required 'path'
command line option. Also updates the issue reviver to add a 'revived' label to
revived issues. Issues with a 'revived' label will no longer be marked as
stale.
PiperOrigin-RevId: 373046772
|
|
|
|
PiperOrigin-RevId: 372376653
|
|
PiperOrigin-RevId: 372166050
|
|
With debugging enabled, go-marshal can generate too much output for
bazel under default configurations, which can cause builds to
fail. The limit defaults to 1 MB.
PiperOrigin-RevId: 372030402
|
|
This is a suite of changes intended to dramatically speed up nogo speed.
First, there are minor changes that help efficiency significantly.
* Gob-based encoding is used internally, and JSON only used for the final
set of findings. This is done to preserve the existing format (which is
consumed by external tooling), and to facilitate manual debugging.
* Unnecessary regex compilation is elided in the configuration, and care is
taken for merges to prevent redundant entries. I'm not sure quite sure how,
but it turns out that this was consumed a significant amount of time,
presumably compiling the same regexes over and over again.
Second, this change enables bazel workers for nogo analyzers.
Workers enable persistent processes instead of creating and tearing down a
sandbox every invocation. A library is introduced to abstraction these details,
and allow the tools to still be written using standard flags, etc.
The key here is that these binaries and the core of nogo become aware of
caches with worker.Cache. This allows us to save significant time loading the
same set of files and findings over and over again. These caches are keyed by
the digests that are provided by bazel, and are capped in overall size.
Note that the worker package attempts to capture output during each run, but
tools are no longer permitted to write to stdout. This necessitated dropping
some spurious output from checklocks.
PiperOrigin-RevId: 370505732
|
|
Presently, the standard library facts are not serialized in a deterministic
order. This means that they have the possibility to change on each iteration,
requiring a large scale re-analysis of all downstream actions, which includes
all packages.
Improve cache-ability of nogo actions by improving the determinism of the both
facts and findings. Internally, default facts should be serialized as a sorted
list for this reason already.
PiperOrigin-RevId: 370188259
|
|
We already have blocking nogo tests which show all findings. This job was
building all nogo targets, and posting all the findings to GitHub as a check
run. Building nogo takes a while so we actually end up wasting a lot of time
doing redundant work.
This is aligned with our goal of moving away from GitHub actions to BuildKite
only.
PiperOrigin-RevId: 370134875
|
|
PiperOrigin-RevId: 369505182
|
|
gohacks.Memmove() takes in the number of bytes to move. The current generated
code passes len(src) and len(dst) as the number of bytes to move.
However, the marshal.Marshallable interface allows passing in larger buffers.
The stated precondition is that the buffer should be "at least" SizeBytes()
in length but it is allowed to be larger.
This change now correctly calls Memmove with the argument for the number of
bytes to move as type.SizeBytes(). This was caught when I made lisafs use the
Unsafe marshalling API and got a lot of memory violations.
PiperOrigin-RevId: 368952642
|
|
PiperOrigin-RevId: 368470656
|
|
Netstack is supposed to be somewhat independent of the rest of gVisor, and
others should be able to use it without pulling in excessive dependencies.
Currently, there is no way to fight dependency creep besides careful code
review.
This change introduces a test rule `netstack_deps_check` that ensures the target
only relies on gVisor targets and a short allowlist of external dependencies.
Users who add a dependency will see an error and have to manually update the
allowlist.
The set of packages to test comes from //runsc, as it uses packages we would
expect users to commonly rely on. It was generated via:
$ find ./runsc -name BUILD | xargs grep tcpip | awk '{print $2}' | sort | uniq
(Note: We considered giving //pkg/tcpip it's own go.mod, but this breaks go
tooling.)
PiperOrigin-RevId: 368456711
|
|
PiperOrigin-RevId: 367517305
|
|
PiperOrigin-RevId: 366555466
|
|
Split usermem package to help remove syserror dependency in go_marshal.
New hostarch package contains code not dependent on syserror.
PiperOrigin-RevId: 365651233
|
|
Stateify methods are always called without holding the appropriate
locks. The system is paused and we know there will be no mutations
when we call Save/Load, so this is perfectly safe. However, checklocks
can't know about this, and it will always complain.
Mark stateify generated methods that touch struct fields as
"checklocksignore" to avoid this.
PiperOrigin-RevId: 364610241
|
|
We were only supporting dynamic struct types. With this change, users can make
any type dynamic. The tool (correctly) blindly just generates the remaining
methods needed to implement Marshallable using the 3 methods defined by the
user on the dynamic type.
This is helpful in situations like:
type StringArray []string
Added a test for such a use case.
PiperOrigin-RevId: 364463164
|
|
This may be useful for tracking down where build tags come from and
understanding tag import issues in generated files.
PiperOrigin-RevId: 364374931
|
|
PiperOrigin-RevId: 362545342
|
|
We can generate more than one apt repo for the same package. If we will
sign a package again, its file will be changed and all hashes that have
been generated before will be invalid.
|
|
This binary is used to recursively enable and generate Merkle tree files
for all files and directories in a file system from inside a gVisor
sandbox.
PiperOrigin-RevId: 362418770
|
|
The dynamic type user defines the marshalling logic, so we don't need to test
for things like alignment, absence of slices, etc.
For dynamic types, the go_marshal generator just generates the missing methods
required to implement marshal.Marshallable.
PiperOrigin-RevId: 361676311
|
|
This validates that struct fields if annotated with "// checklocks:mu" where
"mu" is a mutex field in the same struct then access to the field is only
done with "mu" locked.
All types that are guarded by a mutex must be annotated with
// +checklocks:<mutex field name>
For more details please refer to README.md.
PiperOrigin-RevId: 360729328
|
|
This will fix debian packaging.
Updates #5510
PiperOrigin-RevId: 359563378
|
|
This makes it easier to implement dynamically sized types in go-marshal. You
really only need to implement MarshalBytes, UnmarshalBytes and SizeBytes to
implement the entire interface.
By using the `dynamic` tag, the autogenerator will generate the rest of the
methods for us.
This change also simplifies how KernelIPTGetEntries implements Marshallable
using the newly added utility.
PiperOrigin-RevId: 356397114
|
|
Confirmed gazelle generates no significant noise.
Fix documented gazelle invocation while I'm here.
PiperOrigin-RevId: 355452758
|
|
...and a bunch of other things as I worked through the rot. Notably:
- Upgrade to bazel 4.0.0
- Upgrade to Go 1.15.7
Remove go_branch stderr suppression; this made it quite difficult to see
what was failing while developing this patch.
PiperOrigin-RevId: 355257833
|
|
This is required only for the built-in bazel nogo functionality.
Since we roll these targets manually via the wrappers, we don't need
to use go_tool_library. The inconsistent use of these targets leads
to conflicting instantiations of go_default_library and go_tool_library,
which both contain the same output files.
PiperOrigin-RevId: 355184975
|
|
This change also adds an extra sanity check to the make_apt.sh script,
in order to ensure that this simple mistake does not occur again.
PiperOrigin-RevId: 355101754
|
|
It's unclear why permissions wind up corrupted, but these can be cleared
on any failure, similar to the bazel cache itself:
https://buildkite.com/gvisor/pipeline/builds/2304#_
PiperOrigin-RevId: 355057421
|
|
The GH Build action has been failing with the error message:
```
--- BUILD -c opt //runsc
tee: /proc/self/fd/2: No such device or address
```
tee /dev/fd/2 seems to be the canonical way of copying stdin to stderr.
So use that instead.
PiperOrigin-RevId: 353259087
|
