mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: "Andy Lutomirski" <luto@amacapital.net>
Cc: <binutils@sourceware.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: Avoiding unnecessary jump relocations in gas?
Date: Thu, 07 May 2015 12:52:01 +0100	[thread overview]
Message-ID: <554B6E0102000078000778D0@mail.emea.novell.com> (raw)
In-Reply-To: <CALCETrUXnXVARO=KD+r0YV1wi2vvXF-41VS++F877HsJs1qz9Q@mail.gmail.com>

>>> On 07.05.15 at 08:02, <luto@amacapital.net> wrote:
> AFAICT gas will produce relocations for jumps to global labels in the
> same file.  This doesn't seem directly harmful to me, except that, on
> x86, it forces five-byte jumps instead of two-byte jumps.
> 
> This seems especially unfortunate, since even hidden and protected
> symbols have this problem.
> 
> Given that many users don't want interposition support (especially the
> kernel and anyone using .hidden or .protected), it would be nice to
> have a command-line option to turn this off and probably also to turn
> it off by default for hidden and protected symbols.  Can gas do this?

I've been running with the below changes (taken off of a bigger set
of changes, so the line numbers may look a little odd) for the last
couple of years. I never tried to submit this change because so far
I couldn't find the time to check whether this would have any
unwanted side effects on cases I don't normally use.

--- binutils-2.25/gas/config/tc-i386.c
+++ 2.25/gas/config/tc-i386.c
@@ -8872,6 +8899,22 @@ i386_frag_max_var (fragS *frag)
 int
 md_estimate_size_before_relax (fragS *fragP, segT segment)
 {
+#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
+  asymbol *bfdsym = NULL;
+  elf_symbol_type *elfsym = NULL;
+
+  if (IS_ELF)
+    {
+      bfdsym = symbol_get_bfdsym (fragP->fr_symbol);
+
+      if (S_IS_EXTERNAL (fragP->fr_symbol))
+	{
+	  elfsym = elf_symbol_from (bfd_asymbol_bfd (bfdsym), bfdsym);
+	  gas_assert (elfsym);
+	}
+    }
+#endif
+
   /* We've already got fragP->fr_subtype right;  all we have to do is
      check for un-relaxable symbols.  On an ELF system, we can't relax
      an externally visible symbol, because it may be overridden by a
@@ -8879,10 +8922,12 @@ md_estimate_size_before_relax (fragS *fr
   if (S_GET_SEGMENT (fragP->fr_symbol) != segment
 #if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
       || (IS_ELF
-	  && (S_IS_EXTERNAL (fragP->fr_symbol)
+	  && ((S_IS_EXTERNAL (fragP->fr_symbol)
+	       && elfsym->internal_elf_sym.st_other != STV_PROTECTED
+	       && elfsym->internal_elf_sym.st_other != STV_HIDDEN
+	       && elfsym->internal_elf_sym.st_other != STV_INTERNAL)
 	      || S_IS_WEAK (fragP->fr_symbol)
-	      || ((symbol_get_bfdsym (fragP->fr_symbol)->flags
-		   & BSF_GNU_INDIRECT_FUNCTION))))
+	      || (bfdsym->flags & BSF_GNU_INDIRECT_FUNCTION)))
 #endif
 #if defined (OBJ_COFF) && defined (TE_PE)
       || (OUTPUT_FLAVOR == bfd_target_coff_flavour

But then I also have this (intentionally compiled out)

--- binutils-2.25/gas/symbols.c
+++ 2.25/gas/symbols.c
@@ -2075,13 +2075,43 @@ S_IS_DEFINED (symbolS *s)
 int
 S_FORCE_RELOC (symbolS *s, int strict)
 {
+#if 0/*defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)*/
+  While generally correct, this and the similarly framed code fragments
+  below cause, if enabled, relocations to be dropped that should not be (i.e.
+  BFD_RELOC_X86_64_GOTPCREL) because the side effects of using a certain
+  relocation type are not accounted for in generic_force_reloc (in write.c)
+  except for the vtable relocations. GOT references, at least, would have
+  to fall into this category too, but there does not seem to be a flag in
+  struct fix to indicate such side effects (maybe fx_plt really is, but then
+  it is grossly misnamed), nor does it seem reasonable to enumerate all such
+  relocations in generic_force_reloc.
+  elf_symbol_type *elfsym = NULL;
+#endif
+
   if (LOCAL_SYMBOL_CHECK (s))
     return ((struct local_symbol *) s)->lsy_section == undefined_section;
 
+#if 0/*defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)*/
+  if (IS_ELF && (s->bsym->flags & BSF_GLOBAL))
+    {
+      asymbol *bfdsym = symbol_get_bfdsym (s);
+
+      elfsym = elf_symbol_from (bfd_asymbol_bfd (bfdsym), bfdsym);
+      gas_assert (elfsym);
+    }
+#endif
+
   return ((strict
 	   && ((s->bsym->flags & BSF_WEAK) != 0
 	       || (EXTERN_FORCE_RELOC
-		   && (s->bsym->flags & BSF_GLOBAL) != 0)))
+		   && (s->bsym->flags & BSF_GLOBAL) != 0
+#if 0/*defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)*/
+		   && (elfsym == NULL
+		       || (elfsym->internal_elf_sym.st_other != STV_PROTECTED
+		           && elfsym->internal_elf_sym.st_other != STV_HIDDEN
+			   && elfsym->internal_elf_sym.st_other != STV_INTERNAL))
+#endif
+		   )))
 	  || (s->bsym->flags & BSF_GNU_INDIRECT_FUNCTION) != 0
 	  || s->bsym->section == undefined_section
 	  || bfd_is_com_section (s->bsym->section));

Jan

  reply	other threads:[~2015-05-07 11:52 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-07  6:02 Andy Lutomirski
2015-05-07 11:52 ` Jan Beulich [this message]
2015-05-07 16:21   ` H.J. Lu
2015-05-08  3:22     ` Andy Lutomirski
2015-05-08 12:09       ` H.J. Lu
2015-05-08 20:16         ` H.J. Lu
2015-05-18 19:36           ` Andy Lutomirski
2015-05-18 20:01             ` H. Peter Anvin
2015-05-18 20:11               ` Borislav Petkov
2015-05-18 20:02             ` H.J. Lu
2015-05-18 20:06               ` H. Peter Anvin
2015-05-18 20:25                 ` H.J. Lu
2015-05-18 20:28                   ` H. Peter Anvin
2015-05-18 20:34                     ` H. Peter Anvin
2015-05-20 20:53                       ` Andy Lutomirski
2015-05-20 20:59                         ` H. Peter Anvin
2015-05-20 21:47                           ` Andy Lutomirski
2015-05-20 21:58                             ` H. Peter Anvin
2015-05-20 22:08                             ` H. Peter Anvin
2015-05-20 22:17                               ` Andy Lutomirski
2015-05-20 22:21                                 ` H. Peter Anvin

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=554B6E0102000078000778D0@mail.emea.novell.com \
    --to=jbeulich@suse.com \
    --cc=binutils@sourceware.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    /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

all inboxes | Powered by JetHome®