From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8307 invoked by alias); 13 Jan 2005 23:20:54 -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 7846 invoked from network); 13 Jan 2005 23:20:16 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 13 Jan 2005 23:20:16 -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 j0DNKGCP015748 for ; Thu, 13 Jan 2005 18:20:16 -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 j0DNKFr08079 for ; Thu, 13 Jan 2005 18:20:15 -0500 Received: from rsandifo by localhost with local (Exim 3.35 #1) id 1CpEGE-0003lt-00 for binutils@sources.redhat.com; Thu, 13 Jan 2005 23:20:14 +0000 To: binutils@sources.redhat.com Subject: Follow-up to November's .eh_frame optimisations (2/3) References: <877jmg3ow3.fsf@redhat.com> From: Richard Sandiford Date: Thu, 13 Jan 2005 23:20:00 -0000 In-Reply-To: <877jmg3ow3.fsf@redhat.com> (Richard Sandiford's message of "Thu, 13 Jan 2005 23:11:56 +0000") Message-ID: <873bx43oi9.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/msg00134.txt.bz2 This patch follows on from: http://sources.redhat.com/ml/binutils/2005-01/msg00133.html and is the second of the three. The final patch will need to interpret CFA instructions, some of which have variable length fields, and some of which contain leb128s, which themselves have a variable length. So, the question was, should we just assume that these instructions are valid, or should we take care not to read past the end of the CIE/FDE? I thought it would be better to do the latter. _bfd_elf_discard_section_eh_frame already does many bounds checks, such as making sure that the reported CIE length is within the bounds of the input section. However, once it's tested that, it doesn't usually check whether there are N bytes left in the CIE/FDE before doing "buf += N", or whether an leb128 actually fits within the CIE/FDE. Since the final optimisation needs to do both these things, the patch below adds some helper routines and converts the existing code to use them. I guess this might seem a teensy bit anal ;), but it should make things more robust in the face of true garbage, should anyone care. Tested in the same way as the first patch. OK to install? (BTW, there's a potential conflict with the patch that H.J. posted yesterday, which moved read_*_leb128 to libbfd.c. It should be easy enough to adapt this patch if his goes in first.) Richard * elf-bfd.h (struct cie): Use bfd_vmas for code_align, ra_column and augmentation_size. Use bfd_signed_vmas for data_align. * elf-eh-frame.c (read_unsigned_leb128, read_signed_leb128) (read_uleb128, read_sleb128): Delete in favor of... (read_byte, skip_leb128, read_uleb128, read_sleb128): ...these new functions. Don't read past the end of the enclosing CIE or FDE. (skip_bytes): New utility function. (_bfd_elf_discard_section_eh_frame): Use new functions, adding more sanity checking. (_bfd_elf_write_section_eh_frame): Use new functions. Index: bfd/elf-bfd.h =================================================================== RCS file: /cvs/src/src/bfd/elf-bfd.h,v retrieving revision 1.166 diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.166 elf-bfd.h *** bfd/elf-bfd.h 10 Dec 2004 14:04:56 -0000 1.166 --- bfd/elf-bfd.h 10 Jan 2005 13:09:50 -0000 *************** struct cie *** 270,279 **** struct cie_header hdr; unsigned char version; unsigned char augmentation[20]; ! unsigned int code_align; ! int data_align; ! unsigned int ra_column; ! unsigned int augmentation_size; struct elf_link_hash_entry *personality; unsigned char per_encoding; unsigned char lsda_encoding; --- 270,279 ---- struct cie_header hdr; unsigned char version; unsigned char augmentation[20]; ! bfd_vma code_align; ! bfd_signed_vma data_align; ! bfd_vma ra_column; ! bfd_vma augmentation_size; struct elf_link_hash_entry *personality; unsigned char per_encoding; unsigned char lsda_encoding; *** bfd/elf-eh-frame.c.2 2005-01-07 10:44:35.000000000 +0000 --- bfd/elf-eh-frame.c 2005-01-07 11:12:01.423299862 +0000 *************** *** 26,104 **** #define EH_FRAME_HDR_SIZE 8 ! /* Helper function for reading uleb128 encoded data. */ ! static bfd_vma ! read_unsigned_leb128 (bfd *abfd ATTRIBUTE_UNUSED, ! char *buf, ! unsigned int *bytes_read_ptr) { ! bfd_vma result; ! unsigned int num_read; ! int shift; ! unsigned char byte; ! result = 0; ! shift = 0; ! num_read = 0; ! do { ! byte = bfd_get_8 (abfd, (bfd_byte *) buf); ! buf++; ! num_read++; ! result |= (((bfd_vma) byte & 0x7f) << shift); ! shift += 7; } ! while (byte & 0x80); ! *bytes_read_ptr = num_read; ! return result; } ! /* Helper function for reading sleb128 encoded data. */ ! static bfd_signed_vma ! read_signed_leb128 (bfd *abfd ATTRIBUTE_UNUSED, ! char *buf, ! unsigned int * bytes_read_ptr) { - bfd_vma result; - int shift; - int num_read; unsigned char byte; - - result = 0; - shift = 0; - num_read = 0; do ! { ! byte = bfd_get_8 (abfd, (bfd_byte *) buf); ! buf ++; ! num_read ++; ! result |= (((bfd_vma) byte & 0x7f) << shift); ! shift += 7; ! } while (byte & 0x80); ! if (byte & 0x40) ! result |= (((bfd_vma) -1) << (shift - 7)) << 7; ! *bytes_read_ptr = num_read; ! return result; } ! #define read_uleb128(VAR, BUF) \ ! do \ ! { \ ! (VAR) = read_unsigned_leb128 (abfd, buf, &leb128_tmp); \ ! (BUF) += leb128_tmp; \ ! } \ ! while (0) ! #define read_sleb128(VAR, BUF) \ ! do \ ! { \ ! (VAR) = read_signed_leb128 (abfd, buf, &leb128_tmp); \ ! (BUF) += leb128_tmp; \ ! } \ ! while (0) /* Return 0 if either encoding is variable width, or not yet known to bfd. */ --- 26,110 ---- #define EH_FRAME_HDR_SIZE 8 ! /* If *ITER hasn't reached END yet, read the next byte into *RESULT and ! move onto the next byte. Return true on success. */ ! static inline bfd_boolean ! read_byte (bfd_byte **iter, bfd_byte *end, unsigned char *result) { ! if (*iter >= end) ! return FALSE; ! *result = *((*iter)++); ! return TRUE; ! } ! /* Move *ITER over LENGTH bytes, or up to END, whichever is closer. ! Return true it was possible to move LENGTH bytes. */ ! ! static inline bfd_boolean ! skip_bytes (bfd_byte **iter, bfd_byte *end, bfd_size_type length) ! { ! if ((bfd_size_type) (end - *iter) < length) { ! *iter = end; ! return FALSE; } ! *iter += length; ! return TRUE; } ! /* Move *ITER over an leb128, stopping at END. Return true if the end ! of the leb128 was found. */ ! static bfd_boolean ! skip_leb128 (bfd_byte **iter, bfd_byte *end) { unsigned char byte; do ! if (!read_byte (iter, end, &byte)) ! return FALSE; while (byte & 0x80); ! return TRUE; } ! /* Like skip_leb128, but treat the leb128 as an unsigned value and ! store it in *VALUE. */ ! static bfd_boolean ! read_uleb128 (bfd_byte **iter, bfd_byte *end, bfd_vma *value) ! { ! bfd_byte *start, *p; ! ! start = *iter; ! if (!skip_leb128 (iter, end)) ! return FALSE; ! ! p = *iter; ! *value = *--p; ! while (p > start) ! *value = (*value << 7) | (*--p & 0x7f); ! ! return TRUE; ! } ! ! /* Like read_uleb128, but for signed values. */ ! ! static bfd_boolean ! read_sleb128 (bfd_byte **iter, bfd_byte *end, bfd_signed_vma *value) ! { ! bfd_byte *start, *p; ! ! start = *iter; ! if (!skip_leb128 (iter, end)) ! return FALSE; ! ! p = *iter; ! *value = ((*--p & 0x7f) ^ 0x40) - 0x40; ! while (p > start) ! *value = (*value << 7) | (*--p & 0x7f); ! ! return TRUE; ! } /* Return 0 if either encoding is variable width, or not yet known to bfd. */ *************** _bfd_elf_discard_section_eh_frame *** 279,285 **** struct elf_link_hash_table *htab; struct eh_frame_hdr_info *hdr_info; struct eh_frame_sec_info *sec_info = NULL; - unsigned int leb128_tmp; unsigned int cie_usage_count, offset; unsigned int ptr_size; --- 285,290 ---- *************** _bfd_elf_discard_section_eh_frame *** 351,356 **** --- 356,363 ---- for (;;) { unsigned char *aug; + bfd_byte *start, *end; + bfd_size_type length; if (sec_info->count == sec_info->alloced) { *************** _bfd_elf_discard_section_eh_frame *** 376,394 **** /* If we are at the end of the section, we still need to decide on whether to output or discard last encountered CIE (if any). */ if ((bfd_size_type) (buf - ehbuf) == sec->size) ! 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; --- 383,404 ---- /* If we are at the end of the section, we still need to decide on whether to output or discard last encountered CIE (if any). */ if ((bfd_size_type) (buf - ehbuf) == sec->size) ! { ! hdr.id = (unsigned int) -1; ! end = buf; ! } else { /* Read the length of the entry. */ ! REQUIRE (skip_bytes (&buf, ehbuf + sec->size, 4)); ! hdr.length = bfd_get_32 (abfd, 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); + end = buf + hdr.length; this_inf->offset = last_fde - ehbuf; this_inf->size = 4 + hdr.length; *************** _bfd_elf_discard_section_eh_frame *** 406,413 **** } else { ! hdr.id = bfd_get_32 (abfd, buf); ! buf += 4; REQUIRE (hdr.id != (unsigned int) -1); } } --- 416,423 ---- } else { ! REQUIRE (skip_bytes (&buf, end, 4)); ! hdr.id = bfd_get_32 (abfd, buf - 4); REQUIRE (hdr.id != (unsigned int) -1); } } *************** _bfd_elf_discard_section_eh_frame *** 450,456 **** cie_usage_count = 0; memset (&cie, 0, sizeof (cie)); cie.hdr = hdr; ! cie.version = *buf++; /* Cannot handle unknown versions. */ REQUIRE (cie.version == 1 || cie.version == 3); --- 460,466 ---- cie_usage_count = 0; memset (&cie, 0, sizeof (cie)); cie.hdr = hdr; ! REQUIRE (read_byte (&buf, end, &cie.version)); /* Cannot handle unknown versions. */ REQUIRE (cie.version == 1 || cie.version == 3); *************** _bfd_elf_discard_section_eh_frame *** 465,479 **** /* We cannot merge "eh" CIEs because __EXCEPTION_TABLE__ is private to each CIE, so we don't need it for anything. Just skip it. */ ! buf += ptr_size; SKIP_RELOCS (buf); } ! read_uleb128 (cie.code_align, buf); ! read_sleb128 (cie.data_align, buf); if (cie.version == 1) ! cie.ra_column = *buf++; else ! read_uleb128 (cie.ra_column, buf); ENSURE_NO_RELOCS (buf); cie.lsda_encoding = DW_EH_PE_omit; cie.fde_encoding = DW_EH_PE_omit; --- 475,492 ---- /* We cannot merge "eh" CIEs because __EXCEPTION_TABLE__ is private to each CIE, so we don't need it for anything. Just skip it. */ ! REQUIRE (skip_bytes (&buf, end, ptr_size)); SKIP_RELOCS (buf); } ! REQUIRE (read_uleb128 (&buf, end, &cie.code_align)); ! REQUIRE (read_sleb128 (&buf, end, &cie.data_align)); if (cie.version == 1) ! { ! REQUIRE (buf < end); ! cie.ra_column = *buf++; ! } else ! REQUIRE (read_uleb128 (&buf, end, &cie.ra_column)); ENSURE_NO_RELOCS (buf); cie.lsda_encoding = DW_EH_PE_omit; cie.fde_encoding = DW_EH_PE_omit; *************** _bfd_elf_discard_section_eh_frame *** 484,490 **** if (*aug == 'z') { aug++; ! read_uleb128 (cie.augmentation_size, buf); ENSURE_NO_RELOCS (buf); } --- 497,503 ---- if (*aug == 'z') { aug++; ! REQUIRE (read_uleb128 (&buf, end, &cie.augmentation_size)); ENSURE_NO_RELOCS (buf); } *************** _bfd_elf_discard_section_eh_frame *** 492,503 **** switch (*aug++) { 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; --- 505,516 ---- switch (*aug++) { case 'L': ! REQUIRE (read_byte (&buf, end, &cie.lsda_encoding)); ENSURE_NO_RELOCS (buf); REQUIRE (get_DW_EH_PE_width (cie.lsda_encoding, ptr_size)); break; case 'R': ! REQUIRE (read_byte (&buf, end, &cie.fde_encoding)); ENSURE_NO_RELOCS (buf); REQUIRE (get_DW_EH_PE_width (cie.fde_encoding, ptr_size)); break; *************** _bfd_elf_discard_section_eh_frame *** 505,518 **** { int per_width; ! 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_size_type) per_width - 1))); ENSURE_NO_RELOCS (buf); /* Ensure we have a reloc here, against a global symbol. */ --- 518,532 ---- { int per_width; ! REQUIRE (read_byte (&buf, end, &cie.per_encoding)); per_width = get_DW_EH_PE_width (cie.per_encoding, ptr_size); REQUIRE (per_width); if ((cie.per_encoding & 0xf0) == DW_EH_PE_aligned) ! { ! length = -(buf - ehbuf) & (per_width - 1); ! REQUIRE (skip_bytes (&buf, end, length)); ! } ENSURE_NO_RELOCS (buf); /* Ensure we have a reloc here, against a global symbol. */ *************** _bfd_elf_discard_section_eh_frame *** 545,551 **** cookie->rel++; while (GET_RELOC (buf) != NULL); } ! buf += per_width; } break; default: --- 559,565 ---- cookie->rel++; while (GET_RELOC (buf) != NULL); } ! REQUIRE (skip_bytes (&buf, end, per_width)); } break; default: *************** _bfd_elf_discard_section_eh_frame *** 627,644 **** cie_usage_count++; hdr_info->fde_count++; } if (cie.lsda_encoding != DW_EH_PE_omit) ! { ! unsigned int dummy; - aug = buf; - buf += 2 * get_DW_EH_PE_width (cie.fde_encoding, ptr_size); - if (cie.augmentation[0] == 'z') - read_uleb128 (dummy, buf); - /* If some new augmentation data is added before LSDA - in FDE augmentation area, this need to be adjusted. */ - this_inf->lsda_offset = (buf - aug); - } buf = last_fde + 4 + hdr.length; SKIP_RELOCS (buf); } --- 641,661 ---- cie_usage_count++; hdr_info->fde_count++; } + /* Skip the initial location and address range. */ + start = buf; + length = get_DW_EH_PE_width (cie.fde_encoding, ptr_size); + REQUIRE (skip_bytes (&buf, end, 2 * length)); + + /* Skip the augmentation size, if present. */ + if (cie.augmentation[0] == 'z') + REQUIRE (skip_leb128 (&buf, end)); + + /* Of the supported augmentation characters above, only 'L' + adds augmentation data to the FDE. This code would need to + be adjusted if any future augmentations do the same thing. */ if (cie.lsda_encoding != DW_EH_PE_omit) ! this_inf->lsda_offset = buf - start; buf = last_fde + 4 + hdr.length; SKIP_RELOCS (buf); } *************** _bfd_elf_write_section_eh_frame (bfd *ab *** 850,856 **** struct eh_frame_sec_info *sec_info; struct elf_link_hash_table *htab; struct eh_frame_hdr_info *hdr_info; - unsigned int leb128_tmp; unsigned int ptr_size; struct eh_cie_fde *ent; --- 867,872 ---- *************** _bfd_elf_write_section_eh_frame (bfd *ab *** 952,958 **** { unsigned char *aug; unsigned int action, extra_string, extra_data; ! unsigned int dummy, per_width, per_encoding; /* Need to find 'R' or 'L' augmentation's argument and modify DW_EH_PE_* value. */ --- 968,974 ---- { unsigned char *aug; unsigned int action, extra_string, extra_data; ! unsigned int per_width, per_encoding; /* Need to find 'R' or 'L' augmentation's argument and modify DW_EH_PE_* value. */ *************** _bfd_elf_write_section_eh_frame (bfd *ab *** 966,974 **** buf += 9; aug = buf; buf = strchr (buf, '\0') + 1; ! read_uleb128 (dummy, buf); ! read_sleb128 (dummy, buf); ! read_uleb128 (dummy, buf); if (*aug == 'z') { /* The uleb128 will always be a single byte for the kind --- 982,990 ---- buf += 9; aug = buf; buf = strchr (buf, '\0') + 1; ! skip_leb128 (&buf, end); ! skip_leb128 (&buf, end); ! skip_leb128 (&buf, end); if (*aug == 'z') { /* The uleb128 will always be a single byte for the kind *************** _bfd_elf_write_section_eh_frame (bfd *ab *** 981,986 **** --- 997,1003 ---- memmove (buf + extra_string + extra_data, buf, end - buf); memmove (aug + extra_string, aug, buf - aug); buf += extra_string; + end += extra_string + extra_data; if (ent->add_augmentation_size) {