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

Port backtrace's line-tables-only test over to rustc #122918

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

jieyouxu
Copy link
Contributor

@jieyouxu jieyouxu commented Mar 22, 2024

Part of #122899.

@rustbot
Copy link
Collaborator

rustbot commented Mar 22, 2024

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 22, 2024
@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from 4cc07d7 to f85a455 Compare March 22, 2024 22:54
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from 55327df to 1a3a3df Compare March 23, 2024 00:21
@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Contributor Author

Where line numbers :(

@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from 1a3a3df to 599207e Compare March 23, 2024 01:12
@workingjubilee
Copy link
Contributor

workingjubilee commented Mar 23, 2024

it is starting to seem like "reusing the existing c file" isn't going to work if it requires reusing the existing gcc which might be trash (at least, at generating debuginfo).

@jieyouxu
Copy link
Contributor Author

It was because ui test mode by default has -C strip=debuginfo which... strips debuginfo. If I override it with -C strip=none, then we have debuginfo :)

@workingjubilee
Copy link
Contributor

oh nice.

@jieyouxu jieyouxu marked this pull request as draft March 23, 2024 18:44
@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from 599207e to a35f42c Compare March 23, 2024 18:45
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Mar 23, 2024
@rust-log-analyzer

This comment has been minimized.

@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from 35924aa to 698b9a3 Compare March 24, 2024 16:13
@jieyouxu jieyouxu removed the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Mar 24, 2024
@jieyouxu jieyouxu marked this pull request as ready for review March 24, 2024 16:14
@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from 698b9a3 to c98c3b6 Compare March 24, 2024 16:17
@rust-log-analyzer

This comment has been minimized.

@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from c98c3b6 to d5ba1e7 Compare March 24, 2024 16:45
@jieyouxu jieyouxu marked this pull request as draft March 25, 2024 21:47
@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from d5ba1e7 to 088b979 Compare March 25, 2024 22:07
@jieyouxu jieyouxu marked this pull request as ready for review March 25, 2024 22:07
@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from 340da7e to a07f1f0 Compare April 6, 2024 19:05
@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Contributor Author

jieyouxu commented Apr 6, 2024

So the helper does have line-tables, bar and baz:

{ file: "C:\a\rust\rust\tests\ui\debuginfo\auxiliary\line-tables-only-helper.rs", line: 5 },
{ file: "C:\a\rust\rust\tests\ui\debuginfo\auxiliary\line-tables-only-helper.rs", line: 10 },
backtrace does not contain expected name foo

but where did my foo go :ferrisAware:? @workingjubilee do you have any clue why i686-msvc would yeet foo's line table entry?

(the helper is this:)

//@ compile-flags: -Cstrip=none -Cdebuginfo=line-tables-only

#[no_mangle]
pub fn baz<F>(mut cb: F, data: u32) where F: FnMut(u32) {
    cb(data);
}

#[no_mangle]
pub fn bar<F>(cb: F, data: u32) where F: FnMut(u32) {
    baz(cb, data);
}

#[no_mangle]
pub fn foo<F>(cb: F, data: u32) where F: FnMut(u32) {
    bar(cb, data);
}

pub fn capture_backtrace() -> std::backtrace::Backtrace {
    let mut bt = None;
    foo(|_| bt = Some(std::backtrace::Backtrace::capture()), 42);
    bt.unwrap()
}

@rust-log-analyzer

This comment has been minimized.

@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from d08d266 to 00d3639 Compare April 6, 2024 21:11
@workingjubilee
Copy link
Contributor

terrifying.

@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Contributor Author

jieyouxu commented Apr 6, 2024

Somehow made it even worse on i686-msvc. I'll fix this tmrw 😆

@jieyouxu jieyouxu force-pushed the port-backtrace-line-tables-only branch from 5c400c3 to 6065096 Compare April 7, 2024 13:19
@rust-cloud-vms rust-cloud-vms bot force-pushed the port-backtrace-line-tables-only branch from 6065096 to d4aeff7 Compare April 7, 2024 15:25
Comment on lines +41 to +46
// FIXME(jieyouxu): for some forsaken reason on i686-msvc `foo` doesn't have an entry in the
// line tables?
#[cfg(not(all(target_pointer_width = "32", target_env = "msvc")))]
{
assert_contains(&backtrace, "foo", "line-tables-only-helper.rs", 5);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ferrisExorcism

@jieyouxu
Copy link
Contributor Author

jieyouxu commented Apr 7, 2024

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 7, 2024
@workingjubilee
Copy link
Contributor

Looks good to me, but in practice we're playing roulette.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 9, 2024

📌 Commit d4aeff7 has been approved by workingjubilee

It is now in the queue for this repository.

@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 Apr 9, 2024
@jieyouxu
Copy link
Contributor Author

jieyouxu commented Apr 9, 2024

I'm going to mark this as iffy because... I kinda expect this to bounce in full build CI
@bors rollup=iffy

@bors
Copy link
Contributor

bors commented Apr 9, 2024

⌛ Testing commit d4aeff7 with merge 493ef5d...

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2024
…only, r=workingjubilee

Port backtrace's `line-tables-only` test over to rustc

Part of rust-lang#122899.
@pietroalbini
Copy link
Member

@bors retry

Yielding priority to the stable release.

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Apr 10, 2024

⌛ Testing commit d4aeff7 with merge 93c131e...

@bors
Copy link
Contributor

bors commented Apr 10, 2024

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing 93c131e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 10, 2024
@bors bors merged commit 93c131e into rust-lang:master Apr 10, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 10, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (93c131e): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -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.2% [0.2%, 0.2%] 1
Regressions ❌
(secondary)
2.0% [0.2%, 3.7%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [0.2%, 0.2%] 1

Max RSS (memory usage)

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

Cycles

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

Binary size

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

Bootstrap: 673.348s -> 674.369s (0.15%)
Artifact size: 318.36 MiB -> 318.42 MiB (0.02%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants