This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH v2] gas/ELF: don't accumulate .type settings
On Mon, Jul 08, 2019 at 06:53:01AM +0000, Jan Beulich wrote:
> On 08.07.2019 07:42, Alan Modra wrote:
> > On Wed, Jul 03, 2019 at 08:05:48AM +0000, Jan Beulich wrote:
> >> * config/obj-elf.c (obj_elf_type): Check for conflicts between
> >> old and new types.
> >> * config/tc-hppa.h (md_elf_symbol_type_change): New.
> >> * doc/as.texi: Mention warning behavior for the ELF flavor of
> >> .type.
> >> * testsuite/gas/elf/type-2.e, testsuite/gas/elf/type-2.l,
> >> testsuite/gas/elf/type-2.s: New.
> >> * testsuite/gas/elf/elf.exp: Run new test.
> >
> > aarch64_be-linux-gnu_ilp32 +FAIL: elf type-2 list
> > aarch64-linux +FAIL: elf type-2 list
> > nds32be-elf +FAIL: elf type-2 list
> > nds32le-linux +FAIL: elf type-2 list
> > rl78-elf +FAIL: elf type-2 list
> >
> > Plus a large number of mips-sgi-irix6 tests, all due to "symbol ..
> > already has its type set" warnings.
>
> Well, I can see about looking into these, but not before some time
> next week. Until then all I can suggest is for someone to revert
> the commit if the failures need to be taken care of quickly.
I don't think there is any hurry, but this time I'll fix the fails,
particularly since they involve some MIPS tidies.
> I have
> to admit though that it is quite unobvious to me why the test would
> behave differently for different targets
Exactly why general ELF changes need to be tested on multiple targets.
Lots of things are not obvious. (And I hope I haven't tripped over
some irix peculiarity with this patch..)
----
git commit f2d4ba38f5 caused many failures for mips-sgi-irix targets,
and added a new test that failed for aarch64, nds32, and rl78.
The mips failures are due to BSF_OBJECT being set in many cases for
symbols by the mips .global/.globl directive. This patch removes that
code and instead sets BSF_OBJECT in a target frob_symbol function,
also moving the mips hacks in elf_frob_symbol to the new function.
Note that common symbols are handled fine in elf.c:swap_out_syms
without needing to set BSF_OBJECT, so that old code can disappear.
* config/obj-elf.c (elf_frob_symbol): Remove mips hacks.
* config/tc-mips.h (tc_frob_symbol): Define.
(mips_frob_symbol): Declare.
* config/tc-mips.c (s_mips_globl): Don't set BSF_OBJECT for irix.
(mips_frob_symbol): Fudge symbols for irix here.
* testsuite/gas/elf/type-2.e: Allow random target symbols.
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index 224d4c29e6..af35feeec2 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -2369,23 +2369,6 @@ elf_frob_symbol (symbolS *symp, int *puntp)
as_bad (_("symbol `%s' can not be both weak and common"),
S_GET_NAME (symp));
}
-
-#ifdef TC_MIPS
- /* The Irix 5 and 6 assemblers set the type of any common symbol and
- any undefined non-function symbol to STT_OBJECT. We try to be
- compatible, since newer Irix 5 and 6 linkers care. However, we
- only set undefined symbols to be STT_OBJECT if we are on Irix,
- because that is the only time gcc will generate the necessary
- .global directives to mark functions. */
-
- if (S_IS_COMMON (symp))
- symbol_get_bfdsym (symp)->flags |= BSF_OBJECT;
-
- if (strstr (TARGET_OS, "irix") != NULL
- && ! S_IS_DEFINED (symp)
- && (symbol_get_bfdsym (symp)->flags & BSF_FUNCTION) == 0)
- symbol_get_bfdsym (symp)->flags |= BSF_OBJECT;
-#endif
}
struct group_list
diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 671d74aab7..b7b4b6989a 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -16461,7 +16461,6 @@ s_mips_globl (int x ATTRIBUTE_UNUSED)
char *name;
int c;
symbolS *symbolP;
- flagword flag;
do
{
@@ -16472,14 +16471,6 @@ s_mips_globl (int x ATTRIBUTE_UNUSED)
*input_line_pointer = c;
SKIP_WHITESPACE_AFTER_NAME ();
-#ifdef TE_IRIX
- /* On Irix 5, every global symbol that is not explicitly labelled as
- being a function is apparently labelled as being an object. */
- flag = BSF_OBJECT;
-#else
- flag = BSF_NO_FLAGS;
-#endif
-
if (!is_end_of_line[(unsigned char) *input_line_pointer]
&& (*input_line_pointer != ','))
{
@@ -16493,11 +16484,9 @@ s_mips_globl (int x ATTRIBUTE_UNUSED)
(void) restore_line_pointer (c);
if (sec != NULL && (sec->flags & SEC_CODE) != 0)
- flag = BSF_FUNCTION;
+ symbol_get_bfdsym (symbolP)->flags |= BSF_FUNCTION;
}
- symbol_get_bfdsym (symbolP)->flags |= flag;
-
c = *input_line_pointer;
if (c == ',')
{
@@ -16512,6 +16501,23 @@ s_mips_globl (int x ATTRIBUTE_UNUSED)
demand_empty_rest_of_line ();
}
+#ifdef TE_IRIX
+/* The Irix 5 and 6 assemblers set the type of any common symbol and
+ any undefined non-function symbol to STT_OBJECT. We try to be
+ compatible, since newer Irix 5 and 6 linkers care. */
+
+void
+mips_frob_symbol (symbolS *symp ATTRIBUTE_UNUSED)
+{
+ /* This late in assembly we can set BSF_OBJECT indiscriminately
+ and let elf.c:swap_out_syms sort out the symbol type. */
+ flagword *flags = &symbol_get_bfdsym (symp)->flags;
+ if ((*flags & (BSF_GLOBAL | BSF_WEAK)) != 0
+ || !S_IS_DEFINED (symp))
+ *flags |= BSF_OBJECT;
+}
+#endif
+
static void
s_option (int x ATTRIBUTE_UNUSED)
{
diff --git a/gas/config/tc-mips.h b/gas/config/tc-mips.h
index 615269dd37..a928c8a49c 100644
--- a/gas/config/tc-mips.h
+++ b/gas/config/tc-mips.h
@@ -126,6 +126,11 @@ extern void mips_frob_file (void);
extern void mips_frob_file_after_relocs (void);
#endif
+#ifdef TE_IRIX
+#define tc_frob_symbol(sym, punt) mips_frob_symbol (sym)
+extern void mips_frob_symbol (symbolS *);
+#endif
+
#define tc_fix_adjustable(fixp) mips_fix_adjustable (fixp)
extern int mips_fix_adjustable (struct fix *);
diff --git a/gas/testsuite/gas/elf/type-2.e b/gas/testsuite/gas/elf/type-2.e
index 684727ccf4..57f21068cc 100644
--- a/gas/testsuite/gas/elf/type-2.e
+++ b/gas/testsuite/gas/elf/type-2.e
@@ -1,10 +1,20 @@
+.+: 0+0 +0 +NOTYPE +LOCAL +DEFAULT +UND *
+#...
+.+: 0+0 +0 +OBJECT +LOCAL +DEFAULT +. test1
+#...
+.+: 0+1 +0 +FUNC +LOCAL +DEFAULT +. test2
+#...
+.+: 0+2 +0 +NOTYPE +LOCAL +DEFAULT +. test3
+#...
+.+: 0+3 +0 +NOTYPE +LOCAL +DEFAULT +. test4
+#...
+.+: 0+4 +0 +NOTYPE +LOCAL +DEFAULT +. test5
+#...
+.+: 0+5 +0 +NOTYPE +LOCAL +DEFAULT +. test6
+#...
+.+: 0+6 +0 +OBJECT +LOCAL +DEFAULT +. test7
+#...
+.+: 0+7 +0 +TLS +LOCAL +DEFAULT +. test8
+#...
+.+: 0+8 +0 +IFUNC +LOCAL +DEFAULT +. test9
+#pass
--
Alan Modra
Australia Development Lab, IBM