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

debuginfo: Improvements for stepping experience in GDB #10966

Merged
merged 11 commits into from
Dec 16, 2013

Conversation

michaelwoerister
Copy link
Member

This PR improves the stepping experience in GDB. It contains some fine tuning of line information and makes rustc produce nearly the same IR/DWARF as Clang. The focus of the changes is function prologue handling which has caused some problems in the past (#9641).

It seems that GDB does not properly handle function prologues when the function uses segmented stacks, i.e. it does not recognize that the __morestack check is part of the prologue. When setting a breakpoint like break foo it will set the break point before the arguments of foo() have been loaded and still contain bogus values. For function with the #[no_split_stack] attribute this problem has never occurred for me so I'm pretty sure that segmented stacks are the cause of the problem. @jdm mentioned that segmented stack won't be completely abandoned after all. I'd be grateful if you could tell me about what the future might bring in this regard (@brson, @cmr).

Anyway, this PR should alleviate this problem at least in the case when setting breakpoints using line numbers and also make it less confusing when setting them via function names because then GDB will break before the first statement where one could conceivably argue that arguments need not be initialized yet.

Also, a koala: 🐨

Cheers,
Michael

@emberian
Copy link
Member

So, segmented stacks are fully abandoned, but the stack check prelude isn't going away. We need it to keep memory safety, otherwise stack overflow causes unsafety (and we can't statically assert stack size (yet)).

__morestack right now just aborts. I think we should try and do whatever it is that -fstack-check does in GCC, and I've heard rumors of a __chkstk function (a la windows) that is supposedly more standard.

let scope_line = get_scope_line(cx, top_level_block, loc.line);
// Clang sets this parameter to the opening brace of the function's block, so let's do this too.
let scope_line = span_start(cx, top_level_block.span).line;
let is_local_to_unit = !cx.reachable.contains(&fn_ast_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have a comment about why reachable is appropriate here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I'll do that.

@michaelwoerister
Copy link
Member Author

OK, comments added. Let's see if this works:
r=cmr

@michaelwoerister
Copy link
Member Author

So, segmented stacks are fully abandoned, but the stack check prelude isn't going away. We need it to keep memory safety, otherwise stack overflow causes unsafety (and we can't statically assert stack size (yet)). [...]

Thanks for the info, by the way! We'll see if GDB can handle this better than the current situation.

@alexcrichton
Copy link
Member

This is amazing, awesome work!

@jdm
Copy link
Contributor

jdm commented Dec 14, 2013

🎉

@michaelwoerister
Copy link
Member Author

Thanks, @alexcrichton and @jdm

This test failure seems to be caused by some oddity in LLVM's 32bit code generator. The IR looks the same for 32 and 64 bit but in 32bit mode the DWARF data does not contain an entry for the unique_pointer test function. Adding something useless to the function like let _ = 0; makes the problem go away. I guess this is some kind of optimization removing the function from machine code altogether... I'll have to look into it.

@michaelwoerister
Copy link
Member Author

OK, the test failure has been fixed. There was a bug that lead to source locations only being assigned to CALL instructions but not to INVOKE instructions. Apparently this caused small functions consisting mainly of an INVOKE instruction would be pruned from debug info.

bors added a commit that referenced this pull request Dec 16, 2013
This PR improves the stepping experience in GDB. It contains some fine tuning of line information and makes *rustc* produce nearly the same IR/DWARF as Clang. The focus of the changes is function prologue handling which has caused some problems in the past (#9641).

It seems that GDB does not properly handle function prologues when the function uses segmented stacks, i.e. it does not recognize that the `__morestack` check is part of the prologue. When setting a breakpoint like `break foo` it will set the break point before the arguments of `foo()` have been loaded and still contain bogus values. For function with the #[no_split_stack] attribute this problem has never occurred for me so I'm pretty sure that segmented stacks are the cause of the problem. @jdm mentioned that segmented stack won't be completely abandoned after all. I'd be grateful if you could tell me about what the future might bring in this regard (@brson, @cmr).

Anyway, this PR should alleviate this problem at least in the case when setting breakpoints using line numbers and also make it less confusing when setting them via function names because then GDB will break *before* the first statement where one could conceivably argue that arguments need not be initialized yet.

Also, a koala: 🐨

Cheers,
Michael
@bors bors closed this Dec 16, 2013
@bors bors merged commit 9384de7 into rust-lang:master Dec 16, 2013
@michaelwoerister michaelwoerister deleted the prelude2 branch February 7, 2022 09:28
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 30, 2023
Make [`missing_panics_doc`]  not lint for `todo!()`

closes rust-lang#10966

changelog: [`missing_panics_doc`] now does not lint for `todo!()`
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.

6 participants