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

[lijiang]

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