From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id oC3JMY3taWWdUhQAWB0awg (envelope-from ) for ; Fri, 01 Dec 2023 09:28:29 -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=K+jM9Bk+; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id C8FC91E0C0; Fri, 1 Dec 2023 09:28:29 -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 AE5DD1E00F for ; Fri, 1 Dec 2023 09:28:27 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 67EF03861816 for ; Fri, 1 Dec 2023 14:28:27 +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 1F0DC3858C62 for ; Fri, 1 Dec 2023 14:28:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1F0DC3858C62 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 1F0DC3858C62 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=1701440894; cv=none; b=vr2bA3sn7IwVjIWR+d5RID+uWmz7SriEQE5piNz1MGTaDnPIEAhnJuaO2pZR6ITxLDLrPBNqekD5ntIzn5pW+x8i3t3kCdcLB9GfoB3YL8T+qyic4F5+hHSDX9duZ9FCZig1ottS7AfX5n1QCe1hfHpqFkC7rB9nDQ6f2NVxNno= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701440894; c=relaxed/simple; bh=OzF/nR97/W7s1uYJ2ZXthIt+Tz5t1CUhZcvQ7SUf7RY=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=V517JrnSr8UAZFoHULwAMb3Qi7DzCJpU07thgWS6Ww1l0T2nc6Ou7jYdh5ssVNoy7ONXNh61cwcJaKFMmakVnsC+lUjmjEWwcRApBiH+I2hwQ8+w2vCE8DZX5IT5Wn6gkCliOtwt+Ougi8kjMg1rvppTkvFu66pkhBO4RhUaGnA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701440890; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=q+rowDUsy3DN7GS6XxuqVPFWBxRnmiy3XuoVOkSEz8g=; b=K+jM9Bk+BIFOzYM1C8FFLSo5L8P308QmyjEAgz6bYUnLGw4ZZH404V3zpDKJuetSy4IS9+ 7LEFwmZS5/4NJrHXNZ2F0GWFJjsgQhKvrKbHrloqGMpgyt9rv06hOatzfNNQD9q1IiqZMM ySWSjflCsg2mSrkjUyTkUaE+nomSLeQ= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-261-5gWxmw_tPliw1YXSRF4Uog-1; Fri, 01 Dec 2023 09:28:09 -0500 X-MC-Unique: 5gWxmw_tPliw1YXSRF4Uog-1 Received: by mail-ej1-f72.google.com with SMTP id a640c23a62f3a-a0bd5327164so193383966b.3 for ; Fri, 01 Dec 2023 06:28:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701440888; x=1702045688; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=q+rowDUsy3DN7GS6XxuqVPFWBxRnmiy3XuoVOkSEz8g=; b=YN0yOkXGCHPrDFRYhLg20mlRZXAsNkmu9t48nZNRS4y+Fq4LvB+YXz0DkPIhLTvPtI 8jsglh2u/X/bhrbQ8e0dMeqPzaoxJ+AmmILGXbsrlsJtt2knGjcVBvv/0cwfNFjRoV+v a7Dp+bUAM3B6syq8zXzOtKxU1eFtBD9A8d36NX67DayayflzMP3TeI1XJz8KYofrwLJD fRgJOxai2d8EW4LczIOc3GI3lI8PHYviJKPOs7rp+Id3Slw59Yxl1RAKKoOHva8pU4an qHvhmypXA3jEMUsNWsdKe63+fvvwEzIt2wFThez2WW5IWmdwk3lXHNTYhFvOewcPC+2Z 68rw== X-Gm-Message-State: AOJu0YzTLeDsYg/xRXT1zCnvmDqRkvX1shcNovwtVk5suFbxOEoK0ZJO hZVvwYJYfQxLO9d+mVJ2lH549wAYWaPjadn+qE7Z6kwT8KYorUiGVCYzCX9KAkAUX9lnTeZD3ZF OM03LzAIW0ihdf0IwczkXRw== X-Received: by 2002:a17:906:48d7:b0:a19:a19a:ead1 with SMTP id d23-20020a17090648d700b00a19a19aead1mr960851ejt.138.1701440888033; Fri, 01 Dec 2023 06:28:08 -0800 (PST) X-Google-Smtp-Source: AGHT+IEzeLFmRv8Zrs2R2/uoE1r2TDGxeqWlcTTGqxBb6G9BaluRdtu9LUE3ya6DOYGG0Xfa5M4OJQ== X-Received: by 2002:a17:906:48d7:b0:a19:a19a:ead1 with SMTP id d23-20020a17090648d700b00a19a19aead1mr960836ejt.138.1701440887280; Fri, 01 Dec 2023 06:28:07 -0800 (PST) Received: from [192.168.0.129] (ip-94-112-227-180.bb.vodafone.cz. [94.112.227.180]) by smtp.gmail.com with ESMTPSA id l23-20020a1709062a9700b009fcf9f8e526sm1970951eje.25.2023.12.01.06.28.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 01 Dec 2023 06:28:06 -0800 (PST) Message-ID: <20d2d95d-4ccd-a69d-1694-0d31826677ac@redhat.com> Date: Fri, 1 Dec 2023 15:28:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH v3] gdb/testsuite: add test for backtracing for threaded inferiors from a corefile To: Andrew Burgess , gdb-patches@sourceware.org References: <20231025114253.259603-2-blarsen@redhat.com> <87ttp6nks3.fsf@redhat.com> From: Guinevere Larsen In-Reply-To: <87ttp6nks3.fsf@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, 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 On 28/11/2023 13:07, Andrew Burgess wrote: > 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? Yes, that's exactly it. I tried with hardcoding and changing sleep times and such, but I could never get the order to be consistent, so I figured building the list based on the actual final order of the threads would be better. > >> + >> +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? good idea! I'll do this. > >> + >> + 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? We can, TCL really isn't my forte :). > > >> + ".*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. I'll add some comments and send them in the next version. > >> +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. Will do. I guess I used an outdated test as an example for this one. v4 should go out shortly. -- Cheers, Guinevere Larsen She/Her/Hers > >> +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