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

[Pedro Alves <palves at redhat dot com>]

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