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.

Sign up for GitHub

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

Jump to bottom

Add a test for issue #33172 #97085

Merged
merged 7 commits into from
Aug 5, 2022
Merged

Add a test for issue #33172 #97085

merged 7 commits into from
Aug 5, 2022

Conversation

rylev
Copy link
Member

@rylev rylev commented May 16, 2022 •
edited by wesleywiser
Loading

Adds a test confirming that #33172 has been fixed.

CDB has some surprising results as it looks like the supposedly unmangled static's symbol name is prefixed when it shouldn't be.

r? @wesleywiser

Closes #33172

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 16, 2022
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 23, 2022
@Dylan-DPC Dylan-DPC added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 12, 2022
@rylev rylev mentioned this pull request Jun 20, 2022
Closed
@rylev rylev 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 Jun 20, 2022
@rylev rylev force-pushed the test-issue-33172 branch from 9fbf419 to 3ea686f Compare June 20, 2022 16:47
@wesleywiser
Copy link
Member

Thanks @rylev!

@bors r+

@bors
Copy link
Contributor

bors commented Jun 20, 2022

📌 Commit 3ea686f has been approved by wesleywiser

@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 Jun 20, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 21, 2022
@matthiaskrgr
Rollup merge of rust-lang#97085 - rylev:test-issue-33172, r=wesleywiser
7ec1840 
Add a test for issue rust-lang#33172

Adds a test confirming that rust-lang#33172 has been fixed.

CDB has some surprising results as it looks like the supposedly unmangled static's symbol name is prefixed when it shouldn't be.

r? ``@wesleywiser``

Closes rust-lang#33172
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 21, 2022
Closed
@JohnTitor
Copy link
Member

Failed in rollup: #98321 (comment)
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 21, 2022
@rylev rylev force-pushed the test-issue-33172 branch from c443851 to 1a25ac9 Compare June 21, 2022 16:01
@rust-lang rust-lang deleted a comment from rust-log-analyzer Jun 21, 2022
@rylev rylev 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 Jun 22, 2022
@rylev rylev requested a review from wesleywiser June 23, 2022 10:02
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@wesleywiser
Copy link
Member

@bors r+

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 25, 2022
@Dylan-DPC
Rollup merge of rust-lang#97085 - rylev:test-issue-33172, r=wesleywiser
7070a3f 
Add a test for issue rust-lang#33172

Adds a test confirming that rust-lang#33172 has been fixed.

CDB has some surprising results as it looks like the supposedly unmangled static's symbol name is prefixed when it shouldn't be.

r? ```@wesleywiser```

Closes rust-lang#33172
@Dylan-DPC Dylan-DPC mentioned this pull request Jun 25, 2022
Closed
@JohnTitor
Copy link
Member

Failed in rollup: #98480 (comment)
@bors r- rollup=iffy

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 25, 2022
@wesleywiser
Copy link
Member

Looks like this might be an issue with older gdb version?

NOTE: compiletest thinks it is using GDB version 7011001

We might need to restrict this to a newer version via the min-gdb-version compile test directive.

@rylev rylev force-pushed the test-issue-33172 branch from 23d325e to 6cc3b41 Compare August 4, 2022 09:54
@rylev rylev 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 Aug 4, 2022
@rylev
Copy link
Member Author

rylev commented Aug 4, 2022

@wesleywiser I pushed a change to set the min-version to higher than the default. Unfortunately, I'm having a hard time getting an old version of gdb working with my local test setup to test locally. It would be nice if we could run the debugger suite before it goes into a rollup and potentially fails.

@wesleywiser
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Aug 4, 2022

📌 Commit 6cc3b41 has been approved by wesleywiser

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 Aug 4, 2022
@bors
Copy link
Contributor

bors commented Aug 5, 2022

⌛ Testing commit 6cc3b41 with merge 6bcf01a...

@bors
Copy link
Contributor

bors commented Aug 5, 2022

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing 6bcf01a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 5, 2022
@bors bors merged commit 6bcf01a into rust-lang:master Aug 5, 2022
@rustbot rustbot added this to the 1.64.0 milestone Aug 5, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6bcf01a): comparison url.

Instruction count

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regression found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 1.2% 1.2% 1
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) N/A N/A 0

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: 😿 relevant regression found
mean1 max count2
Regressions 😿
(primary)		 2.1% 2.1% 1
Regressions 😿
(secondary)		 3.0% 3.0% 1
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) 2.1% 2.1% 1

Cycles

Results
  • Primary benchmarks: 🎉 relevant improvement found
  • Secondary benchmarks: 🎉 relevant improvement found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -2.9% -2.9% 1
Improvements 🎉
(secondary)		 -3.8% -3.8% 1
All 😿🎉 (primary) -2.9% -2.9% 1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2 ↩3

  2. number of relevant changes ↩ ↩2 ↩3

@wesleywiser
Copy link
Member

I think it's safe to ignore this perf result since this PR only adds a debuginfo test and touches no other code so by definition, it couldn't have had an effect on compiler performance.

@rylev rylev deleted the test-issue-33172 branch August 5, 2022 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wesleywiser wesleywiser Awaiting requested review from wesleywiser

Assignees

@wesleywiser wesleywiser

Labels
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.

no_mangle static symbols have improper debug info
10 participants
@rylev @wesleywiser @bors @JohnTitor @rust-log-analyzer @rust-timer @apiraino @rust-highfive @rustbot @Dylan-DPC