From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9120 invoked by alias); 29 Apr 2013 17:31:54 -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 9110 invoked by uid 89); 29 Apr 2013 17:31:54 -0000 X-Spam-SWARE-Status: No, score=-7.7 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_PASS,TW_BJ autolearn=ham version=3.3.1 Received: from mga11.intel.com (HELO mga11.intel.com) (192.55.52.93) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 29 Apr 2013 17:31:54 +0000 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 29 Apr 2013 10:31:52 -0700 X-ExtLoop1: 1 Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by fmsmga002.fm.intel.com with ESMTP; 29 Apr 2013 10:31:41 -0700 Received: from irsmsx153.ger.corp.intel.com (163.33.192.75) by IRSMSX104.ger.corp.intel.com (163.33.3.159) with Microsoft SMTP Server (TLS) id 14.1.355.2; Mon, 29 Apr 2013 18:30:01 +0100 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.244]) by IRSMSX153.ger.corp.intel.com ([169.254.9.23]) with mapi id 14.01.0438.000; Mon, 29 Apr 2013 18:30:01 +0100 From: "Blanc, Nicolas" To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH 1/3] Added command remove-symbol-file. Date: Tue, 30 Apr 2013 07:30:00 -0000 Message-ID: <388084C8C1E6A64FA36AD1D656E4856619DF0224@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> <517ABA6A.5070400@redhat.com> In-Reply-To: <517ABA6A.5070400@redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2013-04/txt/msg00879.txt.bz2 Hi Pedro, >> +/* Upon notification of FREE_OBJFILE remove any reference >> + to any user-added file that is about to be freed. */ > > Why only user-added files? I choose to restrict the scope of the command to user-added files in order = to limit potential unforeseen side-effects in a first time. This restriction can be lifted in the future if needed. >> +static void >> +remove_user_added_objfile (struct objfile *objfile) { >> + struct so_list *gdb; >> + >> + if (!objfile) >> + return; >> + >> + if (!(objfile->flags & OBJF_USERLOADED) >> + || !(objfile->flags & OBJF_SHARED)) >> + return; >> + >> + >> + gdb =3D so_list_head; >> + while (gdb) >> + { >> + if (gdb->objfile =3D=3D objfile) >> + gdb->objfile =3D NULL; >> + gdb =3D gdb->next; >> + } > > Or rather/also, this looks a bit weird to me. > Can we ever really ever find a user-loaded file in the so_list_head list?= What would that mean? > IIRC, the only way to get a OBJF_USERLOADED|OBJF_SHARED objfile is throug= h "dll-symbols" (dll_symbol_command), but that doesn't create any entry in = the shared library list. This is a good question. I added this function because update_solib_list ha= ndles the case of user-added files: /* Unless the user loaded it explicitly, free SO's objfile. */ if (gdb->objfile && ! (gdb->objfile->flags & OBJF_USERLOADED) && !solib_used (gdb)) free_objfile (gdb->objfile); If the user-loaded check above is needed then remove_user_added_objfile() i= s also needed. But I don't think it is currently possible that user-loaded files end up in= so_list_head either. >> +static void >> +disable_breakpoints_in_free_objfile (struct objfile * objfile) > > This is clearly mirroring the naming of > disable_breakpoints_in_unloaded_shlib. Should be "in freed objfile". > "in free objfile" would mean something else. Ok > + error (_("USAGE: remove-symbol-file ")); > I'd s/low// here. text_address is clear and common enough that having "l= ow" there makes me go "what does low mean here?" Or just
even. Ok, I am using text_addr now. > + if (objf->flags & OBJF_USERLOADED && objf->addr_low =3D=3D addr) > As I mentioned, the .text address may not be the lower address in the obj= ect at all, so this "addr_low" confused me. > I'd be happier with naming the field for what ig really is, something lik= e "add_symbol_file_addr", with a comment indicating this is related to "add= -symbol-file", but I see you're reusing an existing variable. At the very = least, > the variable's definition should gain a comment explaining its overloadin= g for add-symbol-file. I understand your point. GDB's implementation\doc assume that addr_low is t= he text address. It' not only the case for add-symbol-file but also for info shared. So I've added the following comment to struct objfile for clarification: /* The base address of the object file, which by default is assumed to be the address of the text section. The remove-symbol-file command uses this field to identify the object file to remove. */ CORE_ADDR addr_low; I hope this helps. Thanks, Nicolas -- Pedro Alves 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