Add a backtrace to Allocation, display it in leak reports - #109061

saethlin · 2023-03-12T22:32:07Z

Information like this from diagnostics is indispensable for diagnosing problems that are difficult to reproduce such as https://github.com/rust-lang/miri-test-libstd/actions/runs/4395316008/jobs/7697019211#step:4:770 (which has not been reproduced or diagnosed).

rustbot · 2023-03-12T22:32:12Z

r? @michaelwoerister

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

rustbot · 2023-03-12T22:32:34Z

Failed to set assignee to ghost: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

saethlin · 2023-03-12T22:32:41Z

@bors try @rust-timer queue

bors · 2023-03-12T22:32:49Z

⌛ Trying commit f333ee50016e250b849f35947483f3e28989e3e0 with merge 0a70014f8cc287d3eee38518da13127b0eca278e...

bors · 2023-03-13T01:13:09Z

☀️ Try build successful - checks-actions
Build commit: 0a70014f8cc287d3eee38518da13127b0eca278e (0a70014f8cc287d3eee38518da13127b0eca278e)

bors · 2023-03-13T01:13:09Z

☀️ Try build successful - checks-actions
Build commit: 0a70014f8cc287d3eee38518da13127b0eca278e (0a70014f8cc287d3eee38518da13127b0eca278e)

rust-timer · 2023-03-13T06:18:41Z

