| Age | Commit message (Collapse) | Author |
|---|
|
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
|
|
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
|
|
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 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
|
|
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 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
|
|
For validation, the "on" key in existing YAML files is changed to a literal
string. In the YAML spec, on is a keyword which encodes a boolean value, so
without relying on a specific implementation the YAML files are technically
not encoding an object that complies with the specification.
PiperOrigin-RevId: 350172147
|
|
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
|
|
PiperOrigin-RevId: 337544107
|
|
PiperOrigin-RevId: 336343819
|
|
PiperOrigin-RevId: 336126583
|
|
This change also adds support to go_stateify for detecting an appropriate
receiver name, avoiding a large number of false positives.
PiperOrigin-RevId: 335994587
|
|
PiperOrigin-RevId: 329408633
|
|
This immediately revealed an escape analysis violation (!), where
the sync.Map was being used in a context that escapes were not
allowed. This is a relatively minor fix and is included.
PiperOrigin-RevId: 328611237
|
|
PiperOrigin-RevId: 325280924
|
|
See tools/nogo/README.md.
The checkescape tool is able to perform recursive escape analysis, using the
actual generated binary to confirm the results produced by the compiler itself.
As an initial use case, this replaces the manual escape analysis tests used for
go_marshal, and validates that the CopyIn and CopyOut paths will not require
any allocation or stack splits.
Updates #2243
PiperOrigin-RevId: 307532986
|
