Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix some uses of pointer intrinsics with invalid pointers #53804

Merged
merged 2 commits into from
Sep 16, 2018

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Aug 29, 2018

Found by miri:

  • Vec::into_iter calls ptr::read (and the underlying copy_nonoverlapping) with an unaligned pointer to a ZST. According to LLVM devs, this is UB because it contradicts the metadata we are attaching to that pointer.

  • HashMap creation calls ptr:.write_bytes on a NULL pointer with a count of 0. This is likely not currently UB currently, but it violates the rules we are setting in Rewrite docs for pointer methods #53783, and we might want to exploit those rules later (e.g. with more nonnull attributes for LLVM).

    Probably what HashMap really should do is use NonNull::dangling() instead of 0 for the empty case, but that would require a more careful analysis of the code.

It seems like ideally, we should do a review of usage of such intrinsics all over libstd to ensure that they use valid pointers even when the size is 0. Is it worth opening an issue for that?

@rust-highfive
Copy link
Collaborator

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2018
@@ -2410,9 +2410,8 @@ impl<T> Iterator for IntoIter<T> {
// same pointer.
self.ptr = arith_offset(self.ptr as *const i8, 1) as *mut T;

// Use a non-null pointer value
// (self.ptr might be null because of wrapping)
Some(ptr::read(1 as *mut T))
Copy link
Contributor

@gnzlbg gnzlbg Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible for ptr::read to contain a debug_assert_eq!(ptr.align_offset(align_of::<T>()), 0) ?

If that assert fails, it would currently be UB, so we might as well just panic in debug builds to at least catch these inside of std on CI. Maybe someday users will benefit from these without having to use miri to detect these issues.

Copy link
Member Author

@RalfJung RalfJung Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My plan was to open an E-easy issue (once #53783 lands) to decorate all these functions with a debug_assert! testing non-NULL and being aligned.

I would however use ptr as usize % mem::align_of<T>() == 0, because your test will always fail on miri. Remember, align_offset can spuriously "fail" (return usize::max).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea.

Remember, align_offset can spuriously fail.

I didn't know this :/ whyyyy

Copy link
Member Author

@RalfJung RalfJung Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we cannot implement it completely in miri/CTFE -- allocations do not actually have "real" base addresses there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that makes sense. This is getting off topic, but couldn't miri track the alignment requested for an allocation and use that in align_offset ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could, and it could give a better return value in that case.

But align_offset is also used e.g. on a large u8 allocation to find a subslice that is 256bit-aligned for use with SSE. There is no way for miri to provide an alignment stronger than what the allocation promises.

So maybe it could be made good enough for your test, not sure... the % works today though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a bit worrying that we might prefer slower run-time code because it works on miri to fast run-time code. Does the % lower to the same LLVM-IR than align_offset, at least after running opt ?

An alternative is to use conditional compilation, and use % for miri and align_offset for run-time rust.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how % could be any slower than the intrinsic. If anything, align_offset is doing the harder job (computing which offset to add, not just testing it for 0) so I'd expect it to be slower.

Currently, LLVM optimizes the to the same code to the extend that they get deduplicated.^^

@@ -742,7 +742,9 @@ impl<K, V> RawTable<K, V> {
) -> Result<RawTable<K, V>, CollectionAllocErr> {
unsafe {
let ret = RawTable::new_uninitialized_internal(capacity, fallibility)?;
ptr::write_bytes(ret.hashes.ptr(), 0, capacity);
if capacity > 0 {
Copy link
Contributor

@gnzlbg gnzlbg Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding this workaround, I would prefer if here:

const EMPTY: usize = 1;

we would change EMPTY from a const to a const fn empty<T>() -> *mut T; that just returns NonNull::dangling().as_ptr(). I think EMPTY is only used twice in the whole module.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/// Note: when the pointer is initialized to EMPTY `.ptr()` will return
/// null and the tag functions shouldn't be used.

The table states this, but there are no debug_assert!s preventing misuse AFAICT, might be worth it to add these as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid of touching HashMap, so I'd prefer if someone who knows anything about that code does those changes...

The pointer here was previously NULL, so I don't think it came from EMPTY.

Copy link
Contributor

@gnzlbg gnzlbg Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If capacity == 0, then new_uninitialized_internal can only return (

if capacity == 0 {

) hashes with this value:

hashes: TaggedHashUintPtr::new(EMPTY as *mut HashUint),

which basically calls (

unsafe fn new(ptr: *mut HashUint) -> Self {
):

Unique::new_unchecked(EMPTY as *mut HashUint)

Unique::ptr() will then just return EMPTY as *mut HashUint which is never null, so I have no idea how this pointer there could ever be NULL. It should be EMPTY. That is still wrong because of incorrect alignment, but that's a different type of wrong. Or what am I missing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table states this, but there are no debug_assert!s preventing misuse AFAICT, might be worth it to add these as well.

Well, the tag functions cannot assert anything reasonable because the last bit can be either 0 or 1.

I think this also uses values as sentinel values in ways it should not. But that really needs someone to actually dig in the code. I can open an issue about that if you want, right now I only have time to do the most obvious thing.

I have no idea how this pointer there could ever be NULL. It should be EMPTY.

As the comment you quoted says, ptr() will return NULL for EMPTY.

If EMPTY was properly aligned and ptr did not do anything, we would not even need any test here on write_bytes because non-NULL aligned zero-sized writes are perfectly fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can open an issue about that if you want, right now I only have time to do the most obvious thing.

Yeah, this is already an improvement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I opened #53853.

Anything left to do in this PR?

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 31, 2018

This LGTM. @shepmaster ?

@memoryruins
Copy link
Contributor

It seems like ideally, we should do a review of usage of such intrinsics all over libstd to ensure that they use valid pointers even when the size is 0. Is it worth opening an issue for that?

Sounds like a good idea. It may be worth adding some instructions for anyone interested in reviewing with Miri in the tracking issue. Was it similar to the steps taken in #53564 (comment) and #53566?

@RalfJung
Copy link
Member Author

TBH I think the review has to mostly proceed by looking at the code. And we should find a way to run the libstd tests (and doctests) in miri. Doing that manually seems not worthwhile? But the steps are essentially, run some code in miri and see what happens.^^ miri only detects UB that actually runs, so you basically need an exhaustive test suite to be sure.

Also, which tracking issue?

What seems much more tractable at this point is to add debug_assert! to all these methods (there are not so many), and then run the test suite with debug assertions enabled. I guess I could open an issue with instructions for that. Not sure if it would be E-easy or E-medium?

@cramertj
Copy link
Member

Sorry for the somewhat unrelated question, but what is the point of using ptr::read(NonNull::dangling().as_ptr()) to create a ZST rather than using mem::transmute(()), mem::zeroed(), mem::uninitialized(), or some other runtime-no-op-give-me-an-arbitrary-zero-sized-value operation?

@memoryruins
Copy link
Contributor

memoryruins commented Aug 31, 2018

And we should find a way to run the libstd tests (and doctests) in miri.

👍 would miri fit in a custom test framework? #53410 then allow any crate mark a function as a miri test.

Also, which tracking issue?

I meant if an issue was opened for tracking reviews all over std, it could be added there.

@RalfJung
Copy link
Member Author

I opened an issue to add debug_assert! to all intrinsics, to find issues at least in the libstd test suite: #53871

what is the point of using ptr::read(NonNull::dangling().as_ptr()) to create a ZST rather than using mem::transmute(()), mem::zeroed(), mem::uninitialized(), or some other runtime-no-op-give-me-an-arbitrary-zero-sized-value operation?

Good question. No idea. Again, my intention was to do the most obvious thing and only that.

+1 would miri fit in a custom test framework? #53410 then allow any crate mark a function as a miri test.

Maybe? But really what I mean is that it should run the same test as what currently runs with ./x.py test src/libstd.

@joshlf
Copy link
Contributor

joshlf commented Aug 31, 2018

Sorry for the somewhat unrelated question, but what is the point of using ptr::read(NonNull::dangling().as_ptr()) to create a ZST rather than using mem::transmute(()), mem::zeroed(), mem::uninitialized(), or some other runtime-no-op-give-me-an-arbitrary-zero-sized-value operation?

mem::transmute won't work because it requires that the size of both types is the same, and that code only knows that the type T is zero sized due to a check at runtime. mem::uninitialized probably works, but I'd be worried about running afoul of LLVM poison. mem::zeroed probably works.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 9, 2018

@shepmaster this awaits your review :)

@RalfJung
Copy link
Member Author

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned shepmaster Sep 16, 2018
@nagisa
Copy link
Member

nagisa commented Sep 16, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Sep 16, 2018

📌 Commit 357c5da has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 16, 2018
@bors
Copy link
Contributor

bors commented Sep 16, 2018

⌛ Testing commit 357c5da with merge 8a2dec6...

bors added a commit that referenced this pull request Sep 16, 2018
fix some uses of pointer intrinsics with invalid pointers

[Found by miri](rust-lang/miri#446):

* `Vec::into_iter` calls `ptr::read` (and the underlying `copy_nonoverlapping`) with an unaligned pointer to a ZST. [According to LLVM devs](https://bugs.llvm.org/show_bug.cgi?id=38583), this is UB because it contradicts the metadata we are attaching to that pointer.
* `HashMap` creation calls `ptr:.write_bytes` on a NULL pointer with a count of 0. This is likely not currently UB *currently*, but it violates the rules we are setting in #53783, and we might want to exploit those rules later (e.g. with more `nonnull` attributes for LLVM).

    Probably what `HashMap` really should do is use `NonNull::dangling()` instead of 0 for the empty case, but that would require a more careful analysis of the code.

It seems like ideally, we should do a review of usage of such intrinsics all over libstd to ensure that they use valid pointers even when the size is 0. Is it worth opening an issue for that?
@bors
Copy link
Contributor

bors commented Sep 16, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nagisa
Pushing 8a2dec6 to master...

@bors bors merged commit 357c5da into rust-lang:master Sep 16, 2018
@RalfJung RalfJung deleted the ptr-invalid branch November 9, 2018 15:34
@12101111 12101111 mentioned this pull request Oct 8, 2021
5 tasks
bors-servo added a commit to servo/servo that referenced this pull request Oct 9, 2021
Fix UB in hashglobe

<!-- Please describe your changes on the following line: -->

This is a backport of rust-lang/rust#53804

Currently, this bug cause Firefox crash with Rust 1.56 ( LLVM 13 )

<details>
<summary>backtrace of Firefox</summary>

```
(lldb) bt
* thread #1, name = 'GeckoMain', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
  * frame #0: 0x00007ffff4ff7bde libxul.so`::RustMozCrash(const char *, int, const char *) [inlined] MOZ_Crash(aLine=2220, aReason="attempt to write to unaligned or null pointer") at Assertions.h:256:3
    frame #1: 0x00007ffff4ff7bd4 libxul.so`::RustMozCrash(aFilename="/rustc/1.56.0/library/core/src/intrinsics.rs", aLine=2220, aReason="attempt to write to unaligned or null pointer") at wrappers.cpp:18:3
    frame #2: 0x00007ffff4ff7b53 libxul.so`mozglue_static::panic_hook::h91947f48d75eb4dd(info=<unavailable>) at lib.rs:91:9
    frame #3: 0x00007ffff4ff6e19 libxul.so`core::ops::function::Fn::call::h2f4e62c593234181((null)=<unavailable>, (null)=<unavailable>) at function.rs:70:5
    frame #4: 0x00007ffff5bd055b libxul.so`std::panicking::rust_panic_with_hook::h41696e81832261ff(payload=&mut dyn core::panic::BoxMeUp @ 0x00007f24a7e5fc70, message=Option<&core::fmt::Arguments> @ r13, location=<unavailable>) at panicking.rs:628:17
    frame #5: 0x00007ffff5bd00a2 libxul.so`std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::hea2a534982472bd3 at panicking.rs:519:13
    frame #6: 0x00007ffff5bcc494 libxul.so`std::sys_common::backtrace::__rust_end_short_backtrace::h793de5eec3283122(f=<unavailable>) at backtrace.rs:141:18
    frame #7: 0x00007ffff5bd0039 libxul.so`rust_begin_unwind(info=0x00007fffffff7a78) at panicking.rs:517:5
    frame #8: 0x00007ffff5c2ece1 libxul.so`core::panicking::panic_fmt::h43c4759d9f1ef313(fmt=<unavailable>) at panicking.rs:101:14
    frame #9: 0x00007ffff5c2ebbd libxul.so`core::panicking::panic::hb6dc0edf878703a5(expr=<unavailable>) at panicking.rs:50:5
    frame #10: 0x00007ffff56dd397 libxul.so`core::intrinsics::write_bytes::h481ad0b8372e9e0a(dst=0x0000000000000000, val='\0', count=0) at intrinsics.rs:2220:5
    frame #11: 0x00007ffff589e749 libxul.so`hashglobe::table::RawTable$LT$K$C$V$GT$::new::h04532bdf928a2865(capacity=0) at table.rs:839:13
    frame #12: 0x00007ffff58b38f0 libxul.so`hashglobe::hash_map::HashMap$LT$K$C$V$C$S$GT$::try_with_hasher::h7086fbc016a9427d(hash_builder=<unavailable>) at hash_map.rs:622:20
    frame #13: 0x00007ffff58b3077 libxul.so`hashglobe::hash_map::HashMap$LT$K$C$V$C$S$GT$::with_hasher::h9ee840b6d255f9fa(hash_builder=<unavailable>) at hash_map.rs:628:9
    frame #14: 0x00007ffff5812c99 libxul.so`_$LT$hashglobe..hash_map..HashMap$LT$K$C$V$C$S$GT$$u20$as$u20$core..default..Default$GT$::default::h7a34c6ba884b9658 at hash_map.rs:1329:9
    frame #15: 0x00007ffff58dfb3a libxul.so`_$LT$style..selector_map..MaybeCaseInsensitiveHashMap$LT$style..gecko_string_cache..Atom$C$V$GT$$u20$as$u20$core..default..Default$GT$::default::h2c19828653342158 at selector_map.rs:704:37
    frame #16: 0x00007ffff5978919 libxul.so`_$LT$style..invalidation..stylesheets..StylesheetInvalidationSet$u20$as$u20$core..default..Default$GT$::default::h16e0d0431f387b3d at stylesheets.rs:103:5
    frame #17: 0x00007ffff58d54b9 libxul.so`style::invalidation::stylesheets::StylesheetInvalidationSet::new::h4eedeb3b15c2c2c5 at stylesheets.rs:112:9
    frame #18: 0x00007ffff58e43a6 libxul.so`style::stylesheet_set::DocumentStylesheetSet$LT$S$GT$::new::hf80ba16d4d55a4ca at stylesheet_set.rs:516:28
    frame #19: 0x00007ffff58f269a libxul.so`style::stylist::StylistStylesheetSet::new::h66b5d09ea8a90d6e at stylist.rs:462:30
    frame #20: 0x00007ffff58f26f0 libxul.so`style::stylist::Stylist::new::h4732ca5247e85cd7(device=<unavailable>, quirks_mode=Quirks) at stylist.rs:562:26
    frame #21: 0x00007ffff593d755 libxul.so`style::gecko::data::PerDocumentStyleData::new::h9dc814d46fec8d6c(document=<unavailable>) at data.rs:145:22
    frame #22: 0x00007ffff56781d9 libxul.so`Servo_StyleSet_Init(doc=<unavailable>) at glue.rs:4175:25
    frame #23: 0x00007ffff2b58416 libxul.so`mozilla::ServoStyleSet::ServoStyleSet(this=0x00007fffe3c5ba90, aDocument=0x00007fffd391d560) at ServoStyleSet.cpp:120:17
    frame #24: 0x00007ffff128ba5e libxul.so`mozilla::dom::Document::Init() [inlined] mozilla::detail::UniqueSelector<mozilla::ServoStyleSet>::SingleObject mozilla::MakeUnique<mozilla::ServoStyleSet, mozilla::dom::Document&>(aArgs=0x00007fffd391d560) at UniquePtr.h:609:27
    frame #25: 0x00007ffff128ba46 libxul.so`mozilla::dom::Document::Init(this=0x00007fffd391d560) at Document.cpp:2657:15
    frame #26: 0x00007ffff20847d9 libxul.so`nsHTMLDocument::Init(this=0x00007fffd391d560) at nsHTMLDocument.cpp:146:27
    frame #27: 0x00007ffff208462a libxul.so`NS_NewHTMLDocument(aInstancePtrResult=0x00007fffffff9c60, aLoadedAsData=false) at nsHTMLDocument.cpp:112:22
    frame #28: 0x00007ffff2ea18cf libxul.so`nsContentDLF::CreateBlankDocument(aLoadGroup=0x00007fffc2ec87a0, aPrincipal=0x00007fffe476cdb0, aPartitionedPrincipal=0x00007fffe476cdb0, aContainer=0x00007fffe34752c0) at nsContentDLF.cpp:212:22
    frame #29: 0x00007ffff32d1ea1 libxul.so`nsDocShell::CreateAboutBlankContentViewer(this=0x00007fffe34752c0, aPrincipal=0x00007fffe476cdb0, aPartitionedPrincipal=0x00007fffe476cdb0, aCSP=0x0000000000000000, aBaseURI=0x0000000000000000, aIsInitialDocument=true, aCOEP=0x00007fffffff9d86, aTryToSaveOldPresentation=<unavailable>, aCheckPermitUnload=<unavailable>, aActor=0x0000000000000000) at nsDocShell.cpp:6588:16
    frame #30: 0x00007ffff332380c libxul.so`nsAppShellService::JustCreateTopWindow(this=<unavailable>, aParent=0x0000000000000000, aUrl=<unavailable>, aChromeMask=4161799686, aInitialWidth=<unavailable>, aInitialHeight=<unavailable>, aIsHiddenWindow=<unavailable>, aResult=<unavailable>) at nsAppShellService.cpp:760:22
    frame #31: 0x00007ffff3323b03 libxul.so`nsAppShellService::CreateTopLevelWindow(this=<unavailable>, aParent=0x0000000000000000, aUrl=<unavailable>, aChromeMask=4161799686, aInitialWidth=<unavailable>, aInitialHeight=<unavailable>, aResult=<unavailable>) at nsAppShellService.cpp:173:8
    frame #32: 0x00007ffff35aad11 libxul.so`nsAppStartup::CreateChromeWindow(this=<unavailable>, aParent=<unavailable>, aChromeFlags=4161799686, aOpenWindowInfo=0x0000000000000000, aCancel=<unavailable>, _retval=0x00007fffffff9ef8) at nsAppStartup.cpp:750:15
    frame #33: 0x00007ffff3627118 libxul.so`nsWindowWatcher::CreateChromeWindow(this=<unavailable>, aParentChrome=<unavailable>, aChromeFlags=<unavailable>, aOpenWindowInfo=<unavailable>, aResult=0x00007fffffff9fd0) at nsWindowWatcher.cpp:419:33
    frame #34: 0x00007ffff3626ae6 libxul.so`nsWindowWatcher::OpenWindowInternal(this=<unavailable>, aParent=0x0000000000000000, aUrl=0x00007fffffffa2d8, aName=0x00007fffffffa288, aFeatures=0x00007fffffffa278, aCalledFromJS=<unavailable>, aDialog=<unavailable>, aNavigate=<unavailable>, aArgv=<unavailable>, aIsPopupSpam=<unavailable>, aForceNoOpener=<unavailable>, aForceNoReferrer=<unavailable>, aPrintKind=<unavailable>, aLoadState=<unavailable>, aResult=<unavailable>) at nsWindowWatcher.cpp:947:12
    frame #35: 0x00007ffff3624d83 libxul.so`nsWindowWatcher::OpenWindow(this=0x00007fffe3f1bbe0, aParent=0x0000000000000000, aUrl=0x00007fffffffa2d8, aName=0x00007fffffffa288, aFeatures=0x00007fffffffa278, aArguments=<unavailable>, aResult=<unavailable>) at nsWindowWatcher.cpp:293:3
    frame #36: 0x00007ffff365c15b libxul.so`ShowProfileManager(aProfileSvc=<unavailable>, aNative=0x00007fffe8ce8ec0) at nsAppRunner.cpp:2553:27
    frame #37: 0x00007ffff365ad8f libxul.so`XREMain::XRE_mainStartup(bool*) [inlined] SelectProfile(aProfileSvc=<unavailable>, aNative=<unavailable>, aRootDir=<unavailable>, aLocalDir=<unavailable>, aProfile=<unavailable>, aWasDefaultSelection=<unavailable>) at nsAppRunner.cpp:0:7
    frame #38: 0x00007ffff365ab56 libxul.so`XREMain::XRE_mainStartup(this=<unavailable>, aExitFlag=<unavailable>) at nsAppRunner.cpp:4501:8
    frame #39: 0x00007ffff365fd00 libxul.so`XREMain::XRE_main(this=0x00007fffffffa500, argc=2, argv=0x00007fffffffb6f8, aConfig=0x00007fffffffa690) at nsAppRunner.cpp:5465:12
    frame #40: 0x00007ffff3660175 libxul.so`XRE_main(argc=<unavailable>, argv=<unavailable>, aConfig=<unavailable>) at nsAppRunner.cpp:5536:21
    frame #41: 0x00007ffff3665ff1 libxul.so`mozilla::BootstrapImpl::XRE_main(this=<unavailable>, argc=<unavailable>, argv=<unavailable>, aConfig=<unavailable>) at Bootstrap.cpp:45:12
    frame #42: 0x0000555555579140 firefox`main [inlined] do_main(argc=<unavailable>, argv=0x00007fffffffb6f8, envp=<unavailable>) at nsBrowserApp.cpp:225:22
    frame #43: 0x0000555555579076 firefox`main(argc=<unavailable>, argv=<unavailable>, envp=<unavailable>) at nsBrowserApp.cpp:392:16
```
</details>

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants