From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12678 invoked by alias); 20 Mar 2013 19:50:31 -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 12656 invoked by uid 89); 20 Mar 2013 19:50:21 -0000 X-Spam-SWARE-Status: No, score=-8.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BJ,TW_YM autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 20 Mar 2013 19:50:17 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2KJoGQX031263 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 20 Mar 2013 15:50:16 -0400 Received: from barimba (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r2KJoA36020628 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 20 Mar 2013 15:50:15 -0400 From: Tom Tromey To: Jan Kratochvil Cc: gdb-patches@sourceware.org Subject: Re: RFC: fix latent bug in syms_from_objfile_1 References: <87ip4m7wxy.fsf@fleche.redhat.com> <20130320184034.GA708@host2.jankratochvil.net> Date: Wed, 20 Mar 2013 19:53:00 -0000 In-Reply-To: <20130320184034.GA708@host2.jankratochvil.net> (Jan Kratochvil's message of "Wed, 20 Mar 2013 19:40:34 +0100") Message-ID: <87ip4l7swd.fsf@fleche.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-03/txt/msg00769.txt.bz2 >>>>> "Jan" == Jan Kratochvil writes: Jan> The problem is that alloc_section_addr_info sets NUM_SECTIONS while Jan> they are not yet filled in which leads to those ugly "&& Jan> ->other[x].name" checks... Thanks for the review and the hint. What do you think of this? It passes regtesting and removes all the name checks that I could find. Tom 2013-03-20 Tom Tromey * symfile.c (alloc_section_addr_info): Update header. Handle case where NUM_SECTIONS is 0. Don't set 'num_sections' field. (build_section_addr_info_from_section_table): Set 'num_sections'. (build_section_addr_info_from_bfd): Likewise. (build_section_addr_info_from_objfile): Remove dead loop condition. (free_section_addr_info): Unconditionally call xfree. (relative_addr_info_to_section_offsets, addrs_section_sort) (addr_info_make_relative, syms_from_objfile_1): Remove dead loop condition. (syms_from_objfile_1): Remove dead 'if' condition. Check 'num_sections'. (add_symbol_file_command): Set 'num_sections'. * symfile-mem.c (symbol_file_add_from_memory): Set 'num_sections'. * somread.c (som_symfile_offsets): Remove dead loop condition. * machoread.c (macho_symfile_offsets): Remove dead 'if'. * jit.c (jit_bfd_try_read_symtab): Set 'num_sections'. diff --git a/gdb/jit.c b/gdb/jit.c index ecf7317..1ea8e85 100644 --- a/gdb/jit.c +++ b/gdb/jit.c @@ -923,6 +923,7 @@ JITed symbol file is not an object file, ignoring it.\n")); sai->other[i].sectindex = sec->index; ++i; } + sai->num_sections = i; /* This call does not take ownership of SAI. */ make_cleanup_bfd_unref (nbfd); diff --git a/gdb/machoread.c b/gdb/machoread.c index fe77a2d..eff8bdf 100644 --- a/gdb/machoread.c +++ b/gdb/machoread.c @@ -991,9 +991,6 @@ macho_symfile_offsets (struct objfile *objfile, for (i = 0; i < addrs->num_sections; i++) { - if (addrs->other[i].name == NULL) - continue; - ALL_OBJFILE_OSECTIONS (objfile, osect) { const char *bfd_sect_name = osect->the_bfd_section->name; diff --git a/gdb/somread.c b/gdb/somread.c index d9d3e7b..0aa3dce 100644 --- a/gdb/somread.c +++ b/gdb/somread.c @@ -394,7 +394,7 @@ som_symfile_offsets (struct objfile *objfile, struct section_addr_info *addrs) /* Note: Here is OK to compare with ".text" because this is the name that gdb itself gives to that section, not the SOM name. */ - for (i = 0; i < addrs->num_sections && addrs->other[i].name; i++) + for (i = 0; i < addrs->num_sections; i++) if (strcmp (addrs->other[i].name, ".text") == 0) break; text_addr = addrs->other[i].addr; diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c index bb9bbd8..e148d09 100644 --- a/gdb/symfile-mem.c +++ b/gdb/symfile-mem.c @@ -127,6 +127,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name, sai->other[i].sectindex = sec->index; ++i; } + sai->num_sections = i; objf = symbol_file_add_from_bfd (nbfd, from_tty ? SYMFILE_VERBOSE : 0, sai, OBJF_SHARED, NULL); diff --git a/gdb/symfile.c b/gdb/symfile.c index 2abe3f8..2b9b817 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -197,7 +197,9 @@ find_lowest_section (bfd *abfd, asection *sect, void *obj) *lowest = sect; } -/* Create a new section_addr_info, with room for NUM_SECTIONS. */ +/* Create a new section_addr_info, with room for NUM_SECTIONS. The + new object's 'num_sections' field is set to 0; it must be updated + by the caller. */ struct section_addr_info * alloc_section_addr_info (size_t num_sections) @@ -205,11 +207,14 @@ alloc_section_addr_info (size_t num_sections) struct section_addr_info *sap; size_t size; + /* Make sure the size calculation turns out ok. */ + if (num_sections == 0) + ++num_sections; + size = (sizeof (struct section_addr_info) + sizeof (struct other_sections) * (num_sections - 1)); sap = (struct section_addr_info *) xmalloc (size); memset (sap, 0, size); - sap->num_sections = num_sections; return sap; } @@ -241,6 +246,8 @@ build_section_addr_info_from_section_table (const struct target_section *start, } } + sap->num_sections = oidx; + return sap; } @@ -262,6 +269,9 @@ build_section_addr_info_from_bfd (bfd *abfd) sap->other[i].sectindex = sec->index; i++; } + + sap->num_sections = i; + return sap; } @@ -277,7 +287,7 @@ build_section_addr_info_from_objfile (const struct objfile *objfile) gdb_assert (objfile->num_sections == bfd_count_sections (objfile->obfd)); */ sap = build_section_addr_info_from_bfd (objfile->obfd); - for (i = 0; i < sap->num_sections && sap->other[i].name; i++) + for (i = 0; i < sap->num_sections; i++) { int sectindex = sap->other[i].sectindex; @@ -294,8 +304,7 @@ free_section_addr_info (struct section_addr_info *sap) int idx; for (idx = 0; idx < sap->num_sections; idx++) - if (sap->other[idx].name) - xfree (sap->other[idx].name); + xfree (sap->other[idx].name); xfree (sap); } @@ -446,7 +455,7 @@ relative_addr_info_to_section_offsets (struct section_offsets *section_offsets, memset (section_offsets, 0, SIZEOF_N_SECTION_OFFSETS (num_sections)); /* Now calculate offsets for section that were specified by the caller. */ - for (i = 0; i < addrs->num_sections && addrs->other[i].name; i++) + for (i = 0; i < addrs->num_sections; i++) { struct other_sections *osp; @@ -506,7 +515,7 @@ addrs_section_sort (struct section_addr_info *addrs) /* `+ 1' for the NULL terminator. */ array = xmalloc (sizeof (*array) * (addrs->num_sections + 1)); - for (i = 0; i < addrs->num_sections && addrs->other[i].name; i++) + for (i = 0; i < addrs->num_sections; i++) array[i] = &addrs->other[i]; array[i] = NULL; @@ -605,7 +614,7 @@ addr_info_make_relative (struct section_addr_info *addrs, bfd *abfd) (the loadable section directly below it in memory). this_offset = lower_offset = lower_addr - lower_orig_addr */ - for (i = 0; i < addrs->num_sections && addrs->other[i].name; i++) + for (i = 0; i < addrs->num_sections; i++) { struct other_sections *sect = addrs_to_abfd_addrs[i]; @@ -997,7 +1006,7 @@ syms_from_objfile_1 (struct objfile *objfile, We no longer warn if the lowest section is not a text segment (as happens for the PA64 port. */ - if (addrs && addrs->other[0].name) + if (addrs && addrs->num_sections > 0) addr_info_make_relative (addrs, objfile->obfd); /* Initialize symbol reading routines for this objfile, allow complaints to @@ -2341,6 +2350,7 @@ add_symbol_file_command (char *args, int from_tty) At this point, we don't know what file type this is, so we can't determine what section names are valid. */ } + section_addrs->num_sections = sec_num; if (from_tty && (!query ("%s", ""))) error (_("Not confirmed."));