Re: [PATCH][gdb/testsuite] Detect 'No MPX support'

[Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>]

On 4/12/22 08:03, Tom de Vries wrote:
> On 4/11/22 17:00, Simon Marchi wrote:
>> On 2022-04-11 10:25, Tom de Vries via Gdb-patches wrote:
>>> Hi,
>>>
>>> On openSUSE Leap 15.3, mpx support has been disabled for m32, so I 
>>> run into:
>>> ...
>>> (gdb) run ^M
>>> Starting program: outputs/gdb.arch/i386-mpx/i386-mpx ^M
>>> [Thread debugging using libthread_db enabled]^M
>>> Using host libthread_db library "/lib64/libthread_db.so.1".^M
>>> No MPX support^M
>>> ...
>>> and eventually into all sort of fails in this and other mpx test-cases.
>>>
>>> Fix this by detecting the "No MPX support" message in have_mpx.
>>>
>>> Tested on x86_64-linux with target boards unix and unix/-m32.
>>>
>>> Any comments?
>>>
>>> Thanks,
>>> - Tom
>>>
>>> [gdb/testsuite] Detect 'No MPX support'
>>>
>>> ---
>>>   gdb/testsuite/lib/gdb.exp | 23 +++++++++++++++++++++++
>>>   1 file changed, 23 insertions(+)
>>>
>>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>>> index 2eb711748e7..9eb01e0b4b2 100644
>>> --- a/gdb/testsuite/lib/gdb.exp
>>> +++ b/gdb/testsuite/lib/gdb.exp
>>> @@ -8329,6 +8329,29 @@ gdb_caching_proc have_mpx {
>>>
>>>       remote_file build delete $obj
>>>
>>> +    if { $status == 0 } {
>>> +    verbose "$me:  returning $status" 2
>>> +    return $status
>>> +    }
>>> +
>>> +    # Compile program with -mmpx -fcheck-pointer-bounds, try to trigger
>>> +    # 'No MPX support', in other words, see if kernel supports mpx.
>>> +    set src { int main (void) { return 0; } }
>>> +    set comp_flags {}
>>> +    append comp_flags " additional_flags=-mmpx"
>>> +    append comp_flags " additional_flags=-fcheck-pointer-bounds"
>>> +    if {![gdb_simple_compile $me-2 $src executable $comp_flags]} {
>>> +        return 0
>>> +    }
>>> +
>>> +    set result [remote_exec target $obj]
>>> +    set status [lindex $result 0]
>>> +    set output [lindex $result 1]
>>> +    set status [expr ($status == 0) \
>>> +            && ![string equal $output "No MPX support\r\n"]]
>>> +
>>> +    remote_file build delete $obj
>>> +
>>>       verbose "$me:  returning $status" 2
>>>       return $status
>>>   }
>>
>> It seems fine to me.  I am just wondering:
>>
>>   - Who prints this "No MPX support" string exactly?
> 
> Libmpx.  On gcc-7-branch:
> ...
> ./libmpx/mpxrt/mpxrt.c:      __mpxrt_print (VERB_DEBUG, "No MPX 
> support.\n");
> ./libmpx/mpxrt/mpxrt.c:      __mpxrt_print (VERB_ERROR, "No MPX 
> support\n");
> ./libmpx/mpxrt/mpxrt.c:      __mpxrt_print (VERB_ERROR, "No MPX 
> support\n");
> ...
> 
>    When it is printed,
>>     is the status other than 0?  If so, it wouldn't be necessary to check
>>     for the "No MPX support" output, just check if the program runs
>>     successfully.
> 
> No, the status is the same:
> ...
> $ cat ~/min.c
> int
> main (void)
> {
>    return 0;
> }
> $ gcc -mmpx -fcheck-pointer-bounds ~/min.c
> $ ./a.out; echo $?
> 0
> $ gcc -mmpx -fcheck-pointer-bounds ~/min.c -m32
> $ ./a.out; echo $?
> No MPX support
> 0
> ...
> 
>>   - Why do you need to compile a separate program with -mmpx, why not the
>>     existing test program?
> 
> The existing test program tries to check for cpu support, which requires 
> some very specific code.
> 
> The added test checks for kernel support, which (because we're relying 
> on libmpx output) AFAICT requires no particular program, so I'm using 
> the minimal source possible.  This for clarity, to prevent giving the 
> impression in any way that the source does matter.
> 
> I suppose it would be possible to use a more specific test-case that is 
> supposed to have a different status with and without kernel support. But 
> I don't think it makes sense to invest in that unless we run into 
> trouble with this approach.
> 

Hi,

any further comments?

I realize you already mentioned that it seems fine to you, but given that:
- you still had questions, and
- I'm also planning to apply to gdb-12-branch
I thought I ask.

Thanks,
- Tom