From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x343.google.com (mail-wm1-x343.google.com [IPv6:2a00:1450:4864:20::343]) by sourceware.org (Postfix) with ESMTPS id 6314F3858D35 for ; Mon, 3 Aug 2020 10:28:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 6314F3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x343.google.com with SMTP id c80so13854707wme.0 for ; Mon, 03 Aug 2020 03:28:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Eqv44lsicwbfHthA5Met41MtMCRkNtDmMIFvmtsyCTU=; b=cQWTksnUgzRra1zKCdSlL3+KFsud3YimbiYRTW5mppWzokZgOano9Dy8O5ZSikqvSG AcAazqUUZn35ijsiM425CvQpuiD1ZIYISo/rxM2bBsR3CI5nk66W77qoRrZJQ0y5tYxA KZHvjM38TAGI+qG8W9X2upkwUhVLDmzqYv/bYrngqpMH9PhmGKdDlSoThI7SB6UkpAL1 x53c/AeKlSdl8gA28J7tdXFLB9IoH5fGGI19TXZB1gtifEzl7AV8Mx91RDF4cDQWK2h6 VT65NwC+25W0G6WMb61QuGPrStnvHbfIINmj13rBVYFzJ9ScR/sQpOAq51AQYQOeFK/m me3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Eqv44lsicwbfHthA5Met41MtMCRkNtDmMIFvmtsyCTU=; b=datUhzliRjXlQB3iXVEglCAaNDnHV1KART4AtpC6M8LGsIm8x5s+V2oafT+TsR1pw2 mhARhWxIlnfr3XDvYdAVWy+fr4pH0pG8i4GFZ01imzi4GEVQfIqIpLpiTBZHXkzeuzNH TqEvA8qdacE1f3dNMlLE2ccEXIb+OSmKNxX/q7C7xJ7vIOuB05Zu04vhDdpJ5G2+dRIP 7vabcdfSUQ7Lg2/EWScx/Kqw/RI4TnHq4aO9VerXNhqtjzHYX2bwqWdpnPYF2poyKFrp /HSLf6DVV91UD02ukdIW62HcCzM5p7uaPmdeSeutWjhvB7K0FsbS1QGfLyXsOc1AqW5U bD5g== X-Gm-Message-State: AOAM530acC6dBcS1wYp4tk58e5j2iNYCEQq2Oski2eb5WxGEa4Q7I0aR j6eZSvi4bGcS1L8tNFzECwkshHbGGxQ= X-Google-Smtp-Source: ABdhPJwAn1sxc4X8Yy32TuIJ7q8IfadDIBiQVeFUh9MJ7ti4rK1rwx2oDbVtPjX6rJ33NnY6ISlYYQ== X-Received: by 2002:a05:600c:2189:: with SMTP id e9mr16195439wme.171.1596450509319; Mon, 03 Aug 2020 03:28:29 -0700 (PDT) Received: from localhost (host86-140-161-74.range86-140.btcentralplus.com. [86.140.161.74]) by smtp.gmail.com with ESMTPSA id w10sm23746657wmk.0.2020.08.03.03.28.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Aug 2020 03:28:28 -0700 (PDT) Date: Mon, 3 Aug 2020 11:28:27 +0100 From: Andrew Burgess To: Tankut Baris Aktemur Cc: gdb-patches@sourceware.org Subject: Re: [RFC][PATCH 2/2] gdb/breakpoint: add a '-force' flag to the 'condition' command Message-ID: <20200803102827.GU853475@embecosm.com> References: <94a5e4dc4f144cc0f9c170bdabeec7c0cb4fda4d.1596209606.git.tankut.baris.aktemur@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <94a5e4dc4f144cc0f9c170bdabeec7c0cb4fda4d.1596209606.git.tankut.baris.aktemur@intel.com> X-Operating-System: Linux/5.6.15-200.fc31.x86_64 (x86_64) X-Uptime: 11:22:53 up 15 days, 19:37, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Aug 2020 10:28:32 -0000 * Tankut Baris Aktemur via Gdb-patches [2020-07-31 17:42:29 +0200]: > The previous patch made it possible to define a condition if it's > valid at some locations. If the condition is invalid at all of the > locations, it's rejected. However, there may be cases where the user > knows the condition will be valid at a location in the *future*, > e.g. due to a shared library load. > > To make it possible that such condition is defined, this patch > adds an optional '-force' flag to the 'condition' command. When this > command is passed, the condition is not rejected even when it is > invalid for all the current locations (note that all the locations > would be internally disabled in this case). > > For instance: > > (gdb) break test.c:5 > Breakpoint 1 at 0x1155: file test.c, line 5. > (gdb) cond 1 foo == 42 > No symbol "foo" in current context. > > Defining the condition was not possible because 'foo' is not > available. The user can override this behavior with the '-force' > flag: > > (gdb) cond -force 1 foo == 42 > warning: disabling breakpoint 1.1: No symbol "foo" in current context. > (gdb) info breakpoints > Num Type Disp Enb Address What > 1 breakpoint keep y > stop only if foo == 42 > 1.1 n 0x0000000000001155 in main at test.c:5 > > Now the condition is accepted, but the location is automatically > disabled. If a future location has a context in which 'foo' is > available, that location would be enabled. > > This is an RFC. If the idea of having this '-force' flag makes sense, > I could also work on the same feature for the 'break' command, write > test cases, and submit as a full patch. I'd be happy to see the `-force` flag added, I think adding flags like this makes perfect sense. There's already many flags like this for the break command so I don't see why adding one more should be a problem. I'd be tempted to name the flag for `break` as `-force-condition` to make it clear what's being forced. The flags already accept unique sub-strings, so `-force` would still work, plus there's always tab-completion. > > Another potentially interesting approach is to query the user. > Something along the following idea: > > (gdb) cond 1 foo == 42 > Condition expression is invalid at all locations of breakpoint 1. > Do you want force GDB to nevertheless define the condition, but > disable the current locations? [y/N] y > warning: disabling breakpoint 1.1: No symbol "foo" in current context. > > This query approach may break some scripts because it is changing the > existing behavior. Unless the script is parsing the exact output of GDB this should be a problem. When GDB is run in an non-interactive manor the default answer is assumed to prompts like this, so in the above case the user would still get the `N` behaviour. But I wouldn't rush to add more interactive prompts to GDB personally. These are just my thoughts. Others might feel differently. Thanks, Andrew > > gdb/ChangeLog: > 2020-07-31 Tankut Baris Aktemur > > * breakpoint.h (set_breakpoint_condition): Add a new bool parameter. > * breakpoint.c (set_breakpoint_condition): Take a new bool parameter > to control whether condition definition should be forced even when > the condition expression is invalid in all of the current locations. > (condition_command): Update the call to 'set_breakpoint_condition'. > * ada-lang.c (create_ada_exception_catchpoint): Ditto. > * guile/scm-breakpoint.c (gdbscm_set_breakpoint_condition_x): Ditto. > * python/py-breakpoint.c (bppy_set_condition): Ditto. > > gdb/doc/ChangeLog: > 2020-07-31 Tankut Baris Aktemur > > * gdb.texinfo: Document the '-force' flag of the 'condition' command. > --- > gdb/ada-lang.c | 2 +- > gdb/breakpoint.c | 28 ++++++++++++++++++++++------ > gdb/breakpoint.h | 6 ++++-- > gdb/doc/gdb.texinfo | 6 ++++++ > gdb/guile/scm-breakpoint.c | 2 +- > gdb/python/py-breakpoint.c | 2 +- > 6 files changed, 35 insertions(+), 11 deletions(-) > > diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c > index 29951528e5e..69b3c3c63cd 100644 > --- a/gdb/ada-lang.c > +++ b/gdb/ada-lang.c > @@ -12702,7 +12702,7 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch, > c->excep_string = excep_string; > create_excep_cond_exprs (c.get (), ex_kind); > if (!cond_string.empty ()) > - set_breakpoint_condition (c.get (), cond_string.c_str (), from_tty); > + set_breakpoint_condition (c.get (), cond_string.c_str (), from_tty, false); > install_breakpoint (0, std::move (c), 1); > } > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index 7abfd510abc..5fe9358b381 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -874,7 +874,7 @@ set_breakpoint_location_condition (const char *cond_string, bp_location *loc, > > void > set_breakpoint_condition (struct breakpoint *b, const char *exp, > - int from_tty) > + int from_tty, bool force) > { > if (*exp == 0) > { > @@ -941,9 +941,11 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp, > catch (const gdb_exception_error &e) > { > /* Condition string is invalid. If this happens to > - be the last loc, abandon. */ > + be the last loc, abandon (if not forced) or continue > + (if forced). */ > if (loc->next == nullptr) > - throw; > + if (!force) > + throw; > } > } > > @@ -1023,6 +1025,18 @@ condition_command (const char *arg, int from_tty) > error_no_arg (_("breakpoint number")); > > p = arg; > + > + /* Check if the "-force" flag was passed. */ > + bool force = false; > + const char *tok = skip_spaces (p); > + const char *end_tok = skip_to_space (tok); > + int toklen = end_tok - tok; > + if (toklen >= 1 && strncmp (tok, "-force", toklen) == 0) > + { > + force = true; > + p = end_tok + 1; > + } > + > bnum = get_number (&p); > if (bnum == 0) > error (_("Bad breakpoint argument: '%s'"), arg); > @@ -1042,7 +1056,7 @@ condition_command (const char *arg, int from_tty) > " a %s stop condition defined for this breakpoint."), > ext_lang_capitalized_name (extlang)); > } > - set_breakpoint_condition (b, p, from_tty); > + set_breakpoint_condition (b, p, from_tty, force); > > if (is_breakpoint (b)) > update_global_location_list (UGLL_MAY_INSERT); > @@ -15576,8 +15590,10 @@ then no output is printed when it is hit, except what the commands print.")); > > c = add_com ("condition", class_breakpoint, condition_command, _("\ > Specify breakpoint number N to break only if COND is true.\n\ > -Usage is `condition N COND', where N is an integer and COND is an\n\ > -expression to be evaluated whenever breakpoint N is reached.")); > +Usage is `condition [-force] N COND', where N is an integer and COND\n\ > +is an expression to be evaluated whenever breakpoint N is reached.\n\ > +With the `-force` flag, the condition is defined even when it is\n\ > +invalid for all current locations.")); > set_cmd_completer (c, condition_completer); > > c = add_com ("tbreak", class_breakpoint, tbreak_command, _("\ > diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h > index 145bc8a397a..6acec428a9a 100644 > --- a/gdb/breakpoint.h > +++ b/gdb/breakpoint.h > @@ -1617,9 +1617,11 @@ extern int breakpoints_should_be_inserted_now (void); > in our opinion won't ever trigger. */ > extern void breakpoint_retire_moribund (void); > > -/* Set break condition of breakpoint B to EXP. */ > +/* Set break condition of breakpoint B to EXP. > + If FORCE, define the condition even if it is invalid in > + all of the breakpoint locations. */ > extern void set_breakpoint_condition (struct breakpoint *b, const char *exp, > - int from_tty); > + int from_tty, bool force); > > /* Checks if we are catching syscalls or not. > Returns 0 if not, greater than 0 if we are. */ > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 1b9f76573b9..a81689dab21 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -5453,6 +5453,12 @@ prints an error message: > No symbol "foo" in current context. > @end smallexample > > +@item condition -force @var{bnum} @var{expression} > +When the @code{-force} flag is used, define the condition even if > +@var{expression} is invalid in all the current locations of breakpoint > +@var{bpnum}. This is useful if it is known that @var{expression} will > +be valid for a future shared library location. > + > @noindent > @value{GDBN} does > not actually evaluate @var{expression} at the time the @code{condition} > diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c > index 96b6ca91f8d..7c9707235ec 100644 > --- a/gdb/guile/scm-breakpoint.c > +++ b/gdb/guile/scm-breakpoint.c > @@ -905,7 +905,7 @@ gdbscm_set_breakpoint_condition_x (SCM self, SCM newvalue) > ? nullptr > : gdbscm_scm_to_c_string (newvalue)); > > - set_breakpoint_condition (bp_smob->bp, exp ? exp.get () : "", 0); > + set_breakpoint_condition (bp_smob->bp, exp ? exp.get () : "", 0, false); > > return SCM_UNSPECIFIED; > }); > diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c > index 11345ad3052..56640421994 100644 > --- a/gdb/python/py-breakpoint.c > +++ b/gdb/python/py-breakpoint.c > @@ -461,7 +461,7 @@ bppy_set_condition (PyObject *self, PyObject *newvalue, void *closure) > > try > { > - set_breakpoint_condition (self_bp->bp, exp, 0); > + set_breakpoint_condition (self_bp->bp, exp, 0, false); > } > catch (gdb_exception &ex) > { > -- > 2.17.1 >