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

ICE in debug generation with match guard. #34569

Closed
meh opened this issue Jun 30, 2016 · 10 comments
Closed

ICE in debug generation with match guard. #34569

meh opened this issue Jun 30, 2016 · 10 comments
Assignees
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@meh
Copy link
Contributor

meh commented Jun 30, 2016

This is the full project that causes that failure, make sure to use that commit.

The culprit line is here.

Inside a match:

e if display.randr.as_ref().map_or(false, |rr| e == rr.event(xrandr::RRScreenChangeNotify))

Results in:

!dbg attachment points at wrong subprogram for function
!21995 = distinct !DISubprogram(name: "{{closure}}", linkageName: "_ZN12screenruster6locker8{{impl}}5spawn11{{closure}}11{{closure}}E", scope: !21991, file: !352, line: 214, type: !21992, isLocal: true, isDefinition: true, scopeLine: 214, flags: DIFlagPrototyped, isOptimized: false, templateParams: !457, variables: !457)
i1 (i8*, %"locker::display::Extension"*)* @"_ZN12screenruster6locker6Locker5spawn28_$u7b$$u7b$closure$u7d$$u7d$28_$u7b$$u7b$closure$u7d$$u7d$17hb41874d5593ec665E"
  call void @llvm.dbg.declare(metadata %closure.114** %__debuginfo_env_ptr, metadata !59153, metadata !59154), !dbg !59155
!59155 = !DILocation(line: 214, scope: !21990)
!21990 = distinct !DISubprogram(name: "{{closure}}", linkageName: "_ZN12screenruster6locker8{{impl}}5spawn11{{closure}}11{{closure}}E", scope: !21991, file: !352, line: 214, type: !21992, isLocal: true, isDefinition: true, scopeLine: 214, flags: DIFlagPrototyped, isOptimized: false, templateParams: !457, variables: !457)
!21990 = distinct !DISubprogram(name: "{{closure}}", linkageName: "_ZN12screenruster6locker8{{impl}}5spawn11{{closure}}11{{closure}}E", scope: !21991, file: !352, line: 214, type: !21992, isLocal: true, isDefinition: true, scopeLine: 214, flags: DIFlagPrototyped, isOptimized: false, templateParams: !457, variables: !457)
LLVM ERROR: Broken function found, compilation aborted!

If I change that to:

e if e == display.randr.as_ref().map_or(0, |rr| rr.event(xrandr::RRScreenChangeNotify))

Then it compiles.

I tried to come up with a reduced test case but failed, so sorry for the full project as test case.

@nagisa
Copy link
Member

nagisa commented Jun 30, 2016

Building the project at the specified commit fails with

error: Unable to update https://github.com/meh/x11-rs?branch=dpms#cafb1f92

The branch dpms seems to be deleted for the repository in question and referenced commit does not exist anymore.

I tried using master branch but alas.

@meh
Copy link
Contributor Author

meh commented Jun 30, 2016

@nagisa my bad, forgot to cargo update before committing, I made a rustc-bug branch on screenruster, try using that, also updated the OP.

@nagisa
Copy link
Member

nagisa commented Jun 30, 2016

Minimised:

fn main() {
    match 0 {
        e if (|| { e == 0 })() => unimplemented!(),
        1 => unimplemented!(),
        _ => unimplemented!()
    }
}

On nightly, when compiled with debug info rustc test.rs -g fails with

!dbg attachment points at wrong subprogram for function
!64 = distinct !DISubprogram(name: "{{closure}}", linkageName: "_ZN4test4main11{{closure}}E", scope: !61, file: !47, line: 4, type: !62, isLocal: true, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false, templateParams: !51, variables: !51)
i1 (i8*, i1)* @"_ZN4test4main28_$u7b$$u7b$closure$u7d$$u7d$17h6ca029a2196c88a1E"
  call void @llvm.dbg.declare(metadata %closure** %__debuginfo_env_ptr, metadata !144, metadata !145), !dbg !146
!146 = !DILocation(line: 4, scope: !60)
!60 = distinct !DISubprogram(name: "{{closure}}", linkageName: "_ZN4test4main11{{closure}}E", scope: !61, file: !47, line: 4, type: !62, isLocal: true, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false, templateParams: !51, variables: !51)
!60 = distinct !DISubprogram(name: "{{closure}}", linkageName: "_ZN4test4main11{{closure}}E", scope: !61, file: !47, line: 4, type: !62, isLocal: true, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false, templateParams: !51, variables: !51)
LLVM ERROR: Broken function found, compilation aborted!

The problem seems to be the use of variable bound in match arm (the e) inside a closure that appears in match arm’s if portion.

Works with MIR (-Z orbit).

@nagisa
Copy link
Member

nagisa commented Jun 30, 2016

cc @michaelwoerister

@meh
Copy link
Contributor Author

meh commented Jun 30, 2016

Ah it's a regression, that's why I couldn't come up with a reduced test-case, didn't realize I was using stable to compile the test-case 🐼

@eddyb eddyb added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Jun 30, 2016
@michaelwoerister
Copy link
Member

Thanks for the report and the reduced test case. I'll look into it.

@arielb1 arielb1 added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Jul 7, 2016
@arielb1
Copy link
Contributor

arielb1 commented Jul 7, 2016

1.11 is now beta

@arielb1 arielb1 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 7, 2016
@michaelwoerister
Copy link
Member

I was able to reproduce this and it is very strange that there are two DISubprogram debuginfo entries describing the same closure. This should not happen.

@michaelwoerister michaelwoerister self-assigned this Jul 7, 2016
@michaelwoerister
Copy link
Member

For some reason (which I haven't looked into yet) closure::trans_closure_expr is called twice for the closure in the match guard. I would guess that the error would go away if we prevented closures from being translated more than once. I'll look into it some more tomorrow.

@michaelwoerister
Copy link
Member

OK, I can confirm that prevent double-translation of closures solves this problem.

@rust-lang/compiler Does anybody knowledgeable about the non-MIR match code know whether it is legitimate that the guard is translated twice?

bors added a commit that referenced this issue Jul 9, 2016
trans: Make sure that closures only get translated once.

Fixes #34569.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. 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

5 participants