This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] avoid undefined behavior due to oversized shifts
The sanity checks look reasonable, but I don't believe they're quite
enough. My original statement was a little imprecise: shifts by the
value's exact bitwidth are also undefined behavior (i.e. 1<<32 is UB).
Thus, chunksz == size == sizeof(x) is a legitimate invocation of
get_value() but requires some additional handling in addition to the
On Thu, Jan 3, 2013 at 4:17 AM, nick clifton <email@example.com> wrote:
> Hi Nickolai,
>> In C, shifting a value by more than its bitwidth is undefined behavior.
>> The get_value() function in bfd/elflink.c (invoked as part of
>> bfd_elf_perform_complex_relocation) sometimes shifts a bfd_vma value by
>> more than its bitwidth.
> Actually I think that there is a bigger problems here: If (8 * chunksz) is
> more than the bitwidth of the bfd_vma type then the get_value() function is
> never going to return the value that the user expects. Even if you fix the
> shifting problem.
> A better patch I think therefore would be something like this:
> diff -u -3 -p -r1.458 elflink.c
> --- bfd/elflink.c 19 Dec 2012 19:45:43 -0000 1.458
> +++ bfd/elflink.c 3 Jan 2013 09:19:34 -0000
> @@ -7919,13 +7919,19 @@ get_value (bfd_vma size,
> bfd_vma x = 0;
> + /* Sanity checks. */
> + if (chunksz > sizeof (x)
> + || (chunksz == sizeof (x) && size > chunksz)
> + || size < chunksz
> + || size % chunksz != 0
> + || input_bfd == NULL
> + || location == NULL)
> + abort ();
> Do you agree ?