Ignore functions before main and thread entry points in backtraces #47824
Conversation
kennytm
added
the
S-waiting-on-review
Zoxc
force-pushed
the
backtraces
branch
3 times, most recently
from
ad90937 to
ff74477
Compare
| } | ||
|
|
||
| /// Convenience wrapper for `mark_start` | ||
| #[unstable(feature = "rt", reason = "this is only exported for use in libtest", issue = "0")] |
There was a problem hiding this comment.
Could this reason be updated as I think this is used by threading/main now, right?
There was a problem hiding this comment.
Those are private usages in libstd though, which do not require an export.
| fn mark_start(f: &mut FnMut()) { | ||
| f(); | ||
| unsafe { | ||
| asm!("" ::: "memory" : "volatile"); // A dummy statement to prevent tail call optimization |
There was a problem hiding this comment.
This may not compile on platforms like asm.js, so I wonder if we could take a perhaps different route to implement this? Could this call a #[inline(never)] function which doesn't throw using the notail attribute on the call? I think that'd do the same thing, right?
There was a problem hiding this comment.
f may throw here, so we cannot use notail. We need something that is opaque to LLVM. I suggest we just disable this asm! expressions on platforms which do not support it.
There was a problem hiding this comment.
Ah yes indeed that's why we can't notail the call to f, but I"m thinking something like:
fn mark_start(f: &mut FnMut()) {
f();
#[notail]
foo();
#[inline(never)]
fn foo() {}
}There was a problem hiding this comment.
LLVM could still see that foo has no side effect, and just remove the call.
There was a problem hiding this comment.
Ah indeed, in that case could something more portable than asm! be used, like volatile reads or something like that?
There was a problem hiding this comment.
I don't think volatile operations will work as they could be reordered to happen before the call to f, if f contains no volatile operations itself.
There was a problem hiding this comment.
Does LLVM accept empty inline assembly for asm.js?
There was a problem hiding this comment.
AFAIK no we disable inline assembly completely on asm.js. Does the reordering actually happen?
There was a problem hiding this comment.
I was asking about LLVM not rustc though. I do not know if the reordering could happen given LLVM's current optimizations.
There was a problem hiding this comment.
At least in the tests I was running it wasn't reordering, but I'm not sure if that's a guarantee? I figure we could at least try it as this'll need to be upgraded for emscripten anyway.
|
☔ The latest upstream changes (presumably #48040) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
|
Ping from triage, @Zoxc — will you be able to address the merge conflicts sometime soon? I'm not sure if @alexcrichton wanted you to make changes or not, either... |
|
@Zoxc closing this PR due to inactivity, feel free to open it again when you have some spare time to work on it! |
|
This would be a really welcome change -- are we sure we want to close this? I guess if there's no-one else to pick it up either? |
Anyone is certainly welcome to pick up from when this PR was closed! If fixing it excites you, I say to go for it! |
Split out from #45637.
r? @alexcrichton