From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2068 invoked by alias); 17 Apr 2013 08:10:16 -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 2052 invoked by uid 89); 17 Apr 2013 08:10:15 -0000 X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_TRUST,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD,TW_BJ,TW_XZ autolearn=ham version=3.3.1 Received: from mail-bk0-f73.google.com (HELO mail-bk0-f73.google.com) (209.85.214.73) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 17 Apr 2013 08:10:14 +0000 Received: by mail-bk0-f73.google.com with SMTP id q16so97198bkw.4 for ; Wed, 17 Apr 2013 01:10:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:date:message-id:from:to:subject:x-gm-message-state; bh=yqN4IGBgGrzlrfprvqAmct5PFUOk7uqU0sRaj0wdYFI=; b=EIbuQOnfhzK1YR+GW5uuiACZOLNb5SWSsFqGYGvilDV0T3OG5pTzFOKuHp1HgoY2km eBM2wdEAmRy/o6ZxLtf9Og3SceddHHHqz/sqczbfFGbKidLkWOBAELCynDy1CLh6M7lg 1cDZqra0lAvlPgYuJiLp3BUXVRHs2YNOpJc+1yNZI6EYZToICva0n/rxHfi6KRUjzqdu Nj2W4uDmhc9Xfhi5KkACufPWw74rUs0nu4wB9CZ72KN1di/pNk5rhLwqxUEfF0EeCX3F K4EOWilOl+mQo2qeG8Vmqm/ewsPm9ez4FW6M0+0j5i0MzYnWn6JL9jBTLWVXeTaIsqMN fvHQ== X-Received: by 10.15.110.201 with SMTP id ch49mr6678048eeb.2.1366186212166; Wed, 17 Apr 2013 01:10:12 -0700 (PDT) Received: from corp2gmr1-1.eem.corp.google.com (corp2gmr1-1.eem.corp.google.com [172.25.138.99]) by gmr-mx.google.com with ESMTPS id h48si1259232eeu.0.2013.04.17.01.10.12 (version=TLSv1.1 cipher=AES128-SHA bits=128/128); Wed, 17 Apr 2013 01:10:12 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com (ruffy2.mtv.corp.google.com [172.17.128.107]) by corp2gmr1-1.eem.corp.google.com (Postfix) with ESMTP id 263661CA121; Wed, 17 Apr 2013 01:10:10 -0700 (PDT) Date: Wed, 17 Apr 2013 14:55:00 -0000 Message-Id: From: Doug Evans To: gdb-patches@sourceware.org, tromey@redhat.com Subject: [RFC] gdb_bfd_count_sections snafu X-Gm-Message-State: ALoCoQkDx+D9a5MBmvTSo0FYkUdGpOjqrAKgnthJW5K00Act9mvS4uXqsYs9up39CLDF9O/qeyzBLcuPguogaA9PK0kgNAiHmAM6IpxU4VJoC7zImO0AcUYsfE+jEzrDU4sL2reykK4SUxAcCACrjqAsnIe2nRJo49UHv2S0cSvhwl0xdP3+8aT7ibXuowcOKXDh1UofiEq2g3zsNt6hrcx/Z338E5e7Ww== X-SW-Source: 2013-04/txt/msg00522.txt.bz2 Hi. I was getting different failures for jit.exp depending on whether gdb was compiled with/without optimization. That drove me to dig deeper. It turned out that the objfile->msymbols array was being sorted differently because several symbols like _DYNAMIC had been given different addresses depending on whether gdb was compiled with -O0 or -O2. [And if symbols moved in the resort then jit.c's cached minsyms would get clobbered - but that's another bug. :-)] After a bit more digging I found an out of bounds array access in the section offset computation, and the offset was coming from the stack, so that explained the difference I saw. _DYNAMIC lives in the special bfd abs section, and thus has a special section number computed by gdb_bfd.c:gdb_bfd_section_index. In my case bfd_count_sections was 32 and the section number for _DYNAMIC was 35. int gdb_bfd_section_index (bfd *abfd, asection *section) { if (section == NULL) return -1; else if (section == bfd_com_section_ptr) return bfd_count_sections (abfd) + 1; else if (section == bfd_und_section_ptr) return bfd_count_sections (abfd) + 2; else if (section == bfd_abs_section_ptr) return bfd_count_sections (abfd) + 3; else if (section == bfd_ind_section_ptr) return bfd_count_sections (abfd) + 4; return section->index; } There is a wrapper around bfd_count_sections to include these extra sections: gdb_bfd.c:gdb_bfd_count_sections. int gdb_bfd_count_sections (bfd *abfd) { return bfd_count_sections (abfd) + 4; } HOWEVER, objfile->num_sections is computed with bfd_count_sections not gdb_bfd_count_sections. I didn't dig deeper as I'm not familiar with all the file formats, there are more calls to bfd_count_sections that perhaps should be gdb_bfd_count_sections. This patch is at least, I think(!), a step in the right direction. There's some cleanups I'd like to do but I'll send those separately. This patch does clean up one thing: AFAICT when syms_from_objfile_1 is passed NULL for both addrs and offsets, there's no point in building local_addr to have more than one entry (zero would be fine too I think but space needs to be allocated for at least one entry). if (! addrs && ! offsets) { - local_addr - = alloc_section_addr_info (bfd_count_sections (objfile->obfd)); + local_addr = alloc_section_addr_info (1); make_cleanup (xfree, local_addr); addrs = local_addr; } Anyways, here's the patch. It fixes the jit.exp failure (but alas covers up another bug - the minsym caching bug I reported earlier). Regression tested on amd64-linux. [I'm a bit concerned going forward. I think we need to put in some time to at least add more comments to alert the reader. I can imagine one not always being clear when to use bfd_count_sections and gdb_bfd_count_sections without such comments. If you choose the wrong one the failure could be another bug like this. My preference would be code changes to remove the possibility for picking the wrong one, but that would, I think, be non-trivial. Something for another day unless one wants to take it on now. In the meantime we should at least alert the reader.] 2013-04-17 Doug Evans * objfiles.c (objfile_relocate): Use gdb_bfd_count_sections instead of bfd_count_sections. * solib-target.c (solib_target_relocate_section_addresses): Ditto. * symfile.c (default_symfile_offsets): Ditto. (syms_from_objfile_1): Ditto. Make dummy addrs list an array of one entry, not bfd_count_sections entries. Index: objfiles.c =================================================================== RCS file: /cvs/src/src/gdb/objfiles.c,v retrieving revision 1.158 diff -u -p -r1.158 objfiles.c --- objfiles.c 8 Apr 2013 20:04:42 -0000 1.158 +++ objfiles.c 17 Apr 2013 07:07:07 -0000 @@ -880,7 +880,7 @@ objfile_relocate (struct objfile *objfil addr_info_make_relative (objfile_addrs, debug_objfile->obfd); gdb_assert (debug_objfile->num_sections - == bfd_count_sections (debug_objfile->obfd)); + == gdb_bfd_count_sections (debug_objfile->obfd)); new_debug_offsets = xmalloc (SIZEOF_N_SECTION_OFFSETS (debug_objfile->num_sections)); make_cleanup (xfree, new_debug_offsets); Index: solib-target.c =================================================================== RCS file: /cvs/src/src/gdb/solib-target.c,v retrieving revision 1.28 diff -u -p -r1.28 solib-target.c --- solib-target.c 8 Apr 2013 20:04:42 -0000 1.28 +++ solib-target.c 17 Apr 2013 07:07:07 -0000 @@ -339,7 +339,7 @@ solib_target_relocate_section_addresses it any earlier, since we need to open the file first. */ if (so->lm_info->offsets == NULL) { - int num_sections = bfd_count_sections (so->abfd); + int num_sections = gdb_bfd_count_sections (so->abfd); so->lm_info->offsets = xzalloc (SIZEOF_N_SECTION_OFFSETS (num_sections)); Index: symfile.c =================================================================== RCS file: /cvs/src/src/gdb/symfile.c,v retrieving revision 1.370 diff -u -p -r1.370 symfile.c --- symfile.c 8 Apr 2013 20:04:42 -0000 1.370 +++ symfile.c 17 Apr 2013 07:07:07 -0000 @@ -678,7 +678,7 @@ void default_symfile_offsets (struct objfile *objfile, struct section_addr_info *addrs) { - objfile->num_sections = bfd_count_sections (objfile->obfd); + objfile->num_sections = gdb_bfd_count_sections (objfile->obfd); objfile->section_offsets = (struct section_offsets *) obstack_alloc (&objfile->objfile_obstack, SIZEOF_N_SECTION_OFFSETS (objfile->num_sections)); @@ -948,7 +948,7 @@ syms_from_objfile_1 (struct objfile *obj { /* No symbols to load, but we still need to make sure that the section_offsets table is allocated. */ - int num_sections = bfd_count_sections (objfile->obfd); + int num_sections = gdb_bfd_count_sections (objfile->obfd); size_t size = SIZEOF_N_SECTION_OFFSETS (num_offsets); objfile->num_sections = num_sections; @@ -967,8 +967,7 @@ syms_from_objfile_1 (struct objfile *obj no load address was specified. */ if (! addrs && ! offsets) { - local_addr - = alloc_section_addr_info (bfd_count_sections (objfile->obfd)); + local_addr = alloc_section_addr_info (1); make_cleanup (xfree, local_addr); addrs = local_addr; }