Finished benchmarking commit (0a70014f8cc287d3eee38518da13127b0eca278e): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.8%	[0.6%, 1.0%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.6%	[-1.6%, -1.6%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.1%	[4.1%, 4.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

saethlin · 2023-03-13T17:14:31Z

Based on the cachegrind diff, I believe the regressions above are legitimate and caused by this change. This new commit may fix them.

saethlin · 2023-03-13T17:21:28Z

@bors try @rust-timer queue

bors · 2023-03-13T17:21:37Z

⌛ Trying commit c994d6d0f210fd2c4b6bca144347f90299e010cc with merge 77af1a06420c9a7b396987192c44bcd1e4b46bda...

bors · 2023-03-13T20:01:58Z

☀️ Try build successful - checks-actions
Build commit: 77af1a06420c9a7b396987192c44bcd1e4b46bda (77af1a06420c9a7b396987192c44bcd1e4b46bda)

bors · 2023-03-13T20:01:58Z

☀️ Try build successful - checks-actions
Build commit: 77af1a06420c9a7b396987192c44bcd1e4b46bda (77af1a06420c9a7b396987192c44bcd1e4b46bda)

rust-timer · 2023-03-13T21:40:46Z

Finished benchmarking commit (77af1a06420c9a7b396987192c44bcd1e4b46bda): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.5%, 0.7%]	3
Regressions ❌ (secondary)	0.9%	[0.8%, 0.9%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.4%, -0.2%]	4
All ❌✅ (primary)	0.6%	[0.5%, 0.7%]	3

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.0%	[1.0%, 1.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.0%	[1.3%, 4.9%]	7
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.0%	[1.3%, 4.9%]	7

saethlin · 2023-03-14T01:09:51Z

Based on the cachegrind diffs all the perf results are optimization instability, they don't have anything to with Allocation or CTFE.

saethlin · 2023-04-11T02:52:27Z

I added a flag that does the thing and a test for it. I "blessed" the test with x run and some copy+paste, since x test miri currently doesn't work.

saethlin · 2023-04-16T13:18:05Z

I rebased to pull in the fix that makes Miri tests work again, and re-blessed the middle commit.

oli-obk · 2023-04-16T13:30:43Z

@bors r+

bors · 2023-04-16T13:30:45Z

📌 Commit cd305dd21938dcce7d79a24b17949327ee088ba1 has been approved by oli-obk

It is now in the queue for this repository.

saethlin · 2023-04-16T13:51:30Z

32-bit tests 🤦

Co-authored-by: Ralf Jung <post@ralfj.de>

RalfJung · 2023-04-16T14:17:29Z

@bors r=oli-obk

bors · 2023-04-16T14:17:31Z

📌 Commit fb68292 has been approved by oli-obk

It is now in the queue for this repository.

bors · 2023-04-17T00:22:31Z

⌛ Testing commit fb68292 with merge 23eb90f...

bors · 2023-04-17T02:36:31Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 23eb90f to master...

bors · 2023-04-17T02:36:31Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 23eb90f to master...

rust-timer · 2023-04-17T03:54:27Z

Finished benchmarking commit (23eb90f): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.8%	[2.1%, 3.6%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

# Objective Make it possible to run miri with memory leak detection enabled. It is enabled by default, so it reports errors whenever I run it locally. And running it in CI should help prevent any new leaks from being introduced. See bevyengine#4310 and bevyengine#4959 for previous attempts to enable this. Note that miri has added backtraces to leaked allocations since then (rust-lang/rust#109061), so leaks are much easier to diagnose. ## Solution The first leak: When trying to drop non-send data on the wrong thread, the allocation for the value was being leaked. We can't drop the value from the wrong thread, but we could still deallocate its storage. Rework the thread check in `NonSendData::drop` so that it still deallocates, setting `present = false` to ensure the value's `drop` is not called. Change the `panic!` to a `warn!` so that we can have the check work the same way during unwinding. The second leak: The tests for `Interned` intentionally leak data! So we can't exactly "fix" them, but I'd like miri to stop failing on them. Then I realized that miri does not report leaks for our use of `Interned` for things like `ScheduleLabel`. That's because those values are stored in a `static`, so are still reachable. So, add a `static` to each of the tests with intentional leaks and store the leaked values there. miri will consider those values reachable, and not report any leaks. --------- Co-authored-by: Daniel Skates <zeophlite@gmail.com>

rustbot assigned michaelwoerister Mar 12, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 12, 2023

saethlin unassigned michaelwoerister Mar 12, 2023

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 12, 2023

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 13, 2023

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 13, 2023

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Mar 13, 2023

saethlin force-pushed the leak-backtraces branch from 3ebe6e7 to ba7675b Compare March 16, 2023 02:48

saethlin force-pushed the leak-backtraces branch from 83b9000 to 6770c2c Compare April 11, 2023 02:50

Report a backtrace for memory leaks under Miri

606ca4d

saethlin force-pushed the leak-backtraces branch from 6770c2c to cd305dd Compare April 16, 2023 13:17

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 Apr 16, 2023

This comment has been minimized.

Sign in to view

saethlin and others added 2 commits April 16, 2023 10:00

Add a flag to disable leak backtraces

cbc7f94

Improve doc comment of AllocExtra's backtrace

fb68292

Co-authored-by: Ralf Jung <post@ralfj.de>

saethlin force-pushed the leak-backtraces branch from cd305dd to fb68292 Compare April 16, 2023 14:00

bors added merged-by-bors This PR was explicitly merged by bors. labels Apr 17, 2023

bors merged commit 23eb90f into rust-lang:master Apr 17, 2023

rustbot added this to the 1.71.0 milestone Apr 17, 2023

saethlin deleted the leak-backtraces branch April 17, 2023 14:16

oli-obk mentioned this pull request Apr 23, 2023

Improve memory leak identification rust-lang/miri#1481

Closed

SpecificProtagonist mentioned this pull request May 20, 2026

ignoring memory leak in specific test rust-lang/miri#1754

Closed

ivelieu mentioned this pull request Jun 25, 2026

Ignoring/expecting memory leaks in certain tests/functions rust-lang/miri#5130

Open

chescock mentioned this pull request Jul 13, 2026

Resolve memory leaks in tests reported by miri bevyengine/bevy#24979

Merged

Uh oh!

Uh oh!

Conversation

saethlin commented Mar 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 12, 2023

Uh oh!

rustbot commented Mar 12, 2023

Uh oh!

saethlin commented Mar 12, 2023

Uh oh!

This comment has been minimized.

bors commented Mar 12, 2023

Uh oh!

bors commented Mar 13, 2023

Uh oh!

bors commented Mar 13, 2023

Uh oh!

This comment has been minimized.

rust-timer commented Mar 13, 2023

Overall result: ❌ regressions - no action needed

Uh oh!

saethlin commented Mar 13, 2023

Uh oh!

saethlin commented Mar 13, 2023

Uh oh!

This comment has been minimized.

bors commented Mar 13, 2023

Uh oh!

bors commented Mar 13, 2023

Uh oh!

bors commented Mar 13, 2023

Uh oh!

This comment has been minimized.

rust-timer commented Mar 13, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Uh oh!

saethlin commented Mar 14, 2023

Uh oh!

saethlin commented Apr 11, 2023

Uh oh!

saethlin commented Apr 16, 2023

Uh oh!

oli-obk commented Apr 16, 2023

Uh oh!

bors commented Apr 16, 2023

Uh oh!

This comment has been minimized.

saethlin commented Apr 16, 2023

Uh oh!

RalfJung commented Apr 16, 2023

Uh oh!

bors commented Apr 16, 2023

Uh oh!

bors commented Apr 17, 2023

Uh oh!

bors commented Apr 17, 2023

Uh oh!

bors commented Apr 17, 2023

Uh oh!

rust-timer commented Apr 17, 2023

Overall result: no relevant changes - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

saethlin commented Mar 12, 2023 •

edited

Loading