bfd: xtensa: ignore overflow in hight part of const16 relocation

Message ID 20181207211729.32636-1-jcmvbkbc@gmail.com
State New
Headers show
Series
  • bfd: xtensa: ignore overflow in hight part of const16 relocation
Related show

Commit Message

Max Filippov Dec. 7, 2018, 9:17 p.m.
32-bit constants loaded by two const16 opcodes that involve relocation
(e.g. calculated as a sum of a symbol and a constant) may overflow,
resulting in linking error with the following message:

  dangerous relocation: const16: cannot encode: (_start+0x70000000)

They whould wrap around instead.

bfd/
2018-12-07  Max Filippov  <jcmvbkbc@gmail.com>

	* elf32-xtensa.c (elf_xtensa_do_reloc): Only use bits 16..31 of
	the relocated value for the high part const16 immediate.
---
 bfd/elf32-xtensa.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.11.0

Comments

augustine.sterling@gmail.com Dec. 7, 2018, 9:31 p.m. | #1
On Fri, Dec 7, 2018 at 1:17 PM Max Filippov <jcmvbkbc@gmail.com> wrote:
>

> 32-bit constants loaded by two const16 opcodes that involve relocation

> (e.g. calculated as a sum of a symbol and a constant) may overflow,

> resulting in linking error with the following message:

>

>   dangerous relocation: const16: cannot encode: (_start+0x70000000)

>

> They whould wrap around instead.

>

> bfd/

> 2018-12-07  Max Filippov  <jcmvbkbc@gmail.com>

>

>         * elf32-xtensa.c (elf_xtensa_do_reloc): Only use bits 16..31 of

>         the relocated value for the high part const16 immediate.


This is generally OK. I would probably change the comment to, "Ignore
high sixteen bits". The way it is worded now it sounds like a 32-bit
overflow, so more than ~4GB.

> ---

>  bfd/elf32-xtensa.c | 5 +++--

>  1 file changed, 3 insertions(+), 2 deletions(-)

>

> diff --git a/bfd/elf32-xtensa.c b/bfd/elf32-xtensa.c

> index cf085b7b0751..de960cd3b8fc 100644

> --- a/bfd/elf32-xtensa.c

> +++ b/bfd/elf32-xtensa.c

> @@ -1957,8 +1957,9 @@ elf_xtensa_do_reloc (reloc_howto_type *howto,

>         }

>        else if (opcode == get_const16_opcode ())

>         {

> -         /* ALT used for high 16 bits.  */

> -         newval = relocation >> 16;

> +         /* ALT used for high 16 bits.

> +            Ignore 32-bit overflow.  */

> +         newval = (relocation >> 16) & 0xffff;

>           opnd = 1;

>         }

>        else

> --

> 2.11.0

>
Max Filippov Dec. 7, 2018, 9:38 p.m. | #2
On Fri, Dec 7, 2018 at 1:31 PM augustine.sterling@gmail.com
<augustine.sterling@gmail.com> wrote:
>

> On Fri, Dec 7, 2018 at 1:17 PM Max Filippov <jcmvbkbc@gmail.com> wrote:

> >

> > 32-bit constants loaded by two const16 opcodes that involve relocation

> > (e.g. calculated as a sum of a symbol and a constant) may overflow,

> > resulting in linking error with the following message:

> >

> >   dangerous relocation: const16: cannot encode: (_start+0x70000000)

> >

> > They whould wrap around instead.

> >

> > bfd/

> > 2018-12-07  Max Filippov  <jcmvbkbc@gmail.com>

> >

> >         * elf32-xtensa.c (elf_xtensa_do_reloc): Only use bits 16..31 of

> >         the relocated value for the high part const16 immediate.

>

> This is generally OK. I would probably change the comment to, "Ignore

> high sixteen bits". The way it is worded now it sounds like a 32-bit

> overflow, so more than ~4GB.


It is a 32-bit overflow of the relocated value, and when the value is
shifted right by 16 to extract immediate field for the const16 it doesn't
fit into that 16-bit field.
So the fix is to ignore all bits above the bit 31 of the relocated value,
i.e. to only use bits 16..31 for the const16 immediate.

-- 
Thanks.
-- Max
Max Filippov Dec. 11, 2018, 10:25 p.m. | #3
On Fri, Dec 7, 2018 at 1:38 PM Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Fri, Dec 7, 2018 at 1:31 PM augustine.sterling@gmail.com

> <augustine.sterling@gmail.com> wrote:

> > On Fri, Dec 7, 2018 at 1:17 PM Max Filippov <jcmvbkbc@gmail.com> wrote:

> > This is generally OK. I would probably change the comment to, "Ignore

> > high sixteen bits". The way it is worded now it sounds like a 32-bit

> > overflow, so more than ~4GB.

>

> It is a 32-bit overflow of the relocated value, and when the value is

> shifted right by 16 to extract immediate field for the const16 it doesn't

> fit into that 16-bit field.

> So the fix is to ignore all bits above the bit 31 of the relocated value,

> i.e. to only use bits 16..31 for the const16 immediate.


I've applied this fix to master with the following commit message:

32-bit constants loaded by two const16 opcodes that involve relocation
(e.g. calculated as a sum of a symbol and a constant) may overflow,
resulting in linking error with the following message:

  dangerous relocation: const16: cannot encode: (_start+0x70000000)

They should wrap around instead. Limit const16 opcode immediate field to
16 least significant bits to implement this wrap around.

bfd/
2018-12-11  Max Filippov  <jcmvbkbc@gmail.com>

        * elf32-xtensa.c (elf_xtensa_do_reloc): Limit const16 opcode
        immediate field to 16 least significant bits.

-- 
Thanks.
-- Max

Patch

diff --git a/bfd/elf32-xtensa.c b/bfd/elf32-xtensa.c
index cf085b7b0751..de960cd3b8fc 100644
--- a/bfd/elf32-xtensa.c
+++ b/bfd/elf32-xtensa.c
@@ -1957,8 +1957,9 @@  elf_xtensa_do_reloc (reloc_howto_type *howto,
 	}
       else if (opcode == get_const16_opcode ())
 	{
-	  /* ALT used for high 16 bits.  */
-	  newval = relocation >> 16;
+	  /* ALT used for high 16 bits.
+	     Ignore 32-bit overflow.  */
+	  newval = (relocation >> 16) & 0xffff;
 	  opnd = 1;
 	}
       else