Re: FW: [PATCH V4 6/6] Intel MPX bound violation handling.

[Walfred Tedeschi <walfred.tedeschi@intel.com>]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Eli Zaretskii
> Sent: Thursday, January 21, 2016 5:23 PM
> To: Tedeschi, Walfred
> Cc: palves@redhat.com; brobecker@adacore.com; gdb-patches@sourceware.org
> Subject: Re: [PATCH V4 6/6] Intel MPX bound violation handling.
>
>> From: Walfred Tedeschi <walfred.tedeschi@intel.com>
>> Cc: gdb-patches@sourceware.org, Walfred Tedeschi
>> <walfred.tedeschi@intel.com>
>> Date: Thu, 21 Jan 2016 15:48:25 +0100
>>
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -3,6 +3,21 @@
>>   
>>   *** Changes since GDB 7.10
>>   
>> +* Intel MPX boud violation handler.
>> +
>> +   A boundary violations is presented to the inferior as
>> +   a segmentation fault having SIGCODE 3. In this case
>                                             ^^ Two spaces between sentences, please.
ok!
>> +   GDB  displays also the kind of violation (upper or lower),
>> +   bounds, poiter value and the memory accessed, besides displaying
>                ^^^^^^
> "pointer"
>
>> +   the usual signal received and code location report.
>> +
>> +   As exemplified below:
>> +   Program received signal SIGSEGV, Segmentation fault
>> +   upper bound violation - bounds {lbound = 0x603010, ubound = 0x603023}
>> +   accessing 0x60302f.
>> +   0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> +   c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
> May I suggest to say
>
>    accessing address 0x60302f.
>
> instead?  That would be more clear, I think.
I will change it, yes, it is better.
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -22267,6 +22267,57 @@ whose bounds are to be changed, @var{lbound}
>> and @var{ubound} are new values  for lower and upper bounds respectively.
>>   @end table
>>   
>> +
>> +A boundary violation is presented to the inferior as a segmentation
>> +fault having SIGCODE 3. @value{GDBN} may display additional
>                                          ^^ Two spaces.
>
>> +information is displayed in this case.
> "...may display additional information is displayed..."?  One of the "display" and "displayed" is redundant here, I think.
I will change to "may display additional information "
>> +                                          On @code{STOP} mode
>> +@value{GDBN} will also display the kind of violation: "upper" or
>> +"lower", bounds, pointer value and the address accessed.
>> +On @code{NOSTOP} no additional information will be presented.
> I suggest to say "In STOP mode" and "In NOSTOP mode".  "In", not "On".
Right!
>
>> +The usual output of a segfault is:
>> +@smallexample
>> +Program received signal SIGSEGV, Segmentation fault
>> +0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> +c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> +68        value = *(p + len);
>> +@end smallexample
>> +
>> +In case it is a bound violation it will be presented as:
>> +@smallexample
>> +Program received signal SIGSEGV, Segmentation fault upper bound
>> +violation - bounds @{lbound = 0x603010, ubound = 0x603023@} accessing
>> +0x60302f.
>> +0x0000000000400d7c in upper (p=0x603010, a=0x603030, b=0x603050,
>> +c=0x603070, d=0x603090, len=7) at i386-mpx-sigsegv.c:68
>> +68        value = *(p + len);
>> +@end smallexample
> Why do we need to show here the output when no bound violation happened?
>
> Actually, why not move this description and the example to the "Signals" node?  If I were a user who received such a notification, the "Signals" node is where I would look for the explanations first.
I will move it there. About the example I wanted to show where the 
change will be placed. If you consider that this is superfluous, no 
issue in removing.

Will the snippet bellow be better?

Program received signal SIGSEGV, Segmentation fault upper bound
violation - bounds @{lbound = 0x603010, ubound = 0x603023@} accessing
0x60302f.

> Thanks.

Thanks and regards,
-Fred
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928