From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id a+YHKiPYZWUZkREAWB0awg (envelope-from ) for ; Tue, 28 Nov 2023 07:08:03 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=dZHSETOT; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id A0CE71E11B; Tue, 28 Nov 2023 07:08:03 -0500 (EST) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 8D9431E00F for ; Tue, 28 Nov 2023 07:08:01 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 10FB2384770C for ; Tue, 28 Nov 2023 12:08:01 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id A1F193858C50 for ; Tue, 28 Nov 2023 12:07:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A1F193858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A1F193858C50 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701173269; cv=none; b=d3pIYY/MIlo4LX8VJrbr+f/9+qiKpP8xgFQ7JkOml6a/J/YcjtfqNCC/XXTlniepSslkBv58AKWE8e6niMnjQTzZ7mvk6IDplQEk5zi8JPqCM7tQ/ahcVoNmP96zz5Q/tQPExgiVqeUWwiyRnQv0rimMD0o9Rkoko69ctgkvc5Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701173269; c=relaxed/simple; bh=HJr8tDG+qchI/4i4mI4YfzAtt8esg7ykLFxti+fu2bs=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=T7NdxTxeUGWsxBxX/q3/6WPUgFLwwBQik5yoLhrCm0dkuKMYm0RcjCuhPLkbWHJ15d9xMyzfa/wJTKFOcChRV6PAh5USBl7SxVBdijF2aCD1acKgragtwhif3huA8XYql6j9DfMUaVdE+y79mEuQAFtnyQUGtcQJvPljskdQNb0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701173266; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=BaetLx+k1ODtBz2JHvtgT97rMGMby/wj5GmkFH70DrY=; b=dZHSETOTFH5AtcvSFXyHWzzUv80MY6AsF9+lTWZu9I71Eoc0+jr0hz5eu8lbTluo2T2+x6 cmpkI4I0sCNGROpaJjquBCVEPDG2GP06CkLPtTkPFwu6zu0sb3hm11SOx9pYZaYa3kS9k0 ZszXXQ2JRLJ34pYkX1miH3qKQVsgs08= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-153-WKHAP6B4OvWcdh79r5_kOQ-1; Tue, 28 Nov 2023 07:07:44 -0500 X-MC-Unique: WKHAP6B4OvWcdh79r5_kOQ-1 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-333030e0708so1373071f8f.3 for ; Tue, 28 Nov 2023 04:07:44 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701173263; x=1701778063; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=BaetLx+k1ODtBz2JHvtgT97rMGMby/wj5GmkFH70DrY=; b=jSQ9sln9YVLJAflMUcIOY44rRg4G+2IhesaAjT509tIOihzcMPQZ7SVLjhV/Afd1Re HGJ9DBdB51TOnj4c7Bbu6GpZvOtDNj8l17gMf/7hG8UkrUTEkv9u/pvsuhu732pqb8xr DpgbFzhmI80aDltx1J41phNBv5RukaHXDgb9YuY7EExwj9M3Yrz6edwchQ+H783XOOgE 58EFtKZ3dhz1CVh07ScrVOGIHe17oBscd9lzTScrA/idhSBe9dvOGtWEBjgnHQ98SVUX 93AvfDcku3ANfzpG+WcRPj3xrcH7o0t88WkoXyXRpr74ggFpFLUgeGcoxdoZxzwdbFvf 0UHQ== X-Gm-Message-State: AOJu0YyDlB6tVgzLyeMclnou1cBC1GVFBBayNy3piN92ZUhOK2/VHr+P gUHfNCLeEDRFQOh/0VOqnpn+Nwlk3qf5osWDwyE1+AjEPGazjM8x4XR8oekbbIIznMOCz/mQOPt edHhAx36youT6DRQkEI/rKGMAPRgL8g== X-Received: by 2002:adf:f006:0:b0:333:73e:632d with SMTP id j6-20020adff006000000b00333073e632dmr2938014wro.6.1701173262996; Tue, 28 Nov 2023 04:07:42 -0800 (PST) X-Google-Smtp-Source: AGHT+IHjOEJ0ZhEtXaZHp6Fv//yMyBPWQ3k79UzXlY8MgqwWukFMVJf3wE5aEcnaWBNR9Y0Ar4aa9w== X-Received: by 2002:adf:f006:0:b0:333:73e:632d with SMTP id j6-20020adff006000000b00333073e632dmr2937998wro.6.1701173262346; Tue, 28 Nov 2023 04:07:42 -0800 (PST) Received: from localhost (105.226.159.143.dyn.plus.net. [143.159.226.105]) by smtp.gmail.com with ESMTPSA id u7-20020adfeb47000000b003330a1d35b6sm2167917wrn.115.2023.11.28.04.07.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Nov 2023 04:07:41 -0800 (PST) From: Andrew Burgess To: Guinevere Larsen , gdb-patches@sourceware.org Cc: Guinevere Larsen Subject: Re: [PATCH v3] gdb/testsuite: add test for backtracing for threaded inferiors from a corefile In-Reply-To: <20231025114253.259603-2-blarsen@redhat.com> References: <20231025114253.259603-2-blarsen@redhat.com> Date: Tue, 28 Nov 2023 12:07:40 +0000 Message-ID: <87ttp6nks3.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Guinevere Larsen writes: > This patch is based on an out-of-tree patch that fedora has been > carrying for a while. It tests if GDB is able to properly unwind a > threaded program in the following situations: > * regular threads > * in a signal handler > * in a signal handler executing on an alternate stack > > And the final frame can either be in a syscall or in an infinite loop. > > The test works by running the inferior until a crash to generate a > corefile, or until right before the crash. Then applies a backtrace to > all threads to see if any frame can't be identified, and the order of > the threads in GDB. Finally, it goes thread by thread and tries to > collect a large part of the backtrace, to confirm that everything is > being unwound correctly. > > Co-Authored-By: Andrew Burgess > --- > > Changes for v3: > * Resolved Lancelot's comment > * undid early exit in favor of more readable gdb_test usage to load the > corefile > > Changes for v2: > * Linaro CI identified an issue with the test, which made the test fail > when using read1. Fixed here > * Also added early exit on corefile tests, if the corefile isn't > properly loaded > > --- > gdb/testsuite/gdb.threads/threadcrash.c | 443 ++++++++++++++++++++++ > gdb/testsuite/gdb.threads/threadcrash.exp | 209 ++++++++++ > 2 files changed, 652 insertions(+) > create mode 100644 gdb/testsuite/gdb.threads/threadcrash.c > create mode 100644 gdb/testsuite/gdb.threads/threadcrash.exp > > diff --git a/gdb/testsuite/gdb.threads/threadcrash.c b/gdb/testsuite/gdb.threads/threadcrash.c > new file mode 100644 > index 00000000000..e476ae7b07d > --- /dev/null > +++ b/gdb/testsuite/gdb.threads/threadcrash.c > @@ -0,0 +1,443 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2023 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 . */ > + > +#include > +#include > +#include > +#include > +#include > + > +/* The delay that the main thread gives once all the worker threads have > + reached the barrier before the main thread enters the function on which > + GDB will have placed a breakpoint. */ > + > +#define MAIN_THREAD_DELAY 2 > + > +/* The maximum time we allow this test program to run for before an alarm > + signal is sent and everything will exit. */ > +#define WATCHDOG_ALARM_TIME 600 > + > +/* Aliases for the signals used within this script. Each signal > + corresponds to an action (from the FINAL_ACTION enum) that the signal > + handler will perform. */ > + > +#define SPIN_SIGNAL SIGUSR1 > +#define SYSCALL_SIGNAL SIGUSR2 > + > +/* Describe the final action that a thread should perform. */ > + > +enum final_action > + { > + /* Thread should spin in an infinite loop. */ > + SPIN = 0, > + > + /* Thread should block in a syscall. */ > + SYSCALL, > + > + /* This is just a marker to allow for looping over the enum. */ > + LAST_ACTION > + }; > + > +/* Where should the thread perform this action? */ > + > +enum exec_location > + { > + /* Just a normal thread, on a normal stack. */ > + NORMAL = 0, > + > + /* In a signal handler, but use the normal stack. */ > + SIGNAL_HANDLER, > + > + /* In a signal handler using an alternative stack. */ > + SIGNAL_ALT_STACK, > + > + /* This is just a marker to allow for looping over the enum. */ > + LAST_LOCACTION > + }; > + > +/* A descriptor for a single thread job. We create a new thread for each > + job_description. */ > + > +struct job_description > +{ > + /* What action should this thread perform. */ > + enum final_action action; > + > + /* Where should the thread perform the action. */ > + enum exec_location location; > + > + /* The actual thread handle, so we can join with the thread. */ > + pthread_t thread; > +}; > + > +/* A pthread barrier, used to (try) and synchronise the threads. */ > +pthread_barrier_t global_barrier; > + > +/* Return a list of jobs, and place the length of the list in *COUNT. */ > + > +struct job_description * > +get_job_list (int *count) > +{ > + /* The number of jobs. */ > + int num = LAST_ACTION * LAST_LOCACTION; > + > + /* The uninitialised array of jobs. */ > + struct job_description *list > + = malloc (num * sizeof (struct job_description)); > + assert (list != NULL); > + > + /* Fill the array with all possible jobs. */ > + for (int i = 0; i < (int) LAST_ACTION; ++i) > + for (int j = 0; j < (int) LAST_LOCACTION; ++j) > + { > + int idx = (i * LAST_LOCACTION) + j; > + list[idx].action = (enum final_action) i; > + list[idx].location = (enum exec_location) j; > + } > + > + /* Return the array of jobs. */ > + *count = num; > + return list; > +} > + > +/* This function should never be called. If it is then an assertion will > + trigger. */ > + > +void > +assert_not_reached (void) > +{ > + assert (0); > +} > + > +/* The function for a SPIN action. Just spins in a loop. The LOCATION > + argument exists so GDB can identify the expected context for this > + function. */ > + > +void > +do_spin_task (enum exec_location location) > +{ > + (void) location; > + > + /* Let everyone know that we're about to perform our action. */ > + int res = pthread_barrier_wait (&global_barrier); > + assert (res == PTHREAD_BARRIER_SERIAL_THREAD || res == 0); > + > + while (1) > + { > + /* Nothing. */ > + } > +} > + > +/* The function for a SYSCALL action. Just spins in a loop. The LOCATION > + argument exists so GDB can identify the expected context for this > + function. */ > + > +void > +do_syscall_task (enum exec_location location) > +{ > + (void) location; > + > + /* Let everyone know that we're about to perform our action. */ > + int res = pthread_barrier_wait (&global_barrier); > + assert (res == PTHREAD_BARRIER_SERIAL_THREAD || res == 0); > + > + sleep (600); > +} > + > +/* Return the required size for a sigaltstack. We start with a single > + page, but do check against the system defined minimums. We don't run > + much on the alternative stacks, so we don't need a huge one. */ > + > +size_t > +get_stack_size (void) > +{ > + size_t size = getpagesize (); /* Arbitrary starting size. */ > + if (size < SIGSTKSZ) > + size = SIGSTKSZ; > + if (size < MINSIGSTKSZ) > + size = MINSIGSTKSZ; > + return size; > +} > + > +/* A descriptor for an alternative stack. */ > + > +struct stack_descriptor > +{ > + /* The base address of the alternative stack. This is the address that > + must be freed to release the memory used by this stack. */ > + void *base; > + > + /* The size of this alternative stack. Tracked just so we can query this > + from GDB. */ > + size_t size; > +}; > + > +/* Install an alternative signal stack. Return a descriptor for the newly > + allocated alternative stack. */ > + > +struct stack_descriptor > +setup_alt_stack (void) > +{ > + size_t stack_size = get_stack_size (); > + > + void *stack_area = malloc (stack_size); > + > + stack_t stk; > + stk.ss_sp = stack_area; > + stk.ss_flags = 0; > + stk.ss_size = stack_size; > + > + int res = sigaltstack (&stk, NULL); > + assert (res == 0); > + > + struct stack_descriptor desc; > + desc.base = stack_area; > + desc.size = stack_size; > + > + return desc; > +} > + > +/* Return true (non-zero) if we are currently on the alternative stack, > + otherwise, return false (zero). */ > + > +int > +on_alt_stack_p (void) > +{ > + stack_t stk; > + int res = sigaltstack (NULL, &stk); > + assert (res == 0); > + > + return (stk.ss_flags & SS_ONSTACK) != 0; > +} > + > +/* The signal handler function. All signals call here, so we use SIGNO > + (the signal that was delivered) to decide what action to perform. This > + function might, or might not, have been called on an alternative signal > + stack. */ > + > +void > +signal_handler (int signo) > +{ > + enum exec_location location > + = on_alt_stack_p () ? SIGNAL_ALT_STACK : SIGNAL_HANDLER; > + > + switch (signo) > + { > + case SPIN_SIGNAL: > + do_spin_task (location); > + break; > + > + case SYSCALL_SIGNAL: > + do_syscall_task (location); > + break; > + > + default: > + assert_not_reached (); > + } > +} > + > +/* The thread worker function. ARG is a job_description pointer which > + describes what this thread is expected to do. This function always > + returns a NULL pointer. */ > + > +void * > +thread_function (void *arg) > +{ > + struct job_description *job = (struct job_description *) arg; > + struct stack_descriptor desc = { NULL, 0 }; > + int sa_flags = 0; > + > + switch (job->location) > + { > + case NORMAL: > + /* This thread performs the worker action on the current thread, > + select the correct worker function based on the requested > + action. */ > + switch (job->action) > + { > + case SPIN: > + do_spin_task (NORMAL); > + break; > + > + case SYSCALL: > + do_syscall_task (NORMAL); > + break; > + > + default: > + assert_not_reached (); > + } > + break; > + > + case SIGNAL_ALT_STACK: > + /* This thread is to perform its action in a signal handler on the > + alternative stack. Install the alternative stack now, and then > + fall through to the normal signal handler location code. */ > + desc = setup_alt_stack (); > + assert (desc.base != NULL); > + assert (desc.size > 0); > + sa_flags = SA_ONSTACK; > + > + /* Fall through. */ > + case SIGNAL_HANDLER: > + { > + /* This thread is to perform its action in a signal handler. We > + might have just installed an alternative signal stack. */ > + int signo, res; > + > + /* Select the correct signal number so that the signal handler will > + perform the required action. */ > + switch (job->action) > + { > + case SPIN: > + signo = SPIN_SIGNAL; > + break; > + > + case SYSCALL: > + signo = SYSCALL_SIGNAL; > + break; > + > + default: > + assert_not_reached (); > + } > + > + /* Now setup the signal handler. */ > + struct sigaction sa; > + sa.sa_handler = signal_handler; > + sigfillset (&sa.sa_mask); > + sa.sa_flags = sa_flags; > + res = sigaction (signo, &sa, NULL); > + assert (res == 0); > + > + /* Send the signal to this thread. */ > + res = pthread_kill (job->thread, signo); > + assert (res == 0); > + } > + break; > + > + default: > + assert_not_reached (); > + }; > + > + /* Free the alt-stack if we allocated one, if not DESC.BASE will be > + NULL so this call is fine. */ > + free (desc.base); > + > + /* Thread complete. */ > + return NULL; > +} > + > +void > +start_job (struct job_description *job) > +{ > + int res; > + > + res = pthread_create (&job->thread, NULL, thread_function, job); > + assert (res == 0); > +} > + > +/* Join with the thread for JOB. This will block until the thread for JOB > + has finished. */ > + > +void > +finalise_job (struct job_description *job) > +{ > + int res; > + void *retval; > + > + res = pthread_join (job->thread, &retval); > + assert (res == 0); > + assert (retval == NULL); > +} > + > +/* Function that GDB can place a breakpoint on. */ > + > +void > +breakpt (void) > +{ > + /* Nothing. */ > +} > + > +/* Function that triggers a crash, if the user has setup their environment > + correctly this will dump a core file, which GDB can then examine. */ > + > +void > +crash_function (void) > +{ > + volatile int *p = 0; > + volatile int n = *p; > + (void) n; > +} > + > +/* Entry point. */ > + > +int > +main () > +{ > + int job_count, res; > + struct job_description *jobs = get_job_list (&job_count); > + > + /* This test is going to park some threads inside infinite loops. Just > + in case this program is left running, install an alarm that will cause > + everything to exit. */ > + alarm (WATCHDOG_ALARM_TIME); > + > + /* We want each worker thread (of which there are JOB_COUNT) plus the > + main thread (hence + 1) to wait at the barrier. */ > + res = pthread_barrier_init (&global_barrier, NULL, job_count + 1); > + assert (res == 0); > + > + /* Start all the jobs. */ > + for (int i = 0; i < job_count; ++i) > + start_job (&jobs[i]); > + > + /* Notify all the worker threads that we're waiting for them. */ > + res = pthread_barrier_wait (&global_barrier); > + assert (res == PTHREAD_BARRIER_SERIAL_THREAD || res == 0); > + > + /* All we know at this point is that all the worker threads have reached > + the barrier, which is just before they perform their action. But we > + really want them to start their action. > + > + There's really no way we can be 100% certain that the worker threads > + have started their action, all we can do is wait for a short while and > + hope that the machine we're running on is not too slow. */ > + sleep (MAIN_THREAD_DELAY); > + > + /* A function that GDB can place a breakpoint on. By the time we get > + here we are as sure as we can be that all of the worker threads have > + started and are in their worker action (spinning, or syscall). */ > + breakpt (); > + > + /* If GDB is not attached then this function will cause a crash, which > + can be used to dump a core file, which GDB can then analyse. */ > + crash_function (); > + > + /* Due to the crash we never expect to get here. Plus the worker actions > + never terminate. But for completeness, here's where we join with all > + the worker threads. */ > + for (int i = 0; i < job_count; ++i) > + finalise_job (&jobs[i]); > + > + /* Cleanup the barrier. */ > + res = pthread_barrier_destroy (&global_barrier); > + assert (res == 0); > + > + /* And clean up the jobs list. */ > + free (jobs); > + > + return 0; > +} > diff --git a/gdb/testsuite/gdb.threads/threadcrash.exp b/gdb/testsuite/gdb.threads/threadcrash.exp > new file mode 100644 > index 00000000000..ee81b5c32f2 > --- /dev/null > +++ b/gdb/testsuite/gdb.threads/threadcrash.exp > @@ -0,0 +1,209 @@ > +# This testcase is part of GDB, the GNU debugger. > + > +# Copyright 2023 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 . > + > +# This test case looks at GDB's ability to get correct backtraces for a > +# crashed inferior, recreating it from a live inferior, a corefile and > +# a gcore. Generally fine with this patch, but I do have a small style issue. > + > + > +# First check that we have 7 threads. > + You've commented the first two functions, but I think we can be more descriptive, I'd say: # Check that the inferior has 7 threads, and return the number of # threads (7). > +proc test_thread_count {} { > + set thread_count 0 > + > + gdb_test_multiple "info threads" "getting thread count" -lbl { > + -re "Thread" { > + incr thread_count > + exp_continue > + } > + -re "$::gdb_prompt " { > + gdb_assert {$thread_count == 7} > + } > + } > + > + return $thread_count > +} > + > +# Apply all to quickly check if all expected states are > +# present. Then, save the full desired backtrace in a list > +# so we can check full backtraces later. It took me a while to figure out what this function was doing. I'd say: # Use 'thread apply all backtrace' to check if all the expected # threads are present, and stopped in the expected locations. Set the # global TEST_LIST to a list of regexp that is expected to match the # backtrace of each thread. The order of TEST_LIST will vary based on # the order in which GDB sees the threads. It was this last part that puzzled me on first reading -- why wasn't the global TEST_LIST just setup once at the start of the test? I believe it must be because the threads can show up in different orders, right? > + > +proc thread_apply_all {} { > + global test_list I'd be tempted to clear TEST_LIST within the function, rather than doing it elsewhere ... unless there's a reason why that wouldn't work. But surely, if TEST_LIST has content at this point then adding the items below is going to not do the right thing? > + > + set unwind_fail false > + > + gdb_test_multiple "thread apply all backtrace" \ > + "Get thread information" -lbl { > + -re "#\[0-9\]+\\\?\\\?\[^\n\]*" { > + set unwind_fail true > + exp_continue > + } > + -re "\[^\n\]*syscall_task .location=SIGNAL_ALT_STACK\[^\n\]*" { > + set test_list [linsert $test_list end [multi_line ".*sleep.*" \ Could we use lappend here? Or am I missing something? > + ".*do_syscall_task .location=SIGNAL_ALT_STACK.*" \ > + ".*signal_handler.*" \ > + ".*signal handler called.*" \ > + ".*pthread_kill.*" \ > + ".*thread_function.*"]] > + exp_continue > + } > + -re "\[^\n\]*syscall_task .location=SIGNAL_HANDLER\[^\n\]*" { > + set test_list [linsert $test_list end [multi_line ".*sleep.*" \ > + ".*do_syscall_task .location=SIGNAL_HANDLER.*" \ > + ".*signal_handler.*" \ > + ".*signal handler called.*" \ > + ".*pthread_kill.*" \ > + ".*thread_function.*"]] > + exp_continue > + } > + -re "\[^\n\]*syscall_task .location=NORMAL\[^\n\]*" { > + set test_list [linsert $test_list end [multi_line ".*sleep.*" \ > + ".*do_syscall_task .location=NORMAL.*" \ > + ".*thread_function.*"]] > + exp_continue > + } > + -re "\[^\n\]*spin_task .location=SIGNAL_ALT_STACK\[^\n\]*" { > + set test_list [linsert $test_list end [multi_line ".*do_spin_task .location=SIGNAL_ALT_STACK.*" \ > + ".*signal_handler.*" \ > + ".*signal handler called.*" \ > + ".*pthread_kill.*" \ > + ".*thread_function.*"]] > + exp_continue > + } > + -re "\[^\n\]*spin_task .location=SIGNAL_HANDLER\[^\n\]*" { > + set test_list [linsert $test_list end [multi_line ".*do_spin_task .location=SIGNAL_HANDLER.*" \ > + ".*signal_handler.*" \ > + ".*signal handler called.*" \ > + ".*pthread_kill.*" \ > + ".*thread_function.*"]] > + exp_continue > + } > + -re "\[^\n\]*spin_task .location=NORMAL\[^\n\]*" { > + set test_list [linsert $test_list end [multi_line ".*do_spin_task .location=NORMAL..*" \ > + ".*thread_function.*"]] > + exp_continue > + } > + -re "\[^\n\]*main\[^\n\]*" { > + set test_list [linsert $test_list end ".*main.*"] > + exp_continue > + } > + -re "$::gdb_prompt " { > + pass $gdb_test_name > + } > + } > + > + gdb_assert {$unwind_fail == false} > +} > + And the function comments just ... stop. I think it's clearer if we have a comment that states our intentions. > +proc do_full_test {} { > + global test_list > + set thread_count [test_thread_count] > + > + thread_apply_all > + > + gdb_assert {$thread_count == [llength $test_list]} > + > + for {set i 0} {$i < $thread_count } {incr i} { > + set thread_num [expr [llength $test_list] - $i] > + > + gdb_test "thread apply $thread_num backtrace" [lindex $test_list $i] > + } > +} > + > +proc_with_prefix test_corefile {} { > + set corefile [core_find $::binfile] > + if { $corefile == "" } { > + untested "couldn't generate corefile" > + return > + } > + set corefile [gdb_remote_download host $corefile] > + > + gdb_test "core-file $corefile" \ > + "" \ > + "loading_corefile" \ > + "A program is being debugged already\\\. Kill it\\\? \\\(y or n\\\) " \ > + "y" > + > + do_full_test > +} > + > +proc_with_prefix test_gcore {} { > + > + clean_restart "$::binfile" > + > + gdb_test "handle SIGUSR1 nostop print pass" \ > + ".*SIGUSR1.*No.*Yes.*Yes.*User defined signal 1" \ > + "setup SIGUSR1" > + gdb_test "handle SIGUSR2 nostop print pass" \ > + ".*SIGUSR2.*No.*Yes.*Yes.*User defined signal 2" \ > + "setup SIGUSR2" > + > + gdb_test "run" ".*Segmentation fault.*" "continue to crash" > + > + set gcore_name "${::binfile}.gcore" > + set gcore_supported [gdb_gcore_cmd "$gcore_name" "saving gcore"] > + > + if {!$gcore_supported} { > + unsupported "couldn't generate gcore file" > + return > + } > + > + set corefile [gdb_remote_download host $gcore_name] > + > + gdb_test "core-file $corefile" \ > + "" \ > + "loading_corefile" \ > + "A program is being debugged already\\\. Kill it\\\? \\\(y or n\\\) " \ > + "y" > + > + do_full_test > +} > + > +standard_testfile > + > +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { > + return -1 > +} > + You can still use prepare_for_testing here, I think this should do what you want: if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] { return -1 } The use of 'pthreads' in the flags will cause this to call gdb_compile_pthreads under the hood. > +clean_restart ${binfile} > + > +gdb_test_no_output "set backtrace limit unlimited" > + > +set test_list { } These I'm hoping can be eliminated by moving this line into thread_apply_all. Thanks, Andrew > + > +with_test_prefix "live inferior" { > + gdb_test "handle SIGUSR1 nostop print pass" \ > + ".*SIGUSR1.*No.*Yes.*Yes.*User defined signal 1" \ > + "setup SIGUSR1" > + gdb_test "handle SIGUSR2 nostop print pass" \ > + ".*SIGUSR2.*No.*Yes.*Yes.*User defined signal 2" \ > + "setup SIGUSR2" > + > + gdb_breakpoint "breakpt" > + gdb_test "run" ".*breakpt.*" "run to break function" > + > + do_full_test > +} > + > +set test_list { } > + > +test_corefile > + > +set test_list { } > + > +test_gcore > -- > 2.41.0