[0/5] Various test suite fixes (mostly thread related)

Message ID 20230217203818.11287-1-jhb@FreeBSD.org
Headers
Series Various test suite fixes (mostly thread related) |

Message

John Baldwin Feb. 17, 2023, 8:38 p.m. UTC
  I've been working on a series of patches for the FreeBSD native target
to fix various issues with multiprocessing over the past month or two.
During that time I have fixed a few issues in various tests I've
observed so far.  This series is just some of the test fixes.

Patch 1 fixes a Linux-specific assumption about PTHREAD_STACK_MIN.
There are a couple of other tests that also hardcoded
PTHREAD_STACK_MIN*2, but I haven't observed crashes with those on
FreeBSD so haven't adjusted those.  However, it might be more correct
to apply this same logic to the other tests as well.

Patch 2 fixes some Linux-specific assumptions about system call
catching.  The test still has some failures on FreeBSD that I might
need to adjust in the FreeBSD native target (FreeBSD reports a single
event for exec for example that is just the TARGET_WAITKIND_EXECD but
not a separate syscall exit event, and I think a similar issue might
also exist for fork events.)  However, FreeBSD now passes at least
some of the tests compared to passing zero before.

Patches 3 and 4 fix synchronization issues where tests were assuming
that if the main thread calls pthread_create and then exits, the new
thread will start executing bofore the main thread exits.  That isn't
guaranteed to be true as a given kernel scheduler may create the new
thread and place it on a run queue but not preempt to it and return to
the main thread letting it exit before the new thread runs and reports
an event to say it has been born.  Use a pthread_barrier to ensure the
requirements of the test instead.  There are likely other instances of
this, these are just the two I've noticed so far.  I wonder if Linux
reports thread creation events when the thread is created rather than
when it starts executing which might explain why these tests have this
assumption?

The last patch is a bit more of a RFC and probably not really the way
we want to fix the issue.  Some tests assume that the format of 'info
threads' is to have thread identifiers start with "Thread".  However,
some targets use a different format.  For example FreeBSD's native
target refers to threads as "LWP <nnnn>".  There are some existing
tests that use a looser regex here already rather than requiring the
word "Thread".  I wonder if we want to define a helper macro or
variable or like to contain the regex to use for matching on a valid
thread identifier instead?

John Baldwin (5):
  gdb.threads/multi-create: Double the existing stack size.
  gdb.base/catch-syscall.exp: Remove some Linux-only assumptions.
  gdb.threads/execl.c: Ensure all threads are started before execl.
  gdb.threads/next-bp-other-thread.c: Ensure child thread is started.
  gdb tests: Allow for "LWP" in thread IDs from info threads.

 gdb/testsuite/gdb.base/catch-syscall.c        |  4 ++
 gdb/testsuite/gdb.base/catch-syscall.exp      | 48 +++++++++++++++++--
 .../gdb.multi/multi-target-thread-find.exp    |  2 +-
 gdb/testsuite/gdb.python/py-thrhandle.exp     |  2 +-
 .../gdb.server/stop-reply-no-thread-multi.exp |  8 ++--
 gdb/testsuite/gdb.threads/execl.c             |  8 ++++
 gdb/testsuite/gdb.threads/execl.exp           |  6 +--
 .../gdb.threads/fork-child-threads.exp        |  2 +-
 .../gdb.threads/fork-thread-pending.exp       | 16 +++----
 .../gdb.threads/info-threads-cur-sal.exp      |  8 ++--
 .../gdb.threads/interrupt-while-step-over.exp |  2 +-
 gdb/testsuite/gdb.threads/leader-exit.exp     |  2 +-
 gdb/testsuite/gdb.threads/manythreads.exp     |  2 +-
 gdb/testsuite/gdb.threads/multi-create.c      |  8 +++-
 .../gdb.threads/next-bp-other-thread.c        |  9 ++++
 .../gdb.threads/no-unwaited-for-left.exp      |  4 +-
 gdb/testsuite/gdb.threads/non-ldr-exc-2.exp   |  2 +-
 gdb/testsuite/gdb.threads/pthreads.exp        |  8 ++--
 .../signal-command-handle-nopass.exp          |  2 +-
 ...ignal-command-multiple-signals-pending.exp |  2 +-
 .../signal-delivered-right-thread.exp         |  2 +-
 gdb/testsuite/gdb.threads/signal-sigtrap.exp  |  4 +-
 gdb/testsuite/gdb.threads/staticthreads.exp   |  2 +-
 .../gdb.threads/thread-specific-bp.exp        |  2 +-
 gdb/testsuite/gdb.threads/thread-specific.exp |  4 +-
 gdb/testsuite/gdb.threads/tls.exp             | 12 ++---
 gdb/testsuite/gdb.trace/report.exp            |  2 +-
 gdb/testsuite/gdb.trace/strace.exp            |  2 +-
 28 files changed, 120 insertions(+), 55 deletions(-)
  

Comments

Tom Tromey March 3, 2023, 7:16 p.m. UTC | #1
John> I've been working on a series of patches for the FreeBSD native target
John> to fix various issues with multiprocessing over the past month or two.
John> During that time I have fixed a few issues in various tests I've
John> observed so far.  This series is just some of the test fixes.

Thank you for doing this.

John> Patches 3 and 4 fix synchronization issues where tests were assuming
John> that if the main thread calls pthread_create and then exits, the new
John> thread will start executing bofore the main thread exits.

I like the barrier technique.

John> The last patch is a bit more of a RFC and probably not really the way
John> we want to fix the issue.  Some tests assume that the format of 'info
John> threads' is to have thread identifiers start with "Thread".  However,
John> some targets use a different format.  For example FreeBSD's native
John> target refers to threads as "LWP <nnnn>".  There are some existing
John> tests that use a looser regex here already rather than requiring the
John> word "Thread".  I wonder if we want to define a helper macro or
John> variable or like to contain the regex to use for matching on a valid
John> thread identifier instead?

I think a helper variable would be fine.  We already have a few of these
in gdb.exp, like 'octal' or 'bkptno_numopt_re'.

I think patches 1-4 are OK as-is.

thanks,
Tom