Re: [PATCH][gdb/testsuite] Rewrite catch-follow-exec.exp

[Gary Benson <gbenson@redhat.com>]

Tom de Vries wrote:
> On 10/9/18 3:51 PM, Gary Benson wrote:
> > Tom de Vries wrote:
> > >      append FLAGS " \"$binfile\""
> > >      append FLAGS " -batch"
> > > +    append FLAGS " -ex \"target native\""
> > >      append FLAGS " -ex \"catch exec\""
> > >      append FLAGS " -ex \"set follow-exec-mode new\""
> > 
> > I'm a little confused with this part, doesn't this force the test to
> > run on the host?
> 
> The "target native" was an attempt to fix problems when running with
> --target_board=native-gdbserver. Perhaps it's better to bail out in
> that case, but I haven't yet figured out how to. Any advice here?

Tests that can't run remote usually bail with something like this at
the start:

  if ![isnative] then {
      return
  }

There should probably also be an 'append FLAGS " -nx"' too.

> > > +	    # We're not testing the "status returned by the spawned process",
> > > +	    # because it's currently one, and we suspect it will be zero after
> > > +	    # fixing PR23368 - "gdb goes to into background when hitting exec
> > > +	    # catchpoint with follow-exec-mode new"
> > > +            #gdb_assert { [lindex $result 3] == 0 }
> > 
> > I'm not sure we should commit commented-out code.  Why not have the
> > test assert { [lindex $result 3] == 1 } if that's what's happening
> > now, with the comment reworded to indicate that it might need changing
> > to zero when PR23368 is fixed.  That way, when PR23368 *is* fixed,
> > whoever's fixing it gets a failing test, they investigate, find the
> > comment, and update it as part of their series.
> > 
> 
> Makes sense, will do.

I'm guessing this whole function could be replaced with something more
regular (which would work remote) once PR23368 is fixed?  Something
like this:

  clean_restart ${binfile}
  gdb_test "catch exec" "Catchpoint \[0-9\]+ \\\(exec\\\).*"
  gdb_test "set follow-exec-mode new"
  gdb_run_cmd
   ...

If that is the case, could you note that in that comment?  Or just
paste the URL of this thread in the archive.

I'm happy with this patch with these changes, but I'm not a maintainer
so one of those guys will have to give the final approval.

Thanks,
Gary