Date: Sun, 20 Aug 2017 12:42:01 +0200 From: Oliver Pinter <oliver.pinter@hardenedbsd.org> To: Konstantin Belousov <kib@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r322720 - head/sys/amd64/amd64 Message-ID: <CAPQ4ffu9p5KRyMfa7v_cvVHZr_qSvw0CTji0HF0wsdX%2B6OD0AQ@mail.gmail.com> In-Reply-To: <201708200952.v7K9qPP4036384@repo.freebsd.org> References: <201708200952.v7K9qPP4036384@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sunday, August 20, 2017, Konstantin Belousov <kib@freebsd.org> wrote: > Author: kib > Date: Sun Aug 20 09:52:25 2017 > New Revision: 322720 > URL: https://svnweb.freebsd.org/changeset/base/322720 > > Log: > Simplify amd64 trap(). > > - Use more relevant name 'signo' instead of 'i' for the local variable > which contains a signal number to send for the current exception. > - Eliminate two labels 'userout' and 'out' which point to the very end > of the trap() function. Instead use return directly. > - Re-indent the prot_fault_translation block by reducing if() nesting. > - Some more monor style changes. > > Requested and reviewed by: bde > Tested by: pho > Sponsored by: The FreeBSD Foundation > MFC after: 1 week > > Modified: > head/sys/amd64/amd64/trap.c > > Modified: head/sys/amd64/amd64/trap.c > ============================================================ > ================== > --- head/sys/amd64/amd64/trap.c Sun Aug 20 09:42:09 2017 (r322719) > +++ head/sys/amd64/amd64/trap.c Sun Aug 20 09:52:25 2017 (r322720) > @@ -172,12 +172,12 @@ trap(struct trapframe *frame) > #ifdef KDB > register_t dr6; > #endif > - int i, ucode; > + int signo, ucode; > u_int type; > > td = curthread; > p = td->td_proc; > - i = 0; > + signo = 0; > ucode = 0; > addr = 0; > > @@ -186,22 +186,20 @@ trap(struct trapframe *frame) > > #ifdef SMP > /* Handler for NMI IPIs used for stopping CPUs. */ > - if (type == T_NMI) { > - if (ipi_nmi_handler() == 0) > - goto out; > - } > -#endif /* SMP */ > + if (type == T_NMI && ipi_nmi_handler() == 0) > + return; > +#endif > > #ifdef KDB > if (kdb_active) { > kdb_reenter(); > - goto out; > + return; > } > #endif > > if (type == T_RESERVED) { > trap_fatal(frame, 0); > - goto out; > + return; > } > > if (type == T_NMI) { > @@ -214,18 +212,18 @@ trap(struct trapframe *frame) > */ > if (pmc_intr != NULL && > (*pmc_intr)(PCPU_GET(cpuid), frame) != 0) > - goto out; > + return; > #endif > > #ifdef STACK > if (stack_nmi_handler(frame) != 0) > - goto out; > + return; > #endif > } > > if (type == T_MCHK) { > mca_intr(); > - goto out; > + return; > } > > if ((frame->tf_rflags & PSL_I) == 0) { > @@ -269,7 +267,7 @@ trap(struct trapframe *frame) > > switch (type) { > case T_PRIVINFLT: /* privileged instruction fault */ > - i = SIGILL; > + signo = SIGILL; > ucode = ILL_PRVOPC; > break; > > @@ -281,41 +279,41 @@ trap(struct trapframe *frame) > fill_frame_regs(frame, ®s); > if (dtrace_pid_probe_ptr != NULL && > dtrace_pid_probe_ptr(®s) == 0) > - goto out; > + return; > } > #endif > frame->tf_rflags &= ~PSL_T; > - i = SIGTRAP; > + signo = SIGTRAP; > ucode = (type == T_TRCTRAP ? TRAP_TRACE : > TRAP_BRKPT); > break; > > case T_ARITHTRAP: /* arithmetic trap */ > ucode = fputrap_x87(); > if (ucode == -1) > - goto userout; > - i = SIGFPE; > + return; > + signo = SIGFPE; > break; > > case T_PROTFLT: /* general protection fault */ > - i = SIGBUS; > + signo = SIGBUS; > ucode = BUS_OBJERR; > break; > case T_STKFLT: /* stack fault */ > case T_SEGNPFLT: /* segment not present fault */ > - i = SIGBUS; > + signo = SIGBUS; > ucode = BUS_ADRERR; > break; > case T_TSSFLT: /* invalid TSS fault */ > - i = SIGBUS; > + signo = SIGBUS; > ucode = BUS_OBJERR; > break; > case T_ALIGNFLT: > - i = SIGBUS; > + signo = SIGBUS; > ucode = BUS_ADRALN; > break; > case T_DOUBLEFLT: /* double fault */ > default: > - i = SIGBUS; > + signo = SIGBUS; > ucode = BUS_OBJERR; > break; > > @@ -325,67 +323,64 @@ trap(struct trapframe *frame) > */ > if (*p->p_sysent->sv_trap != NULL && > (*p->p_sysent->sv_trap)(td) == 0) > - goto userout; > + return; > > addr = frame->tf_addr; > - i = trap_pfault(frame, TRUE); > - if (i == -1) > - goto userout; > - if (i == 0) > - goto user; > - > - if (i == SIGSEGV) > + signo = trap_pfault(frame, TRUE); > + if (signo == -1) > + return; > + if (signo == 0) > + goto userret; > + if (signo == SIGSEGV) { > ucode = SEGV_MAPERR; > - else { > - if (prot_fault_translation == 0) { > - /* > - * Autodetect. > - * This check also covers the > images > - * without the ABI-tag ELF note. > - */ > - if (SV_CURPROC_ABI() == > SV_ABI_FREEBSD > - && p->p_osrel >= > P_OSREL_SIGSEGV) { > - i = SIGSEGV; > - ucode = SEGV_ACCERR; > - } else { > - i = SIGBUS; > - ucode = BUS_PAGE_FAULT; > - } > - } else if (prot_fault_translation == 1) { > - /* > - * Always compat mode. > - */ > - i = SIGBUS; > - ucode = BUS_PAGE_FAULT; > - } else { > - /* > - * Always SIGSEGV mode. > - */ > - i = SIGSEGV; > + } else if (prot_fault_translation == 0) { > + /* > + * Autodetect. This check also covers > + * the images without the ABI-tag ELF > + * note. > + */ > + if (SV_CURPROC_ABI() == SV_ABI_FREEBSD && > + p->p_osrel >= P_OSREL_SIGSEGV) { > + signo = SIGSEGV; > ucode = SEGV_ACCERR; > + } else { > + signo = SIGBUS; > + ucode = BUS_PAGE_FAULT; > } > + } else if (prot_fault_translation == 1) { > + /* > + * Always compat mode. > + */ > + signo = SIGBUS; > + ucode = BUS_PAGE_FAULT; > + } else { > + /* > + * Always SIGSEGV mode. > + */ > + signo = SIGSEGV; > + ucode = SEGV_ACCERR; > } > break; > > case T_DIVIDE: /* integer divide fault */ > ucode = FPE_INTDIV; > - i = SIGFPE; > + signo = SIGFPE; > break; > > #ifdef DEV_ISA > case T_NMI: > nmi_handle_intr(type, frame); > - goto out; > -#endif /* DEV_ISA */ > + return; > +#endif > > case T_OFLOW: /* integer overflow fault */ > ucode = FPE_INTOVF; > - i = SIGFPE; > + signo = SIGFPE; > break; > > case T_BOUND: /* bounds check fault */ > ucode = FPE_FLTSUB; > - i = SIGFPE; > + signo = SIGFPE; > break; > > case T_DNA: > @@ -393,18 +388,18 @@ trap(struct trapframe *frame) > KASSERT(PCB_USER_FPU(td->td_pcb), > ("kernel FPU ctx has leaked")); > fpudna(); > - goto userout; > + return; > > case T_FPOPFLT: /* FPU operand fetch fault */ > ucode = ILL_COPROC; > - i = SIGILL; > + signo = SIGILL; > break; > > case T_XMMFLT: /* SIMD floating-point exception */ > ucode = fputrap_sse(); > if (ucode == -1) > - goto userout; > - i = SIGFPE; > + return; > + signo = SIGFPE; > break; > #ifdef KDTRACE_HOOKS > case T_DTRACE_RET: > @@ -412,8 +407,8 @@ trap(struct trapframe *frame) > fill_frame_regs(frame, ®s); > if (dtrace_return_probe_ptr != NULL && > dtrace_return_probe_ptr(®s) == 0) > - goto out; > - goto userout; > + return; > + return; This part of code - the double return - looks weird. Probably it was left here to document the original "behavior". > #endif > } > } else { > @@ -424,13 +419,13 @@ trap(struct trapframe *frame) > switch (type) { > case T_PAGEFLT: /* page fault */ > (void) trap_pfault(frame, FALSE); > - goto out; > + return; > > case T_DNA: > if (PCB_USER_FPU(td->td_pcb)) > panic("Unregistered use of FPU in kernel"); > fpudna(); > - goto out; > + return; > > case T_ARITHTRAP: /* arithmetic trap */ > case T_XMMFLT: /* SIMD floating-point exception */ > @@ -440,7 +435,7 @@ trap(struct trapframe *frame) > * registration for FPU traps is overkill. > */ > trap_fatal(frame, 0); > - goto out; > + return; > > case T_STKFLT: /* stack fault */ > case T_PROTFLT: /* general protection fault */ > @@ -460,35 +455,35 @@ trap(struct trapframe *frame) > */ > if (frame->tf_rip == (long)doreti_iret) { > frame->tf_rip = (long)doreti_iret_fault; > - goto out; > + return; > } > if (frame->tf_rip == (long)ld_ds) { > frame->tf_rip = (long)ds_load_fault; > - goto out; > + return; > } > if (frame->tf_rip == (long)ld_es) { > frame->tf_rip = (long)es_load_fault; > - goto out; > + return; > } > if (frame->tf_rip == (long)ld_fs) { > frame->tf_rip = (long)fs_load_fault; > - goto out; > + return; > } > if (frame->tf_rip == (long)ld_gs) { > frame->tf_rip = (long)gs_load_fault; > - goto out; > + return; > } > if (frame->tf_rip == (long)ld_gsbase) { > frame->tf_rip = (long)gsbase_load_fault; > - goto out; > + return; > } > if (frame->tf_rip == (long)ld_fsbase) { > frame->tf_rip = (long)fsbase_load_fault; > - goto out; > + return; > } > if (curpcb->pcb_onfault != NULL) { > frame->tf_rip = (long)curpcb->pcb_onfault; > - goto out; > + return; > } > break; > > @@ -504,7 +499,7 @@ trap(struct trapframe *frame) > */ > if (frame->tf_rflags & PSL_NT) { > frame->tf_rflags &= ~PSL_NT; > - goto out; > + return; > } > break; > > @@ -525,7 +520,7 @@ trap(struct trapframe *frame) > * processor doesn't > */ > load_dr6(rdr6() & ~0xf); > - goto out; > + return; > } > /* > * FALLTHROUGH (TRCTRAP kernel mode, kernel > address) > @@ -540,27 +535,27 @@ trap(struct trapframe *frame) > dr6 = rdr6(); > load_dr6(dr6 & ~0x4000); > if (kdb_trap(type, dr6, frame)) > - goto out; > + return; > #endif > break; > > #ifdef DEV_ISA > case T_NMI: > nmi_handle_intr(type, frame); > - goto out; > -#endif /* DEV_ISA */ > + return; > +#endif > } > > trap_fatal(frame, 0); > - goto out; > + return; > } > > /* Translate fault for emulators (e.g. Linux) */ > - if (*p->p_sysent->sv_transtrap) > - i = (*p->p_sysent->sv_transtrap)(i, type); > + if (*p->p_sysent->sv_transtrap != NULL) > + signo = (*p->p_sysent->sv_transtrap)(signo, type); > > ksiginfo_init_trap(&ksi); > - ksi.ksi_signo = i; > + ksi.ksi_signo = signo; > ksi.ksi_code = ucode; > ksi.ksi_trapno = type; > ksi.ksi_addr = (void *)addr; > @@ -568,8 +563,8 @@ trap(struct trapframe *frame) > uprintf("pid %d comm %s: signal %d err %lx code %d type %d > " > "addr 0x%lx rsp 0x%lx rip 0x%lx " > "<%02x %02x %02x %02x %02x %02x %02x %02x>\n", > - p->p_pid, p->p_comm, i, frame->tf_err, ucode, type, > addr, > - frame->tf_rsp, frame->tf_rip, > + p->p_pid, p->p_comm, signo, frame->tf_err, ucode, type, > + addr, frame->tf_rsp, frame->tf_rip, > fubyte((void *)(frame->tf_rip + 0)), > fubyte((void *)(frame->tf_rip + 1)), > fubyte((void *)(frame->tf_rip + 2)), > @@ -581,14 +576,10 @@ trap(struct trapframe *frame) > } > KASSERT((read_rflags() & PSL_I) != 0, ("interrupts disabled")); > trapsignal(td, &ksi); > - > -user: > +userret: > userret(td, frame); > KASSERT(PCB_USER_FPU(td->td_pcb), > ("Return from trap with kernel FPU ctx leaked")); > -userout: > -out: > - return; > } > > /* > _______________________________________________ > svn-src-head@freebsd.org <javascript:;> mailing list > https://lists.freebsd.org/mailman/listinfo/svn-src-head > To unsubscribe, send any mail to "svn-src-head-unsubscribe@freebsd.org > <javascript:;>" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPQ4ffu9p5KRyMfa7v_cvVHZr_qSvw0CTji0HF0wsdX%2B6OD0AQ>