From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21590 invoked by alias); 12 Aug 2013 07:50:37 -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 21573 invoked by uid 89); 12 Aug 2013 07:50:36 -0000 X-Spam-SWARE-Status: No, score=-6.1 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_DNSWL_MED,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 Received: from e28smtp04.in.ibm.com (HELO e28smtp04.in.ibm.com) (122.248.162.4) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 12 Aug 2013 07:50:35 +0000 Received: from /spool/local by e28smtp04.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 12 Aug 2013 13:12:49 +0530 Received: from d28dlp01.in.ibm.com (9.184.220.126) by e28smtp04.in.ibm.com (192.168.1.134) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 12 Aug 2013 13:12:48 +0530 Received: from d28relay04.in.ibm.com (d28relay04.in.ibm.com [9.184.220.61]) by d28dlp01.in.ibm.com (Postfix) with ESMTP id 3D216E0054 for ; Mon, 12 Aug 2013 13:20:49 +0530 (IST) Received: from d28av06.in.ibm.com (d28av06.in.ibm.com [9.184.220.48]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r7C7oPBU37028036 for ; Mon, 12 Aug 2013 13:20:25 +0530 Received: from d28av06.in.ibm.com (localhost [127.0.0.1]) by d28av06.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r7C7oRHj009851 for ; Mon, 12 Aug 2013 13:20:28 +0530 Received: from d23ml188.in.ibm.com (d23ml188.in.ibm.com [9.182.8.144]) by d28av06.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id r7C7oROL009845; Mon, 12 Aug 2013 13:20:27 +0530 In-Reply-To: <201308082201.r78M1L4R020039@d06av02.portsmouth.uk.ibm.com> References: from "Raunaq 12" at Aug 08, 2013 05:32:49 PM <201308082201.r78M1L4R020039@d06av02.portsmouth.uk.ibm.com> Subject: Re: [PATCH 2/5] powerpc64-aix processing xlC generated line table X-KeepSent: 8944CD0B:3B517EDB-65257BC5:0028EBFF; type=4; name=$KeepSent To: "Ulrich Weigand" Cc: brobecker@adacore.com, gdb-patches@sourceware.org, palves@redhat.com (Pedro Alves), tromey@redhat.com Message-ID: From: Raunaq 12 Date: Mon, 12 Aug 2013 07:50:00 -0000 MIME-Version: 1.0 Content-type: multipart/mixed; Boundary="0__=EABBF156DFBB6D6F8f9e8a93df938690918cEABBF156DFBB6D6F" Content-Disposition: inline X-TM-AS-MML: No X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13081207-5564-0000-0000-000009388548 X-Virus-Found: No X-SW-Source: 2013-08/txt/msg00291.txt.bz2 --0__=EABBF156DFBB6D6F8f9e8a93df938690918cEABBF156DFBB6D6F Content-type: text/plain; charset=US-ASCII Content-length: 4844 "Ulrich Weigand" wrote on 08/09/2013 03:31:21 AM: > Raunaq 12 wrote: > > "Ulrich Weigand" wrote on 08/07/2013 08:53:42 PM: > > > Did you confirm -via test suite runs- that: > > > - this patch improves results when using XLC, and > > > - this patch does not regress when using GCC? > > > > Yes, I did test it. Running the entire test bucket when test cases > > are compiled with xlc is a pain as many compiler options used to > > compile the test programs are not compatible with xlc. > > Well, the test suite is supposed to handle different compilers, > but since only very few people test with anything but GCC, > support for other compilers tends to bit-rot. As a suggestion > to help with future maintainability of GDB on AIX, it would be > nice if you could go through the test suite at some point and > clean those problems up ... Agreed, that would greatly help with future maintainablity of GDB on AIX. Will plan to take that task up once I am done fixing some other issues faced by GDB on AIX like stabstring support for XLC++ compiled binaries. > > So this must be taken care of before we call arrange_linetable to take > > care of the "line number 0" function markers. > > Well, I guess what I'm concerned about is that your patch eliminates > the possibility for arrange_linetable to see separate functions in > the line table (since the line number 0 markers have actually been > erased). Since arrange_linetable wants to sort the line table by > function, I'd have expected this to cause issues. (Unless there is > no real reason any more to perform this sorting?) > > > So I cannot make these changes in arrange_linetable(). > > It's probably easier to show what I had been thinking of in the form > of a patch. Note that this is completely untested and just intended > to show the idea: > > > Index: gdb/xcoffread.c > =================================================================== > RCS file: /cvs/src/src/gdb/xcoffread.c,v > retrieving revision 1.114 > diff -u -p -r1.114 xcoffread.c > --- gdb/xcoffread.c 6 May 2013 19:38:04 -0000 1.114 > +++ gdb/xcoffread.c 8 Aug 2013 21:52:42 -0000 > @@ -438,6 +438,7 @@ arrange_linetable (struct linetable *old > struct linetable_entry *fentry; /* function entry vector */ > int fentry_size; /* # of function entries */ > struct linetable *newLineTb; /* new line table */ > + int extra_lines = 0; > > #define NUM_OF_FUNCTIONS 20 > > @@ -459,6 +460,12 @@ arrange_linetable (struct linetable *old > fentry[function_count].line = ii; > fentry[function_count].pc = oldLineTb->item[ii].pc; > ++function_count; > + > + /* If the function was compiled with XLC, we may have to add an > + extra line entry later. Reserve space for that. */ > + if (ii + 1 < oldLineTb->nitems > + && oldLineTb->item[ii].pc != oldLineTb->item[ii + 1].pc) > + extra_lines++; > } > } > > @@ -475,7 +482,8 @@ arrange_linetable (struct linetable *old > newLineTb = (struct linetable *) > xmalloc > (sizeof (struct linetable) + > - (oldLineTb->nitems - function_count) * sizeof (struct linetable_entry)); > + ((oldLineTb->nitems - function_count + extra_lines) > + * sizeof (struct linetable_entry))); > > /* If line table does not start with a function beginning, copy up until > a function begin. */ > @@ -490,6 +498,17 @@ arrange_linetable (struct linetable *old > > for (ii = 0; ii < function_count; ++ii) > { > + /* If the function was compiled with XLC, we may have to add an > + extra line to cover the function prologue. */ > + jj = fentry[ii].line; > + if (jj + 1 < oldLineTb->nitems > + && oldLineTb->item[jj].pc != oldLineTb->item[jj + 1].pc) > + { > + newLineTb->item[newline] = oldLineTb->item[jj]; > + newLineTb->item[newline].line = oldLineTb->item[jj + 1].line; > + newline++; > + } > + > for (jj = fentry[ii].line + 1; > jj < oldLineTb->nitems && oldLineTb->item[jj].line != 0; > ++jj, ++newline) > > Does this also solve the problem you were trying to address? Thanks for taking time to write this Ulrich. This does solve the problem but one extra line had to be added to update the number of items in the line table as seen below, - newLineTb->nitems = oldLineTb->nitems - function_count; + newLineTb->nitems = oldLineTb->nitems - function_count + extra_lines; Added this line and ran the related testcases. Results same as before. Can we go ahead and check this in? If so, please find the changelog entry and attached patch below. --- ChangeLog :- * xcoffread.c (arrange_linetable): Add fix to correctly handle line tables generated by XLC compiled binaries. (See attached file: gdb-7.6-xcoff_xlc_lines.patch) Thanks, Raunaq M. Bathija --0__=EABBF156DFBB6D6F8f9e8a93df938690918cEABBF156DFBB6D6F Content-type: application/octet-stream; name="gdb-7.6-xcoff_xlc_lines.patch" Content-Disposition: attachment; filename="gdb-7.6-xcoff_xlc_lines.patch" Content-transfer-encoding: base64 Content-length: 3083 SW5kZXg6IC4vZ2RiL3hjb2ZmcmVhZC5jDQo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09DQotLS0gLi9nZGIub3JpZy94Y29mZnJlYWQuYwkyMDEzLTA4LTEyIDEw OjQ3OjU4LjAwMDAwMDAwMCArMDYwMA0KKysrIC4vZ2RiL3hjb2ZmcmVhZC5j CQkyMDEzLTA4LTEyIDEyOjQ2OjU0LjAwMDAwMDAwMCArMDYwMA0KQEAgLTQz OCw2ICs0MzgsNyBAQA0KICAgc3RydWN0IGxpbmV0YWJsZV9lbnRyeSAqZmVu dHJ5OwkvKiBmdW5jdGlvbiBlbnRyeSB2ZWN0b3IgKi8NCiAgIGludCBmZW50 cnlfc2l6ZTsJCS8qICMgb2YgZnVuY3Rpb24gZW50cmllcyAqLw0KICAgc3Ry dWN0IGxpbmV0YWJsZSAqbmV3TGluZVRiOwkvKiBuZXcgbGluZSB0YWJsZSAq Lw0KKyAgaW50IGV4dHJhX2xpbmVzID0gMDsNCiANCiAjZGVmaW5lIE5VTV9P Rl9GVU5DVElPTlMgMjANCiANCkBAIC00NTksNiArNDYwLDEyIEBADQogCSAg ZmVudHJ5W2Z1bmN0aW9uX2NvdW50XS5saW5lID0gaWk7DQogCSAgZmVudHJ5 W2Z1bmN0aW9uX2NvdW50XS5wYyA9IG9sZExpbmVUYi0+aXRlbVtpaV0ucGM7 DQogCSAgKytmdW5jdGlvbl9jb3VudDsNCisNCisJICAvKiBJZiB0aGUgZnVu Y3Rpb24gd2FzIGNvbXBpbGVkIHdpdGggWExDLCB3ZSBtYXkgaGF2ZSB0byBh ZGQgYW4NCisgICAgICAgICAgICAgZXh0cmEgbGluZSBlbnRyeSBsYXRlci4g IFJlc2VydmUgc3BhY2UgZm9yIHRoYXQuICAqLw0KKwkgIGlmIChpaSArIDEg PCBvbGRMaW5lVGItPm5pdGVtcw0KKwkgICAgICAmJiBvbGRMaW5lVGItPml0 ZW1baWldLnBjICE9IG9sZExpbmVUYi0+aXRlbVtpaSArIDFdLnBjKQ0KKwkg ICAgZXh0cmFfbGluZXMrKzsNCiAJfQ0KICAgICB9DQogDQpAQCAtNDc1LDcg KzQ4Miw3IEBADQogICBuZXdMaW5lVGIgPSAoc3RydWN0IGxpbmV0YWJsZSAq KQ0KICAgICB4bWFsbG9jDQogICAgIChzaXplb2YgKHN0cnVjdCBsaW5ldGFi bGUpICsNCi0gICAgKG9sZExpbmVUYi0+bml0ZW1zIC0gZnVuY3Rpb25fY291 bnQpICogc2l6ZW9mIChzdHJ1Y3QgbGluZXRhYmxlX2VudHJ5KSk7DQorICAg IChvbGRMaW5lVGItPm5pdGVtcyAtIGZ1bmN0aW9uX2NvdW50ICsgZXh0cmFf bGluZXMpICogc2l6ZW9mIChzdHJ1Y3QgbGluZXRhYmxlX2VudHJ5KSk7DQog DQogICAvKiBJZiBsaW5lIHRhYmxlIGRvZXMgbm90IHN0YXJ0IHdpdGggYSBm dW5jdGlvbiBiZWdpbm5pbmcsIGNvcHkgdXAgdW50aWwNCiAgICAgIGEgZnVu Y3Rpb24gYmVnaW4uICAqLw0KQEAgLTQ5MCwxMyArNDk3LDI2IEBADQogDQog ICBmb3IgKGlpID0gMDsgaWkgPCBmdW5jdGlvbl9jb3VudDsgKytpaSkNCiAg ICAgew0KKyAgICAgIC8qIElmIHRoZSBmdW5jdGlvbiB3YXMgY29tcGlsZWQg d2l0aCBYTEMsIHdlIG1heSBoYXZlIHRvIGFkZCBhbg0KKyAgICAgICAgIGV4 dHJhIGxpbmUgdG8gY292ZXIgdGhlIGZ1bmN0aW9uIHByb2xvZ3VlLiAgKi8N CisgICAgICBqaiA9IGZlbnRyeVtpaV0ubGluZTsNCisgICAgICBpZiAoamog KyAxIDwgb2xkTGluZVRiLT5uaXRlbXMNCisJICAmJiBvbGRMaW5lVGItPml0 ZW1bampdLnBjICE9IG9sZExpbmVUYi0+aXRlbVtqaiArIDFdLnBjKQ0KKwl7 DQorCSAgbmV3TGluZVRiLT5pdGVtW25ld2xpbmVdID0gb2xkTGluZVRiLT5p dGVtW2pqXTsNCisJICBuZXdMaW5lVGItPml0ZW1bbmV3bGluZV0ubGluZSA9 IG9sZExpbmVUYi0+aXRlbVtqaiArIDFdLmxpbmU7DQorCSAgbmV3bGluZSsr Ow0KKwl9DQorDQogICAgICAgZm9yIChqaiA9IGZlbnRyeVtpaV0ubGluZSAr IDE7DQogCSAgIGpqIDwgb2xkTGluZVRiLT5uaXRlbXMgJiYgb2xkTGluZVRi LT5pdGVtW2pqXS5saW5lICE9IDA7DQogCSAgICsramosICsrbmV3bGluZSkN CiAJbmV3TGluZVRiLT5pdGVtW25ld2xpbmVdID0gb2xkTGluZVRiLT5pdGVt W2pqXTsNCiAgICAgfQ0KICAgeGZyZWUgKGZlbnRyeSk7DQotICBuZXdMaW5l VGItPm5pdGVtcyA9IG9sZExpbmVUYi0+bml0ZW1zIC0gZnVuY3Rpb25fY291 bnQ7DQorICAvKiBUaGUgbnVtYmVyIG9mIGl0ZW1zIGluIHRoZSBsaW5lIHRh YmxlIG11c3QgaW5jbHVkZSB0aGVzZQ0KKyAgICAgZXh0cmEgbGluZXMgd2hp Y2ggd2VyZSBhZGRlZCBpbiBjYXNlIG9mIFhMQyBjb21waWxlZCBmdW5jdGlv bnMuICAqLw0KKyAgbmV3TGluZVRiLT5uaXRlbXMgPSBvbGRMaW5lVGItPm5p dGVtcyAtIGZ1bmN0aW9uX2NvdW50ICsgZXh0cmFfbGluZXM7DQogICByZXR1 cm4gbmV3TGluZVRiOw0KIH0NCiANCg== --0__=EABBF156DFBB6D6F8f9e8a93df938690918cEABBF156DFBB6D6F--