[Crash-utility] [PATCH v3] x86_64_exception_frame only performs EFRAME_VERIFY if it is the only flag

[HAGIO KAZUHITO(萩尾 一仁)]

Hi,

sorry for the delay, applied.
https://github.com/crash-utility/crash/commit/915cc069ec0338b2e5342fbd02cbfb921d5494af

Thanks,
Kazu

-----Original Message-----
> Hi, David Mair
> 
> Thank you for the fix.
> 
> 在 2020年09月11日 00:00, crash-utility-request at redhat.com 写道:
> > Message: 2
> > Date: Thu, 10 Sep 2020 09:35:34 +0000
> > From: HAGIO KAZUHITO(?????) 	<k-hagio-ab at nec.com>
> > To: "Discussion list for crash utility usage,	maintenance and
> > 	development"	<crash-utility at redhat.com>
> > Subject: Re: [Crash-utility] [PATCH v3] x86_64_exception_frame only
> > 	performs EFRAME_VERIFY if it is the only flag
> > Message-ID:
> > 	<OSBPR01MB19919A04D573C74CD0727B82DD270 at OSBPR01MB1991.jpnprd01.prod.outlook.com>
> >
> > Content-Type: text/plain; charset="iso-2022-jp"
> >
> > -----Original Message-----
> >> Hi Kazu,
> >>
> >> I swear that last week I couldn't build past:
> >>
> >> verify_addr = (local - bt->stackbuf) + bt->stackbase;
> >>
> >> without a gcc error on the two char * in the parentheses being
> >> used in a ulong assignment. Last week it required casts on both
> >> char * to build for me. This week build of the same workspace
> >> of expanded source works through the same line without any casts.
> >> Below is a patch to replace the one in this thread's header post.
> >> It is cleaned of the casts I previously had in the line above.
> >> It builds fixes the problems I described in this thread's header
> >> and is unmodified other than removal of the casts I previously
> >> had in the line above.
> > OK, so for the following patch and the v3 commit message [1]
> > without its last paragraph about the cast to uintptr_t,
> >
> > Acked-by: Kazuhito Hagio <k-hagio-ab at nec.com>
> >
> > Please wait for another ack.
> >
> > [1] https://www.redhat.com/archives/crash-utility/2020-September/msg00016.html
> >
> > Thanks,
> > Kazu
> >
> >> I'm sorry for the trouble getting it tidy.
> >>
> >> Signed-off-by: David Mair <dmair at suse.com>
> >> ---
> >> diff --git a/x86_64.c b/x86_64.c
> >> index fc05e8a..dec3730 100644
> >> --- a/x86_64.c
> >> +++ b/x86_64.c
> >> @@ -4412,15 +4412,20 @@ x86_64_exception_frame(ulong flags, ulong kvaddr, char *local,
> >>          long r8, r9, r10, r11, r12, r13, r14, r15;
> >>  	struct machine_specific *ms;
> >>  	struct syment *sp;
> >> -	ulong offset;
> >> +	ulong offset, verify_addr;
> >>  	char *pt_regs_buf;
> >>  	long verified;
> >>  	long err;
> >>  	char buf[BUFSIZE];
> >>
> >> -	if (flags == EFRAME_VERIFY) {
> >> -		if (!accessible(kvaddr) ||
> >> -		    !accessible(kvaddr + SIZE(pt_regs) - sizeof(long)))
> 
> Actually, that would be good to have a simple code comment for this change.
> 
> Anyway, this fix looks good to me. Acked-by: Lianbo Jiang <lijiang at redhat.com>
> 
> >> +	if (flags & EFRAME_VERIFY) {
> >> +		if (kvaddr)
> >> +			verify_addr = kvaddr;
> >> +		else
> >> +			verify_addr = (local - bt->stackbuf) + bt->stackbase;
> >> +
> >> +		if (!accessible(verify_addr) ||
> >> +		    !accessible(verify_addr + SIZE(pt_regs) - sizeof(long)))
> >>  			return FALSE;
> >>  	}
> >>
> >>
> >> --
> >> Crash-utility mailing list
> >> Crash-utility at redhat.com
> >> https://www.redhat.com/mailman/listinfo/crash-utility