[PATCH 3/8] x86: honor signedness of PC-relative relocations

H.J. Lu hjl.tools@gmail.com
Fri Apr 23 13:18:02 GMT 2021


On Fri, Apr 23, 2021 at 1:35 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> PR gas/27763
>
> While the comment in output_jump() was basically correct prior to the
> introduction of 64-bit mode, both that and the not-JMP-like behavior of
> XBEGIN require adjustments: Branches with 32-bit displacement do not
> wrap at 4G in 64-bit mode, and XBEGIN with 16-bit operand size doesn't
> wrap at 64k. Similarly %rip-relative addressing doesn't wrap at 4G.
>
> The new testcase points out that for PE/COFF object_64bit didn't get
> set so far, preventing in particular the check at the end of
> md_convert_frag() to take effect.
>
> For Mach-O the new testcase fails (bogusly), in that only the first two
> of the expected errors get raised. Since for Mach-O many testcases
> already fail, and since an x86_64-darwin target can't even be configured
> for, I didn't think I need to bother.
>
> Note that there are further issues in this area, in particular for
> branches with operand size overrides. Such branches, which truncate
> %rip / %eip, can't be correctly expressed with ordinary PC-relative
> relocations. It's not really clear what to do with them - perhaps the
> best we can do is to carry through all associated relocations, leaving
> it to the linker (or even loader) to decide (once the final address
> layout is known). Same perhaps goes for relocations associated with
> 32-bit addressing in 64-bit mode.
>
> gas/
> 2021-04-XX  Jan Beulich  <jbeulich@suse.com>
>
>         PR gas/27763
>         * config/tc-i386.c (output_jump): Also mark 2-byte relocs as
>         signed for XBEGIN. Also mark 4-byte relocs as signed for 64-bit.
>         (output_disp): Also mark 4-byte relocs as signed for 64-bit.
>         (md_estimate_size_before_relax): Move local variable fixP. Set
>         it from fix_new() return values. Mark 4-byte relocs as signed
>         for 64-bit.
>         * testsuite/gas/i386/pcrel64.s, testsuite/gas/i386/pcrel64.l: New.
>         * testsuite/gas/i386/i386.exp: Run new tests.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -8927,11 +8927,26 @@ output_jump (void)
>    fixP = fix_new_exp (frag_now, p - frag_now->fr_literal, size,
>                       i.op[0].disps, 1, jump_reloc);
>
> -  /* All jumps handled here are signed, but don't use a signed limit
> -     check for 32 and 16 bit jumps as we want to allow wrap around at
> -     4G and 64k respectively.  */
> -  if (size == 1)
> -    fixP->fx_signed = 1;
> +  /* All jumps handled here are signed, but don't unconditionally use a
> +     signed limit check for 32 and 16 bit jumps as we want to allow wrap
> +     around at 4G (outside of 64-bit mode) and 64k (except for XBEGIN)
> +     respectively.  */
> +  switch (size)
> +    {
> +    case 1:
> +      fixP->fx_signed = 1;
> +      break;
> +
> +    case 2:
> +      if (i.tm.base_opcode == 0xc7f8)
> +       fixP->fx_signed = 1;
> +      break;
> +
> +    case 4:
> +      if (flag_code == CODE_64BIT)
> +       fixP->fx_signed = 1;
> +      break;
> +    }
>  }
>
>  static void
> @@ -10020,6 +10035,11 @@ output_disp (fragS *insn_start_frag, off
>               fixP = fix_new_exp (frag_now, p - frag_now->fr_literal,
>                                   size, i.op[n].disps, pcrel,
>                                   reloc_type);
> +
> +             if (flag_code == CODE_64BIT && size == 4 && pcrel
> +                 && !i.prefix[ADDR_PREFIX])
> +               fixP->fx_signed = 1;
> +
>               /* Check for "call/jmp *mem", "mov mem, %reg",
>                  "test %reg, mem" and "binop mem, %reg" where binop
>                  is one of adc, add, and, cmp, or, sbb, sub, xor
> @@ -12255,6 +12275,7 @@ md_estimate_size_before_relax (fragS *fr
>        enum bfd_reloc_code_real reloc_type;
>        unsigned char *opcode;
>        int old_fr_fix;
> +      fixS *fixP = NULL;
>
>        if (fragP->fr_var != NO_RELOC)
>         reloc_type = (enum bfd_reloc_code_real) fragP->fr_var;
> @@ -12276,10 +12297,10 @@ md_estimate_size_before_relax (fragS *fr
>           /* Make jmp (0xeb) a (d)word displacement jump.  */
>           opcode[0] = 0xe9;
>           fragP->fr_fix += size;
> -         fix_new (fragP, old_fr_fix, size,
> -                  fragP->fr_symbol,
> -                  fragP->fr_offset, 1,
> -                  reloc_type);
> +         fixP = fix_new (fragP, old_fr_fix, size,
> +                         fragP->fr_symbol,
> +                         fragP->fr_offset, 1,
> +                         reloc_type);
>           break;
>
>         case COND_JUMP86:
> @@ -12306,8 +12327,6 @@ md_estimate_size_before_relax (fragS *fr
>         case COND_JUMP:
>           if (no_cond_jump_promotion && fragP->fr_var == NO_RELOC)
>             {
> -             fixS *fixP;
> -
>               fragP->fr_fix += 1;
>               fixP = fix_new (fragP, old_fr_fix, 1,
>                               fragP->fr_symbol,
> @@ -12323,16 +12342,23 @@ md_estimate_size_before_relax (fragS *fr
>           opcode[0] = TWO_BYTE_OPCODE_ESCAPE;
>           /* We've added an opcode byte.  */
>           fragP->fr_fix += 1 + size;
> -         fix_new (fragP, old_fr_fix + 1, size,
> -                  fragP->fr_symbol,
> -                  fragP->fr_offset, 1,
> -                  reloc_type);
> +         fixP = fix_new (fragP, old_fr_fix + 1, size,
> +                         fragP->fr_symbol,
> +                         fragP->fr_offset, 1,
> +                         reloc_type);
>           break;
>
>         default:
>           BAD_CASE (fragP->fr_subtype);
>           break;
>         }
> +
> +      /* All jumps handled here are signed, but don't unconditionally use a
> +        signed limit check for 32 and 16 bit jumps as we want to allow wrap
> +        around at 4G (outside of 64-bit mode) and 64k.  */
> +      if (size == 4 && flag_code == CODE_64BIT)
> +       fixP->fx_signed = 1;
> +
>        frag_wane (fragP);
>        return fragP->fr_fix - old_fr_fix;
>      }
> @@ -13964,9 +13990,11 @@ i386_target_format (void)
>  # if defined (TE_PE) || defined (TE_PEP)
>      case bfd_target_coff_flavour:
>        if (flag_code == CODE_64BIT)
> -       return use_big_obj ? "pe-bigobj-x86-64" : "pe-x86-64";
> -      else
> -       return use_big_obj ? "pe-bigobj-i386" : "pe-i386";
> +       {
> +         object_64bit = 1;
> +         return use_big_obj ? "pe-bigobj-x86-64" : "pe-x86-64";
> +       }
> +      return use_big_obj ? "pe-bigobj-i386" : "pe-i386";
>  # elif defined (TE_GO32)
>      case bfd_target_coff_flavour:
>        return "coff-go32";
> --- a/gas/testsuite/gas/i386/i386.exp
> +++ b/gas/testsuite/gas/i386/i386.exp
> @@ -760,6 +760,7 @@ if [gas_64_check] then {
>      } else {
>        run_dump_test "x86-64-w64-pcrel"
>      }
> +    run_list_test "pcrel64" "-al"
>      run_dump_test "x86-64-rip"
>      run_dump_test "x86-64-rip-intel"
>      run_dump_test "x86-64-stack"
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/pcrel64.l
> @@ -0,0 +1,54 @@
> +.*: Assembler messages:
> +.*:16: Error: .*
> +.*:17: Error: .*
> +.*:13: Error: .*
> +.*:15: Error: .*
> +.*:18: Error: .*
> +.*:19: Error: .*
> +.*:20: Error: .*
> +GAS LISTING .*
> +
> +
> +[      ]*[0-9]+[       ]+\.text
> +[      ]*[0-9]+[       ]+\.code64
> +[      ]*[0-9]+[       ]+pcrel:
> +[      ]*[0-9]+ \?\?\?\? E8..8000[     ]+call  target
> +[      ]*[0-9]+[       ]+00
> +[      ]*[0-9]+ \?\?\?\? E9..8000[     ]+jmp   target
> +[      ]*[0-9]+[       ]+00
> +[      ]*[0-9]+ \?\?\?\? 0F84..80[     ]+jz    target
> +[      ]*[0-9]+[       ]+0000
> +[      ]*[0-9]+ \?\?\?\? C7F8..80[     ]+xbegin        target
> +[      ]*[0-9]+[       ]+0000
> +[      ]*[0-9]+ \?\?\?\? 8B05..80[     ]+mov   target\(%rip\), %eax
> +[      ]*[0-9]+[       ]+0000
> +[      ]*[0-9]+ \?\?\?\? 678B05..[     ]+mov   target\(%eip\), %eax
> +[      ]*[0-9]+[       ]+800000
> +[      ]*[0-9]+ \?\?\?\? 48C7C0..[     ]+mov   \$target-., %rax
> +[      ]*[0-9]+[       ]+800000
> +[      ]*[0-9]+ \?\?\?\? B8..8000[     ]+mov   \$target-., %eax
> +[      ]*[0-9]+[       ]+00
> +[      ]*[0-9]+[       ]*
> +[      ]*[0-9]+ \?\?\?\? 66C7F8..[     ]+data16 xbegin target
> +[      ]*[0-9]+[       ]+80
> +[      ]*[0-9]+[       ]*
> +[      ]*[0-9]+ \?\?\?\? E8...000[     ]+call  target\+0x7ffff000
> +[      ]*[0-9]+[       ]+80
> +[      ]*[0-9]+ \?\?\?\? E9000000[     ]+jmp   target\+0x7ffff000
> +[      ]*[0-9]+[       ]+00
> +[      ]*[0-9]+ \?\?\?\? 0F840000[     ]+jz    target\+0x7ffff000
> +[      ]*[0-9]+[       ]+0000
> +[      ]*[0-9]+ \?\?\?\? C7F8...0[     ]+xbegin        target\+0x7ffff000
> +[      ]*[0-9]+[       ]+0080
> +[      ]*[0-9]+ \?\?\?\? 8B05...0[     ]+mov   target\+0x7ffff000\(%rip\), %eax
> +[      ]*[0-9]+[       ]+0080
> +[      ]*[0-9]+ \?\?\?\? 48C7C0..[     ]+mov   \$target\+0x7ffff000-., %rax
> +[      ]*[0-9]+[       ]+.00080
> +[      ]*[0-9]+[       ]*
> +[      ]*[0-9]+ \?\?\?\? 678B05..[     ]+mov   target\+0x7ffff000\(%eip\), %eax
> +[      ]*[0-9]+[       ]+.00080
> +[      ]*[0-9]+ \?\?\?\? B8...000[     ]+mov   \$target\+0x7ffff000-., %eax
> +[      ]*[0-9]+[       ]+80
> +[      ]*[0-9]+[       ]*
> +[      ]*[0-9]+ \?\?\?\? CCCCCCCC[     ]+\.fill 0x8000, 1, 0xcc
> +#pass
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/pcrel64.s
> @@ -0,0 +1,27 @@
> +       .text
> +       .code64
> +pcrel:
> +       call    target
> +       jmp     target
> +       jz      target
> +       xbegin  target
> +       mov     target(%rip), %eax
> +       mov     target(%eip), %eax
> +       mov     $target-., %rax
> +       mov     $target-., %eax
> +
> +       data16 xbegin target
> +
> +       call    target+0x7ffff000
> +       jmp     target+0x7ffff000
> +       jz      target+0x7ffff000
> +       xbegin  target+0x7ffff000
> +       mov     target+0x7ffff000(%rip), %eax
> +       mov     $target+0x7ffff000-., %rax
> +
> +       mov     target+0x7ffff000(%eip), %eax
> +       mov     $target+0x7ffff000-., %eax
> +
> +       .fill 0x8000, 1, 0xcc
> +target:
> +       ret
>

Please include testcases in:

https://sourceware.org/pipermail/binutils/2021-April/116252.html

OK with that change.

Thanks.

-- 
H.J.


More information about the Binutils mailing list