[1/8] x86-64: have value properly checked when resolving fixup

Message ID 8566732d-093c-6ae3-56ab-78715bb00f78@suse.com
State New
Headers show
Series
  • x86: assorted relocation handling related adjustments (part II)
Related show

Commit Message

Luis Machado via Binutils April 23, 2021, 8:34 a.m.
Constants not known at the time an individual insn gets assembled and
going into a sign-extended field still shouldn't be silently truncated
at the time the respective fixup gets resolved.

gas/
2021-04-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (md_apply_fix): Mark BFD_RELOC_X86_64_32S as
	signed.
	* testsuite/gas/i386/x86-64-addr32-bad.s,
	testsuite/gas/i386/x86-64-addr32-bad.l: New.
	* testsuite/gas/i386/i386.exp: Run new test.

---
I wonder whether this isn't also related to PR gas/27763.

I won't exclude that more relocation types ought to have values checked
here (hence using switch() right away), but e.g. BFD_RELOC_{8,16,32}
don't need to be - values not valid there get taken care of by
write.c:fixup_segment().

Comments

Luis Machado via Binutils April 23, 2021, 1:12 p.m. | #1
On Fri, Apr 23, 2021 at 1:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> Constants not known at the time an individual insn gets assembled and

> going into a sign-extended field still shouldn't be silently truncated

> at the time the respective fixup gets resolved.

>

> gas/

> 2021-04-XX  Jan Beulich  <jbeulich@suse.com>

>

>         * config/tc-i386.c (md_apply_fix): Mark BFD_RELOC_X86_64_32S as

>         signed.

>         * testsuite/gas/i386/x86-64-addr32-bad.s,

>         testsuite/gas/i386/x86-64-addr32-bad.l: New.

>         * testsuite/gas/i386/i386.exp: Run new test.

>

> ---

> I wonder whether this isn't also related to PR gas/27763.

>

> I won't exclude that more relocation types ought to have values checked

> here (hence using switch() right away), but e.g. BFD_RELOC_{8,16,32}

> don't need to be - values not valid there get taken care of by

> write.c:fixup_segment().

>

> --- a/gas/config/tc-i386.c

> +++ b/gas/config/tc-i386.c

