| Message ID | yjt2d2abqz9x.fsf@ruffy.mtv.corp.google.com |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 5931 invoked by alias); 1 Oct 2014 21:02:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 5919 invoked by uid 89); 1 Oct 2014 21:02:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-lb0-f201.google.com Received: from mail-lb0-f201.google.com (HELO mail-lb0-f201.google.com) (209.85.217.201) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 01 Oct 2014 21:02:38 +0000 Received: by mail-lb0-f201.google.com with SMTP id u10so33017lbd.4 for <gdb-patches@sourceware.org>; Wed, 01 Oct 2014 14:02:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-type; bh=YF/7PeZKqz1sWC90k6FuU9PyUo0KeMj3Sv5w2Q+8ZCQ=; b=fHw6V5Epp/p3i6qni2uZ0eMXneaob4rnzVkOoPxANuMCL2THV4kU9cKybGcuQ/UEpg nENE1bjOOyG/jL8LpML5SKZjt8agTjY7ofn7JKGCcDJ0QukZ1CpTsRhxtp5aPaGB1Y9S qL7AMj+VK93Nr++8nCNGNS2W7h8ms1bgzM4qdThIywyOxBCuycqU2GFnTpV+3DoesteT nHUHDFYxQP7qS4zPG1ubK2tfJ/9J54Tlq+pcl7LqS8J+09xIPbtMU3KZgLiHfScYnR2R xzCS/bwNPa5C4tTjpMxuCojbwfIwbeASOReelyMJHgvPKyHbjbOfXYwYiO8To7haEMbU EwXg== X-Gm-Message-State: ALoCoQm+2VX1sT/h+2ki2nDM+MK7nSEWMTpyDU5eU8Rz97omLOqEkrBCa3lf5oVOh6812FMd1ZXrWnTglnGc8ngW6ktaB/Lah4auDrBPwHkE/+MBPxg+19pcvrS7mw6al48yT5QVG5fdGeHCZ2pP3wkDI8T1OFbNFzcSOjoTXmbmDklZihUAuJ8= X-Received: by 10.180.109.67 with SMTP id hq3mr2971887wib.1.1412197355297; Wed, 01 Oct 2014 14:02:35 -0700 (PDT) Received: from corpmail-nozzle1-1.hot.corp.google.com ([100.108.1.104]) by gmr-mx.google.com with ESMTPS id t6si488529qcd.0.2014.10.01.14.02.34 for <gdb-patches@sourceware.org> (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 01 Oct 2014 14:02:35 -0700 (PDT) Received: from ruffy.mtv.corp.google.com ([172.17.128.44]) by corpmail-nozzle1-1.hot.corp.google.com with ESMTPS id MAWbjHnX.1; Wed, 01 Oct 2014 14:02:35 -0700 From: Doug Evans <dje@google.com> To: gdb-patches@sourceware.org Subject: [PATCH] Don't run forever in gdb.base/structs.c Date: Wed, 01 Oct 2014 14:02:34 -0700 Message-ID: <yjt2d2abqz9x.fsf@ruffy.mtv.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes |
Commit Message
Doug Evans
Oct. 1, 2014, 9:02 p.m. UTC
Hi. If gdb crashes during testing tests may be left to free-run, eating cpu. This patch fixes one of the more egregious cases since several versions of the program are built. I've got patches to fix others. Just seeing if folks want to comment on this first. IWBN to have the harness itself cleanup, and I think there's something we can do there, but that's not always robust either, and I think multiple levels of robustness would be useful. Since this testcase is an egregious one, and since this patch simple, I'm starting with this. I thought of trying to make the loop limit something less random, but that itself introduces complications, e.g., if one wants to extend the test. So I kept it simple. One could instead reorganize the test, but it's not worth it, at least not at this point. 2014-10-01 Doug Evans <dje@google.com> * gdb.base/structs.c (main): Don't run forever.
Comments
On 10/01/2014 10:02 PM, Doug Evans wrote: > If gdb crashes during testing tests may be left to free-run, eating cpu. > > This patch fixes one of the more egregious cases since several versions > of the program are built. > > I've got patches to fix others. > Just seeing if folks want to comment on this first. > > IWBN to have the harness itself cleanup, and I think there's something > we can do there, but that's not always robust either, and I think > multiple levels of robustness would be useful. Agreed. > Since this testcase is an egregious one, and since this patch simple, > I'm starting with this. Looks fine with me. We already do something like this in many tests even. E.g., of the top of my head: $ grep -rn "Don't run forever. Run just short of it :)" * gdb.base/watch_thread_num.c:55: /* Don't run forever. Run just short of it :) */ gdb.mi/nsintrall.c:55: /* Don't run forever. Run just short of it :) */ gdb.mi/nsmoribund.c:35: /* Don't run forever. Run just short of it :) */ gdb.threads/pending-step.c:54: /* Don't run forever. Run just short of it :) */ gdb.threads/watchthreads.c:71: /* Don't run forever. Run just short of it :) */ gdb.threads/threadapply.c:72: /* Don't run forever. Run just short of it :) */ gdb.threads/thread-specific.c:42: /* Don't run forever. Run just short of it :) */ gdb.threads/thread-specific.c:56: /* Don't run forever. Run just short of it :) */ gdb.threads/schedlock.c:55: /* Don't run forever. Run just short of it :) */ In a few other tests, we use "alarm()", though IMO it's best to avoid that if possible, to expose the test on as much targets as possible. E.g., alarm() IIRC isn't available on mingw unless you specify __USE_MINGW_ALARM. Bare metal targets may have trouble with it too, etc. Thanks, Pedro Alves
On Wed, Oct 1, 2014 at 4:51 PM, Pedro Alves <palves@redhat.com> wrote: > On 10/01/2014 10:02 PM, Doug Evans wrote: > >> If gdb crashes during testing tests may be left to free-run, eating cpu. >> >> This patch fixes one of the more egregious cases since several versions >> of the program are built. >> >> I've got patches to fix others. >> Just seeing if folks want to comment on this first. >> >> IWBN to have the harness itself cleanup, and I think there's something >> we can do there, but that's not always robust either, and I think >> multiple levels of robustness would be useful. > > Agreed. > >> Since this testcase is an egregious one, and since this patch simple, >> I'm starting with this. > > Looks fine with me. Committed, thanks. > We already do something like this in many tests even. E.g., of the > top of my head: > > $ grep -rn "Don't run forever. Run just short of it :)" * > gdb.base/watch_thread_num.c:55: /* Don't run forever. Run just short of it :) */ > gdb.mi/nsintrall.c:55: /* Don't run forever. Run just short of it :) */ > gdb.mi/nsmoribund.c:35: /* Don't run forever. Run just short of it :) */ > gdb.threads/pending-step.c:54: /* Don't run forever. Run just short of it :) */ > gdb.threads/watchthreads.c:71: /* Don't run forever. Run just short of it :) */ > gdb.threads/threadapply.c:72: /* Don't run forever. Run just short of it :) */ > gdb.threads/thread-specific.c:42: /* Don't run forever. Run just short of it :) */ > gdb.threads/thread-specific.c:56: /* Don't run forever. Run just short of it :) */ > gdb.threads/schedlock.c:55: /* Don't run forever. Run just short of it :) */ > > In a few other tests, we use "alarm()", though IMO it's best to avoid > that if possible, to expose the test on as much targets as possible. > E.g., alarm() IIRC isn't available on mingw unless you > specify __USE_MINGW_ALARM. Bare metal targets may have trouble > with it too, etc. Agreed.
On 10/02/2014 09:10 PM, Doug Evans wrote: > On Wed, Oct 1, 2014 at 4:51 PM, Pedro Alves <palves@redhat.com> wrote: >> On 10/01/2014 10:02 PM, Doug Evans wrote: >> >>> If gdb crashes during testing tests may be left to free-run, eating cpu. >>> >>> This patch fixes one of the more egregious cases since several versions >>> of the program are built. >>> >>> I've got patches to fix others. >>> Just seeing if folks want to comment on this first. >>> >>> IWBN to have the harness itself cleanup, and I think there's something >>> we can do there, but that's not always robust either, and I think >>> multiple levels of robustness would be useful. >> >> Agreed. >> >>> Since this testcase is an egregious one, and since this patch simple, >>> I'm starting with this. >> >> Looks fine with me. > > Committed, thanks. Thank you. Guess this could be material for: https://sourceware.org/gdb/wiki/GDBTestcaseCookbook ? >> In a few other tests, we use "alarm()", though IMO it's best to avoid >> that if possible, to expose the test on as much targets as possible. >> E.g., alarm() IIRC isn't available on mingw unless you >> specify __USE_MINGW_ALARM. Bare metal targets may have trouble >> with it too, etc. > > Agreed. Expanding on that a bit: TBC, I see no problem with alarm() if the test is already using other posix-ish things, like pthread_create, or signal. I'd avoid iff the test otherwise could be mostly plain C/C++ and OS independent, like structs.c. Thanks, Pedro Alves
On Fri, Oct 3, 2014 at 1:57 AM, Pedro Alves <palves@redhat.com> wrote: > On 10/02/2014 09:10 PM, Doug Evans wrote: >> On Wed, Oct 1, 2014 at 4:51 PM, Pedro Alves <palves@redhat.com> wrote: >>> On 10/01/2014 10:02 PM, Doug Evans wrote: >>> >>>> If gdb crashes during testing tests may be left to free-run, eating cpu. >>>> >>>> This patch fixes one of the more egregious cases since several versions >>>> of the program are built. >>>> >>>> I've got patches to fix others. >>>> Just seeing if folks want to comment on this first. >>>> >>>> IWBN to have the harness itself cleanup, and I think there's something >>>> we can do there, but that's not always robust either, and I think >>>> multiple levels of robustness would be useful. >>> >>> Agreed. >>> >>>> Since this testcase is an egregious one, and since this patch simple, >>>> I'm starting with this. >>> >>> Looks fine with me. >> >> Committed, thanks. > > Thank you. Guess this could be material for: > > https://sourceware.org/gdb/wiki/GDBTestcaseCookbook Good idea. Done.
diff --git a/gdb/testsuite/gdb.base/structs.c b/gdb/testsuite/gdb.base/structs.c index 60772bb..d0b69a8 100644 --- a/gdb/testsuite/gdb.base/structs.c +++ b/gdb/testsuite/gdb.base/structs.c @@ -425,12 +425,14 @@ int main() Fun17(foo17); Fun18(foo18); - /* An infinite loop that first clears all the variables and then + /* An (almost-)infinite loop that first clears all the variables and then calls each function. This "hack" is to make testing random functions easier - "advance funN" is guaranteed to have always - been preceded by a global variable clearing zed call. */ + been preceded by a global variable clearing zed call. + We don't let this run forever in case gdb crashes while testing, + we don't want to be left eating all cpu on the user's system. */ - while (1) + for (i = 0; i < 1000000; ++i) { zed (); L1 = fun1();