Replace some cfg(debug) with cfg(debug_assertions) (#3242)

* Replace some cfg(debug) with cfg(debug_assertions)

Cargo nor rustc ever sets `cfg(debug)` automatically, so it's expected
that these usages were intended to be `cfg(debug_assertions)`.

* Fix MachBuffer debug-assertion invariant checks.

We should only check invariants when we expect them to be true --
specifically, before the branch-simplification algorithm runs. At other
times, they may be temporarily violated: e.g., after
`add_{cond,uncond}_branch()` but before emitting the branch bytes. This
is the expected sequence, and the rest of the code is consistent with
that.

Some of the checks also were not quite right (w.r.t. the written
invariants); specifically, we should not check validity of a label's
offset when the label has been aliased to another label.

It seems that this is an unfortunate consequence of leftover
debug-assertions that weren't actually being run, so weren't kept
up-to-date. Should no longer happen now that we actually check these!

Co-authored-by: Chris Fallin <chris@cfallin.org>
This commit is contained in:
Alex Crichton
2021-08-25 22:15:24 -05:00
committed by GitHub
parent da5c82b786
commit 6fbddc1931
2 changed files with 16 additions and 14 deletions

View File

@@ -321,8 +321,15 @@ impl<I: VCodeInst> MachBuffer<I> {
/// Debug-only: check invariants of labels and branch-records described /// Debug-only: check invariants of labels and branch-records described
/// under "Branch-optimization Correctness" above. /// under "Branch-optimization Correctness" above.
#[cfg(debug)] ///
/// These invariants are checked at branch-simplification
/// time. Note that they may be temporarily violated at other
/// times, e.g. after calling `add_{cond,uncond}_branch()` and
/// before emitting branch bytes.
fn check_label_branch_invariants(&self) { fn check_label_branch_invariants(&self) {
if !cfg!(debug_assertions) {
return;
}
let cur_off = self.cur_offset(); let cur_off = self.cur_offset();
// Check that every entry in latest_branches has *correct* // Check that every entry in latest_branches has *correct*
// labels_at_this_branch lists. We do not check completeness because // labels_at_this_branch lists. We do not check completeness because
@@ -340,10 +347,15 @@ impl<I: VCodeInst> MachBuffer<I> {
} }
} }
// Check that every label is unresolved, or resolved at or before // Check that every label is unresolved, or resolved at or
// cur_offset. If at cur_offset, must be in `labels_at_tail`. // before cur_offset. If at cur_offset, must be in
// `labels_at_tail`. We skip labels that are aliased to
// others already.
for (i, &off) in self.label_offsets.iter().enumerate() { for (i, &off) in self.label_offsets.iter().enumerate() {
let label = MachLabel(i as u32); let label = MachLabel(i as u32);
if self.label_aliases[i] != UNKNOWN_LABEL {
continue;
}
debug_assert!(off == UNKNOWN_LABEL_OFFSET || off <= cur_off); debug_assert!(off == UNKNOWN_LABEL_OFFSET || off <= cur_off);
if off == cur_off { if off == cur_off {
debug_assert!( debug_assert!(
@@ -363,11 +375,6 @@ impl<I: VCodeInst> MachBuffer<I> {
} }
} }
#[cfg(not(debug))]
fn check_label_branch_invariants(&self) {
// Nothing.
}
/// Current offset from start of buffer. /// Current offset from start of buffer.
pub fn cur_offset(&self) -> CodeOffset { pub fn cur_offset(&self) -> CodeOffset {
self.data.len() as CodeOffset self.data.len() as CodeOffset
@@ -527,7 +534,6 @@ impl<I: VCodeInst> MachBuffer<I> {
self.optimize_branches(); self.optimize_branches();
// Post-invariant: by `optimize_branches()` (see argument there). // Post-invariant: by `optimize_branches()` (see argument there).
self.check_label_branch_invariants();
} }
/// Lazily clear `labels_at_tail` if the tail offset has moved beyond the /// Lazily clear `labels_at_tail` if the tail offset has moved beyond the
@@ -594,7 +600,6 @@ impl<I: VCodeInst> MachBuffer<I> {
} }
// Post-invariant: no mutations to branches/labels data structures. // Post-invariant: no mutations to branches/labels data structures.
self.check_label_branch_invariants();
} }
/// Inform the buffer of an unconditional branch at the given offset, /// Inform the buffer of an unconditional branch at the given offset,
@@ -628,7 +633,6 @@ impl<I: VCodeInst> MachBuffer<I> {
// Post-invariant: we asserted branch start is current tail; the list of // Post-invariant: we asserted branch start is current tail; the list of
// labels at branch is cloned from list of labels at current tail. // labels at branch is cloned from list of labels at current tail.
self.check_label_branch_invariants();
} }
/// Inform the buffer of a conditional branch at the given offset, /// Inform the buffer of a conditional branch at the given offset,
@@ -662,7 +666,6 @@ impl<I: VCodeInst> MachBuffer<I> {
// Post-invariant: we asserted branch start is current tail; labels at // Post-invariant: we asserted branch start is current tail; labels at
// branch list is cloned from list of labels at current tail. // branch list is cloned from list of labels at current tail.
self.check_label_branch_invariants();
} }
fn truncate_last_branch(&mut self) { fn truncate_last_branch(&mut self) {
@@ -757,7 +760,6 @@ impl<I: VCodeInst> MachBuffer<I> {
// (see comments to `add_{cond,uncond}_branch()`). // (see comments to `add_{cond,uncond}_branch()`).
// - The buffer is truncated to just before the last branch, and the // - The buffer is truncated to just before the last branch, and the
// fixup record referring to that last branch is removed. // fixup record referring to that last branch is removed.
self.check_label_branch_invariants();
} }
fn optimize_branches(&mut self) { fn optimize_branches(&mut self) {

View File

@@ -212,7 +212,7 @@ fn build_function_addr_map(
}); });
} }
if cfg!(debug) { if cfg!(debug_assertions) {
// fn_map is sorted by the generated field -- see FunctionAddressMap::instructions. // fn_map is sorted by the generated field -- see FunctionAddressMap::instructions.
for i in 1..fn_map.len() { for i in 1..fn_map.len() {
assert_le!(fn_map[i - 1].generated, fn_map[i].generated); assert_le!(fn_map[i - 1].generated, fn_map[i].generated);