> @@ -12576,7 +12576,18 @@ md_apply_fix (fixS *fixP, valueT *valP,

>

>    /* Are we finished with this relocation now?  */

>    if (fixP->fx_addsy == NULL)

> -    fixP->fx_done = 1;

> +    {

> +      fixP->fx_done = 1;

> +      switch (fixP->fx_r_type)

> +       {

> +       case BFD_RELOC_X86_64_32S:

> +         fixP->fx_signed = 1;

> +         break;

> +

> +       default:

> +         break;

> +       }

> +    }

>  #if defined (OBJ_COFF) && defined (TE_PE)

>    else if (fixP->fx_addsy != NULL && S_IS_WEAK (fixP->fx_addsy))

>      {

> --- a/gas/testsuite/gas/i386/i386.exp

> +++ b/gas/testsuite/gas/i386/i386.exp

> @@ -750,6 +750,7 @@ if [gas_64_check] then {

>      run_dump_test "x86_64-intel"

>      run_dump_test "x86-64-addr32"

>      run_dump_test "x86-64-addr32-intel"

> +    run_list_test "x86-64-addr32-bad" "-al"

>      run_dump_test "x86-64-opcode"

>      run_dump_test "x86-64-intel64"

>      if { ! [istarget "*-*-*cygwin*"] && ![istarget "*-*-mingw*"] } then {

> --- /dev/null

> +++ b/gas/testsuite/gas/i386/x86-64-addr32-bad.l

> @@ -0,0 +1,29 @@

> +.*: Assembler messages:

> +.*:3: Error:.* [0x]*88888888 .*

> +.*:7: Error:.* [0x]*99999999 .*

> +.*:11: Error:.* [0x]*99999999 .*

> +GAS LISTING .*

> +

> +

> +[      ]*[0-9]+[       ]+\.text

> +[      ]*[0-9]+[       ]+addr32:

> +[      ]*[0-9]+[       ]+lea   0x88888888\(%rax\), %rax

> +[      ]*[0-9]+[       ]+\?\?\?\? 8D808888[    ]+lea   0x88888888\(%rax\), %eax

> +[      ]*[0-9]+[       ]+8888

> +[      ]*[0-9]+[       ]+\?\?\?\? 67488D80[    ]+lea   0x88888888\(%eax\), %rax

> +[      ]*[0-9]+[       ]+88888888 *

> +[      ]*[0-9]+[       ]*

> +[      ]*[0-9]+[       ]+\?\?\?\? 488D8099[    ]+lea   value\(%rax\), %rax

> +[      ]*[0-9]+[       ]+999999

> +[      ]*[0-9]+[       ]+\?\?\?\? 8D809999[    ]+lea   value\(%rax\), %eax

> +[      ]*[0-9]+[       ]+9999

> +[      ]*[0-9]+[       ]+\?\?\?\? 67488D80[    ]+lea   value\(%eax\), %rax

> +[      ]*[0-9]+[       ]+99999999 *

> +[      ]*[0-9]+[       ]*

> +[      ]*[0-9]+[       ]+\?\?\?\? 48C7C099[    ]+mov   \$value, %rax

> +[      ]*[0-9]+[       ]+999999

> +[      ]*[0-9]+[       ]+\?\?\?\? B8999999[    ]+mov   \$value, %eax

> +[      ]*[0-9]+[       ]+99

> +[      ]*[0-9]+[       ]*

> +[      ]*[0-9]+[       ]+\.equ value, 0x99999999

> +#pass

> --- /dev/null

> +++ b/gas/testsuite/gas/i386/x86-64-addr32-bad.s

> @@ -0,0 +1,15 @@

> +       .text

> +addr32:

> +       lea     0x88888888(%rax), %rax

> +       lea     0x88888888(%rax), %eax

> +       lea     0x88888888(%eax), %rax

> +

> +       lea     value(%rax), %rax

> +       lea     value(%rax), %eax

> +       lea     value(%eax), %rax

> +

> +       mov     $value, %rax

> +       mov     $value, %eax

> +

> +       .equ    value, 0x99999999

> +       .end

>


OK.

Thanks.

-- 
H.J.

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -12576,7 +12576,18 @@  md_apply_fix (fixS *fixP, valueT *valP,
 
   /* Are we finished with this relocation now?  */
   if (fixP->fx_addsy == NULL)
-    fixP->fx_done = 1;
+    {
+      fixP->fx_done = 1;
+      switch (fixP->fx_r_type)
+	{
+	case BFD_RELOC_X86_64_32S:
+	  fixP->fx_signed = 1;
+	  break;
+
+	default:
+	  break;
+	}
+    }
 #if defined (OBJ_COFF) && defined (TE_PE)
   else if (fixP->fx_addsy != NULL && S_IS_WEAK (fixP->fx_addsy))
     {
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -750,6 +750,7 @@  if [gas_64_check] then {
     run_dump_test "x86_64-intel"
     run_dump_test "x86-64-addr32"
     run_dump_test "x86-64-addr32-intel"
+    run_list_test "x86-64-addr32-bad" "-al"
     run_dump_test "x86-64-opcode"
     run_dump_test "x86-64-intel64"
     if { ! [istarget "*-*-*cygwin*"] && ![istarget "*-*-mingw*"] } then {
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-addr32-bad.l
@@ -0,0 +1,29 @@ 
+.*: Assembler messages:
+.*:3: Error:.* [0x]*88888888 .*
+.*:7: Error:.* [0x]*99999999 .*
+.*:11: Error:.* [0x]*99999999 .*
+GAS LISTING .*
+
+
+[ 	]*[0-9]+[ 	]+\.text
+[ 	]*[0-9]+[ 	]+addr32:
+[ 	]*[0-9]+[ 	]+lea	0x88888888\(%rax\), %rax
+[ 	]*[0-9]+[ 	]+\?\?\?\? 8D808888[ 	]+lea	0x88888888\(%rax\), %eax
+[ 	]*[0-9]+[ 	]+8888
+[ 	]*[0-9]+[ 	]+\?\?\?\? 67488D80[ 	]+lea	0x88888888\(%eax\), %rax
+[ 	]*[0-9]+[ 	]+88888888 *
+[ 	]*[0-9]+[ 	]*
+[ 	]*[0-9]+[ 	]+\?\?\?\? 488D8099[ 	]+lea	value\(%rax\), %rax
+[ 	]*[0-9]+[ 	]+999999
+[ 	]*[0-9]+[ 	]+\?\?\?\? 8D809999[ 	]+lea	value\(%rax\), %eax
+[ 	]*[0-9]+[ 	]+9999
+[ 	]*[0-9]+[ 	]+\?\?\?\? 67488D80[ 	]+lea	value\(%eax\), %rax
+[ 	]*[0-9]+[ 	]+99999999 *
+[ 	]*[0-9]+[ 	]*
+[ 	]*[0-9]+[ 	]+\?\?\?\? 48C7C099[ 	]+mov	\$value, %rax
+[ 	]*[0-9]+[ 	]+999999
+[ 	]*[0-9]+[ 	]+\?\?\?\? B8999999[ 	]+mov	\$value, %eax
+[ 	]*[0-9]+[ 	]+99
+[ 	]*[0-9]+[ 	]*
+[ 	]*[0-9]+[ 	]+\.equ	value, 0x99999999
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-addr32-bad.s
@@ -0,0 +1,15 @@ 
+	.text
+addr32:
+	lea	0x88888888(%rax), %rax
+	lea	0x88888888(%rax), %eax
+	lea	0x88888888(%eax), %rax
+
+	lea	value(%rax), %rax
+	lea	value(%rax), %eax
+	lea	value(%eax), %rax
+
+	mov	$value, %rax
+	mov	$value, %eax
+
+	.equ	value, 0x99999999
+	.end