Page MenuHomeFreeBSD

follow-up to r332730 and r332752: set kdb_why to "trap" for fatal traps
ClosedPublic

Authored by avg on May 14 2018, 6:18 PM.

Details

Summary

This change affects arm, arm64 and mips achitectures.
Additonally, it removes redundant checks for kdb_active where it
already results in kdb_reenter() and adds kdb_reenter() calls
where they were missing.

Some architectures check the return value of kdb_trap(), but some don't.
I haven't changed any of that.

Some trap handling routines have a return code.
I am not sure if I provided correct ones for returns after kdb_reenter().
It should not return unless kdb_jmpbufp is NULL for some reason.

Test Plan

Only compile tested for all affected architectures.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

avg created this revision.May 14 2018, 6:18 PM
jhb accepted this revision.May 14 2018, 8:47 PM

Overall I think these look ok. I haven't tested them though.

sys/arm/arm/trap-v6.c
602 ↗(On Diff #42537)

It seems like the existing kdb_reenter() logic in this file probably should be moved to abort_fatal() similar to your change for the trap-v4.c. That is probably for someone on arm@ to test and fix, but that would seem to be more consistent with how kdb_reenter() is used on other platforms.

This revision is now accepted and ready to land.May 14 2018, 8:47 PM
skra added a comment.May 15 2018, 6:35 AM
In D15431#325512, @jhb wrote:

Overall I think these look ok. I haven't tested them though.

sys/arm/arm/trap-v6.c
602 ↗(On Diff #42537)

If you mean the use of kdb_reenter() in abort_handler(), then the logic mirrors the one used in i386 trap().

eadler accepted this revision.May 15 2018, 7:29 AM
eadler added a subscriber: eadler.
eadler added inline comments.
sys/arm64/arm64/trap.c
159 ↗(On Diff #42537)

shouldn't this be under the ifdef as well or am I mis-reading?

avg added inline comments.May 15 2018, 7:37 AM
sys/arm64/arm64/trap.c
159 ↗(On Diff #42537)

Yes, it should. Thank you.

avg updated this revision to Diff 42570.May 15 2018, 8:43 AM

place 'handled' under KDB, fix a whitespace issue near #ifdef

This revision now requires review to proceed.May 15 2018, 8:43 AM
jhb added inline comments.May 15 2018, 10:59 PM
sys/arm/arm/trap-v6.c
602 ↗(On Diff #42537)

Nevermind, I had misread abort_handler() as being one of the sub-handlers in aborts[].

This revision was not accepted when it landed; it landed in state Needs Review.May 16 2018, 6:52 AM
This revision was automatically updated to reflect the committed changes.