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

Workaround ODR violations in enum debuginfo #27689

Merged
merged 1 commit into from
Aug 16, 2015
Merged

Conversation

dotdash
Copy link
Contributor

@dotdash dotdash commented Aug 12, 2015

When using a generic enum type that was defined in an external crate,
our debuginfo currently claims that the concrete type (e.g. Option)
was defined in the current crate, where it was first used.

This means that if there are multiple crates that all use, for example,
Option values, they'll have conflicting debuginfo, each crate
claiming to have defined that type. This doesn't cause problems in
regular builds, but with LTO enabled, LLVM complains because it tries to
merge the debuginfo for those types and sees the ODR violations.

Since I couldn't find a way to get the file info for the external crate
that actually defined the enum, I'm working around the issue by using
"" as the file for enum types. We'll want to re-visit and fix
this later, but this at least this fixes the ICE. And with the file
being unknown instead of wrong, the debuginfo isn't really worse than
before either.

Fixes #26447

When using a generic enum type that was defined in an external crate,
our debuginfo currently claims that the concrete type (e.g. Option<i32>)
was defined in the current crate, where it was first used.

This means that if there are multiple crates that all use, for example,
Option<i32> values, they'll have conflicting debuginfo, each crate
claiming to have defined that type. This doesn't cause problems in
regular builds, but with LTO enabled, LLVM complains because it tries to
merge the debuginfo for those types and sees the ODR violations.

Since I couldn't find a way to get the file info for the external crate
that actually defined the enum, I'm working around the issue by using
"<unknown>" as the file for enum types. We'll want to re-visit and fix
this later, but this at least this fixes the ICE. And with the file
being unknown instead of wrong, the debuginfo isn't really worse than
before either.

Fixes rust-lang#26447
@rust-highfive
Copy link
Collaborator

r? @jroesch

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

@dotdash
Copy link
Contributor Author

dotdash commented Aug 12, 2015

r? @michaelwoerister

@michaelwoerister
Copy link
Member

@dotdash: From your point view, is there a reason why you'd rather associate enum types to a file named <unknown> instead of not associating them with any source location at all (like we do with structs, i.e. use NO_FILE_METADATA)? Because if there isn't then I'd rather leave off the source location altogether.

@dotdash
Copy link
Contributor Author

dotdash commented Aug 14, 2015

Unfortunately, unless I missed something, this kind of debuginfo requires file metadata, otherwise I would have just removed it.

@michaelwoerister
Copy link
Member

Yes, I think I remember now that I've run into this oddity myself some time ago. We should try and this restriction removed from LLVM.
Thanks for the PR!

@michaelwoerister
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 16, 2015

📌 Commit d17d2dd has been approved by michaelwoerister

@bors
Copy link
Contributor

bors commented Aug 16, 2015

⌛ Testing commit d17d2dd with merge 2f60268...

bors added a commit that referenced this pull request Aug 16, 2015
When using a generic enum type that was defined in an external crate,
our debuginfo currently claims that the concrete type (e.g. Option<i32>)
was defined in the current crate, where it was first used.

This means that if there are multiple crates that all use, for example,
Option<i32> values, they'll have conflicting debuginfo, each crate
claiming to have defined that type. This doesn't cause problems in
regular builds, but with LTO enabled, LLVM complains because it tries to
merge the debuginfo for those types and sees the ODR violations.

Since I couldn't find a way to get the file info for the external crate
that actually defined the enum, I'm working around the issue by using
"<unknown>" as the file for enum types. We'll want to re-visit and fix
this later, but this at least this fixes the ICE. And with the file
being unknown instead of wrong, the debuginfo isn't really worse than
before either.

Fixes #26447
@bors bors merged commit d17d2dd into rust-lang:master Aug 16, 2015
@pnkfelix
Copy link
Member

Was this the last known bug blocking make check from working with --enable-debug on all targets?

(I would like to try to get a buildbot machine going that tests --enable-debug, as discussed on #27010 and #17166 )

@dotdash
Copy link
Contributor Author

dotdash commented Aug 17, 2015

@pnkfelix it was the last one I'm aware of.

@dotdash dotdash deleted the die_odr branch January 15, 2016 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sepcomp-lib-lto test fails on master with --enable-debug
6 participants