From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3051 invoked by alias); 13 Jan 2005 23:12:13 -0000 Mailing-List: contact binutils-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sources.redhat.com Received: (qmail 3014 invoked from network); 13 Jan 2005 23:12:03 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 13 Jan 2005 23:12:03 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id j0DNBwIX013988 for ; Thu, 13 Jan 2005 18:12:03 -0500 Received: from localhost (mail@vpn50-41.rdu.redhat.com [172.16.50.41]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id j0DNBvr05557 for ; Thu, 13 Jan 2005 18:11:57 -0500 Received: from rsandifo by localhost with local (Exim 3.35 #1) id 1CpE8C-0003lj-00 for binutils@sources.redhat.com; Thu, 13 Jan 2005 23:11:56 +0000 To: binutils@sources.redhat.com Subject: Follow-up to November's .eh_frame optimisations (1/3) From: Richard Sandiford Date: Thu, 13 Jan 2005 23:12:00 -0000 Message-ID: <877jmg3ow3.fsf@redhat.com> User-Agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2005-01/txt/msg00133.txt.bz2 A couple of months ago, I submitted a patch to extend the linker .eh_frame optimisations: http://sources.redhat.com/ml/binutils/2004-11/msg00226.html The idea was to change FDEs from an absolute to a PC-relative encoding in cases where the original CIEs had no 'R' augmentation. Doing that increased the size of the CIEs and FDEs (because of the new augmentation info) but reduced the number of relocs needed. At the time, I promised to extend the patch so that it would only grow the CIEs and FDEs if there wasn't enough padding to hold the new data. I've finally got around to doing that. The full patch needs to parse the CFA instructions, so that means more sanity checks in _bfd_elf_discard_section_eh_frame. Most of the existing tests have the form: if (something_that_shouldn't_happen) goto free_no_table; but for one particular type of test, the goto is wrapped up in an assert-like macro called ENSURE_NO_RELOCS(). I tried just using gotos for the new checks, but I thought it made the code quite a bit harder to follow. Things seemed much more readable when using assert-like statements instead. This first patch therefore paves the way by adding a new assert-like macro called REQUIRE() and, for consistency, uses it for the existing goto-based tests. Tested on mips64-linux-gnu (gas, ld and binutils) and with a gcc bootstrap and regression test on mips-sgi-irix6.5. OK to install? Richard * elf-eh-frame.c (_bfd_elf_discard_section_eh_frame): Use an assert-style REQUIRE() macro to handle sanity checks. *** bfd/elf-eh-frame.c.1 2005-01-07 10:36:19.000000000 +0000 --- bfd/elf-eh-frame.c 2005-01-07 10:44:35.000000000 +0000 *************** _bfd_elf_discard_section_eh_frame *** 265,270 **** --- 265,276 ---- bfd_boolean (*reloc_symbol_deleted_p) (bfd_vma, void *), struct elf_reloc_cookie *cookie) { + #define REQUIRE(COND) \ + do \ + if (!(COND)) \ + goto free_no_table; \ + while (0) + bfd_byte *ehbuf = NULL, *buf; bfd_byte *last_cie, *last_fde; struct eh_cie_fde *ent, *last_cie_inf, *this_inf; *************** _bfd_elf_discard_section_eh_frame *** 296,303 **** /* Read the frame unwind information from abfd. */ ! if (!bfd_malloc_and_get_section (abfd, sec, &ehbuf)) ! goto free_no_table; if (sec->size >= 4 && bfd_get_32 (abfd, ehbuf) == 0 --- 302,308 ---- /* Read the frame unwind information from abfd. */ ! REQUIRE (bfd_malloc_and_get_section (abfd, sec, &ehbuf)); if (sec->size >= 4 && bfd_get_32 (abfd, ehbuf) == 0 *************** _bfd_elf_discard_section_eh_frame *** 310,317 **** /* If .eh_frame section size doesn't fit into int, we cannot handle it (it would need to use 64-bit .eh_frame format anyway). */ ! if (sec->size != (unsigned int) sec->size) ! goto free_no_table; ptr_size = (elf_elfheader (abfd)->e_ident[EI_CLASS] == ELFCLASS64) ? 8 : 4; --- 315,321 ---- /* If .eh_frame section size doesn't fit into int, we cannot handle it (it would need to use 64-bit .eh_frame format anyway). */ ! REQUIRE (sec->size == (unsigned int) sec->size); ptr_size = (elf_elfheader (abfd)->e_ident[EI_CLASS] == ELFCLASS64) ? 8 : 4; *************** _bfd_elf_discard_section_eh_frame *** 322,338 **** cie_usage_count = 0; sec_info = bfd_zmalloc (sizeof (struct eh_frame_sec_info) + 99 * sizeof (struct eh_cie_fde)); ! if (sec_info == NULL) ! goto free_no_table; sec_info->alloced = 100; #define ENSURE_NO_RELOCS(buf) \ ! if (cookie->rel < cookie->relend \ ! && (cookie->rel->r_offset \ ! < (bfd_size_type) ((buf) - ehbuf)) \ ! && cookie->rel->r_info != 0) \ ! goto free_no_table #define SKIP_RELOCS(buf) \ while (cookie->rel < cookie->relend \ --- 326,340 ---- cie_usage_count = 0; sec_info = bfd_zmalloc (sizeof (struct eh_frame_sec_info) + 99 * sizeof (struct eh_cie_fde)); ! REQUIRE (sec_info); sec_info->alloced = 100; #define ENSURE_NO_RELOCS(buf) \ ! REQUIRE (!(cookie->rel < cookie->relend \ ! && (cookie->rel->r_offset \ ! < (bfd_size_type) ((buf) - ehbuf)) \ ! && cookie->rel->r_info != 0)) #define SKIP_RELOCS(buf) \ while (cookie->rel < cookie->relend \ *************** _bfd_elf_discard_section_eh_frame *** 357,364 **** sizeof (struct eh_frame_sec_info) + ((sec_info->alloced + 99) * sizeof (struct eh_cie_fde))); ! if (sec_info == NULL) ! goto free_no_table; memset (&sec_info->entry[sec_info->alloced], 0, 100 * sizeof (struct eh_cie_fde)); --- 359,365 ---- sizeof (struct eh_frame_sec_info) + ((sec_info->alloced + 99) * sizeof (struct eh_cie_fde))); ! REQUIRE (sec_info); memset (&sec_info->entry[sec_info->alloced], 0, 100 * sizeof (struct eh_cie_fde)); *************** _bfd_elf_discard_section_eh_frame *** 378,404 **** hdr.id = (unsigned int) -1; else { ! if ((bfd_size_type) (buf + 4 - ehbuf) > sec->size) ! /* No space for CIE/FDE header length. */ ! goto free_no_table; ! hdr.length = bfd_get_32 (abfd, buf); - if (hdr.length == 0xffffffff) - /* 64-bit .eh_frame is not supported. */ - goto free_no_table; buf += 4; ! if ((bfd_size_type) (buf - ehbuf) + hdr.length > sec->size) ! /* CIE/FDE not contained fully in this .eh_frame input section. */ ! goto free_no_table; this_inf->offset = last_fde - ehbuf; this_inf->size = 4 + hdr.length; if (hdr.length == 0) { ! /* CIE with length 0 must be only the last in the section. */ ! if ((bfd_size_type) (buf - ehbuf) < sec->size) ! goto free_no_table; ENSURE_NO_RELOCS (buf); sec_info->count++; /* Now just finish last encountered CIE processing and break --- 379,403 ---- hdr.id = (unsigned int) -1; else { ! /* Read the length of the entry. */ ! REQUIRE ((bfd_size_type) (buf - ehbuf) + 4 <= sec->size); hdr.length = bfd_get_32 (abfd, buf); buf += 4; ! ! /* 64-bit .eh_frame is not supported. */ ! REQUIRE (hdr.length != 0xffffffff); ! ! /* The CIE/FDE must be fully contained in this input section. */ ! REQUIRE ((bfd_size_type) (buf - ehbuf) + hdr.length <= sec->size); this_inf->offset = last_fde - ehbuf; this_inf->size = 4 + hdr.length; if (hdr.length == 0) { ! /* A zero-length CIE should only be found at the end of ! the section. */ ! REQUIRE ((bfd_size_type) (buf - ehbuf) == sec->size); ENSURE_NO_RELOCS (buf); sec_info->count++; /* Now just finish last encountered CIE processing and break *************** _bfd_elf_discard_section_eh_frame *** 409,416 **** { hdr.id = bfd_get_32 (abfd, buf); buf += 4; ! if (hdr.id == (unsigned int) -1) ! goto free_no_table; } } --- 408,414 ---- { hdr.id = bfd_get_32 (abfd, buf); buf += 4; ! REQUIRE (hdr.id != (unsigned int) -1); } } *************** _bfd_elf_discard_section_eh_frame *** 455,464 **** cie.version = *buf++; /* Cannot handle unknown versions. */ ! if (cie.version != 1 && cie.version != 3) ! goto free_no_table; ! if (strlen (buf) > sizeof (cie.augmentation) - 1) ! goto free_no_table; strcpy (cie.augmentation, buf); buf = strchr (buf, '\0') + 1; --- 453,460 ---- cie.version = *buf++; /* Cannot handle unknown versions. */ ! REQUIRE (cie.version == 1 || cie.version == 3); ! REQUIRE (strlen (buf) < sizeof (cie.augmentation)); strcpy (cie.augmentation, buf); buf = strchr (buf, '\0') + 1; *************** _bfd_elf_discard_section_eh_frame *** 498,511 **** case 'L': cie.lsda_encoding = *buf++; ENSURE_NO_RELOCS (buf); ! if (get_DW_EH_PE_width (cie.lsda_encoding, ptr_size) == 0) ! goto free_no_table; break; case 'R': cie.fde_encoding = *buf++; ENSURE_NO_RELOCS (buf); ! if (get_DW_EH_PE_width (cie.fde_encoding, ptr_size) == 0) ! goto free_no_table; break; case 'P': { --- 494,505 ---- case 'L': cie.lsda_encoding = *buf++; ENSURE_NO_RELOCS (buf); ! REQUIRE (get_DW_EH_PE_width (cie.lsda_encoding, ptr_size)); break; case 'R': cie.fde_encoding = *buf++; ENSURE_NO_RELOCS (buf); ! REQUIRE (get_DW_EH_PE_width (cie.fde_encoding, ptr_size)); break; case 'P': { *************** _bfd_elf_discard_section_eh_frame *** 514,521 **** cie.per_encoding = *buf++; per_width = get_DW_EH_PE_width (cie.per_encoding, ptr_size); ! if (per_width == 0) ! goto free_no_table; if ((cie.per_encoding & 0xf0) == DW_EH_PE_aligned) buf = (ehbuf + ((buf - ehbuf + per_width - 1) --- 508,514 ---- cie.per_encoding = *buf++; per_width = get_DW_EH_PE_width (cie.per_encoding, ptr_size); ! REQUIRE (per_width); if ((cie.per_encoding & 0xf0) == DW_EH_PE_aligned) buf = (ehbuf + ((buf - ehbuf + per_width - 1) *************** _bfd_elf_discard_section_eh_frame *** 608,621 **** else { /* Ensure this FDE uses the last CIE encountered. */ ! if (last_cie == NULL ! || hdr.id != (unsigned int) (buf - 4 - last_cie)) ! goto free_no_table; ENSURE_NO_RELOCS (buf); ! if (GET_RELOC (buf) == NULL) ! /* This should not happen. */ ! goto free_no_table; if ((*reloc_symbol_deleted_p) (buf - ehbuf, cookie)) /* This is a FDE against a discarded section. It should --- 601,611 ---- else { /* Ensure this FDE uses the last CIE encountered. */ ! REQUIRE (last_cie); ! REQUIRE (hdr.id == (unsigned int) (buf - 4 - last_cie)); ENSURE_NO_RELOCS (buf); ! REQUIRE (GET_RELOC (buf)); if ((*reloc_symbol_deleted_p) (buf - ehbuf, cookie)) /* This is a FDE against a discarded section. It should *************** free_no_table: *** 693,698 **** --- 683,690 ---- hdr_info->table = FALSE; hdr_info->last_cie.hdr.length = 0; return FALSE; + + #undef REQUIRE } /* This function is called for .eh_frame_hdr section after