Follow-up to November's .eh_frame optimisations (2/3)

[Richard Sandiford <rsandifo at redhat dot com>]

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)
  		{