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

Epilogue instructions don't set SourceLoc to return inst SourceLoc #1145

Closed
bjorn3 opened this issue Oct 25, 2019 · 3 comments
Closed

Epilogue instructions don't set SourceLoc to return inst SourceLoc #1145

bjorn3 opened this issue Oct 25, 2019 · 3 comments
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift Issues related to the Cranelift code generator

Comments

@bjorn3
Copy link
Contributor

bjorn3 commented Oct 25, 2019

  • What are the steps to reproduce the issue?

    test compile
    target x86_64
    
    function u0:0() system_v {
    ebb0:
    nop
    @0001 return
    }
    
  • What do you expect to happen? What does actually happen? Does it panic, and
    if so, with which assertion?

    test compile
    target x86_64
    function u0:0(i64 fp [%rbp]) -> i64 fp [%rbp] system_v {
        ss0 = incoming_arg 16, offset -16
    
                                    ebb0(v4: i64 [%rbp]):
    [RexOp1pushq#50]                    x86_push v4
    [RexOp1copysp#8089]                 copy_special %rsp -> %rbp
    [-]                                 nop
    [RexOp1popq#58,%rbp]                v5 = x86_pop.i64 ; <-- this should use @0001 as srcloc
    @0001 [Op1ret#c3]                   return v5
    }
    

    This causes debuggers to switch between function start and end as current position with cg_clif generated executables when returning from a function.

  • Which Cranelift version / commit hash / branch are you using? 387593d

@bjorn3
Copy link
Contributor Author

bjorn3 commented Oct 25, 2019

I get that this standard form can be useful when somebody barely posts any information otherwise, but it is a lot of work to fill in.

@bnjbvr
Copy link
Member

bnjbvr commented Oct 28, 2019

I get that this standard form can be useful when somebody barely posts any information otherwise, but it is a lot of work to fill in.

With all the information that you've provided in the OP, this issue is actually perfect. Indeed it's more work to fill for the reporter, but if it makes it easier to understand what the problem is and how to fix it (which is the case here in my opinion), then it's a net win for everyone. Of course we're not married to the issue templates in their current forms, and we'll see how everyone feels about it. We can amend them, remove them, etc.

@alexcrichton alexcrichton transferred this issue from bytecodealliance/cranelift Feb 28, 2020
@alexcrichton alexcrichton added bug Incorrect behavior in the current implementation that needs fixing cranelift Issues related to the Cranelift code generator labels Feb 28, 2020
@bjorn3
Copy link
Contributor Author

bjorn3 commented Mar 20, 2021

Fixed with new x64 backend.

@bjorn3 bjorn3 closed this as completed Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

No branches or pull requests

3 participants