Re: [PATCH] Add system test before "set remote system-call-allowed 1"

[Hui Zhu <hui_zhu at mentor dot com>]

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,