From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6376 invoked by alias); 23 Aug 2010 19:30:19 -0000 Received: (qmail 6286 invoked by uid 22791); 23 Aug 2010 19:30:18 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.35) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 23 Aug 2010 19:30:11 +0000 Received: from hpaq2.eem.corp.google.com (hpaq2.eem.corp.google.com [172.25.149.2]) by smtp-out.google.com with ESMTP id o7NJU8fd017501 for ; Mon, 23 Aug 2010 12:30:08 -0700 Received: from vws18 (vws18.prod.google.com [10.241.21.146]) by hpaq2.eem.corp.google.com with ESMTP id o7NJTiSh014791 for ; Mon, 23 Aug 2010 12:30:07 -0700 Received: by vws18 with SMTP id 18so4276837vws.15 for ; Mon, 23 Aug 2010 12:30:06 -0700 (PDT) MIME-Version: 1.0 Received: by 10.220.159.14 with SMTP id h14mr3325201vcx.115.1282591806839; Mon, 23 Aug 2010 12:30:06 -0700 (PDT) Received: by 10.220.189.205 with HTTP; Mon, 23 Aug 2010 12:30:06 -0700 (PDT) In-Reply-To: <20100823185008.GA2926@host1.dyn.jankratochvil.net> References: <20100823185008.GA2926@host1.dyn.jankratochvil.net> Date: Mon, 23 Aug 2010 19:30:00 -0000 Message-ID: Subject: Re: [patch] Fix DW_OP_call2 and DW_OP_call4 for max-cache-age 0 From: Doug Evans To: Jan Kratochvil Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true 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: 2010-08/txt/msg00395.txt.bz2 Hi. Comments inline. On Mon, Aug 23, 2010 at 11:50 AM, Jan Kratochvil wrote: > Hi, > > as discussed on #gdb when you set max-cache-age 0 DW_OP_call{2,4} crashed= GDB. > > I admit I rather did not test max-cache-age 0 globally. > > No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu. > > OK to check-in? > > The problem is not reproducible with max-cache-age 1. =A0Without the > dw2_do_instantiate_symtab patch part GDB no longer crashes on max-cache-a= ge 0 > but it will then error out on DW_OP_call{2,4} with that: > + =A0 =A0error (_("Dwarf Error: Cannot read CU for DIE at 0x%x referenced= " > + =A0 =A0 =A0 =A0 =A0 =A0"in module %s"), > > There is a bit weird that functions with parameter per_cu have also param= eter > objfile when there is per_cu->objfile. =A0It is because per_cu->objfile i= s there > only since cf9fe5cf2be8b76820a05c966cdca6df5c2eee24 (Tom Tromey 2010-07-1= 3). > > > Thanks, > Jan > > > gdb/ > 2010-08-23 =A0Jan Kratochvil =A0 > > =A0 =A0 =A0 =A0* dwarf2read.c (dw2_do_instantiate_symtab): Move the > =A0 =A0 =A0 =A0age_cached_comp_units call to the top, extend its comment. > =A0 =A0 =A0 =A0(dwarf2_fetch_die_location_block): Initialize cu later. = =A0Call > =A0 =A0 =A0 =A0dw2_setup and dw2_do_instantiate_symtab if PER_CU->CU is N= ULL. > > gdb/testsuite/ > 2010-08-23 =A0Jan Kratochvil =A0 > > =A0 =A0 =A0 =A0* gdb.dwarf2/dw2-op-call.exp (maintenance set dwarf2 max-c= ache-age 0): > =A0 =A0 =A0 =A0New test. > > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -1636,6 +1636,11 @@ dw2_do_instantiate_symtab (struct objfile *objfile, > =A0{ > =A0 struct cleanup *back_to; > > + =A0/* Age the cache, releasing compilation units that have not been used > + =A0 =A0 recently. =A0Age them first so that we do not age out the reque= sted PER_CU > + =A0 =A0 unit if DWARF2_MAX_CACHE_AGE is too low. =A0*/ > + =A0age_cached_comp_units (); Aging cached units first feels weird (if not wrong at least weird); we may toss out something we're about to want. At the least IWBN to elaborate on why this fixes things. > + > =A0 back_to =3D make_cleanup (dwarf2_release_queue, NULL); > > =A0 queue_comp_unit (per_cu, objfile); > @@ -1647,10 +1652,6 @@ dw2_do_instantiate_symtab (struct objfile *objfile, > > =A0 process_queue (objfile); > > - =A0/* Age the cache, releasing compilation units that have not > - =A0 =A0 been used recently. =A0*/ > - =A0age_cached_comp_units (); > - > =A0 do_cleanups (back_to); > =A0} > > @@ -12720,11 +12721,22 @@ struct dwarf2_locexpr_baton > =A0dwarf2_fetch_die_location_block (unsigned int offset, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct dw= arf2_per_cu_data *per_cu) > =A0{ > - =A0struct dwarf2_cu *cu =3D per_cu->cu; > + =A0struct dwarf2_cu *cu; > =A0 struct die_info *die; > =A0 struct attribute *attr; > =A0 struct dwarf2_locexpr_baton retval; > > + =A0if (per_cu->cu =3D=3D NULL) > + =A0 =A0{ > + =A0 =A0 =A0dw2_setup (per_cu->objfile); > + =A0 =A0 =A0dw2_do_instantiate_symtab (per_cu->objfile, per_cu); > + =A0 =A0} > + =A0if (per_cu->cu =3D=3D NULL) > + =A0 =A0error (_("Dwarf Error: Cannot read CU for DIE at 0x%x referenced= " > + =A0 =A0 =A0 =A0 =A0 =A0"in module %s"), > + =A0 =A0 =A0 =A0 =A0offset, per_cu->objfile->name); > + > + =A0cu =3D per_cu->cu; > =A0 die =3D follow_die_offset (offset, &cu); > =A0 if (!die) > =A0 =A0 error (_("Dwarf Error: Cannot find DIE at 0x%x referenced in modu= le %s"), > --- a/gdb/testsuite/gdb.dwarf2/dw2-op-call.exp > +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-call.exp > @@ -36,6 +36,9 @@ if =A0{ [gdb_compile "${srcdir}/${subdir}/${srcfile}" "= ${objdir}/${subdir}/${execu > > =A0clean_restart $executable > > +# Additional test to verify the referenced CU is not aged out. > +gdb_test_no_output "maintenance set dwarf2 max-cache-age 0" > + > =A0gdb_test "p array1" " =3D 1" > =A0gdb_test "p array2" " =3D 2" "array2 using DW_OP_call2" > =A0gdb_test "p array3" " =3D 3" "array3 using DW_OP_call4" > The comment reads better to me if "Additional test" is removed. I.e. "Verify the ...". It's not really an "additional test".