[igt-dev] [PATCH i-g-t] lib/igt_core: Detect gdb harder

[Chris Wilson]

Quoting Ville Syrjälä (2020-01-22 17:21:26)
> On Wed, Jan 22, 2020 at 05:09:19PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjala (2020-01-22 16:43:32)
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > The current gdb detection only works for the parent igt process,
> > > but none of its children will see the gdb and so won't trap properly
> > > in igt_fail_assert(). Also we will not detect gdb if it was attached
> > > after the fact. Fix all of that by looking for the "TracerPid"
> > > information in /proc/<pid>/status. We'll leave the current "assume
> > > parent may be gdb" approach as a fallback.
> > > 
> > > Also annoyingly by default gdb will only track a single process.
> > > To make it track all of them, and let them all run simultanously
> > > one needs the following incantations:
> > >  set detach-on-fork off
> > >  set schedule-multiple on
> > > 
> > > Maybe that will save someone from having to trawl as many
> > > docs/gogole hits as I did.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > >  lib/igt_core.c | 38 ++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 36 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > index 0a0068946a6a..109b5926eac4 100644
> > > --- a/lib/igt_core.c
> > > +++ b/lib/igt_core.c
> > > @@ -1609,12 +1609,12 @@ void igt_describe_f(const char *fmt, ...)
> > >         assert(ret < sizeof(__current_description));
> > >  }
> > >  
> > > -static bool running_under_gdb(void)
> > > +static bool is_gdb(pid_t pid)
> > >  {
> > >         char pathname[30], buf[1024];
> > >         ssize_t len;
> > >  
> > > -       sprintf(pathname, "/proc/%d/exe", getppid());
> > > +       sprintf(pathname, "/proc/%d/exe", pid);
> > >         len = readlink(pathname, buf, sizeof(buf) - 1);
> > >         if (len < 0)
> > >                 return false;
> > > @@ -1624,6 +1624,40 @@ static bool running_under_gdb(void)
> > >         return strncmp(basename(buf), "gdb", 3) == 0;
> > >  }
> > >  
> > > +static pid_t tracer_pid(void)
> > > +{
> > > +       char pathname[30];
> > > +       pid_t pid = 0;
> > > +       FILE *f;
> > > +
> > > +       sprintf(pathname, "/proc/%d/status", getpid());
> > > +
> > > +       f = fopen(pathname, "r");
> > > +       if (!f)
> > > +               return getppid();
> > > +
> > > +       for (;;) {
> > > +               char buf[1024];
> > > +               char *s;
> > > +
> > > +               s = fgets(buf, sizeof(buf), f);
> > 
> > I'd prefer fgetline, /proc/<pid>status is >1024 bytes so there's always
> > that chance we cut the TracerPid line.
> 
> fgets() reads at most one line (which means my 1024 copypasta is
> probably a bit much). Not sure what fgetline() is.

I meant getline. fgets breaks on newlines as well? man says it does,
fair enough. I'd make that 3 things, but I'm going to forget at least
two of them by tomorrow.
-Chris