Re: [PATCH v2 3/3] Add test for thread names

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 15-11-26 06:45 AM, Pedro Alves wrote:
> On 11/25/2015 09:48 PM, Simon Marchi wrote:
> 
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  pthread_t threads[NUM_THREADS];
>> +  struct thread_data args[NUM_THREADS];
>> +  pthread_barrier_t barrier;
>> +  int i;
>> +  const char *names[] = { "carrot", "potato", "celery" };
> 
> Add an alarm call so the process doesn't run forever if something
> goes wrong and it gets detached / reparented to init.

Since the main function doesn't wait forever, I don't know if it's necessary.
If gdb crashes and leaves the process running, the process should end by
itself (unless something is really wrong with the threads and the barrier).

I'll add it anyway, it doesn't hurt and it's a good practice.

>> +
>> +  /* Make sure that NAMES contains NUM_THREADS elements.  */
>> +  assert (sizeof (names) == sizeof(names[0]) * NUM_THREADS);
>> +
>> +  assert (0 == pthread_barrier_init (&barrier, NULL, NUM_THREADS + 1));
> 
> There should be no side-effects in assert calls:
> 
>      res = pthread_barrier_init (&barrier, NULL, NUM_THREADS + 1));
>      assert (res == 0);

Right, if asserts are disabled, that code is left out.  All fixed.

>> +
>> +  pthread_setname_np (pthread_self (), "main");
>> +
>> +  for (i = 0; i < NUM_THREADS; i++)
>> +    {
>> +      struct thread_data *arg = &args[i];
>> +
>> +      arg->name = names[i];
>> +      arg->barrier = &barrier;
>> +
>> +      assert (0 == pthread_create (&threads[i], NULL, thread_func, arg));
> 
> Ditto.
> 
>> +    }
>> +
>> +  pthread_barrier_wait (&barrier);
>> +
>> +  all_threads_ready ();
>> +
>> +  return 0;
>> +}
> 
> OK with above fixed.
> 
>> +set expected_thread_list "\\* 1   .*\"main\" all_threads_ready.*\n"
>> +append expected_thread_list "  2   .*\"carrot\".*\n"
>> +append expected_thread_list "  3   .*\"potato\".*\n"
>> +append expected_thread_list "  4   .*\"celery\".*"
>> +
>> +gdb_test "info threads" "$expected_thread_list" "list threads"
> 
> Note you can do this with multi_line.  E.g.:
> 
> gdb_test "info threads" \
>     [multi_line \
>          "\\* 1   .*\"main\" all_threads_ready.*" \
>          "  2   .*\"carrot\".*" \
> 	 "  3   .*\"potato\".*" \
> 	 "  4   .*\"celery\".*"] \
>     "list threads"

Ok.


I modified names.c significantly, so could you give it another quick look?

Changes:

- Added alarm call.
- Added some assert calls
- I was seeing some sigsegv and sigill in __run_exit_handlers when running the binary,
  I think it was because I was not joining the threads.  I added a second barrier call
  and am now joining the threads properly.
- Define _GNU_SOURCE to remove warning about pthread_setname_np.


>From 41b75663742b06550911bdf2b6aee704fca66645 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Thu, 26 Nov 2015 09:49:04 -0500
Subject: [PATCH] Add test for thread names

I couldn't find a test that verified the thread name functionality, so I
created a new one.

A target board can define gdb,no_thread_names if it doesn't support thread
names and wants to skip the tests that uses them.

This test has been made with Linux in mind.  Not all platforms use
pthread_setname_np to set the thread name, but some #ifdefs can be added
later in order to support other platforms.

Tested on x86-64 Ubuntu 14.04, native and remote.

gdb/testsuite/ChangeLog:

	* gdb.threads/names.exp: New file.
	* gdb.threads/names.c: New file.
	* README: Mention gdb,no_thread_names.
---
 gdb/testsuite/ChangeLog             |  6 +++
 gdb/testsuite/README                |  3 ++
 gdb/testsuite/gdb.threads/names.c   | 97 +++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.threads/names.exp | 38 +++++++++++++++
 4 files changed, 144 insertions(+)
 create mode 100644 gdb/testsuite/gdb.threads/names.c
 create mode 100644 gdb/testsuite/gdb.threads/names.exp

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 9f6d7e6..461565f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2015-11-26  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* gdb.threads/names.exp: New file.
+	* gdb.threads/names.c: New file.
+	* README: Mention gdb,no_thread_names.
+
 2015-11-26  Markus Metzger  <markus.t.metzger@intel.com>

 	PR 19297
diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index 70f65cd..77ac74e 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -369,6 +369,9 @@ gdb,predefined_tsv

   The predefined trace state variables the board has.

+gdb,no_thread_names
+
+  The target doesn't support thread names.

 Testsuite Organization
 **********************
diff --git a/gdb/testsuite/gdb.threads/names.c b/gdb/testsuite/gdb.threads/names.c
new file mode 100644
index 0000000..130ddc1
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/names.c
@@ -0,0 +1,97 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include <assert.h>
+
+#define NUM_THREADS 3
+
+struct thread_data
+{
+  const char *name;
+  pthread_barrier_t *barrier;
+};
+
+static void *
+thread_func (void *varg)
+{
+  struct thread_data *arg = (struct thread_data *) varg;
+  int res;
+
+  res = pthread_setname_np (pthread_self (), arg->name);
+  assert (res == 0);
+
+  pthread_barrier_wait (arg->barrier);
+
+  pthread_barrier_wait (arg->barrier);
+
+  return NULL;
+}
+
+static void
+all_threads_ready (void)
+{
+}
+
+int
+main (int argc, char **argv)
+{
+  pthread_t threads[NUM_THREADS];
+  struct thread_data args[NUM_THREADS];
+  pthread_barrier_t barrier;
+  int i, res;
+  const char *names[] = { "carrot", "potato", "celery" };
+
+  alarm (20);
+
+  /* Make sure that NAMES contains NUM_THREADS elements.  */
+  assert (sizeof (names) == sizeof (names[0]) * NUM_THREADS);
+
+  res = pthread_barrier_init (&barrier, NULL, NUM_THREADS + 1);
+  assert (res == 0);;
+
+  res = pthread_setname_np (pthread_self (), "main");
+  assert (res == 0);
+
+  for (i = 0; i < NUM_THREADS; i++)
+    {
+      struct thread_data *arg = &args[i];
+
+      arg->name = names[i];
+      arg->barrier = &barrier;
+
+      res = pthread_create (&threads[i], NULL, thread_func, arg);
+      assert (res == 0);
+    }
+
+  pthread_barrier_wait (&barrier);
+
+  all_threads_ready ();
+
+  pthread_barrier_wait (&barrier);
+
+  for (i = 0; i < NUM_THREADS; i++)
+    {
+      res = pthread_join (threads[i], NULL);
+      assert (res == 0);
+    }
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/names.exp b/gdb/testsuite/gdb.threads/names.exp
new file mode 100644
index 0000000..f6bbdb4
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/names.exp
@@ -0,0 +1,38 @@
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Verify that thread name features work properly (e.g. they show up in info
+# threads).
+
+if [target_info exists gdb,no_thread_names] {
+    continue
+}
+
+standard_testfile
+
+if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] {
+    return -1
+}
+
+if ![runto "all_threads_ready"] {
+    continue
+}
+
+gdb_test "info threads" \
+    [multi_line "\\* 1   .*\"main\" all_threads_ready.*" \
+		"  2   .*\"carrot\".*"  \
+		"  3   .*\"potato\".*"  \
+		"  4   .*\"celery\".*" ] \
+    "list threads"
-- 
2.5.1