Re: [PATCH] BFD: Don't crash if the size of a reloc is 3.

[Alan Modra <amodra at gmail dot com>]

On Fri, Jul 20, 2018 at 06:41:40PM +0200, John Darrington wrote:
> * bfd/libbfd.c (bfd_get_24): New function.
> * bfd/libbfd.c (bfd_put_24): New function.
> * bfd/bfd-in2.h: regen.
> * bfd/reloc.c (_bfd_clear_contents): Deal with the case when reloc size is 3

ChangeLog entries are relative to the ChangeLog file.  Lose "bfd/".

> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h

Normally you shouldn't post regenerated files.

> diff --git a/bfd/libbfd.c b/bfd/libbfd.c
> index 971be4f..d31a2dc 100644
> --- a/bfd/libbfd.c
> +++ b/bfd/libbfd.c
> @@ -458,6 +458,29 @@ DESCRIPTION
>  .#define bfd_get_signed_16(abfd, ptr) \
>  .  BFD_SEND (abfd, bfd_getx_signed_16, (ptr))
>  .
> +.#define bfd_put_24(abfd, val, ptr) \
> +.do \
> +.{ \
> +.if (bfd_big_endian ((abfd))) \
> +.{ \

Missing indentation, and it's nice to line up continuation backslashes.
The result ought to be

#define bfd_put_24(abfd, val, ptr) \
  do							\
    {							\
      if (bfd_big_endian ((abfd)))			\
	{						\
	  bfd_put_8 ((abfd), (val) >> 0,  (ptr) + 2);	\
...

> +.  bfd_put_8 ((abfd), (val) >> 0,  (ptr) + 2); \
> +.  bfd_put_8 ((abfd), (val) >> 8,  (ptr) + 1); \
> +.  bfd_put_8 ((abfd), (val) >> 16, (ptr) + 0); \
> +.} \
> +.else \
> +.{ \
> +.  bfd_put_8 ((abfd), (val) >> 0,  (ptr) + 0); \
> +.  bfd_put_8 ((abfd), (val) >> 8,  (ptr) + 1); \
> +.  bfd_put_8 ((abfd), (val) >> 16, (ptr) + 2); \
> +.} \
> +.} \
> +.while (0)
> +.
> +.#define bfd_get_24(abfd, ptr) \
> +.  ((bfd_get_8 ((abfd), (ptr) + 0) << (bfd_big_endian ((abfd)) ? 16 : 0)) | \

No trailing operators.  "|" belongs on the next line.

> +.   (bfd_get_8 ((abfd), (ptr) + 1) << 8) | \

Ditto.

> +.   (bfd_get_8 ((abfd), (ptr) + 2) << (bfd_big_endian ((abfd)) ? 0 : 16))) \
> +.
>  .#define bfd_put_32(abfd, val, ptr) \
>  .  BFD_SEND (abfd, bfd_putx32, ((val),(ptr)))
>  .#define bfd_put_signed_32 \
> diff --git a/bfd/reloc.c b/bfd/reloc.c
> index 68bc8a8..535c43d 100644
> --- a/bfd/reloc.c
> +++ b/bfd/reloc.c
> @@ -1636,6 +1636,9 @@ _bfd_clear_contents (reloc_howto_type *howto,
>      case 2:
>        x = bfd_get_16 (input_bfd, location);
>        break;
> +    case 3:
> +      x = bfd_get_24 (input_bfd, location);
> +      break;
>      case 4:
>        x = bfd_get_32 (input_bfd, location);
>        break;
> @@ -1670,6 +1673,9 @@ _bfd_clear_contents (reloc_howto_type *howto,
>      case 2:
>        bfd_put_16 (input_bfd, x, location);
>        break;
> +    case 3:
> +      bfd_put_24 (input_bfd, x, location);
> +      break;
>      case 4:
>        bfd_put_32 (input_bfd, x, location);
>        break;

You have some more things to fix in reloc.c.  _bfd_relocate_contents
needs a similar fix to the above, and DOIT in bfd_perform_relocation
and bfd_install_relocation ought to both handle "case 5" and use
bfd_get/put_24.

-- 
Alan Modra
Australia Development Lab, IBM