Skip to content

Generator drop glue jumps from is_cleanup:false -> is_cleanup:true and the other way #58892

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
bjorn3 opened this issue Mar 3, 2019 · 3 comments
Closed

Generator drop glue jumps from is_cleanup:false -> is_cleanup:true and the other way #58892

bjorn3 opened this issue Mar 3, 2019 · 3 comments
Assignees
@matthewjasper
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bjorn3
Copy link
Member

bjorn3 commented Mar 3, 2019 

#![feature(generators, generator_trait)]

fn main() {
    let mut foo = || {
        yield;
    };
}

I only tested this with -Cpanic=abort.

Wrong jumps: bb1 -> bb5, bb4 -> bb6, bb5 -> bb2.

Note: made mir dump below using println!("{:#?}"), because rustc_mir::util::write_mir_pretty doesn't work correctly for shims.

Mir for `Instance { def: DropGlue(DefId(2/0:691 ~ core[479c]::ptr[0]::real_drop_in_place[0]), Some([generator@/Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6 {()}])), substs: [[generator@/Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6 {()}]] } _ZN4core3ptr18real_drop_in_place17h0fdaf8ce1f626024E` 
Mir {
    basic_blocks: [
        BasicBlockData {
            statements: [],
            terminator: Some(
                Terminator {
                    source_info: SourceInfo {
                        span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
                        scope: scope[0]
                    },
                    kind: switchInt(((*_1).0: u32)) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]
                }
            ),
            is_cleanup: false
        },
        BasicBlockData {
            statements: [],
            terminator: Some(
                Terminator {
                    source_info: SourceInfo {
                        span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:10:5: 10:6,
                        scope: scope[0]
                    },
                    kind: goto -> bb5
                }
            ),
            is_cleanup: false
        },
        BasicBlockData {
            statements: [],
            terminator: Some(
                Terminator {
                    source_info: SourceInfo {
                        span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
                        scope: scope[0]
                    },
                    kind: return
                }
            ),
            is_cleanup: false
        },
        BasicBlockData {
            statements: [],
            terminator: Some(
                Terminator {
                    source_info: SourceInfo {
                        span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
                        scope: scope[0]
                    },
                    kind: return
                }
            ),
            is_cleanup: false
        },
        BasicBlockData {
            statements: [],
            terminator: Some(
                Terminator {
                    source_info: SourceInfo {
                        span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
                        scope: scope[0]
                    },
                    kind: goto -> bb6
                }
            ),
            is_cleanup: false
        },
        BasicBlockData {
            statements: [],
            terminator: Some(
                Terminator {
                    source_info: SourceInfo {
                        span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:10:5: 10:6,
                        scope: scope[0]
                    },
                    kind: goto -> bb2
                }
            ),
            is_cleanup: true
        },
        BasicBlockData {
            statements: [],
            terminator: Some(
                Terminator {
                    source_info: SourceInfo {
                        span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
                        scope: scope[0]
                    },
                    kind: goto -> bb3
                }
            ),
            is_cleanup: true
        },
        BasicBlockData {
            statements: [
                StorageLive(_3)
            ],
            terminator: Some(
                Terminator {
                    source_info: SourceInfo {
                        span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
                        scope: scope[0]
                    },
                    kind: goto -> bb1
                }
            ),
            is_cleanup: false
        },
        BasicBlockData {
            statements: [],
            terminator: Some(
                Terminator {
                    source_info: SourceInfo {
                        span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
                        scope: scope[0]
                    },
                    kind: return
                }
            ),
            is_cleanup: false
        }
    ],
    phase: Validated,
    source_scopes: [
        SourceScopeData {
            span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
            parent_scope: None
        }
    ],
    source_scope_local_data: Set(
        [
            SourceScopeLocalData {
                lint_root: HirId {
                    owner: DefIndex(0:3),
                    local_id: 8
                },
                safety: Safe
            }
        ]
    ),
    promoted: [],
    yield_ty: None,
    generator_drop: None,
    generator_layout: Some(
        GeneratorLayout {
            fields: []
        }
    ),
    local_decls: [
        LocalDecl {
            mutability: Mut,
            is_user_variable: None,
            internal: false,
            is_block_tail: None,
            ty: (),
            user_ty: UserTypeProjections {
                contents: []
            },
            name: None,
            source_info: SourceInfo {
                span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
                scope: scope[0]
            },
            visibility_scope: scope[0]
        },
        LocalDecl {
            mutability: Mut,
            is_user_variable: None,
            internal: false,
            is_block_tail: None,
            ty: *mut [generator@/Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6 {()}],
            user_ty: UserTypeProjections {
                contents: []
            },
            name: None,
            source_info: SourceInfo {
                span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
                scope: scope[0]
            },
            visibility_scope: scope[0]
        },
        LocalDecl {
            mutability: Mut,
            is_user_variable: None,
            internal: false,
            is_block_tail: None,
            ty: (),
            user_ty: UserTypeProjections {
                contents: []
            },
            name: None,
            source_info: SourceInfo {
                span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:9:9: 9:14,
                scope: scope[0]
            },
            visibility_scope: scope[0]
        },
        LocalDecl {
            mutability: Mut,
            is_user_variable: None,
            internal: false,
            is_block_tail: None,
            ty: (),
            user_ty: UserTypeProjections {
                contents: []
            },
            name: None,
            source_info: SourceInfo {
                span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:9:9: 9:14,
                scope: scope[0]
            },
            visibility_scope: scope[0]
        },
        LocalDecl {
            mutability: Mut,
            is_user_variable: None,
            internal: false,
            is_block_tail: None,
            ty: (),
            user_ty: UserTypeProjections {
                contents: []
            },
            name: None,
            source_info: SourceInfo {
                span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:22: 8:22,
                scope: scope[0]
            },
            visibility_scope: scope[0]
        }
    ],
    user_type_annotations: [],
    arg_count: 1,
    upvar_decls: [],
    spread_arg: None,
    control_flow_destroyed: [],
    span: /Users/bjorn/Documents/rustc_codegen_cranelift/rust/src/test/run-pass/generator/panic-drops.rs:8:19: 10:6,
    cache: Cache {
        predecessors: RwLock(
            RefCell {
                value: None
            }
        )
    }
}
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 3, 2019

