From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32670 invoked by alias); 25 Jun 2012 11:39:32 -0000 Received: (qmail 32395 invoked by uid 22791); 25 Jun 2012 11:39:28 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KAM_STOCKTIP,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-vb0-f41.google.com (HELO mail-vb0-f41.google.com) (209.85.212.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 25 Jun 2012 11:38:55 +0000 Received: by vbkv13 with SMTP id v13so2350682vbk.0 for ; Mon, 25 Jun 2012 04:38:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding:x-system-of-record :x-gm-message-state; bh=3nfVVFG7C0hRcbZVxlhCAQI/SMA8ZrRH6V+xJ4QceAI=; b=m5aXE92V7g5NAMb+27vYkbkL22a4yr+kqU2jg0Ej6GbnOL0xaANuXHNAaYd3tOeo2q JlB08rOcy6T9KaineNKSIT1sKdDfsU8ggDd8IXGNEsWRL8sTI0J5b/XTz53g700Q5zzx dvBZzAQmXmc2VY48ABWMJy8U+pz/0xQ0laQpW+3dmixNF2aZYtPa7nTsfU/4NaknES91 hpkxsgAP1daQbp6LUgGGUkhc3M7UZTg4peqw7yj9G3LBW1VF3Bk8hggSWHMB7+LFPYMM 0036LhZGvlpBhj0VMkkPUYasEXsqVOptIPwSOiNcZO+uJlqhxavH9b0t/iLKhp2JfnUF q2tA== Received: by 10.52.95.171 with SMTP id dl11mr6246531vdb.120.1340624334331; Mon, 25 Jun 2012 04:38:54 -0700 (PDT) MIME-Version: 1.0 Received: by 10.52.95.171 with SMTP id dl11mr6246515vdb.120.1340624334061; Mon, 25 Jun 2012 04:38:54 -0700 (PDT) Received: by 10.52.26.132 with HTTP; Mon, 25 Jun 2012 04:38:53 -0700 (PDT) In-Reply-To: <20120624183352.GA15915@host2.jankratochvil.net> References: <20120605011446.670FD1E123B@ruffy2.mtv.corp.google.com> <20120622193539.GA7344@host2.jankratochvil.net> <20120624183352.GA15915@host2.jankratochvil.net> Date: Mon, 25 Jun 2012 11:39:00 -0000 Message-ID: Subject: Re: [RFA] Fix inconsistency in blockvector addrmap vs non-addrmap handling From: Doug Evans To: Jan Kratochvil Cc: gdb-patches@sourceware.org, tromey@redhat.com, palves@redhat.com Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-Gm-Message-State: ALoCoQkuKSWXU1Midd22/xyQaC95zbWONdbHktc5bBzTU1q58YpPUnD79/l5xkuDEW1Z0LwkAsdrZRSaDTNi6VHCLDsk/7oNMCaUBM2qcNjIMEcXBAiveOrHevpvoPe2oet3p6YuiOFYNUgRlIjtjpS+A7jUfnmfc3IAVTjuuRh4fOdjlxVZ7wgbMtzvRk20PCZIhxE+iV56 X-IsSubscribed: yes 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 X-SW-Source: 2012-06/txt/msg00764.txt.bz2 On Sun, Jun 24, 2012 at 11:33 AM, Jan Kratochvil wrote: > On Fri, 22 Jun 2012 22:51:41 +0200, Doug Evans wrote: >> But remember that the static block =A0(or global block) doesn't exist >> until the call to end_symtab, and, barring even more work, you need to >> get the range into pending_addrmap before the call to make_blockvector >> where pending_addrmap is turned into a fixed addrmap. > > One could use either callback for end_sym there or split end_sym (which I= have > chosen). > > Unfortunately I do not have time now to finish a proper testcase also > exploiting the discontiguous CU so posting just the fix here. =A0I should= get to > the testcase back on Tuesday. =A0In fact I do not know if this fix works = at all > for the discontiguous CU case. > > No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu agains= t the > patch: > =A0 =A0 =A0 =A0http://sourceware.org/ml/gdb-patches/2012-06/msg00109.html > =A0 =A0 =A0 =A0Subject: [RFA] Fix inconsistency in blockvector addrmap vs= non-addrmap handling > > > Thanks, > Jan > > > gdb/ > 2012-06-24 =A0Jan Kratochvil =A0 > > =A0 =A0 =A0 =A0* buildsym.c (end_symtab): Split it to ... > =A0 =A0 =A0 =A0(end_symtab_get_static_block): ... this ... > =A0 =A0 =A0 =A0(end_symtab_from_static_block): ... and this function. > =A0 =A0 =A0 =A0(end_symtab): New function. > =A0 =A0 =A0 =A0* buildsym.h (end_symtab_get_static_block) > =A0 =A0 =A0 =A0(end_symtab_from_static_block): New declarations. > =A0 =A0 =A0 =A0* dwarf2read.c (process_full_comp_unit): New variable stat= ic_block. > =A0 =A0 =A0 =A0Set its valid CU ranges. > > diff --git a/gdb/buildsym.c b/gdb/buildsym.c > index f1fb4be..2de2ef8 100644 > --- a/gdb/buildsym.c > +++ b/gdb/buildsym.c > @@ -923,38 +923,20 @@ block_compar (const void *ap, const void *bp) > =A0 =A0 =A0 =A0 =A0- (BLOCK_START (b) < BLOCK_START (a))); > =A0} > > -/* Finish the symbol definitions for one main source file, close off > - =A0 all the lexical contexts for that file (creating struct block's for > - =A0 them), then make the struct symtab for that file and put it in the > - =A0 list of all such. > - > - =A0 END_ADDR is the address of the end of the file's text. =A0SECTION is > - =A0 the section number (in objfile->section_offsets) of the blockvector > - =A0 and linetable. > +/* Implementation of the first part of end_symtab. =A0It allows modifying > + =A0 STATIC_BLOCK before it gets finalized by end_symtab_from_static_blo= ck. > + =A0 If the returned value is NULL there is no blockvector created for > + =A0 this symtab (you still must call end_symtab_from_static_block). =A0= */ > > - =A0 Note that it is possible for end_symtab() to return NULL. =A0In > - =A0 particular, for the DWARF case at least, it will return NULL when > - =A0 it finds a compilation unit that has exactly one DIE, a > - =A0 TAG_compile_unit DIE. =A0This can happen when we link in an object > - =A0 file that was compiled from an empty source file. =A0Returning NULL > - =A0 is probably not the correct thing to do, because then gdb will > - =A0 never know about this empty file (FIXME). =A0*/ > - > -struct symtab * > -end_symtab (CORE_ADDR end_addr, struct objfile *objfile, int section) > +struct block * > +end_symtab_get_static_block (CORE_ADDR end_addr, struct objfile *objfile) > =A0{ > - =A0struct symtab *symtab =3D NULL; > - =A0struct blockvector *blockvector; > - =A0struct subfile *subfile; > - =A0struct context_stack *cstk; > - =A0struct subfile *nextsub; > - > =A0 /* Finish the lexical context of the last function in the file; pop > =A0 =A0 =A0the context stack. =A0*/ > > =A0 if (context_stack_depth > 0) > =A0 =A0 { > - =A0 =A0 =A0cstk =3D pop_context (); > + =A0 =A0 =A0struct context_stack *cstk =3D pop_context (); > =A0 =A0 =A0 /* Make a block for the local symbols within. =A0*/ > =A0 =A0 =A0 finish_block (cstk->name, &local_symbols, cstk->old_blocks, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0cstk->start_addr, end_addr, objfil= e); > @@ -1021,15 +1003,41 @@ end_symtab (CORE_ADDR end_addr, struct objfile *o= bjfile, int section) > =A0 =A0 { > =A0 =A0 =A0 /* Ignore symtabs that have no functions with real debugging > =A0 =A0 =A0 =A0 =A0info. =A0*/ > + =A0 =A0 =A0return NULL; > + =A0 =A0} > + =A0else > + =A0 =A0{ > + =A0 =A0 =A0/* Define the STATIC_BLOCK. =A0*/ > + =A0 =A0 =A0return finish_block (NULL, &file_symbols, NULL, last_source_= start_addr, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0end_addr, objfile); > + =A0 =A0} > +} > + > +/* Implementation of the second part of end_symtab. =A0Pass STATIC_BLOCK > + =A0 as value returned by end_symtab_get_static_block. =A0*/ > + > +struct symtab * > +end_symtab_from_static_block (struct block *static_block, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct objfile = *objfile, int section) > +{ > + =A0struct symtab *symtab =3D NULL; > + =A0struct blockvector *blockvector; > + =A0struct subfile *subfile; > + =A0struct subfile *nextsub; > + > + =A0if (static_block =3D=3D NULL) > + =A0 =A0{ > + =A0 =A0 =A0/* Ignore symtabs that have no functions with real debugging > + =A0 =A0 =A0 =A0 info. =A0*/ > =A0 =A0 =A0 blockvector =3D NULL; > =A0 =A0 } > =A0 else > =A0 =A0 { > - =A0 =A0 =A0/* Define the STATIC_BLOCK & GLOBAL_BLOCK, and build the > + =A0 =A0 =A0CORE_ADDR end_addr =3D BLOCK_END (static_block); > + > + =A0 =A0 =A0/* Define after STATIC_BLOCK also GLOBAL_BLOCK, and build the > =A0 =A0 =A0 =A0 =A0blockvector. =A0*/ > - =A0 =A0 =A0finish_block (0, &file_symbols, 0, last_source_start_addr, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 end_addr, objfile); > - =A0 =A0 =A0finish_block_internal (0, &global_symbols, 0, last_source_st= art_addr, > + =A0 =A0 =A0finish_block_internal (NULL, &global_symbols, NULL, last_sou= rce_start_addr, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 end_addr, objfile= , 1); > =A0 =A0 =A0 blockvector =3D make_blockvector (objfile); > =A0 =A0 } > @@ -1219,6 +1227,36 @@ end_symtab (CORE_ADDR end_addr, struct objfile *ob= jfile, int section) > =A0 return symtab; > =A0} > > +/* Finish the symbol definitions for one main source file, close off > + =A0 all the lexical contexts for that file (creating struct block's for > + =A0 them), then make the struct symtab for that file and put it in the > + =A0 list of all such. > + > + =A0 END_ADDR is the address of the end of the file's text. =A0SECTION is > + =A0 the section number (in objfile->section_offsets) of the blockvector > + =A0 and linetable. > + > + =A0 Note that it is possible for end_symtab() to return NULL. =A0In > + =A0 particular, for the DWARF case at least, it will return NULL when > + =A0 it finds a compilation unit that has exactly one DIE, a > + =A0 TAG_compile_unit DIE. =A0This can happen when we link in an object > + =A0 file that was compiled from an empty source file. =A0Returning NULL > + =A0 is probably not the correct thing to do, because then gdb will > + =A0 never know about this empty file (FIXME). > + > + =A0 If you need to modify STATIC_BLOCK before it is finalized you should > + =A0 call end_symtab_get_static_block and end_symtab_from_static_block > + =A0 yourself. =A0*/ > + > +struct symtab * > +end_symtab (CORE_ADDR end_addr, struct objfile *objfile, int section) > +{ > + =A0struct block *static_block; > + > + =A0static_block =3D end_symtab_get_static_block (end_addr, objfile); > + =A0return end_symtab_from_static_block (static_block, objfile, section); > +} > + > =A0/* Push a context block. =A0Args are an identifying nesting level > =A0 =A0(checkable when you pop it), and the starting PC address of this > =A0 =A0context. =A0*/ > diff --git a/gdb/buildsym.h b/gdb/buildsym.h > index 4448267..89324e9 100644 > --- a/gdb/buildsym.h > +++ b/gdb/buildsym.h > @@ -260,6 +260,11 @@ extern char *pop_subfile (void); > > =A0extern struct symtab *end_symtab (CORE_ADDR end_addr, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct= objfile *objfile, int section); > +extern struct block *end_symtab_get_static_block (CORE_ADDR end_addr, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 struct objfile *objfile); > +extern struct symtab *end_symtab_from_static_block (struct block *static= _block, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct objfile *objfile, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 int section); > > =A0/* Defined in stabsread.c. =A0*/ > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index aa42b4c..62119ad 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -5790,6 +5790,7 @@ process_full_comp_unit (struct dwarf2_per_cu_data *= per_cu, > =A0 struct symtab *symtab; > =A0 struct cleanup *back_to, *delayed_list_cleanup; > =A0 CORE_ADDR baseaddr; > + =A0struct block *static_block; > > =A0 baseaddr =3D ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfi= le)); > > @@ -5820,7 +5821,15 @@ process_full_comp_unit (struct dwarf2_per_cu_data = *per_cu, > =A0 =A0 =A0it, by scanning the DIE's below the compilation unit. =A0*/ > =A0 get_scope_pc_bounds (cu->dies, &lowpc, &highpc, cu); > > - =A0symtab =3D end_symtab (highpc + baseaddr, objfile, SECT_OFF_TEXT (ob= jfile)); > + =A0static_block =3D end_symtab_get_static_block (highpc + baseaddr, obj= file); > + > + =A0/* This CU may have discontiguous DW_AT_ranges and the CU may have a= ddresses > + =A0 =A0 not belonging to any of its child DIEs (such as virtual method = tables). > + =A0 =A0 Assign such addresses to STATIC_BLOCK's addrmap. =A0*/ > + =A0dwarf2_record_block_ranges (cu->dies, static_block, baseaddr, cu); > + > + =A0symtab =3D end_symtab_from_static_block (static_block, objfile, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0SECT_OFF_TEXT (objfile)); > > =A0 if (symtab !=3D NULL) > =A0 =A0 { This is on the right track, but it's incomplete (IMO). - Why call get_scope_pc_bounds if we're going to then explicitly attach DW_TAG_compile_unit's low_pc/high_pc/ranges attributes to STATIC_BLOCK? [we're processing these attributes twice, blech] - If it does actually fix the bug in question it seems more accidental than on purpose. - e.g. if the DW_TAG_compile_unit's DW_AT_ranges doesn't include some min= sym, what happens? - If we really want to take this step then start,end on global,static blocks needs to go. Until then we're just satisfying a desire to apply dwarf correctly without really accomplishing anything.