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

[Simon Marchi <simark@simark.ca>]

On 2022-04-13 04:58, Tom de Vries via Gdb-patches wrote:
> 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.

It's fine with me, thanks.

Simon