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

mcount_return_trampoline: Don't disable interrupts #10

Closed
wants to merge 2 commits into from
Closed

mcount_return_trampoline: Don't disable interrupts #10

wants to merge 2 commits into from

Conversation

mkroening
Copy link
Member

@mkroening mkroening commented Jul 15, 2021

This fixes #8 for me.

@mkroening mkroening marked this pull request as draft July 15, 2021 09:10
@mkroening mkroening changed the title mcount_return_trampoline: Correctly restore stack mcount_return_trampoline: Fix pushing rflags Jul 15, 2021
@mkroening mkroening changed the title mcount_return_trampoline: Fix pushing rflags mcount_return_trampoline: Fix restoring stack Jul 15, 2021
@mkroening mkroening marked this pull request as ready for review July 15, 2021 09:23
@tlambertz
Copy link
Collaborator

Huh yeah, seems I missed that one. Now wondering how it ever worked?
The latest commit was done a few months after my thesis was complete and I was cleaning up my working directory, might have not tested it fully?

If i recall correctly, the reason for the changes in the latest commit were some changes in libhermit which lead to frequent crashes (tracing interrupt-based filesystem writes?). Something with interrupt at inopportune moments, trashing the stack/flags register.

@mkroening
Copy link
Member Author

Ah, I see. That makes sense. I have only tested with the rust example, and that one always crashes for me on the latest commit without this fix, as explained in #8.

@mkroening
Copy link
Member Author

Ah, nevermind, I did not test this change correctly. I'll rework this.

@mkroening mkroening marked this pull request as draft July 15, 2021 11:39
@mkroening mkroening changed the title mcount_return_trampoline: Fix restoring stack mcount_return_trampoline: Don't disable interrupts Jul 15, 2021
@mkroening
Copy link
Member Author

Ah, sorry. I must have been not very concentrated. Anyway, I found the issue now. The stack has been handled correctly.

The problem is calling cli in user space, which is not possible. When you tested this in libhermit you might have tested kernel space only. Removing cli makes the example work for me. Why would we need to disable interrupts there in the first place? Any interrupt handler would make sure not to clobber any registers, wouldn't it?

@mkroening mkroening marked this pull request as ready for review July 15, 2021 13:27
@tlambertz
Copy link
Collaborator

I think one issue I tried to fix was that an interrupt was arriving while the kernel was in mcount(). The interrupt in turn called mcount(), which then overwrote the return pointer of the first mcount() with the trampoline, which broke things.

@mkroening
Copy link
Member Author

Ah, so then the problem is the access to RETSTACK? I was trying to reproduce the issue to be sure, but I can't get RustyHermit to build with rftrace.

@mkroening mkroening marked this pull request as draft November 5, 2021 09:46
@mkroening mkroening closed this Nov 27, 2023
@mkroening mkroening deleted the segmentation-fault-fix branch December 4, 2023 13:49
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.

Segmentation fault
2 participants