Re: [PATCH V5 5/5] ntel MPX bound violation handling

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

Am 2/2/2016 um 11:38 AM schrieb Pedro Alves:
> On 02/02/2016 10:09 AM, Walfred Tedeschi wrote:
>> Am 1/28/2016 um 2:42 PM schrieb Pedro Alves:
>> Pedro,
>>
>> I have tried to address all your comments.
>
> I quickly skimmed v6 (will have to look in more detail), but looks like
> you missed the renaming comments.  See them quoted below.
>
>> For the tests I also verified that there was only one line in the log as
>> shown in the wiki.
>
> Thanks.
>
>> Have though some comments below:
>>
>
>>> # Function called when a segmentation fault with
>>> # SIGCODE 3 (SIG_CODE_BOUNDARY_FAULT) is received by the inferior.
>>>
>>> But, see below.
>
>>>> +
>>>> +static void
>>>> +handle_segmentation_faults (struct ui_out *uiout)
>>>
>>> "handle_segmentation_fault", singular.
>
> ...
>
>>> And then the gdbarch hook can be renamed to a more generic
>>> gdbarch_handle_segmentation_fault.
>
> ...
>
> The comments thoughout should be
>>> updated then, like, for this function:
>>>
>>> /* Some targets/architectures can do extra processing/display of
>>>      segmentation faults.  E.g., Intel MPX boundary faults.
>>>      Call the architecture dependent function to handle the fault.  */
>>>
>>> static void
>>> handle_segmentation_fault (struct ui_out *uiout)
>>> {
>>>
>
> (You did update the comment.)
>
>>>> +for {set i 0} {$i < 15} {incr i} {
>>>> +    set message "MPX signal segv Upper: ${i}"
>>>> +    gdb_test_multiple "continue" "$message ${i}" {
>>>> +        -re $segv_upper_bound {
>>>> +            pass "$message"
>>>> +            }
>>>> +        -re ".*$inferior_exited_re normally.*$gdb_prompt $" {
>>>> +            fail "$message"
>>>
>>> The pass/fail calls are missing ${i}.  Please make sure test
>>> messages are unique in gdb.sum:
>>
>> In fail and pass i think we do not need the ${i} it is added at the
>> message level already see:
>>
>> set message "MPX signal segv Upper: ${i}"
>
> In that case, you shouldn't have an "${i}" in this line:
>
>     +    gdb_test_multiple "continue" "$message ${i}" {
>
> because then that expands to "... ${i} ${i}".
>
> Thanks,
> Pedro Alves
>

Pedro,

Thanks a lot!

I will do it still today.

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