x86 build breakage

Message ID ZvQB46h-9gv7Es7T@squeak.grove.modra.org
State New
Headers
Series x86 build breakage |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Alan Modra Sept. 25, 2024, 12:28 p.m. UTC
  OK to apply the following?

From 3ef6914d084c0f42404676e3fd7b8d7d34a33ec1 Mon Sep 17 00:00:00 2001
From: Alan Modra <amodra@gmail.com>
Date: Wed, 25 Sep 2024 21:32:03 +0930
Subject: Re: x86: Enable TLS relocation check only for ELF

Some configurations (eg. i386-bsd, i386-msdos) broke with commit
9e0840deed38, due to the gotrel array being unused.  Fix that.
Also invert the preprocessor test around lex_got to make it positive
logic and remove the LEX_AT condition which is no longer necessary.
(The only x86 config files defining LEX_AT also define TE_PE.)
  

Comments

Jan Beulich Sept. 25, 2024, 2:16 p.m. UTC | #1
On 25.09.2024 14:28, Alan Modra wrote:
> OK to apply the following?

Certainly.

> From 3ef6914d084c0f42404676e3fd7b8d7d34a33ec1 Mon Sep 17 00:00:00 2001
> From: Alan Modra <amodra@gmail.com>
> Date: Wed, 25 Sep 2024 21:32:03 +0930
> Subject: Re: x86: Enable TLS relocation check only for ELF
> 
> Some configurations (eg. i386-bsd, i386-msdos) broke with commit
> 9e0840deed38, due to the gotrel array being unused.  Fix that.

I can't help the impression that the array was unused for such targets also
before x86_report_tls_error()'s uses appeared. I wonder what's different
between then and now (yet likely not worthwhile to actually waste time on
figuring out).

> Also invert the preprocessor test around lex_got to make it positive
> logic and remove the LEX_AT condition which is no longer necessary.
> (The only x86 config files defining LEX_AT also define TE_PE.)

Thanks for that - I don't recall how many times I sighed over that
needlessly inverted logic.

There's another related use of LEX_AT also in x86_cons(). Curiously that
#if condition doesn't fully match the other one. Can I maybe talk you into
pruning that as well in the same commit?

Jan
  
Alan Modra Sept. 25, 2024, 11:04 p.m. UTC | #2
On Wed, Sep 25, 2024 at 04:16:59PM +0200, Jan Beulich wrote:
> On 25.09.2024 14:28, Alan Modra wrote:
> > OK to apply the following?
> 
> Certainly.
> 
> > From 3ef6914d084c0f42404676e3fd7b8d7d34a33ec1 Mon Sep 17 00:00:00 2001
> > From: Alan Modra <amodra@gmail.com>
> > Date: Wed, 25 Sep 2024 21:32:03 +0930
> > Subject: Re: x86: Enable TLS relocation check only for ELF
> > 
> > Some configurations (eg. i386-bsd, i386-msdos) broke with commit
> > 9e0840deed38, due to the gotrel array being unused.  Fix that.
> 
> I can't help the impression that the array was unused for such targets also
> before x86_report_tls_error()'s uses appeared. I wonder what's different
> between then and now (yet likely not worthwhile to actually waste time on
> figuring out).

Yes, the error was there prior to the commit I blamed, but masked by
another error.  I've fixed the commit subject and message, writing:

    x86 TLS relocation checks
    
    Some configurations (eg. i386-bsd, i386-msdos) broke with the addition
    of the TLS relocation checking.  The "x86_elf_abi undeclared" error
    has been fixed, but "gotrel defined but not used" remains.  Fix that.
    Also invert the preprocessor test around lex_got to make it positive
    logic and remove the LEX_AT condition which is no longer necessary.
    (The only x86 config files defining LEX_AT also define TE_PE.)

> > Also invert the preprocessor test around lex_got to make it positive
> > logic and remove the LEX_AT condition which is no longer necessary.
> > (The only x86 config files defining LEX_AT also define TE_PE.)
> 
> Thanks for that - I don't recall how many times I sighed over that
> needlessly inverted logic.
> 
> There's another related use of LEX_AT also in x86_cons(). Curiously that
> #if condition doesn't fully match the other one. Can I maybe talk you into
> pruning that as well in the same commit?

Done.
  

Patch

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 25ecaa66104..3f8224e0720 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -1327,6 +1327,8 @@  static htab_t op_hash;
 /* Hash table for register lookup.  */
 static htab_t reg_hash;
 
+#if (defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF) || defined (OBJ_MACH_O) \
+     || defined (TE_PE))
 static const struct
 {
   const char *str;
@@ -1414,6 +1416,7 @@  gotrel[] =
 #undef OPERAND_TYPE_IMM32_32S_64_DISP32_64
 #undef OPERAND_TYPE_IMM64_DISP64
 };
+#endif
 
   /* Various efficient no-op patterns for aligning code labels.
      Note: Don't try to assemble the instructions in the comments.
@@ -12836,10 +12839,8 @@  x86_address_bytes (void)
   return stdoutput->arch_info->bits_per_address / 8;
 }
 
-#if (!(defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF) || defined (OBJ_MACH_O)) \
-     || defined (LEX_AT)) && !defined (TE_PE)
-# define lex_got(reloc, adjust, types) NULL
-#else
+#if (defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF) || defined (OBJ_MACH_O) \
+     || defined (TE_PE))
 /* Parse operands of the form
    <symbol>@GOTOFF+<nnn>
    and similar .plt or .got references.
@@ -12937,6 +12938,8 @@  lex_got (enum bfd_reloc_code_real *rel,
   /* Might be a symbol version string.  Don't as_bad here.  */
   return NULL;
 }
+#else
+# define lex_got(reloc, adjust, types) NULL
 #endif
 
 bfd_reloc_code_real_type