public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [PATCH 1/3] x86-64: improve handling of branches to absolute addresses
Date: Thu, 30 Jun 2022 10:50:32 -0700	[thread overview]
Message-ID: <CAMe9rOpVa0NmsoVsyFV30p==Vm0HigNDvV_kB_orB7qNrU3kRg@mail.gmail.com> (raw)
In-Reply-To: <e0071110-098e-93df-839d-df47d390bce3@suse.com>

On Thu, Jun 30, 2022 at 5:08 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> There are two related problems here: The use of "addr32" on a direct
> branch would, besides causing a warning, result in operands to be
> permitted which mistakenly are refused without "addr32". Plus at some
> point not too long ago I'm afraid it may have been me who regressed the
> relocation addends emitted for such branches. Correct both problems,
> adding a testcase to guard against regressing this again.
> ---
> In principle things like "JECXZ <absolute>" should be permitted as well,
> I think. Whether the destination is within reach can only be determined
> by the linker. But that likely being a more intrusive change, I guess we
> can leave this as is until someone really needs it to work.
>
> If "ELF: emit symbol table when there are relocations" (submitted
> earlier) goes in before this change, I'd be inclined to drop the label
> again from the new testcase. The original lack of a label there was how
> I noticed that other issue, so the testcase here could at once serve to
> test that changed behavior as well.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -4975,7 +4975,9 @@ md_assemble (char *line)
>    if (i.imm_operands)
>      optimize_imm ();
>
> -  if (i.disp_operands && !want_disp32 (current_templates->start))
> +  if (i.disp_operands && !want_disp32 (current_templates->start)
> +      && (!current_templates->start->opcode_modifier.jump
> +         || i.jumpabsolute || i.types[0].bitfield.baseindex))
>      {
>        for (j = 0; j < i.operands; ++j)
>         {
> @@ -5985,7 +5987,9 @@ optimize_disp (void)
>             /* Optimize 64-bit displacement to 32-bit for 64-bit BFD.  */
>             if ((i.types[op].bitfield.disp32
>                  || (flag_code == CODE_64BIT
> -                    && want_disp32 (current_templates->start)))
> +                    && want_disp32 (current_templates->start)
> +                    && (!current_templates->start->opcode_modifier.jump
> +                        || i.jumpabsolute || i.types[op].bitfield.baseindex)))
>                 && fits_in_unsigned_long (op_disp))
>               {
>                 /* If this operand is at most 32 bits, convert
> --- a/gas/testsuite/gas/i386/i386.exp
> +++ b/gas/testsuite/gas/i386/i386.exp
> @@ -1314,6 +1314,7 @@ if [gas_64_check] then {
>         run_dump_test "x86-64-branch-3"
>         run_list_test "x86-64-branch-4" "-al -mintel64"
>         run_list_test "x86-64-branch-5" "-al"
> +       run_dump_test "x86-64-branch-6"
>
>         run_dump_test "x86-64-rip-2"
>
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-branch-6.d
> @@ -0,0 +1,21 @@
> +#objdump: -r
> +#name: x86-64 branch 6
> +#warning_output: x86-64-branch-6.e
> +
> +.*: +file format .*
> +
> +RELOCATION RECORDS FOR \[\.text\]:
> +OFFSET +TYPE +VALUE *
> +0+01 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+11 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+21 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+31 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+07 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+0c R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+17 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+1c R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+27 R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+2c R_X86_64_PC32 +\*ABS\*\+0x000000008765431d
> +0+37 R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +0+3c R_X86_64_PC32 +\*ABS\*\+0x000000087654320c
> +#pass
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-branch-6.s
> @@ -0,0 +1,18 @@
> +       .text
> +
> +branch_6:
> +       call    0x87654321
> +       je      0x87654321
> +       jmp     0x87654321
> +
> +       call    0x876543210
> +       je      0x876543210
> +       jmp     0x876543210
> +
> +       addr32 call     0x87654321
> +       addr32 je       0x87654321
> +       addr32 jmp      0x87654321
> +
> +       addr32 call     0x876543210
> +       addr32 je       0x876543210
> +       addr32 jmp      0x876543210
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-branch-6.e
> @@ -0,0 +1,7 @@
> +.*: Assembler messages:
> +.*:12: Warning: skipping prefixes on `call'
> +.*:13: Warning: skipping prefixes on `je'
> +.*:14: Warning: skipping prefixes on `jmp'
> +.*:16: Warning: skipping prefixes on `call'
> +.*:17: Warning: skipping prefixes on `je'
> +.*:18: Warning: skipping prefixes on `jmp'
>

OK.

Thanks.

-- 
H.J.

  reply	other threads:[~2022-06-30 17:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-30 12:06 [PATCH 0/3] gas/x86: displacement handling adjustments Jan Beulich
2022-06-30 12:08 ` [PATCH 1/3] x86-64: improve handling of branches to absolute addresses Jan Beulich
2022-06-30 17:50   ` H.J. Lu [this message]
2022-06-30 12:08 ` [PATCH 2/3] x86: restore masking of displacement kinds Jan Beulich
2022-06-30 22:47   ` H.J. Lu
2022-06-30 12:10 ` [PATCH 3/3] x86: fold Disp32S and Disp32 Jan Beulich
2022-06-30 22:51   ` H.J. Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMe9rOpVa0NmsoVsyFV30p==Vm0HigNDvV_kB_orB7qNrU3kRg@mail.gmail.com' \
    --to=hjl.tools@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=jbeulich@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).