From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32599 invoked by alias); 22 Feb 2010 19:14:09 -0000 Received: (qmail 32557 invoked by uid 22791); 22 Feb 2010 19:14:06 -0000 X-SWARE-Spam-Status: No, hits=-1.1 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,SARE_MSGID_LONG40 X-Spam-Check-By: sourceware.org Received: from mail-bw0-f212.google.com (HELO mail-bw0-f212.google.com) (209.85.218.212) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 22 Feb 2010 19:14:01 +0000 Received: by bwz4 with SMTP id 4so2103732bwz.8 for ; Mon, 22 Feb 2010 11:13:58 -0800 (PST) MIME-Version: 1.0 Received: by 10.204.48.202 with SMTP id s10mr917726bkf.34.1266866038384; Mon, 22 Feb 2010 11:13:58 -0800 (PST) In-Reply-To: <201002221827.51014.pedro@codesourcery.com> References: <90baa01f1002181132v2faf5c3fm4b368ff0324fc3b2@mail.gmail.com> <201002221827.51014.pedro@codesourcery.com> Date: Mon, 22 Feb 2010 19:14:00 -0000 Message-ID: <90baa01f1002221113n82a30dred0208e4f201faca@mail.gmail.com> Subject: Re: Fwd: [patch]: Fix crash in objc and breakpoints From: Kai Tietz To: Pedro Alves Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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-02/txt/msg00547.txt.bz2 Hello Pedro, 2010/2/22 Pedro Alves : > On Thursday 18 February 2010 19:32:11, Kai Tietz wrote: > >> =A0 =A0 =A0 =A0* source.c (line_info): Initialize pspace by default >> =A0 =A0 =A0 =A0current_program_space. >> =A0 =A0 =A0 =A0* frame.c (find_frame_sal): Likewise. >> =A0 =A0 =A0 =A0* linespec.c (decode_line_2): Likewise. >> =A0 =A0 =A0 =A0(decode_objc): Likewise. >> >> Ok for apply? > > Sorry, not yet. > > The patch is mangled and I can't apply it as is, > but I'll try to make an effort to read it anyway. > > Could you please, please also use (cvs) diff's -p switch > for future patches? =A0It makes diffs so much more readable. > Thanks. > Ok, I use in general quilt for patches, but of course I can sent them in future by using cvs -p for diff. >> >> Index: src/gdb/frame.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- src.orig/gdb/frame.c =A0 =A0 =A0 =A02010-01-29 16:28:43.000000000 +0= 100 >> +++ src/gdb/frame.c =A0 =A0 2010-02-18 10:49:42.745803800 +0100 >> @@ -1857,6 +1857,8 @@ >> =A0 =A0 =A0 =A0 =A0 the call site is. =A0Do not pretend to. =A0This is j= arring, but >> =A0 =A0 =A0 =A0 =A0 we can't do much better. =A0*/ >> =A0 =A0 =A0 =A0sal->pc =3D get_frame_pc (frame); >> + =A0 =A0 =A0/* Initialize pspace by default. =A0*/ >> + =A0 =A0 =A0sal->pspace =3D current_program_space; >> > > Did you try frame->pspace? Yes, I did. But it hadn't solved the issue. >> =A0 =A0 =A0 return; >> =A0 =A0 } >> Index: src/gdb/linespec.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- src.orig/gdb/linespec.c =A0 =A0 2010-02-18 10:41:31.000000000 +0100 >> +++ src/gdb/linespec.c =A02010-02-18 10:52:50.980178800 +0100 >> @@ -513,7 +513,9 @@ >> =A0 while (i < nelts) >> =A0 =A0 { >> =A0 =A0 =A0 init_sal (&return_values.sals[i]); =A0 =A0 =A0 /* Initialize= to zeroes. >> */ >> + =A0 =A0 =A0return_values.sals[i].pspace =3D current_program_space; >> =A0 =A0 =A0 init_sal (&values.sals[i]); >> + =A0 =A0 =A0values.sals[i].pspace =3D current_program_space; >> =A0 =A0 =A0 if (sym_arr[i] && SYMBOL_CLASS (sym_arr[i]) =3D=3D LOC_BLOCK) >> =A0 =A0 =A0 =A0values.sals[i] =3D find_function_start_sal (sym_arr[i], >> funfirstline); >> =A0 =A0 =A0 i++; > > Sorry, I don't like these workarounds. =A0Why was this needed? > There must be a code path that didn't set the pspace on a > valid sal. =A0What was it? > > I think you're patching this: > > =A0i =3D 0; > =A0while (i < nelts) > =A0 =A0{ > =A0 =A0 =A0init_sal (&return_values.sals[i]); =A0 =A0 =A0 =A0/* Initializ= e to zeroes. =A0*/ > =A0 =A0 =A0init_sal (&values.sals[i]); > =A0 =A0 =A0if (sym_arr[i] && SYMBOL_CLASS (sym_arr[i]) =3D=3D LOC_BLOCK) > =A0 =A0 =A0 =A0values.sals[i] =3D find_function_start_sal (sym_arr[i], fu= nfirstline); > =A0 =A0 =A0i++; > =A0 =A0} > > and find_function_start_sal should be returning a sal with > the correct pspace, so this must be about the sals that > aren't LOC_BLOCK? =A0What are those? =A0Below there's this > > =A0while (i < nelts) > =A0 =A0 =A0 =A0{ > =A0 =A0 =A0 =A0 =A0if (sym_arr[i] && SYMBOL_CLASS (sym_arr[i]) =3D=3D LOC= _BLOCK) > =A0 =A0 =A0 =A0 =A0 =A0{ > ... > =A0 =A0 =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 =A0else > =A0 =A0 =A0 =A0 =A0 =A0printf_unfiltered (_("?HERE\n")); > =A0 =A0 =A0 =A0 =A0i++; > > Are we hitting this? =A0Sounds like something similar to PR10966. =A0Does > the workaround that has been applied on the branch for this PR happen > to fix this? The patch in PR/10966 looks similar but didn't solved the issue, For some of those place I added a default initialization of sal's pspace are maybe superflous, but I saw the issue just remaining for obj-c. I tested that the necessary patches to solve my issue are in frame.c (find_frame_sal) and linespec.c (decode_line_2), The other seems to be not really necessary. >> @@ -1206,6 +1208,7 @@ >> =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 ¤t_target); >> >> =A0 =A0 =A0 =A0 =A0init_sal (&values.sals[0]); >> + =A0 =A0 =A0 =A0 values.sals[0].pspace =3D current_program_space; >> =A0 =A0 =A0 =A0 =A0values.sals[0].pc =3D pc; >> =A0 =A0 =A0 =A0} >> =A0 =A0 =A0 return values; > > > >> Index: src/gdb/source.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- src.orig/gdb/source.c =A0 =A0 =A0 2010-01-12 16:54:43.000000000 +0100 >> +++ src/gdb/source.c =A0 =A02010-02-18 10:46:36.183303800 +0100 >> @@ -1467,6 +1467,7 @@ >> =A0 int i; >> >> =A0 init_sal (&sal); =A0 =A0 =A0 =A0 =A0 =A0 /* initialize to zeroes */ >> + =A0sal.pspace =3D current_program_space; /* initialize as default. =A0= */ > > Likewise. =A0Isn't this a problem with the `if (arg =3D=3D 0)' branch bel= ow > only? =A0It looks like it. > >> >> =A0 if (arg =3D=3D 0) >> =A0 =A0 { >> >> > > Do you have a testcase (doesn't have to be in .exp form, just > something I could try) for these issues yet? =A0You seem to > be covering different problems with the same patch. Sadly the test-scenario for this issue is a bit big. I'll try to make a testcase for it, but AFAI had seen it happens for me with Obj-C and shared object files. I hope it isn't a Windows specific thing, but I wouldn't assume so. Kai --=20 | (\_/) This is Bunny. Copy and paste | (=3D'.'=3D) Bunny into your signature to help | (")_(") him gain world domination