I noticed this, because cg_clif crashed with a verifier error about a jump to an undefined bb. (cg_clif omits is_cleanup blocks because it doesn't support unwinding yet anyway)

@jonas-schievink jonas-schievink added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html labels Mar 3, 2019
@matthewjasper matthewjasper self-assigned this Mar 3, 2019
@matthewjasper
Copy link
Contributor

Pretty printed shim (not fully transformed) 
// MIR for `main::{{closure}}`
// source = MirSource { instance: Item(DefId(0/1:9 ~ generator_drop_shim[317d]::main[0]::{{closure}}[0])), promoted: None }
// pass_name = generator_drop
// disambiguator = 0
// generator_layout = GeneratorLayout { fields: [] }

fn main::{{closure}}(_1: *mut [generator@src/generator-drop-shim.rs:4:19: 6:6 {()}]) -> () {
    let mut _0: ();                      // return place
    let mut _2: ();
    let mut _3: ();
    let mut _4: ();

    bb0: {                              
        switchInt(((*_1).0: u32)) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; // bb0[0]: scope 0 at src/generator-drop-shim.rs:4:19: 6:6
    }

    bb1: {                              
        goto -> bb5;                     // bb1[0]: scope 0 at src/generator-drop-shim.rs:6:5: 6:6
    }

    bb2: {                              
        return;                          // bb2[0]: scope 0 at src/generator-drop-shim.rs:4:19: 6:6
    }

    bb3: {                              
        return;                          // bb3[0]: scope 0 at src/generator-drop-shim.rs:4:19: 6:6
    }

    bb4: {                              
        goto -> bb6;                     // bb4[0]: scope 0 at src/generator-drop-shim.rs:4:19: 6:6
    }

    bb5: {                              
        goto -> bb2;                     // bb5[0]: scope 0 at src/generator-drop-shim.rs:6:5: 6:6
    }

    bb6: {                               // cleanup
        goto -> bb3;                     // bb6[0]: scope 0 at src/generator-drop-shim.rs:4:19: 6:6
    }

    bb7: {                              
        StorageLive(_3);                 // bb7[0]: scope 0 at src/generator-drop-shim.rs:4:19: 6:6
        goto -> bb1;                     // bb7[1]: scope 0 at src/generator-drop-shim.rs:4:19: 6:6
    }

    bb8: {                              
        return;                          // bb8[0]: scope 0 at src/generator-drop-shim.rs:4:19: 6:6
    }
}

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 3, 2019

Forgot -Zdump-mir existed :)

@matthewjasper matthewjasper mentioned this issue Mar 3, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue Mar 19, 2019
@Centril
Rollup merge of rust-lang#58902 - matthewjasper:generator-cleanup-blo…
fdb2ecd 
…cks, r=davidtwco

Fixes for the generator transform

* Moves cleanup annotations in pretty printed MIR so that they can be tested
* Correctly determines which drops are in cleanup blocks when elaborating generator drops
* Use the correct state for poisoning a generator

Closes rust-lang#58892
bors added a commit that referenced this issue Mar 21, 2019
@bors
Auto merge of #58902 - matthewjasper:generator-cleanup-blocks, r=davi…
20958fc 
…dtwco

Fixes for the generator transform

* Moves cleanup annotations in pretty printed MIR so that they can be tested
* Correctly determines which drops are in cleanup blocks when elaborating generator drops
* Use the correct state for poisoning a generator

Closes #58892
@bjorn3 bjorn3 mentioned this issue Jun 10, 2019
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@matthewjasper matthewjasper

Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jonas-schievink @bjorn3 @matthewjasper