From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 44296 invoked by alias); 12 Jan 2019 01:40:36 -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 44285 invoked by uid 89); 12 Jan 2019 01:40:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-spam-relays-external:209.85.128.68, H*RU:209.85.128.68 X-HELO: mail-wm1-f68.google.com Received: from mail-wm1-f68.google.com (HELO mail-wm1-f68.google.com) (209.85.128.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 12 Jan 2019 01:40:33 +0000 Received: by mail-wm1-f68.google.com with SMTP id m22so4025892wml.3 for ; Fri, 11 Jan 2019 17:40:33 -0800 (PST) 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:user-agent; bh=sLRABGpeN2S/W94BAD1vYtza31xDolivbivj+OcMNTU=; b=HqvTEAY/Utslj6XDln2Dr+B7/qzLUgy71GDpvqTBUHMRmDmDHKpNEaUXr2anZOC6xq 5/SXTVjStWR6mPUhe+nYN+oTIVeSpNnq/tPmLJ94moIHFXvXXeJsx39KQ5Jea/awi9Rj Rx+Qg2evlHsr9LWSYQPH6ktsp1dvwSaMQIecyEd2ptC5xvoonm93KjApKIlvkJ5UbeHE STgi5O6FGraiHhw94LKcRXj1D8R/h7gXzVrQ4zNPS21KhpyFqrlS2iUuew0mBJUOlLsY bC5YhduONXy1t996f4pQP5DQV7YmwDkJOLPPMQKYqFE86eVzsQheEwwX9SMK4tbjd3C1 /gGA== Return-Path: Received: from localhost (host86-172-198-47.range86-172.btcentralplus.com. [86.172.198.47]) by smtp.gmail.com with ESMTPSA id c21sm5108786wre.71.2019.01.11.17.40.29 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 11 Jan 2019 17:40:29 -0800 (PST) Date: Sat, 12 Jan 2019 01:40:00 -0000 From: Andrew Burgess To: Simon Marchi Cc: Pedro Alves , "gdb-patches@sourceware.org" Subject: Re: [PATCH] Fix MI output for multi-location breakpoints Message-ID: <20190112014028.GK3456@embecosm.com> References: <20190111001516.4761-1-simon.marchi@ericsson.com> <20190111123420.GI3456@embecosm.com> <5cdbc81d-5198-f592-c142-8768991c306a@redhat.com> <973aa853-7a1b-4f7a-fd09-b99698aa6363@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <973aa853-7a1b-4f7a-fd09-b99698aa6363@ericsson.com> X-Fortune: Anything that is good and useful is made of chocolate. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00262.txt.bz2 * Simon Marchi [2019-01-11 23:36:16 +0000]: > On 2019-01-11 1:39 p.m., Pedro Alves wrote: > > On 01/11/2019 12:34 PM, Andrew Burgess wrote: > >> * Simon Marchi [2019-01-11 00:15:34 +0000]: > >> > >>> [CCing Pedro because we had some discussions earlier about that offline] > > > > > > Thanks. This was also recently-ish discussed in PR9659. > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=9659 > > Ahh thanks for the reference, I couldn't remember where you had already > wrote about that. > > > My original concern with MI bumps for individual MI fixes is that they > > force an all-or-nothing approach on the frontends. Let me expand. > > > > Suppose a frontend developer only cares about the multi-location > > fix, and not any of the other (supposed) fixes that go into MI3 that > > make it backwards incompatible. It was with that in mind that I had > > suggested at > > to consider going with the "-fix-break-list-bug" solution first. > > I agree this would be nice. > > > That would be usable with MI2 and also be enabled by default with > > MI3 (with no way to disable). Then later on, when we get rid of > > MI2, the "-fix-break-list-bug" setting disappears. > > Well this addresses my concern that frontends won't need to use > -fix-break-list-bug until the end of time, so I am ok with it. > > > But I suppose that that's really an unnecessary complication if we're > > not really going to massively change MI every other release, and if > > migrating a frontend to a new MI version isn't expected to be that > > complicated. We probably aren't and it probably isn't. > > I'll at least give it a try, implementing it is probably not hard. If it doesn't > add too much maintenance burden, I'm not against it. If I do it for this bug, it > will pave the path for future bug fixes, so hopefully it will be smoother next time. > > > So all things considered, it's fine with me to go your route directly > > without a "-fix-break-list-bug" step. > > As I said, I'll give it a try. I intend to name it -fix-multi-location-breakpoint-output. > Instead of adding a flag for this specific issue, should we consider adding a generic mechanism that allows single commands to be run with a different MI version? My first thought was to add (in mi-parse.c:mi_parse) a new flag '--mi', like we already have '--thread' and '--language', which would let you pick a different MI version just for this command. So you could say: -break-insert --mi 3 LOCATION And get MI3 for this command, even if you are currently running at MI2 by default. Conversely, if a UI developer has mostly moved to MI3, but break has not been updated yet, they could (assuming their default is now MI3) do this: -break-insert --mi 2 LOCATION and get the old behaviour. The problem with the above, is that a user can also do: break LOCATION and run the console command, but also get the formatted output. I'm slightly tempted so say that we could ignore this case. If you use a CLI command then you get whatever the default is, only pure MI commands would allow per-command switching... An alternative, but similarly generic approach would be to allow recursive MI invocation, with something like this (assuming MI2 is the current default): -interpreter-exec mi3 "-break-insert LOCATION" Again, this would allow the interpreter to be switched up and down as needed on a command-by-command basis. The problem with the second approach is that it currently segfaults, I assume we don't currently expect recursive MI invocation. I started working on a patch for the first approach before realising the problem with CLI commands. I haven't looked at the cause of the segfault in the second approach yet. Do you think there's any benefit to adopting a more general solution to this issue? Thanks, Andrew > > I agree with Andrew below though. Bumping the MI version this late in > > the cycle is probably not a good idea. > > I agree, I intend to merge a fix for this after 8.3 has branched. > > > If we want to fix this bug for 8.3, we could merge the fix while > > leaving MI2 as the default, declare MI3 stable, and then bump the > > WIP MI version to MI4. I.e., the comments in the code that talk > > about things to fix for MI3 should become references to MI4 instead. > > Yes, although I would wait until 8.3 is branched before merging it. > > Btw I realized the output with this patch is not good. For -break-list with two multi-location > breakpoints, it results in something like: > > body=[ > bkpt={ ... }, > locations={ ... }, > bkpt={ ... }, > locations={ ... }, > ] > > Where I was aiming for: > > body=[ > bkpt={ > ..., > locations={ ... }, > }, > bkpt={ > ..., > locations={ ... }, > }, > ] > > The next version will fix this. > > Simon