gdb: implement ::re_set method for catchpoint classHEADmaster

It is possible to attach a condition to a catchpoint.  This can't be
done when the catchpoint is created, but can be done with the
'condition' command, this is documented in the GDB manual:

     You can also use the 'if' keyword with the 'watch' command.  The
  'catch' command does not recognize the 'if' keyword; 'condition' is the
  only way to impose a further condition on a catchpoint.

A GDB crash was reported against Fedora GDB where a user had attached
a condition to a catchpoint and then restarted the inferior.  When the
catchpoint was hit GDB would immediately segfault.  I was able to
reproduce the failure on upstream GDB:

  (gdb) file ./some/binary
  (gdb) catch syscall write
  (gdb) run
  ...
  Catchpoint 1 (returned from syscall write), 0x00007ffff7b594a7 in write () from /lib64/libc.so.6
  (gdb) condition 1 $_streq((char *) $rsi, "foobar") == 0
  (gdb) run
  ...
  Fatal signal: Segmentation fault
  ...

What happened here is that on the system in question we had debug
information available for both the main application and also for
libc.

When the condition was attached GDB was stopped inside libc and as the
debug information was available GDB found a reference to the 'char'
type (for the cast) inside libc's debug information.

When the inferior is restarted GDB discards all of the objfiles
associated with shared libraries, and this includes libc.  As such the
'char' type, which is objfile owned, is discarded and the reference to
it from the catchpoint's condition expression becomes invalid.

Now, if it were a breakpoint instead of a catchpoint, what would
happen is that after the shared library objfiles had been discarded
we'd call the virtual breakpoint::re_set method on the breakpoint, and
this would update the breakpoint's condition expression.  This is
because user breakpoints are actually instances of the code_breakpoint
class and the code_breakpoint::re_set method contains the code to
recompute the breakpoint's condition expression.

However, catchpoints are instances of the catchpoint class which
inherits from the base breakpoint class.  The catchpoint class does
not override breakpoint::re_set, and breakpoint::re_set is empty!

The consequence of this is that catchpoint condition expressions are
never recomputed, and the dangling pointer to the now deleted, objfile
owned type 'char' is left around, and, when the catchpoint is hit, the
invalid pointer is used when GDB tries to evaluate the condition
expression.

In this commit I have implemented catchpoint::re_set.  This is pretty
simple and just recomputes the condition expression as you'd expect.
If the condition doesn't evaluate then the catchpoint is marked as
disabled_by_cond.

I have also made breakpoint::re_set pure virtual.  With the addition
of catchpoint::re_set every sub-class of breakpoint now implements the
::re_set method, and if new sub-classes are added in the future I
think that they _must_ implement ::re_set in order to avoid this
problem.  As such falling back to an empty breakpoint::re_set doesn't
seem helpful.

For testing I have not relied on stopping in libc and having libc
debug information available, this doesn't seem like a good idea for
the GDB testsuite.  Instead I create a (rather pointless) condition
check that uses a type defined only within a shared library.  When the
inferior is restarted the catchpoint will temporarily be marked as
disabled_by_cond (due to the type not being available), but once the
shared library is loaded again the catchpoint will be re-enabled.
Without the fixes above then the same crashing behaviour can be
observed.

One point of note: the dangling pointer of course exposes undefined
behaviour, with no guarantee of a crash.  Though a crash is what I
usually see I have see GDB throw random errors from the expression
evaluation code, and once, I saw no problem at all!  If you recompile
GDB with the address sanitizer, or run under valgrind, then the bug
will be exposed every time.

After fixing this bug I checked bugzilla and found PR gdb/29960 which
is the same bug.  I was able to reproduce the bug before this commit,
and after this commit GDB is no longer crashing.

Before:

  (gdb) file /tmp/hello.x
  Reading symbols from /tmp/hello.x...
  (gdb) run
  Starting program: /tmp/hello.x
  Hello World
  [Inferior 1 (process 1101855) exited normally]
  (gdb) catch syscall 1
  Catchpoint 1 (syscall 'write' [1])
  (gdb) condition 1 write.fd == 1
  (gdb) run
  Starting program: /tmp/hello.x

  Fatal signal: Segmentation fault
  ...

And after:

  (gdb) file /tmp/hello.x
  Reading symbols from /tmp/hello.x...
  (gdb) run
  Starting program: /tmp/hello.x
  Hello World
  Args: ( 0 , 1 , 2 , 3 , 4 , 5 , 6 , 7 )
  [Inferior 1 (process 1102373) exited normally]
  (gdb) catch syscall 1
  Catchpoint 1 (syscall 'write' [1])
  (gdb) condition 1 write.fd == 1
  (gdb) r
  Starting program: /tmp/hello.x
  Error in testing condition for breakpoint 1:
  Attempt to extract a component of a value that is not a structure.

  Catchpoint 1 (call to syscall write), 0x00007ffff7eb94a7 in write ()
     from /lib64/libc.so.6
  (gdb) ptype write
  type = <unknown return type> ()
  (gdb)

Notice we get the error now when the condition fails to evaluate.
This seems reasonable given that 'write' will be a function, and
indeed the final 'ptype' shows that it's a function, not a struct.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29960

Reviewed-By: Tom de Vries <tdevries@suse.de>

0 files changed, 0 insertions, 0 deletions