Skip to content

Instantly share code, notes, and snippets.

@kazcw
Last active November 16, 2018 22:38
Show Gist options
  • Save kazcw/6849c43796d51d3e56ad12bf691ac2e6 to your computer and use it in GitHub Desktop.
Save kazcw/6849c43796d51d3e56ad12bf691ac2e6 to your computer and use it in GitHub Desktop.
proposed policy on reachable UB in Bitcoin Core

Classification of undefined behavior that is not demonstrably unreachable in production:

Unportable (across toolchains as well as platforms)

Classes of "technically undefined" behavior that a lot of software depends on, so the supported toolchain/platform combinations wouldn't be able to get away with breaking it.

Examples: Out-of-bounds pointer created, but not dereferenced

Heuristic: Includes any UB that is not detectable with the selected sanitizers; hopefully clang would not fight so dirtily as to optimize based on behavior they don't offer static analysis for. And hopefully gcc won't do anything clang can't warn us about...

Policy: Avoid in new code, but fixing in committed code is not a priority (Label: refactoring)

Rationale: As far as practical, compiling in circumstances different from those applicable to official release builds should not produce broken results, but code churn concerns must also be considered.

Brittle (breakable by apparently-correct code or toolchain changes)

Instances of undefined behavior of classes that the supported toolchains' optimizers actually use to slice and dice code, but are not currently misoptimized on reachable-in-production code paths.

Examples: Uninitialized read; Signed integer overflow; Strict aliasing violation.

Heuristic: Can write a test that fails under selected sanitizers, in code paths reachable in production

Policy: Severity of issues in this category varies, and can be difficult to assess correctly. All issues of this type should be fixed; importance must be estimated carefully on a case-by-case basis.

Rationale: Code in this category works today, but may be broken by apparently-correct changes in usage, or even changes in the compiler's inlining decisions.

Broken

Does the wrong thing.

Heuristic: Usually demonstrable in functional/unit tests, but not always: e.g. leaking uninitialized memory to a peer would be Broken, but could be impossible to demonstrate in tests without sanitizers.

Choice of sanitizers considered important:

TBD. Maybe start with all clang sanitizers, and add considered exceptions lazily as issues come up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment