From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 63256 invoked by alias); 10 Apr 2018 20:25:47 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 63246 invoked by uid 89); 10 Apr 2018 20:25:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=H*RU:209.85.192.193, Hx-spam-relays-external:209.85.192.193, pending, luckily X-HELO: mail-pf0-f193.google.com Received: from mail-pf0-f193.google.com (HELO mail-pf0-f193.google.com) (209.85.192.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 10 Apr 2018 20:25:45 +0000 Received: by mail-pf0-f193.google.com with SMTP id f15so9206287pfn.0 for ; Tue, 10 Apr 2018 13:25:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:subject:in-reply-to:cc:from:to:message-id :mime-version:content-transfer-encoding; bh=R4Utk573uMZrig1UclMBHiwtcYAh8oOxMkVt3SiNxXk=; b=eASXSfsb9cbg5EGciHB4wTi7ZMXY4uoltpFWU+kmWTjEuytxQ17D541rKyK/GcmENd 56PmIZePiRlilL0QdNGb9XijwyBpHVnWWTaZ2KBuszgfEWsMcbjTsaciRNdA2DJHqYeR KTj+uohwVlRv3rdM5eHOMcHLQrAMkgkOeiPZCQNXPxgAWICy2xFBr0IPIj9MNX0YlLFT 7QxBeGYx8NdIHKXT2IdhaKmR4eyRds1XhogJW2Q0MWg5y3/wZviSbJ9dYmAwz3WG3WjK NpAJjaPJD3xY3jfpDD2IKC0VttjCQc69DavsKv/ocVM4BWten69SvWuFb7COQTNRV/HW /n7Q== X-Gm-Message-State: ALQs6tDI6h1b1flYujFrJ8eGPxJ8BNpR833huJUqCvcw+76OcHlp/aKM v7e/LXhxOOkWUh01xW85VUuuWz1ISsw= X-Google-Smtp-Source: AIpwx4+jg3NRyVLQzjUlZKDZe/JnjUm8UcgF5R6hrtBGBiXQyBChWz6GjgFYL63PiCjqSHMZ8ZTppg== X-Received: by 10.98.93.149 with SMTP id n21mr1554692pfj.222.1523391943291; Tue, 10 Apr 2018 13:25:43 -0700 (PDT) Received: from localhost ([12.206.222.5]) by smtp.gmail.com with ESMTPSA id f13sm2788919pfj.170.2018.04.10.13.25.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 Apr 2018 13:25:42 -0700 (PDT) Date: Tue, 10 Apr 2018 20:25:00 -0000 X-Google-Original-Date: Tue, 10 Apr 2018 13:18:05 PDT (-0700) Subject: Re: [PATCH 2/3] gdb/testsuite: Filter out some registers for riscv In-Reply-To: <20180409222607.GN13407@embecosm.com> CC: gdb-patches@sourceware.org From: Palmer Dabbelt To: andrew.burgess@embecosm.com Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2018-04/txt/msg00193.txt.bz2 On Mon, 09 Apr 2018 15:26:07 PDT (-0700), andrew.burgess@embecosm.com wrote: > * Palmer Dabbelt [2018-04-09 14:28:33 -0700]: > >> On Mon, 09 Apr 2018 08:15:28 PDT (-0700), andrew.burgess@embecosm.com wrote: >> > On riscv the cycle counter, and instructions retired counter CSRs are >> > read only, this causes problems in the gdb.base/callfuncs.exp test, as >> > the values in these CSRs change after an inferior call, the check that >> > no target registers have been modified then fails. >> > >> > Luckily the test already has a mechanism in place for filtering out >> > registers that are modified (and can't be restored) by an inferior call, >> > so this commit adds the problem registers into this list for riscv. >> > >> > In the future we may end up needing to filter out more CSRs, but right >> > now, for the targets I have access too, these are the only ones causing >> > problems. >> > >> > gdb/testsuite/ChangeLog: >> > >> > * gdb.base/callfuncs.exp (fetch_all_registers): Add riscv register >> > filter pattern. >> > --- >> > gdb/testsuite/ChangeLog | 5 +++++ >> > gdb/testsuite/gdb.base/callfuncs.exp | 10 ++++++++++ >> > 2 files changed, 15 insertions(+) >> > >> > diff --git a/gdb/testsuite/gdb.base/callfuncs.exp b/gdb/testsuite/gdb.base/callfuncs.exp >> > index 94636938752..c5e39918c2a 100644 >> > --- a/gdb/testsuite/gdb.base/callfuncs.exp >> > +++ b/gdb/testsuite/gdb.base/callfuncs.exp >> > @@ -285,6 +285,16 @@ proc fetch_all_registers {test} { >> > } >> > exp_continue >> > } >> > + -re "^\(?:cycle\|instret\)\[ \t\]+\[^\r\n\]+\r\n" { >> > + if [istarget "riscv*-*-*"] { >> > + # Filter out the cycle counter and instructions >> > + # retired counter CSRs which are read-only, giving >> > + # spurious differences. >> > + } else { >> > + lappend all_registers_lines $expect_out(0,string) >> > + } >> > + exp_continue >> > + } >> > -re "^\[^ \t\]+\[ \t\]+\[^\r\n\]+\r\n" { >> > lappend all_registers_lines $expect_out(0,string) >> > exp_continue >> >> I think we only want to check the X and F registers here -- essentially >> every CSR is a special register where you can't really rely on the value not >> being changed somewhere by hardware. For example: >> >> * The interrupt pending bits could flip at any point, even if interrupts are >> disabled. >> * The floating-point dirty and exception state bits could change if a >> floating-point instruction executes. >> * The various trap CSRs (epc, badaddr, cause, etc) get set whenever a trap >> is executed. > > I'm not sure about the second case. If GDB is stopped and we perform > an inferior call, then ideally the entire floating point state, > including contents of things like fcsr would be reset, otherwise, when > we continue the behaviour might not be as we expect. > > I do agree with you that the two registers I've filtered so far > probably aren't enough, but I'm really reluctant to _only_ check X and > F registers. I think a better selection would be X, F, and read/write > user CSRs. Which means I need to build the list of CSRs to filter, I > was hoping to put that off for another day for now... > > Let me know how you'd feel about leaving this as it is for now, and > extending the filter list at a later date. I think it's fine for now, we can fix it when another test fails :)