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

regression 1.50: Duplicate symbol linker error #80951

Closed
rylev opened this issue Jan 12, 2021 · 13 comments
Closed

regression 1.50: Duplicate symbol linker error #80951

rylev opened this issue Jan 12, 2021 · 13 comments
Assignees
Labels
E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@rylev
Copy link
Member

rylev commented Jan 12, 2021

In the latest crater run, a crate that compiles on 1.49, did not compile on 1.50 with a linker error you can see here.

@rustbot modify labels: +regression-from-stable-to-beta

@rustbot rustbot added regression-from-stable-to-beta Performance or correctness regression from stable to beta. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 12, 2021
@nagisa
Copy link
Member

nagisa commented Jan 12, 2021

Seems like a not-genuine-regression, its just a case of cargo building and linking two different versions of the same crate (secp256k1), which appears to export no_mangle symbols if the error message looks right.

@Mark-Simulacrum Mark-Simulacrum added this to the 1.50.0 milestone Jan 14, 2021
@wesleywiser
Copy link
Member

Discussed in the release triage meeting. The above theory sounds plausible but we want to make sure there's not an actual issue here.

One possibility @Mark-Simulacrum mentioned is that our rules around no_mangle symbols being exported has changed somehow in this release. @petrochenkov do you happen to know if something like that has changed in this release?

@petrochenkov
Copy link
Contributor

No, I didn't follow the no_mangle changes.

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jan 27, 2021
@apiraino
Copy link
Contributor

Removing I-prioritize label as it looks this is on the radar along with the other crater run issues

@Mark-Simulacrum Mark-Simulacrum added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 27, 2021
@Mark-Simulacrum
Copy link
Member

I can confirm this is a regression from 1.49. However, I'm having trouble bisecting this - nightlies seem to fail with the linker error for a while back; I ran bisect up to 2020-07-20.

Not sure what to do - maybe worth pinging cleanup group to try to minify at least.

@Mark-Simulacrum Mark-Simulacrum added E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example P-high High priority labels Jan 27, 2021
@apiraino
Copy link
Contributor

@rustbot ping cleanup

@rustbot rustbot added the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label Jan 27, 2021
@rustbot
Copy link
Collaborator

rustbot commented Jan 27, 2021

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @JamesPatrickGill @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @shekohex @sinato @smmalis37 @steffahn @Stupremee @tamuhey @turboladen @woshilapin @yerke

@steffahn
Copy link
Member

https://github.com/devrandom/utxo-oracle does compile for me with stable i.e. 1.49. If fails with nightly with beta but also with 1.48 or 1.47.

@steffahn
Copy link
Member

So this is not a regression, AFAICT.

With stable 1.49 (e1884a8) as well as current master (613ef74), the linking error when compiling utxo-oracle only appears with a compiler that was built with the channel = "dev" or channel = "beta" option set (in config.toml). Using channel = "stable" does not result in a linker error. (I didn’t actually test "beta" on e1884a8.)

I also tested current beta (1cd0303) and it doesn’t produce the problem with channel = "stable", so it seems likely that 1.50 won’t regress this build.

Finally, as a double check, 1.48 (7eac88a) does produce the linking failure for me even with channel = "stable". I didn’t go further yet in bisecting where the error disappeared between 1.48 and 1.49.

@Mark-Simulacrum
Copy link
Member

Interesting. Can we try to figure out what is causing that dependence on the channel?

@apiraino apiraino added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Feb 11, 2021
@pnkfelix
Copy link
Member

pnkfelix commented Dec 2, 2022

Visiting for P-high review.

There seems to be a lot of indication that this is not a true regression, but the oddities in terms of how the channel name is affecting the results here is worrisome.

Self-assigning for follow-up investigation.

@pnkfelix pnkfelix self-assigned this Dec 2, 2022
@pnkfelix
Copy link
Member

pnkfelix commented Dec 2, 2022

Also, review determined this no longer warrants P-high priority, not without stronger signal that this represents an actual regression.

@rustbot label: -P-high +P-medium

@rustbot rustbot added P-medium Medium priority and removed P-high High priority labels Dec 2, 2022
@jackh726
Copy link
Contributor

No additional info here in about 3 years, closing for no reproduction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants