From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27925 invoked by alias); 25 Apr 2013 12:49:03 -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 27916 invoked by uid 89); 25 Apr 2013 12:49:03 -0000 X-Spam-SWARE-Status: No, score=-6.5 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,MIME_QP_LONG_LINE,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_PASS,TW_BJ,TW_RG autolearn=ham version=3.3.1 Received: from mga14.intel.com (HELO mga14.intel.com) (143.182.124.37) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 25 Apr 2013 12:49:02 +0000 Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga102.ch.intel.com with ESMTP; 25 Apr 2013 05:48:59 -0700 X-ExtLoop1: 1 Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by azsmga001.ch.intel.com with ESMTP; 25 Apr 2013 05:48:57 -0700 Received: from irsmsx105.ger.corp.intel.com (163.33.3.28) by IRSMSX101.ger.corp.intel.com (163.33.3.153) with Microsoft SMTP Server (TLS) id 14.1.355.2; Thu, 25 Apr 2013 13:48:57 +0100 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.244]) by IRSMSX105.ger.corp.intel.com ([169.254.7.14]) with mapi id 14.01.0438.000; Thu, 25 Apr 2013 13:48:56 +0100 From: "Blanc, Nicolas" To: "gdb-patches@sourceware.org" CC: Tom Tromey , Pedro Alves Subject: RE: [PATCH 1/3] Added command remove-symbol-file. Date: Thu, 25 Apr 2013 19:24:00 -0000 Message-ID: <388084C8C1E6A64FA36AD1D656E4856619DEF314@IRSMSX102.ger.corp.intel.com> References: <1366098721-18302-1-git-send-email-nicolas.blanc@intel.com> <1366098721-18302-2-git-send-email-nicolas.blanc@intel.com> <87fvygnfl0.fsf@fleche.redhat.com> In-Reply-To: <87fvygnfl0.fsf@fleche.redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2013-04/txt/msg00780.txt.bz2 > Nicolas> +/* Disable any breakpoints and tracepoints that are in SOLIB up= on > Nicolas> + notification of UNLOADED_SHLIB. Only apply to enabled=20 > Nicolas> +breakpoints, Tom> Two periods after space -- Yao pointed out one instance of this, but t= here are a couple. Nicolas> I'll fixed those. > Nicolas> + ALL_OBJFILE_OSECTIONS (objfile, osect) > Nicolas> + { > Nicolas> + if (obj_section_addr (osect) <=3D loc_addr > Nicolas> + && loc_addr < obj_section_endaddr (osect)) > Nicolas> + { > Nicolas> + loc->shlib_disabled =3D 1; > Nicolas> + loc->inserted =3D 0; > Nicolas> + observer_notify_breakpoint_modified (loc->owner); > Nicolas> + break; > Nicolas> + } Tom> I liked Yao's comment about notifying just once per breakpoint. It do= esn't seem difficult to do that. Nicolas> Ok I'll use a list of breakpoints to collect first the breakpoints= to notify. Tom> It's curious that this new function and breakpoint_free_objfile don't = overlap at all. Nicolas> First disable_breakpoints_in_free_objfile() is called and then bre= akpoint_free_objfile() from free_objfile(). Nicolas> Now I could test moving the code from breakpoint_free_objfile() to= disable_breakpionts_in_free_objfile() and delete breakpoint_free_objfile(). > Nicolas> - observer_attach_solib_unloaded=20 > Nicolas> (clear_dangling_display_expressions); > Nicolas> + observer_attach_free_objfile=20 > Nicolas> + (clear_dangling_display_expressions); > Tom> It isn't totally clear to me that these are equivalent. > Tom> From browsing solib.c it seems that an solib can be freed without th= e objfiles being purged. > Tom> I am not sure but I think maybe the path core_close -> clear_solib c= an do this. If so, maybe this is simply a bug there; I am not sure. Nicolas> Indeed, an object file is shared with another SO or it was loaded = by the user then free_so() will be called but not free_objfile(). Nicolas> In that case clear_dangling_disaplay_expression() won't be called.= I think it's ok because the dangling reference that needs Nicolas> to be cleaned-up is the object file not the SO. > Nicolas> +static void remove_symbol_file_command (char *, int); Tom >I don't think you really need this declaration. It's better to leave = them out unless they are needed for something specific, like mutual recursi= on. (There are lots of leftover ones, which I've always imagined to be fro= m a time when GCC's prototype warnings worked differently.) Nicolas> Ok, I'll see what happens when I remove it. > Nicolas> + addr_low =3D parse_and_eval_address (arg); Tom> It seems like this parses and evaluates this argument twice. Tom> I think it would be better to take another approach, so that it is jus= t evaluated once. Nicolas> You're right. I can take the value later on from the subsequent lo= op. > Nicolas> + /* Set the low address of the object for identification. */= =20=20 > Nicolas> + objf->addr_low =3D addr_low; Tom> Is there any way to display this information? Nicolas> Unfortunately the answer no. "info files" does not list user-added= files. GDB is currently lacking commands to show user-added files. Tom> Otherwise, how should users find it? Nicolas> The user knows the address because he typed it in for adding the f= ile. Note that when the user needs adding a file manually then he has a way= outside GDB to learn about the memory mapping. > Nicolas> +static void > Nicolas> +remove_symbol_file_command (char *args, int from_tty) > [...] > Nicolas> + argv =3D gdb_buildargv (args); make_cleanup_freeargv (argv); Tom> If the syntax is really just an expression as an argument, then don't = bother with gdb_buildargv. Using gdb_buildargv means that complicated expr= essions will have to be quoted unusually. Tom>I realize this is how add-symbol-file works, but I think it is a bad ap= proach. Nicolas> Ok, I'll do that. > Nicolas> + if (!objf) Tom> We recently decided to use the wordier "objf !=3D NULL" style. Nicolas> Ok. > Nicolas> + c =3D add_cmd ("remove-symbol-file", class_files,=20 > Nicolas> + remove_symbol_file_command, _("\ Tom> I think this line is too long. Nicolas> Will be fixed. Thanks for your feedback. It's much appreciated. Nicolas Intel GmbH Dornacher Strasse 1 85622 Feldkirchen/Muenchen, Deutschland Sitz der Gesellschaft: Feldkirchen bei Muenchen Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk Registergericht: Muenchen HRB 47456 Ust.-IdNr./VAT Registration No.: DE129385895 Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052