This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Follow-up to November's .eh_frame optimisations (1/3)
- From: Richard Sandiford <rsandifo at redhat dot com>
- To: binutils at sources dot redhat dot com
- Date: Thu, 13 Jan 2005 23:11:56 +0000
- Subject: Follow-up to November's .eh_frame optimisations (1/3)
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