Add system test before "set remote system-call-allowed 1"

Message ID 5371910C.9020607@mentor.com
State Committed
Headers

Commit Message

Hui Zhu May 13, 2014, 3:27 a.m. UTC
  This patch is update version according to the discussion in
https://www.sourceware.org/ml/gdb-patches/2009-11/msg00090.html.
If test get the target doesn't support fileio system according to the
remote log.   It will set this test as "unsupported".

Before I made this patch, I want add a check before all of tests in this
file.  But I found that the target maybe support one call but not others.
For example: my target support Fwrite, Fopen and so on.  But not 
Fgettimeofday.
And it doesn't support Fsystem NULL but it support Fsystem not NULL.
So I think if we want to check target support fileio, we need check them
one by one.

Thanks,
Hui

2014-05-13  Nathan Sidwell  <nathan@codesourcery.com>
	    Hui Zhu  <hui@codesourcery.com>

	* gdb.base/fileio.exp: Add test for shell not available as well as
	available.
	* gdb.base/fileio.c (test_system): Check for shell twice.
  

Comments

Hui Zhu May 27, 2014, 10:12 a.m. UTC | #1
Ping.

Thanks,
Hui

On Tue, May 13, 2014 at 11:27 AM, Hui Zhu <hui_zhu@mentor.com> wrote:
> This patch is update version according to the discussion in
> https://www.sourceware.org/ml/gdb-patches/2009-11/msg00090.html.
> If test get the target doesn't support fileio system according to the
> remote log.   It will set this test as "unsupported".
>
> Before I made this patch, I want add a check before all of tests in this
> file.  But I found that the target maybe support one call but not others.
> For example: my target support Fwrite, Fopen and so on.  But not
> Fgettimeofday.
> And it doesn't support Fsystem NULL but it support Fsystem not NULL.
> So I think if we want to check target support fileio, we need check them
> one by one.
>
> Thanks,
> Hui
>
> 2014-05-13  Nathan Sidwell  <nathan@codesourcery.com>
>             Hui Zhu  <hui@codesourcery.com>
>
>         * gdb.base/fileio.exp: Add test for shell not available as well as
>         available.
>         * gdb.base/fileio.c (test_system): Check for shell twice.
>
> --- a/gdb/testsuite/gdb.base/fileio.c
> +++ b/gdb/testsuite/gdb.base/fileio.c
> @@ -55,7 +55,11 @@ time(time_t *t);
>  Not applicable.
>
>  system (const char * string);
> -1) Invalid string/command. -  returns 127.  */
> +1) See if shell available - returns 0
> +2) See if shell available - returns !0
> +3) Execute simple shell command - returns 0
> +4) Invalid string/command. -  returns 127.  */
> +
>  static const char *strerrno (int err);
>
>  /* Note that OUTDIR is defined by the test suite.  */
> @@ -375,21 +379,26 @@ test_system ()
>     */
>    int ret;
>
> -  /* Test for shell */
> +  /* Test for shell (testsuite should have it disabled).  */
>    ret = system (NULL);
> -  printf ("system 1: ret = %d %s\n", ret, ret != 0 ? "OK" : "");
> +  printf ("system 1: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
> +  stop ();
> +  /* Test for shell again (the testsuite will have enabled it now).  */
> +  ret = system (NULL);
> +  printf ("system 2: ret = %d %s\n", ret, ret != 0 ? "OK" : "");
>    stop ();
>    /* This test prepares the directory for test_rename() */
>    sprintf (sys, "mkdir -p %s/%s %s/%s", OUTDIR, TESTSUBDIR, OUTDIR,
> TESTDIR2);
>    ret = system (sys);
>    if (ret == 127)
> -    printf ("system 2: ret = %d /bin/sh unavailable???\n", ret);
> +    printf ("system 3: ret = %d /bin/sh unavailable???\n", ret);
>    else
> -    printf ("system 2: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
> +    printf ("system 3: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
>    stop ();
>    /* Invalid command (just guessing ;-) ) */
>    ret = system ("wrtzlpfrmpft");
> -  printf ("system 3: ret = %d %s\n", ret, WEXITSTATUS (ret) == 127 ? "OK" :
> "");
> +  printf ("system 4: ret = %d %s\n", ret,
> +         WEXITSTATUS (ret) == 127 ? "OK" : "");
>    stop ();
>  }
>
> --- a/gdb/testsuite/gdb.base/fileio.exp
> +++ b/gdb/testsuite/gdb.base/fileio.exp
> @@ -180,19 +180,34 @@ gdb_test continue \
>  "Continuing\\..*isatty 5:.*OK$stop_msg" \
>  "Isatty (open file)"
>
> -gdb_test continue \
> -"Continuing\\..*system 1:.*OK$stop_msg" \
> -"System says shell is available"
> +gdb_test_no_output "set debug remote 1"
> +set msg "System says shell is not available"
> +gdb_test_multiple "continue" $msg {
> +    -re "Continuing\\..*Fsystem.*system 1:.*OK$stop_msg\r\n$gdb_prompt $" {
> +       pass $msg
> +    }
> +    -re ".*Fsystem.*$gdb_prompt $" {
> +       fail $msg
> +    }
> +    -re ".*$gdb_prompt $" {
> +       unsupported $msg
> +    }
> +}
> +gdb_test_no_output "set debug remote 0"
>
>  gdb_test_no_output "set remote system-call-allowed 1"
>
>  gdb_test continue \
>  "Continuing\\..*system 2:.*OK$stop_msg" \
> +"System says shell is available"
> +
> +gdb_test continue \
> +"Continuing\\..*system 3:.*OK$stop_msg" \
>  "System(3) call"
>
>  # Is this ok?  POSIX says system returns a waitpid status?
>  gdb_test continue \
> -"Continuing\\..*system 3:.*OK$stop_msg" \
> +"Continuing\\..*system 4:.*OK$stop_msg" \
>  "System with invalid command returns 127"
>
>  gdb_test continue \
  
Pedro Alves May 29, 2014, 11:12 a.m. UTC | #2
On 05/13/2014 04:27 AM, Hui Zhu wrote:
> This patch is update version according to the discussion in
> https://www.sourceware.org/ml/gdb-patches/2009-11/msg00090.html.
> If test get the target doesn't support fileio system according to the
> remote log.   It will set this test as "unsupported".
> 
> Before I made this patch, I want add a check before all of tests in this
> file.  But I found that the target maybe support one call but not others.
> For example: my target support Fwrite, Fopen and so on.  But not 
> Fgettimeofday.
> And it doesn't support Fsystem NULL but it support Fsystem not NULL.

So IIUC, the test will still have system (NULL) FAIL on your
target, right?

> So I think if we want to check target support fileio, we need check them
> one by one.

Against native, we'll get a new UNSUPPORTED, right?

> @@ -375,21 +379,26 @@ test_system ()
>      */
>     int ret;
> 
> -  /* Test for shell */
> +  /* Test for shell (testsuite should have it disabled).  */

This comment confused me a bit, as I didn't recall that
this is disabled by default.  So, I'd prefer:

  /* Test for shell ('set remote system-call-allowed' is disabled
     by default).  */

>     ret = system (NULL);
> -  printf ("system 1: ret = %d %s\n", ret, ret != 0 ? "OK" : "");
> +  printf ("system 1: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
> +  stop ();
> +  /* Test for shell again (the testsuite will have enabled it now).  */
> +  ret = system (NULL);
> +  printf ("system 2: ret = %d %s\n", ret, ret != 0 ? "OK" : "");
>     stop ();
>     /* This test prepares the directory for test_rename() */
>     sprintf (sys, "mkdir -p %s/%s %s/%s", OUTDIR, TESTSUBDIR, OUTDIR, 
> TESTDIR2);
>     ret = system (sys);
>     if (ret == 127)
> -    printf ("system 2: ret = %d /bin/sh unavailable???\n", ret);
> +    printf ("system 3: ret = %d /bin/sh unavailable???\n", ret);
>     else
> -    printf ("system 2: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
> +    printf ("system 3: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
>     stop ();
>     /* Invalid command (just guessing ;-) ) */
>     ret = system ("wrtzlpfrmpft");
> -  printf ("system 3: ret = %d %s\n", ret, WEXITSTATUS (ret) == 127 ? 
> "OK" : "");
> +  printf ("system 4: ret = %d %s\n", ret,
> +	  WEXITSTATUS (ret) == 127 ? "OK" : "");
>     stop ();
>   }
> 
> --- a/gdb/testsuite/gdb.base/fileio.exp
> +++ b/gdb/testsuite/gdb.base/fileio.exp
> @@ -180,19 +180,34 @@ gdb_test continue \
>   "Continuing\\..*isatty 5:.*OK$stop_msg" \
>   "Isatty (open file)"
> 
> -gdb_test continue \
> -"Continuing\\..*system 1:.*OK$stop_msg" \
> -"System says shell is available"
> +gdb_test_no_output "set debug remote 1"
> +set msg "System says shell is not available"
> +gdb_test_multiple "continue" $msg {
> +    -re "Continuing\\..*Fsystem.*system 1:.*OK$stop_msg\r\n$gdb_prompt $" {
> +	pass $msg
> +    }
> +    -re ".*Fsystem.*$gdb_prompt $" {
> +	fail $msg
> +    }
> +    -re ".*$gdb_prompt $" {
> +	unsupported $msg
> +    }

No need for leading '.*' :

    -re "Fsystem.*$gdb_prompt $" {
	fail $msg
    }
    -re "$gdb_prompt $" {
	unsupported $msg
    }

OK with those changes.
  
Hui Zhu June 4, 2014, 6:44 a.m. UTC | #3
On 05/29/14 19:12, Pedro Alves wrote:
> On 05/13/2014 04:27 AM, Hui Zhu wrote:
>> This patch is update version according to the discussion in
>> https://www.sourceware.org/ml/gdb-patches/2009-11/msg00090.html.
>> If test get the target doesn't support fileio system according to the
>> remote log.   It will set this test as "unsupported".
>>
>> Before I made this patch, I want add a check before all of tests in this
>> file.  But I found that the target maybe support one call but not others.
>> For example: my target support Fwrite, Fopen and so on.  But not
>> Fgettimeofday.
>> And it doesn't support Fsystem NULL but it support Fsystem not NULL.
>
> So IIUC, the test will still have system (NULL) FAIL on your
> target, right?

It will use fileio if argument of Fsystem is not NULL.
But will not use fileio if its argument is NULL.

>
>> So I think if we want to check target support fileio, we need check them
>> one by one.
>
> Against native, we'll get a new UNSUPPORTED, right?

With current patch, we will get a new UNSUPPORTED because it just check 
if target support fileio system(NULL) without "set remote 
system-call-allowed 1".


>
>> @@ -375,21 +379,26 @@ test_system ()
>>       */
>>      int ret;
>>
>> -  /* Test for shell */
>> +  /* Test for shell (testsuite should have it disabled).  */
>
> This comment confused me a bit, as I didn't recall that
> this is disabled by default.  So, I'd prefer:
>
>    /* Test for shell ('set remote system-call-allowed' is disabled
>       by default).  */

Updated.

>
>>      ret = system (NULL);
>> -  printf ("system 1: ret = %d %s\n", ret, ret != 0 ? "OK" : "");
>> +  printf ("system 1: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
>> +  stop ();
>> +  /* Test for shell again (the testsuite will have enabled it now).  */
>> +  ret = system (NULL);
>> +  printf ("system 2: ret = %d %s\n", ret, ret != 0 ? "OK" : "");
>>      stop ();
>>      /* This test prepares the directory for test_rename() */
>>      sprintf (sys, "mkdir -p %s/%s %s/%s", OUTDIR, TESTSUBDIR, OUTDIR,
>> TESTDIR2);
>>      ret = system (sys);
>>      if (ret == 127)
>> -    printf ("system 2: ret = %d /bin/sh unavailable???\n", ret);
>> +    printf ("system 3: ret = %d /bin/sh unavailable???\n", ret);
>>      else
>> -    printf ("system 2: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
>> +    printf ("system 3: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
>>      stop ();
>>      /* Invalid command (just guessing ;-) ) */
>>      ret = system ("wrtzlpfrmpft");
>> -  printf ("system 3: ret = %d %s\n", ret, WEXITSTATUS (ret) == 127 ?
>> "OK" : "");
>> +  printf ("system 4: ret = %d %s\n", ret,
>> +	  WEXITSTATUS (ret) == 127 ? "OK" : "");
>>      stop ();
>>    }
>>
>> --- a/gdb/testsuite/gdb.base/fileio.exp
>> +++ b/gdb/testsuite/gdb.base/fileio.exp
>> @@ -180,19 +180,34 @@ gdb_test continue \
>>    "Continuing\\..*isatty 5:.*OK$stop_msg" \
>>    "Isatty (open file)"
>>
>> -gdb_test continue \
>> -"Continuing\\..*system 1:.*OK$stop_msg" \
>> -"System says shell is available"
>> +gdb_test_no_output "set debug remote 1"
>> +set msg "System says shell is not available"
>> +gdb_test_multiple "continue" $msg {
>> +    -re "Continuing\\..*Fsystem.*system 1:.*OK$stop_msg\r\n$gdb_prompt $" {
>> +	pass $msg
>> +    }
>> +    -re ".*Fsystem.*$gdb_prompt $" {
>> +	fail $msg
>> +    }
>> +    -re ".*$gdb_prompt $" {
>> +	unsupported $msg
>> +    }
>
> No need for leading '.*' :
>
>      -re "Fsystem.*$gdb_prompt $" {
> 	fail $msg
>      }
>      -re "$gdb_prompt $" {
> 	unsupported $msg
>      }
>
> OK with those changes.
>

Committed as 
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9f5a4cef68413e211bc765e969bf6778150231db

Thanks,
Hui
  
Pedro Alves June 4, 2014, 4:29 p.m. UTC | #4
On 06/04/2014 07:44 AM, Hui Zhu wrote:
> On 05/29/14 19:12, Pedro Alves wrote:
>> On 05/13/2014 04:27 AM, Hui Zhu wrote:
>>> This patch is update version according to the discussion in
>>> https://www.sourceware.org/ml/gdb-patches/2009-11/msg00090.html.
>>> If test get the target doesn't support fileio system according to the
>>> remote log.   It will set this test as "unsupported".
>>>
>>> Before I made this patch, I want add a check before all of tests in this
>>> file.  But I found that the target maybe support one call but not others.
>>> For example: my target support Fwrite, Fopen and so on.  But not
>>> Fgettimeofday.
>>> And it doesn't support Fsystem NULL but it support Fsystem not NULL.
>>
>> So IIUC, the test will still have system (NULL) FAIL on your
>> target, right?
> 
> It will use fileio if argument of Fsystem is not NULL.
> But will not use fileio if its argument is NULL.

I mean, you still get one FAIL when you run the test
against your target, even with your patch applied.

Otherwise, I think I'm missing something.

Hmm, looking again at the patch, I'm pretty much confused.  :-/

gdb_test continue \
-"Continuing\\..*system 1:.*OK$stop_msg" \
-"System says shell is available"
+gdb_test_no_output "set debug remote 1"
+set msg "System says shell is not available"
+gdb_test_multiple "continue" $msg {
+    -re "Continuing\\..*Fsystem.*system 1:.*OK$stop_msg\r\n$gdb_prompt $" {
                         ^^^^^^^

What's this "Fsystem" supposed to be matching here?  At this point
"set remote system-call-allowed 1" has NOT been issued yet, so
how come we expect to see that?

+       pass $msg
+    }
+    -re ".*Fsystem.*$gdb_prompt $" {
+       fail $msg
+    }
+    -re "$gdb_prompt $" {
+       unsupported $msg
+    }
+}
+gdb_test_no_output "set debug remote 0"
  
Hui Zhu June 8, 2014, 10:05 a.m. UTC | #5
On 06/05/14 00:29, Pedro Alves wrote:
> On 06/04/2014 07:44 AM, Hui Zhu wrote:
>> On 05/29/14 19:12, Pedro Alves wrote:
>>> On 05/13/2014 04:27 AM, Hui Zhu wrote:
>>>> This patch is update version according to the discussion in
>>>> https://www.sourceware.org/ml/gdb-patches/2009-11/msg00090.html.
>>>> If test get the target doesn't support fileio system according to the
>>>> remote log.   It will set this test as "unsupported".
>>>>
>>>> Before I made this patch, I want add a check before all of tests in this
>>>> file.  But I found that the target maybe support one call but not others.
>>>> For example: my target support Fwrite, Fopen and so on.  But not
>>>> Fgettimeofday.
>>>> And it doesn't support Fsystem NULL but it support Fsystem not NULL.
>>>
>>> So IIUC, the test will still have system (NULL) FAIL on your
>>> target, right?
>>
>> It will use fileio if argument of Fsystem is not NULL.
>> But will not use fileio if its argument is NULL.
>
> I mean, you still get one FAIL when you run the test
> against your target, even with your patch applied.

No.  It will not because the fail just happen when the target use remote 
file io to handle this system call.

>
> Otherwise, I think I'm missing something.
>
> Hmm, looking again at the patch, I'm pretty much confused.  :-/
>
> gdb_test continue \
> -"Continuing\\..*system 1:.*OK$stop_msg" \
> -"System says shell is available"
> +gdb_test_no_output "set debug remote 1"
> +set msg "System says shell is not available"
> +gdb_test_multiple "continue" $msg {
> +    -re "Continuing\\..*Fsystem.*system 1:.*OK$stop_msg\r\n$gdb_prompt $" {
>                           ^^^^^^^
>
> What's this "Fsystem" supposed to be matching here?  At this point
> "set remote system-call-allowed 1" has NOT been issued yet, so
> how come we expect to see that?

I am so sorry that I didn't explain this patch very clear.
The test before "set remote system-call-allowed 1" is to test the 
function that introduced in 
https://sourceware.org/gdb/current/onlinedocs/gdb/system.html "Due to 
security concerns, the system call is by default refused by GDB. The 
user has to allow this call explicitly with the set remote 
system-call-allowed 1 command."

So 3 part of this test is for:

gdb_test_no_output "set debug remote 1"
# Open the switch to check if this target use file io to handle this 
system or not.

set msg "System says shell is not available"
gdb_test_multiple "continue" $msg {
     -re "Continuing\\..*Fsystem.*system 1:.*OK$stop_msg\r\n$gdb_prompt $" {
	pass $msg
     }

Target uses file io handle system and it gets OK.
Then the test pass.

     -re ".*Fsystem.*$gdb_prompt $" {
	fail $msg
     }

Target uses file io handle system and it doesn't OK.
Then the test fail.

     -re "$gdb_prompt $" {
	unsupported $msg
     }

Target doesn't use file io.  The test is not support.

}

Thanks,
Hui

>
> +       pass $msg
> +    }
> +    -re ".*Fsystem.*$gdb_prompt $" {
> +       fail $msg
> +    }
> +    -re "$gdb_prompt $" {
> +       unsupported $msg
> +    }
> +}
> +gdb_test_no_output "set debug remote 0"
>
  
Pedro Alves June 9, 2014, 9:29 a.m. UTC | #6
On 06/08/2014 11:05 AM, Hui Zhu wrote:
> On 06/05/14 00:29, Pedro Alves wrote:
>> On 06/04/2014 07:44 AM, Hui Zhu wrote:
>>> On 05/29/14 19:12, Pedro Alves wrote:
>>>> On 05/13/2014 04:27 AM, Hui Zhu wrote:
>>>>> This patch is update version according to the discussion in
>>>>> https://www.sourceware.org/ml/gdb-patches/2009-11/msg00090.html.
>>>>> If test get the target doesn't support fileio system according to the
>>>>> remote log.   It will set this test as "unsupported".
>>>>>
>>>>> Before I made this patch, I want add a check before all of tests in this
>>>>> file.  But I found that the target maybe support one call but not others.
>>>>> For example: my target support Fwrite, Fopen and so on.  But not
>>>>> Fgettimeofday.
>>>>> And it doesn't support Fsystem NULL but it support Fsystem not NULL.
>>>>
>>>> So IIUC, the test will still have system (NULL) FAIL on your
>>>> target, right?
>>>
>>> It will use fileio if argument of Fsystem is not NULL.
>>> But will not use fileio if its argument is NULL.
>>
>> I mean, you still get one FAIL when you run the test
>> against your target, even with your patch applied.
> 
> No.  It will not because the fail just happen when the target use remote 
> file io to handle this system call.

I think I understand now.  So in this test:

~~~
 gdb_test_no_output "set remote system-call-allowed 1"

 gdb_test continue \
 "Continuing\\..*system 2:.*OK$stop_msg" \
 "System says shell is available"
~~~

your target is not sending an Fsystem packet at all,
and it happens that you target's "system" implementation
returns != 0, indicating that there's a shell on the target.
Correct?

Odd target behavior...  Why would this be desirable instead of
a target bug?  But I digress.

>> Otherwise, I think I'm missing something.
>>
>> Hmm, looking again at the patch, I'm pretty much confused.  :-/

...

>> What's this "Fsystem" supposed to be matching here?  At this point
>> "set remote system-call-allowed 1" has NOT been issued yet, so
>> how come we expect to see that?

...

> set msg "System says shell is not available"
> gdb_test_multiple "continue" $msg {
>      -re "Continuing\\..*Fsystem.*system 1:.*OK$stop_msg\r\n$gdb_prompt $" {
> 	pass $msg
>      }
> 
> Target uses file io handle system and it gets OK.
> Then the test pass.

Ah, I see now.  I managed to get confused and assume that
Fsystem was GDB's reply, instead of the request the target
sends...

Thanks,
  
Hui Zhu June 9, 2014, 3:56 p.m. UTC | #7
On 06/09/14 17:29, Pedro Alves wrote:
> On 06/08/2014 11:05 AM, Hui Zhu wrote:
>> On 06/05/14 00:29, Pedro Alves wrote:
>>> On 06/04/2014 07:44 AM, Hui Zhu wrote:
>>>> On 05/29/14 19:12, Pedro Alves wrote:
>>>>> On 05/13/2014 04:27 AM, Hui Zhu wrote:
>>>>>> This patch is update version according to the discussion in
>>>>>> https://www.sourceware.org/ml/gdb-patches/2009-11/msg00090.html.
>>>>>> If test get the target doesn't support fileio system according to the
>>>>>> remote log.   It will set this test as "unsupported".
>>>>>>
>>>>>> Before I made this patch, I want add a check before all of tests in this
>>>>>> file.  But I found that the target maybe support one call but not others.
>>>>>> For example: my target support Fwrite, Fopen and so on.  But not
>>>>>> Fgettimeofday.
>>>>>> And it doesn't support Fsystem NULL but it support Fsystem not NULL.
>>>>>
>>>>> So IIUC, the test will still have system (NULL) FAIL on your
>>>>> target, right?
>>>>
>>>> It will use fileio if argument of Fsystem is not NULL.
>>>> But will not use fileio if its argument is NULL.
>>>
>>> I mean, you still get one FAIL when you run the test
>>> against your target, even with your patch applied.
>>
>> No.  It will not because the fail just happen when the target use remote
>> file io to handle this system call.
>
> I think I understand now.  So in this test:
>
> ~~~
>   gdb_test_no_output "set remote system-call-allowed 1"
>
>   gdb_test continue \
>   "Continuing\\..*system 2:.*OK$stop_msg" \
>   "System says shell is available"
> ~~~
>
> your target is not sending an Fsystem packet at all,
> and it happens that you target's "system" implementation
> returns != 0, indicating that there's a shell on the target.
> Correct?
>
> Odd target behavior...  Why would this be desirable instead of
> a target bug?  But I digress.

I checked all the gdbrsp packages with the target of this test.
It will not use fileio if the argument of system is NULL.
It will use fileio if the argument is not NULL.

I think idea of target's designer is that system NULL can be handled in 
target very well.  So it will not use fileio in this moment.

I have two ideas on this test:
1. What about I add another test the will do a system(!NULL) before "set 
remote system-call-allowed 1"?

2. Other gdb_test of this test will meet the same issue that target 
doesn't support fileio with the system call but just get a pass.
What about I change other gdb_test to the test like what I didn't in 
current patch?



>
>>> Otherwise, I think I'm missing something.
>>>
>>> Hmm, looking again at the patch, I'm pretty much confused.  :-/
>
> ...
>
>>> What's this "Fsystem" supposed to be matching here?  At this point
>>> "set remote system-call-allowed 1" has NOT been issued yet, so
>>> how come we expect to see that?
>
> ...
>
>> set msg "System says shell is not available"
>> gdb_test_multiple "continue" $msg {
>>       -re "Continuing\\..*Fsystem.*system 1:.*OK$stop_msg\r\n$gdb_prompt $" {
>> 	pass $msg
>>       }
>>
>> Target uses file io handle system and it gets OK.
>> Then the test pass.
>
> Ah, I see now.  I managed to get confused and assume that
> Fsystem was GDB's reply, instead of the request the target
> sends...

Yes, and GDB doesn't have a switch or something to control except 
"remote system-call-allowed".
Then gdbserver can attack the file system of GDB if it want.

>
> Thanks,
>
  

Patch

--- a/gdb/testsuite/gdb.base/fileio.c
+++ b/gdb/testsuite/gdb.base/fileio.c
@@ -55,7 +55,11 @@  time(time_t *t);
  Not applicable.

  system (const char * string);
-1) Invalid string/command. -  returns 127.  */
+1) See if shell available - returns 0
+2) See if shell available - returns !0
+3) Execute simple shell command - returns 0
+4) Invalid string/command. -  returns 127.  */
+
  static const char *strerrno (int err);

  /* Note that OUTDIR is defined by the test suite.  */
@@ -375,21 +379,26 @@  test_system ()
     */
    int ret;

-  /* Test for shell */
+  /* Test for shell (testsuite should have it disabled).  */
    ret = system (NULL);
-  printf ("system 1: ret = %d %s\n", ret, ret != 0 ? "OK" : "");
+  printf ("system 1: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
+  stop ();
+  /* Test for shell again (the testsuite will have enabled it now).  */
+  ret = system (NULL);
+  printf ("system 2: ret = %d %s\n", ret, ret != 0 ? "OK" : "");
    stop ();
    /* This test prepares the directory for test_rename() */
    sprintf (sys, "mkdir -p %s/%s %s/%s", OUTDIR, TESTSUBDIR, OUTDIR, 
TESTDIR2);
    ret = system (sys);
    if (ret == 127)
-    printf ("system 2: ret = %d /bin/sh unavailable???\n", ret);
+    printf ("system 3: ret = %d /bin/sh unavailable???\n", ret);
    else
-    printf ("system 2: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
+    printf ("system 3: ret = %d %s\n", ret, ret == 0 ? "OK" : "");
    stop ();
    /* Invalid command (just guessing ;-) ) */
    ret = system ("wrtzlpfrmpft");
-  printf ("system 3: ret = %d %s\n", ret, WEXITSTATUS (ret) == 127 ? 
"OK" : "");
+  printf ("system 4: ret = %d %s\n", ret,
+	  WEXITSTATUS (ret) == 127 ? "OK" : "");
    stop ();
  }

--- a/gdb/testsuite/gdb.base/fileio.exp
+++ b/gdb/testsuite/gdb.base/fileio.exp
@@ -180,19 +180,34 @@  gdb_test continue \
  "Continuing\\..*isatty 5:.*OK$stop_msg" \
  "Isatty (open file)"

-gdb_test continue \
-"Continuing\\..*system 1:.*OK$stop_msg" \
-"System says shell is available"
+gdb_test_no_output "set debug remote 1"
+set msg "System says shell is not available"
+gdb_test_multiple "continue" $msg {
+    -re "Continuing\\..*Fsystem.*system 1:.*OK$stop_msg\r\n$gdb_prompt $" {
+	pass $msg
+    }
+    -re ".*Fsystem.*$gdb_prompt $" {
+	fail $msg
+    }
+    -re ".*$gdb_prompt $" {
+	unsupported $msg
+    }
+}
+gdb_test_no_output "set debug remote 0"

  gdb_test_no_output "set remote system-call-allowed 1"

  gdb_test continue \
  "Continuing\\..*system 2:.*OK$stop_msg" \
+"System says shell is available"
+
+gdb_test continue \
+"Continuing\\..*system 3:.*OK$stop_msg" \
  "System(3) call"

  # Is this ok?  POSIX says system returns a waitpid status?
  gdb_test continue \
-"Continuing\\..*system 3:.*OK$stop_msg" \
+"Continuing\\..*system 4:.*OK$stop_msg" \
  "System with invalid command returns 127"

  gdb_test continue \