public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] ld: provide __ehdr_start magic symbol
@ 2012-06-19 21:29 Roland McGrath
  2012-06-20  2:30 ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: Roland McGrath @ 2012-06-19 21:29 UTC (permalink / raw)
  To: binutils; +Cc: Mark Seaborn

This adds a new linker feature for ELF targets.  It provides
(i.e. defines if referenced) the special symbol "__ehdr_start" to
the place in the address space where the ELF file header appears (if
that's also in the same segment where the program headers appear).
(I also have a patch for gold, which I'll submit separately after
the feature is agreed to and committed for BFD ld.)

This provides a direct way for a program to examine its own headers.
The usual use of this is to look at the program headers, e.g. to find
PT_TLS and use its parameters in TLS initialization.  Today, a program
has to rely on the system program loader to supply the AT_PHDR et al
fields in the auxv.  But it's not possible to rely on that in all
circumstances, and this is something that is statically knowable at
link time, so there's no reason it ought to depend on a runtime feature.

It's also possible to define symbols giving the start and end of the
phdrs image.  But that is inadvisable because of tools that can change
the headers after link time.  For example, prelink will sometimes
insert a new phdr; objcopy can convert an ELFCLASS64 file to an
ELFCLASS32 file, changing the position and size of the phdrs.  In the
usual layouts, where the ehdr and phdrs both appear in the same
PT_LOAD segment, looking at the ehdr to find the phdrs from its
e_phoff field (and verify their size with e_phentsize, etc.) is the
only really robust approach.

I tried to make the definition consistent with the symbols defined
by PROVIDE and similar magic features like __start_*/__stop_*.
That is, STT_NOTYPE, STB_GLOBAL, STV_DEFAULT.

It might be possible to define this symbol entirely in the default
linker script.  But I was not confident that there would be a way to
do this that might not wind up defining it incorrectly in some
situation where the headers are not actually visible in the program
image, e.g. under strange combinations of --section-start et al
options.

The __executable_start symbol winds up with the same value as
__ehdr_start in the usual layouts on the most common targets, but it
does not always match up for all targets or all layout details that
might be used (and doesn't exist at all in shared objects).  So I
think the new symbol really is necessary.

I did find anywhere in ld.texinfo that documents the existing
magical symbols like __executable_start, etext, etc.  So I did
not add any mention of this to the manual.


Ok for trunk?


Thanks,
Roland


bfd/
2012-06-19  Roland McGrath  <mcgrathr@google.com>

	* elf.c (assign_file_positions_for_non_load_sections): Define
	__ehdr_start symbol if it's referenced and there's a PT_LOAD
	segment that covers both the file and program headers.

ld/
2012-06-19  Roland McGrath  <mcgrathr@google.com>

	* NEWS: Mention __ehdr_start.

ld/testsuite/
2012-06-19  Roland McGrath  <mcgrathr@google.com>

	* ld-elf/ehdr_start.s: New file.
	* ld-elf/ehdr_start.d: New file.

diff --git a/bfd/elf.c b/bfd/elf.c
index 0296ef5..99771f5 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -4826,6 +4826,7 @@ assign_file_positions_for_non_load_sections (bfd *abfd,
   Elf_Internal_Phdr *phdrs;
   Elf_Internal_Phdr *p;
   struct elf_segment_map *m;
+  struct elf_segment_map *hdrs_segment;
   bfd_vma filehdr_vaddr, filehdr_paddr;
   bfd_vma phdrs_vaddr, phdrs_paddr;
   file_ptr off;
@@ -4883,6 +4884,7 @@ assign_file_positions_for_non_load_sections (bfd *abfd,
   filehdr_paddr = 0;
   phdrs_vaddr = bed->maxpagesize + bed->s->sizeof_ehdr;
   phdrs_paddr = 0;
+  hdrs_segment = NULL;
   phdrs = elf_tdata (abfd)->phdr;
   for (m = elf_tdata (abfd)->segment_map, p = phdrs;
        m != NULL;
@@ -4903,12 +4905,36 @@ assign_file_positions_for_non_load_sections (bfd *abfd,
 	  phdrs_paddr = p->p_paddr;
 	  if (m->includes_filehdr)
 	    {
+	      hdrs_segment = m;
 	      phdrs_vaddr += bed->s->sizeof_ehdr;
 	      phdrs_paddr += bed->s->sizeof_ehdr;
 	    }
 	}
     }
 
+  if (hdrs_segment != NULL && link_info != NULL)
+    {
+      /* There is a segment that contains both the file headers and the
+	 program headers, so provide a symbol __ehdr_start pointing there.
+	 A program can use this to examine itself robustly.  */
+
+      struct elf_link_hash_entry *hash
+	= elf_link_hash_lookup (elf_hash_table (link_info), "__ehdr_start",
+				FALSE, FALSE, TRUE);
+      if (hash != NULL
+	  && (hash->root.type == bfd_link_hash_new
+	      || hash->root.type == bfd_link_hash_undefined
+	      || hash->root.type == bfd_link_hash_undefweak
+	      || hash->root.type == bfd_link_hash_common))
+	{
+	  hash->root.type = bfd_link_hash_defined;
+	  hash->root.u.def.value = filehdr_vaddr;
+	  hash->root.u.def.section = bfd_abs_section_ptr;
+	  hash->def_regular = 1;
+	  hash->non_elf = 0;
+	}
+    }
+
   for (m = elf_tdata (abfd)->segment_map, p = phdrs;
        m != NULL;
        m = m->next, p++)
diff --git a/ld/NEWS b/ld/NEWS
index cb2d428..8701c5c 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -1,5 +1,9 @@
 -*- text -*-
 
+* Add a linker-provided symbol when producing ELF output, '__ehdr_start'
+  to point to the ELF file header (and nearby program headers) in the
+  program's memory image.
+
 * Add support for S12X processor.
 
 * Add support for the VLE extension to the PowerPC architecture.
diff --git a/ld/testsuite/ld-elf/ehdr_start.d b/ld/testsuite/ld-elf/ehdr_start.d
new file mode 100644
index 0000000..a81577e
--- /dev/null
+++ b/ld/testsuite/ld-elf/ehdr_start.d
@@ -0,0 +1,7 @@
+#source: ehdr_start.s
+#ld:
+#nm: -n
+
+#...
+[0-9a-f]*000 A __ehdr_start
+#pass
diff --git a/ld/testsuite/ld-elf/ehdr_start.s b/ld/testsuite/ld-elf/ehdr_start.s
new file mode 100644
index 0000000..2086801
--- /dev/null
+++ b/ld/testsuite/ld-elf/ehdr_start.s
@@ -0,0 +1,9 @@
+	.text
+	.globl _start
+_start:
+	.space 16
+
+	.section .rodata,"a",%progbits
+	.globl foo
+foo:
+	.dc.a __ehdr_start

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-19 21:29 [PATCH] ld: provide __ehdr_start magic symbol Roland McGrath
@ 2012-06-20  2:30 ` Alan Modra
  2012-06-20 17:04   ` Roland McGrath
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2012-06-20  2:30 UTC (permalink / raw)
  To: Roland McGrath; +Cc: binutils, Mark Seaborn

On Tue, Jun 19, 2012 at 02:28:45PM -0700, Roland McGrath wrote:
> +  if (hdrs_segment != NULL && link_info != NULL)
> +    {
> +      /* There is a segment that contains both the file headers and the
> +	 program headers, so provide a symbol __ehdr_start pointing there.
> +	 A program can use this to examine itself robustly.  */
> +
> +      struct elf_link_hash_entry *hash
> +	= elf_link_hash_lookup (elf_hash_table (link_info), "__ehdr_start",
> +				FALSE, FALSE, TRUE);
> +      if (hash != NULL
> +	  && (hash->root.type == bfd_link_hash_new
> +	      || hash->root.type == bfd_link_hash_undefined
> +	      || hash->root.type == bfd_link_hash_undefweak
> +	      || hash->root.type == bfd_link_hash_common))

For other linker PROVIDEd symbols, we don't test for undefweak.  ie. a
weak reference stays undefined.

> +	{
> +	  hash->root.type = bfd_link_hash_defined;
> +	  hash->root.u.def.value = filehdr_vaddr;
> +	  hash->root.u.def.section = bfd_abs_section_ptr;

I've just recently changed a lot of places where the linker defines
symbols that hold a virtual address to *not* use absolute symbols.
Absolute syms shouldn't be relocated by ld.so.  They are currently,
but if this is ever fixed your __ehdr_start sym won't relocate when a
shared lib or pie is loaded at some address other than the one it was
linked at.

Maybe
	  if (hdrs_segment->count != 0)
	    {
	      asection *s = hdrs_segment->sections[0];
	      hash->root.u.def.value = filehdr_vaddr - s->vma;
	      hash->root.u.def.section = s;
	    }
and look through other elf_segment_map entries for the lowest vma
section if you happen to have ehdr in a segment by itself, falling
back to bfd_abs_section_ptr if there are no sections.

> +	  hash->def_regular = 1;
> +	  hash->non_elf = 0;
> +	}
> +    }
> +

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-20  2:30 ` Alan Modra
@ 2012-06-20 17:04   ` Roland McGrath
  2012-06-21  2:12     ` Alan Modra
  2012-06-21  3:34     ` Alan Modra
  0 siblings, 2 replies; 14+ messages in thread
From: Roland McGrath @ 2012-06-20 17:04 UTC (permalink / raw)
  To: Roland McGrath, binutils, Mark Seaborn

On Tue, Jun 19, 2012 at 7:29 PM, Alan Modra <amodra@gmail.com> wrote:
> For other linker PROVIDEd symbols, we don't test for undefweak.  ie. a
> weak reference stays undefined.

That's not what I see.

	$ cat s.s
	.section frobozz,"a",%progbits
	.weak etext
	.weak __start_frobozz
	.weak __stop_frobozz
	.dc.a __start_frobozz
	.dc.a __stop_frobozz
	.dc.a etext
	$ ./gas/as-new -o s.o s.s
	$ nm s.o
			 w __start_frobozz
			 w __stop_frobozz
			 w etext
	$ ./ld/ld-new -o s s.o
	./ld/ld-new: warning: cannot find entry symbol _start; defaulting to
0000000000400078
	$ nm s
	0000000000601000 R __bss_start
	0000000000400078 A __start_frobozz
	0000000000400090 R __stop_frobozz
	0000000000601000 R _edata
	0000000000601000 R _end
			 U _start
	0000000000400078 R etext
	$

> I've just recently changed a lot of places where the linker defines
> symbols that hold a virtual address to *not* use absolute symbols.
> Absolute syms shouldn't be relocated by ld.so.  They are currently,
> but if this is ever fixed your __ehdr_start sym won't relocate when a
> shared lib or pie is loaded at some address other than the one it was
> linked at.

I guess I'd expect PIC code using this symbol to always use PC-relative or
GOT-relative relocs (i.e. __attribute__ ((visibility ("hidden"))) in C) and
thus get correct link-time resolution.  Using a dynamic reloc for
__ehdr_start is liable to find one belonging to another object that has
sloppy exports.

> Maybe
>          if (hdrs_segment->count != 0)
>            {
>              asection *s = hdrs_segment->sections[0];
>              hash->root.u.def.value = filehdr_vaddr - s->vma;
>              hash->root.u.def.section = s;
>            }
> and look through other elf_segment_map entries for the lowest vma
> section if you happen to have ehdr in a segment by itself, falling
> back to bfd_abs_section_ptr if there are no sections.

The headers won't ever be inside a section, I don't think.  (I don't know
how to write a linker script such that they would be.)  Do you really mean
that it should use a section-relative symbol with a value outside the section?
I don't think that's really kosher ELF (elflint certainly doesn't like it).


Thanks,
Roland

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-20 17:04   ` Roland McGrath
@ 2012-06-21  2:12     ` Alan Modra
  2012-06-21 16:49       ` Roland McGrath
  2012-06-21  3:34     ` Alan Modra
  1 sibling, 1 reply; 14+ messages in thread
From: Alan Modra @ 2012-06-21  2:12 UTC (permalink / raw)
  To: Roland McGrath; +Cc: binutils, Mark Seaborn

On Wed, Jun 20, 2012 at 10:04:17AM -0700, Roland McGrath wrote:
> On Tue, Jun 19, 2012 at 7:29 PM, Alan Modra <amodra@gmail.com> wrote:
> > For other linker PROVIDEd symbols, we don't test for undefweak.  ie. a
> > weak reference stays undefined.
> 
> That's not what I see.

Hmm.  I was looking at ldexp.c:exp_fold_tree_1<etree_provide> where we
have

	      h = bfd_link_hash_lookup (link_info.hash, tree->assign.dst,
					FALSE, FALSE, TRUE);
	      if (h == NULL
		  || (h->type != bfd_link_hash_new
		      && h->type != bfd_link_hash_undefined
		      && h->type != bfd_link_hash_common))
		{
		  /* Do nothing.  The symbol was never referenced, or was
		     defined by some object.  */
		  break;
		}

"we don't test for undefweak" was correct, but not the conclusion that
undef weaks stay undefined.  What happens is that
bfd_record_link_assignment gets in first and flips
bfd_link_hash_undefweak to bfd_link_hash_new..  So I suppose it
doesn't really matter that your code differs from the ldexp.c code.

> The headers won't ever be inside a section, I don't think.

Right.

> Do you really mean
> that it should use a section-relative symbol with a value outside the section?

Yes.

> I don't think that's really kosher ELF (elflint certainly doesn't like it).

IMNSHO, elflint is wrong.  If you accept that it is wrong for ld.so to
relocate absolute symbols, then ld must generate section relative
symbols with values outside of any section in some circumstances.  (Or
ld needs to generate sections to cover gaps in the image, or you need
a special section, SHN_REL say, like SHN_ABS but relocate with the
image.)

I'm not suggesting that glibc ld.so should be changed.  That obviously
can't happen yet, and perhaps never should, but I'd like to eventually
fix GNU ld for all architectures.  We've had bug reports before about
absolute symbols where people want to legitimately use absolute
symbols to mark memory mapped IO or separate address spaces, for sizes
and suchlike.  They can't because ld generates absolute symbols for
normal addresses, and that forces the kernel loader, embedded loader
or whatever, to relocate absolute symbols.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-20 17:04   ` Roland McGrath
  2012-06-21  2:12     ` Alan Modra
@ 2012-06-21  3:34     ` Alan Modra
  1 sibling, 0 replies; 14+ messages in thread
From: Alan Modra @ 2012-06-21  3:34 UTC (permalink / raw)
  To: binutils

On Wed, Jun 20, 2012 at 10:04:17AM -0700, Roland McGrath wrote:
> 	0000000000400078 A __start_frobozz
> 	0000000000400090 R __stop_frobozz

Applied after running usual set of tests.

ld/
	* ldlang.c (lang_insert_orphan): Don't make __start_<sec> symbol
	absolute, and remove unnecessary alignment.
ld/testsuite/
	* ld-gc/start.d: Update.

Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.392
diff -u -p -r1.392 ldlang.c
--- ld/ldlang.c	15 Jun 2012 15:13:32 -0000	1.392
+++ ld/ldlang.c	21 Jun 2012 03:29:03 -0000
@@ -1790,17 +1790,12 @@ lang_insert_orphan (asection *s,
       if (*ps == '\0')
 	{
 	  char *symname;
-	  etree_type *e_align;
 
 	  symname = (char *) xmalloc (ps - secname + sizeof "__start_" + 1);
 	  symname[0] = bfd_get_symbol_leading_char (link_info.output_bfd);
 	  sprintf (symname + (symname[0] != 0), "__start_%s", secname);
-	  e_align = exp_unop (ALIGN_K,
-			      exp_intop ((bfd_vma) 1 << s->alignment_power));
-	  lang_add_assignment (exp_assign (".", e_align));
 	  lang_add_assignment (exp_provide (symname,
-					    exp_unop (ABSOLUTE,
-						      exp_nameop (NAME, ".")),
+					    exp_nameop (NAME, "."),
 					    FALSE));
 	}
     }
Index: ld/testsuite/ld-gc/start.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-gc/start.d,v
retrieving revision 1.4
diff -u -p -r1.4 start.d
--- ld/testsuite/ld-gc/start.d	10 Feb 2011 07:24:05 -0000	1.4
+++ ld/testsuite/ld-gc/start.d	21 Jun 2012 03:29:03 -0000
@@ -5,5 +5,5 @@
 #notarget: *-*-*aout *-*-*oldld frv-*-linux*
 
 #...
-[0-9a-f]+ A +__start__foo
+[0-9a-f]+ D +__start__foo
 #...

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-21  2:12     ` Alan Modra
@ 2012-06-21 16:49       ` Roland McGrath
  2012-06-22  2:24         ` Alan Modra
  2012-06-24 22:03         ` Hans-Peter Nilsson
  0 siblings, 2 replies; 14+ messages in thread
From: Roland McGrath @ 2012-06-21 16:49 UTC (permalink / raw)
  To: Roland McGrath, binutils, Mark Seaborn

On Wed, Jun 20, 2012 at 7:12 PM, Alan Modra <amodra@gmail.com> wrote:
> "we don't test for undefweak" was correct, but not the conclusion that
> undef weaks stay undefined.  What happens is that
> bfd_record_link_assignment gets in first and flips
> bfd_link_hash_undefweak to bfd_link_hash_new..  So I suppose it
> doesn't really matter that your code differs from the ldexp.c code.

I don't understand the phases of the linker enough to know why it matters
differently here than there.  But removing the undefweak test from the
condition I added makes it fail to define __ehdr_start for a weak
reference, so I'm leaving it in.

>> Do you really mean
>> that it should use a section-relative symbol with a value outside the section?
>
> Yes.

I think this notion is quite misguided, but that issue is entirely separate
from what this change is about.  So I'm happy to make my change comport
with the new norm you've established, departure though it is from prior
behavior and specifications.  I'll discuss the section-vs-absolute issue in
a separate thread.


Is this version OK to commit?


Thanks,
Roland


bfd/
2012-06-21  Roland McGrath  <mcgrathr@google.com>

	* elf.c (assign_file_positions_for_non_load_sections): Define
	__ehdr_start symbol if it's referenced and there's a PT_LOAD
	segment that covers both the file and program headers.

ld/
2012-06-21  Roland McGrath  <mcgrathr@google.com>

	* NEWS: Mention __ehdr_start.

ld/testsuite/
2012-06-21  Roland McGrath  <mcgrathr@google.com>

	* ld-elf/ehdr_start.s: New file.
	* ld-elf/ehdr_start.d: New file.

diff --git a/bfd/elf.c b/bfd/elf.c
index 0296ef5..172782a 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -4826,6 +4826,7 @@ assign_file_positions_for_non_load_sections (bfd *abfd,
   Elf_Internal_Phdr *phdrs;
   Elf_Internal_Phdr *p;
   struct elf_segment_map *m;
+  struct elf_segment_map *hdrs_segment;
   bfd_vma filehdr_vaddr, filehdr_paddr;
   bfd_vma phdrs_vaddr, phdrs_paddr;
   file_ptr off;
@@ -4883,6 +4884,7 @@ assign_file_positions_for_non_load_sections (bfd *abfd,
   filehdr_paddr = 0;
   phdrs_vaddr = bed->maxpagesize + bed->s->sizeof_ehdr;
   phdrs_paddr = 0;
+  hdrs_segment = NULL;
   phdrs = elf_tdata (abfd)->phdr;
   for (m = elf_tdata (abfd)->segment_map, p = phdrs;
        m != NULL;
@@ -4903,12 +4905,66 @@ assign_file_positions_for_non_load_sections (bfd *abfd,
 	  phdrs_paddr = p->p_paddr;
 	  if (m->includes_filehdr)
 	    {
+	      hdrs_segment = m;
 	      phdrs_vaddr += bed->s->sizeof_ehdr;
 	      phdrs_paddr += bed->s->sizeof_ehdr;
 	    }
 	}
     }

+  if (hdrs_segment != NULL && link_info != NULL)
+    {
+      /* There is a segment that contains both the file headers and the
+	 program headers, so provide a symbol __ehdr_start pointing there.
+	 A program can use this to examine itself robustly.  */
+
+      struct elf_link_hash_entry *hash
+	= elf_link_hash_lookup (elf_hash_table (link_info), "__ehdr_start",
+				FALSE, FALSE, TRUE);
+      /* If the symbol was referenced and not defined, define it.  */
+      if (hash != NULL
+	  && (hash->root.type == bfd_link_hash_new
+	      || hash->root.type == bfd_link_hash_undefined
+	      || hash->root.type == bfd_link_hash_undefweak
+	      || hash->root.type == bfd_link_hash_common))
+	{
+	  /* The headers never sit inside any section, so this is really a
+	     case for which an absolute symbol (SHN_ABS) is appropriate.
+	     But the norm for linker-provided symbols now is that they are
+	     associated with some section, even if the address is outside
+	     the bounds of that section.  So pick an arbitrary section to
+	     associate this symbol with.  */
+
+	  asection *s = NULL;
+	  if (hdrs_segment->count != 0)
+	    /* The segment contains sections, so use the first one.  */
+	    s = hdrs_segment->sections[0];
+	  else
+	    /* Use the first (i.e. lowest-addressed) section in any segment.  */
+	    for (m = elf_tdata (abfd)->segment_map; m != NULL; m = m->next)
+	      if (m->count != 0)
+		{
+		  s = m->sections[0];
+		  break;
+		}
+
+	  if (s != NULL)
+	    {
+	      hash->root.u.def.value = filehdr_vaddr - s->vma;
+	      hash->root.u.def.section = s;
+	    }
+	  else
+	    {
+	      hash->root.u.def.value = filehdr_vaddr;
+	      hash->root.u.def.section = bfd_abs_section_ptr;
+	    }
+
+	  hash->root.type = bfd_link_hash_defined;
+	  hash->def_regular = 1;
+	  hash->non_elf = 0;
+	}
+    }
+
   for (m = elf_tdata (abfd)->segment_map, p = phdrs;
        m != NULL;
        m = m->next, p++)
diff --git a/ld/NEWS b/ld/NEWS
index cb2d428..8701c5c 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -1,5 +1,9 @@
 -*- text -*-

+* Add a linker-provided symbol when producing ELF output, '__ehdr_start'
+  to point to the ELF file header (and nearby program headers) in the
+  program's memory image.
+
 * Add support for S12X processor.

 * Add support for the VLE extension to the PowerPC architecture.
diff --git a/ld/testsuite/ld-elf/ehdr_start.d b/ld/testsuite/ld-elf/ehdr_start.d
new file mode 100644
index 0000000..3965eeb
--- /dev/null
+++ b/ld/testsuite/ld-elf/ehdr_start.d
@@ -0,0 +1,7 @@
+#source: ehdr_start.s
+#ld:
+#nm: -n
+
+#...
+[0-9a-f]*000 [ADRT] __ehdr_start
+#pass
diff --git a/ld/testsuite/ld-elf/ehdr_start.s b/ld/testsuite/ld-elf/ehdr_start.s
new file mode 100644
index 0000000..2086801
--- /dev/null
+++ b/ld/testsuite/ld-elf/ehdr_start.s
@@ -0,0 +1,9 @@
+	.text
+	.globl _start
+_start:
+	.space 16
+
+	.section .rodata,"a",%progbits
+	.globl foo
+foo:
+	.dc.a __ehdr_start

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-21 16:49       ` Roland McGrath
@ 2012-06-22  2:24         ` Alan Modra
  2012-06-22 16:53           ` Roland McGrath
  2012-06-24 22:03         ` Hans-Peter Nilsson
  1 sibling, 1 reply; 14+ messages in thread
From: Alan Modra @ 2012-06-22  2:24 UTC (permalink / raw)
  To: Roland McGrath; +Cc: binutils, Mark Seaborn

On Thu, Jun 21, 2012 at 09:48:39AM -0700, Roland McGrath wrote:
> I think this notion is quite misguided, but that issue is entirely separate
> from what this change is about.  So I'm happy to make my change comport
> with the new norm you've established, departure though it is from prior
> behavior and specifications.  I'll discuss the section-vs-absolute issue in
> a separate thread.

Please do read some old threads first.  You'll find me trying to justify
existing ld.so behaviour even though I knew it was wrong.

> Is this version OK to commit?

Yes, but without the following comment, which presumes we've had our
discussion and found absolute symbols quite appropriate for virtual
addresses.

> +	  /* The headers never sit inside any section, so this is really a
> +	     case for which an absolute symbol (SHN_ABS) is appropriate.
> +	     But the norm for linker-provided symbols now is that they are
> +	     associated with some section, even if the address is outside
> +	     the bounds of that section.  So pick an arbitrary section to
> +	     associate this symbol with.  */

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-22  2:24         ` Alan Modra
@ 2012-06-22 16:53           ` Roland McGrath
  0 siblings, 0 replies; 14+ messages in thread
From: Roland McGrath @ 2012-06-22 16:53 UTC (permalink / raw)
  To: Roland McGrath, binutils, Mark Seaborn

Committed.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-21 16:49       ` Roland McGrath
  2012-06-22  2:24         ` Alan Modra
@ 2012-06-24 22:03         ` Hans-Peter Nilsson
  2012-06-25 22:36           ` Roland McGrath
  1 sibling, 1 reply; 14+ messages in thread
From: Hans-Peter Nilsson @ 2012-06-24 22:03 UTC (permalink / raw)
  To: mcgrathr; +Cc: binutils, mseaborn

> From: Roland McGrath <mcgrathr@google.com>
> Date: Thu, 21 Jun 2012 18:48:39 +0200

> ld/testsuite/
> 2012-06-21  Roland McGrath  <mcgrathr@google.com>
> 
> 	* ld-elf/ehdr_start.s: New file.
> 	* ld-elf/ehdr_start.d: New file.

This new test fails for arm-unknown-eabi,
mipsisa32r2el-unknown-linux-gnu and cris-axis-elf, probably
others.

For arm-unknown-eabi and cris-axis-elf, the nm regexp just
doesn't match:

Executing on host: sh -c {/tmp/hpautotest-binutils/arm-unknown-eabi/ld/../binutils/nm-new  -n tmpdir/dump > tmpdir/dump.out 2>ld.tmp}  /dev/null  (timeout = 300)
extra regexps in /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-elf/ehdr_start.d starting with "^[0-9a-f]*000 [ADRT] __ehdr_start$"
EOF from tmpdir/dump.out
FAIL: ld-elf/ehdr_start

For mipsisa32r2el-unknown-linux-gnu, the linker complains; I
think you need to add __start:

Executing on host: sh -c {./ld-new  -L/tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-elf   -o tmpdir/dump tmpdir/dump0.o  2>&1}  /dev/null ld.tmp (timeout = 300)
./ld-new: warning: cannot find entry symbol __start; defaulting to 0000000000400090
succeeded with: <./ld-new: warning: cannot find entry symbol __start; defaulting to 0000000000400090>, expected: <>

brgds, H-P

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-24 22:03         ` Hans-Peter Nilsson
@ 2012-06-25 22:36           ` Roland McGrath
  2012-06-26  4:22             ` Hans-Peter Nilsson
  0 siblings, 1 reply; 14+ messages in thread
From: Roland McGrath @ 2012-06-25 22:36 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: binutils, mseaborn

On Sun, Jun 24, 2012 at 3:03 PM, Hans-Peter Nilsson
<hans-peter.nilsson@axis.com> wrote:
> This new test fails for arm-unknown-eabi,
> mipsisa32r2el-unknown-linux-gnu and cris-axis-elf, probably
> others.

For arm-unknown-eabi and cris-axis-elf, the default layout does not put the
headers in the address space, so this test cannot succeed.  I added a
"#notarget:" line for those.  For mipsisa32r2el-unknown-linux-gnu, it was
indeed just the entry symbol warning, which I fixed with an explicit -e.

Here's what I'm committing.


Thanks,
Roland


ld/testsuite/
2012-06-25  Roland McGrath  <mcgrathr@google.com>

	* ld-elf/ehdr_start.d (ld): Add explicit -e _start.
	(notarget): Add arm*-*-eabi* cris-*-*.

diff --git a/ld/testsuite/ld-elf/ehdr_start.d b/ld/testsuite/ld-elf/ehdr_start.d
index 3965eeb..05c6a07 100644
--- a/ld/testsuite/ld-elf/ehdr_start.d
+++ b/ld/testsuite/ld-elf/ehdr_start.d
@@ -1,6 +1,7 @@
 #source: ehdr_start.s
-#ld:
+#ld: -e _start
 #nm: -n
+#notarget: arm*-*-eabi* cris-*-*

 #...
 [0-9a-f]*000 [ADRT] __ehdr_start

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-25 22:36           ` Roland McGrath
@ 2012-06-26  4:22             ` Hans-Peter Nilsson
  2012-06-27  1:19               ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: Hans-Peter Nilsson @ 2012-06-26  4:22 UTC (permalink / raw)
  To: mcgrathr; +Cc: binutils, mseaborn

> From: Roland McGrath <mcgrathr@google.com>
> Date: Tue, 26 Jun 2012 00:36:22 +0200

> On Sun, Jun 24, 2012 at 3:03 PM, Hans-Peter Nilsson
> <hans-peter.nilsson@axis.com> wrote:
> > This new test fails for arm-unknown-eabi,
> > mipsisa32r2el-unknown-linux-gnu and cris-axis-elf, probably
> > others.
> 
> For arm-unknown-eabi and cris-axis-elf, the default layout does not put the
> headers in the address space, so this test cannot succeed.  I added a
> "#notarget:" line for those.

No; you excluded cris-*-*, which also excludes the succeeding
cris-axis-linux-gnu.  Instead only targets expected to hold
phdrs in the address space should be included.  Other such tests
use "#target: *-*-linux* *-*-gnu*", which seems appropriate here
too.

brgds, H-P

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-26  4:22             ` Hans-Peter Nilsson
@ 2012-06-27  1:19               ` Alan Modra
  2012-06-27  1:30                 ` Roland McGrath
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2012-06-27  1:19 UTC (permalink / raw)
  To: binutils; +Cc: mcgrathr

On Tue, Jun 26, 2012 at 06:21:43AM +0200, Hans-Peter Nilsson wrote:
> cris-axis-linux-gnu.  Instead only targets expected to hold
> phdrs in the address space should be included.  Other such tests
> use "#target: *-*-linux* *-*-gnu*", which seems appropriate here
> too.

Good idea.  Current testsuite delta looks like:

arc-elf  +FAIL: ld-elf/ehdr_start
avr-elf  +FAIL: ld-elf/ehdr_start
bfin-elf  +FAIL: ld-elf/ehdr_start
cr16-elf  +FAIL: ld-elf/ehdr_start
crx-elf  +FAIL: ld-elf/ehdr_start
d10v-elf  +FAIL: ld-elf/ehdr_start
d30v-elf  +FAIL: ld-elf/ehdr_start
dlx-elf  +FAIL: ld-elf/ehdr_start
fr30-elf  +FAIL: ld-elf/ehdr_start
frv-elf  +FAIL: ld-elf/ehdr_start
frv-linux  +FAIL: ld-elf/ehdr_start
h8300-elf  +FAIL: ld-elf/ehdr_start
i386-linuxaout  +FAIL: rep prefix on ret
i586-aout  +FAIL: rep prefix on ret
i586-coff  +FAIL: rep prefix on ret
i686-pe  +FAIL: rep prefix on ret
i960-elf  +FAIL: ld-elf/ehdr_start
ip2k-elf  +FAIL: ld-elf/ehdr_start
iq2000-elf  +FAIL: ld-elf/ehdr_start
lm32-elf  +FAIL: ld-elf/ehdr_start
m32c-elf  +FAIL: ld-elf/ehdr_start
m32r-elf  +FAIL: ld-elf/ehdr_start
mcore-elf  +FAIL: ld-elf/ehdr_start
mep-elf  +FAIL: ld-elf/ehdr_start
microblaze-elf  +FAIL: ld-elf/ehdr_start
mn10200-elf  +FAIL: ld-elf/ehdr_start
mn10300-elf  +FAIL: ld-elf/ehdr_start
ms1-elf  +FAIL: ld-elf/ehdr_start
msp430-elf  +FAIL: ld-elf/ehdr_start
or32-elf  +FAIL: ld-elf/ehdr_start
pj-elf  +FAIL: ld-elf/ehdr_start
rx-elf  +FAIL: ld-elf/ehdr_start
sh64-elf  +FAIL: ld-elf/ehdr_start
sh-rtems  +FAIL: ld-elf/ehdr_start
spu-elf  +FAIL: ld-elf/ehdr_start
tx39-elf  +FAIL: ld-elf/ehdr_start
v850-elf  +FAIL: ld-elf/ehdr_start
x86_64-mingw32  +FAIL: rep prefix on ret
xstormy16-elf  +FAIL: ld-elf/ehdr_start

The frv-linux failure for ehdr_start is
tmpdir/dump0.o: In function `foo':
(.rodata+0x0): cannot emit fixups in read-only section
./ld-new: final link failed: Nonrepresentable section on output

The "rep prefix on ret" failures are all nop padding like:
extra lines in dump.out starting with "^   2:   90                      nop$"
EOF from /src/binutils-current/gas/testsuite/gas/i386/rep-ret.d

gas/testsuite/
	* gas/i386/rep-ret.s: Zero pad section.
	* gas/i386/rep-ret.d: Update.
ld/testsuite/
	* ld-elf/ehdr_start.s: Use data rather than rodata.
	* ld-elf/ehdr_start.d: Run on linux and gnu targets only.

The above fixes the failures, except for the frv-linux one, which now
fails with

./ld-new: BFD (GNU Binutils) 2.22.52.20120627 assertion fail /src/binutils-current/bfd/elf32-frv.c:1333
./ld-new: BFD (GNU Binutils) 2.22.52.20120627 assertion fail /src/binutils-current/bfd/elf32-frv.c:1324
LINKER BUG: .rofixup section size mismatch
./ld-new: final link failed: Nonrepresentable section on output


Index: gas/testsuite/gas/i386/rep-ret.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/i386/rep-ret.d,v
retrieving revision 1.1
diff -u -p -r1.1 rep-ret.d
--- gas/testsuite/gas/i386/rep-ret.d	22 Jun 2012 21:54:05 -0000	1.1
+++ gas/testsuite/gas/i386/rep-ret.d	27 Jun 2012 00:45:38 -0000
@@ -7,3 +7,4 @@ Disassembly of section .text:
 
 0+000 <foo>:
 \s*[0-9a-f]+:\s+f3 c3\s+repz ret\s*
+	\.\.\.
Index: gas/testsuite/gas/i386/rep-ret.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/i386/rep-ret.s,v
retrieving revision 1.1
diff -u -p -r1.1 rep-ret.s
--- gas/testsuite/gas/i386/rep-ret.s	22 Jun 2012 21:54:05 -0000	1.1
+++ gas/testsuite/gas/i386/rep-ret.s	27 Jun 2012 00:45:38 -0000
@@ -1,2 +1,3 @@
 	.text
 foo:	rep ret
+	.p2align 4,0
Index: ld/testsuite/ld-elf/ehdr_start.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-elf/ehdr_start.d,v
retrieving revision 1.2
diff -u -p -r1.2 ehdr_start.d
--- ld/testsuite/ld-elf/ehdr_start.d	25 Jun 2012 22:36:21 -0000	1.2
+++ ld/testsuite/ld-elf/ehdr_start.d	27 Jun 2012 00:45:40 -0000
@@ -1,7 +1,7 @@
 #source: ehdr_start.s
 #ld: -e _start
 #nm: -n
-#notarget: arm*-*-eabi* cris-*-*
+#target: *-*-linux* *-*-gnu*
 
 #...
 [0-9a-f]*000 [ADRT] __ehdr_start
Index: ld/testsuite/ld-elf/ehdr_start.s
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-elf/ehdr_start.s,v
retrieving revision 1.1
diff -u -p -r1.1 ehdr_start.s
--- ld/testsuite/ld-elf/ehdr_start.s	22 Jun 2012 16:52:33 -0000	1.1
+++ ld/testsuite/ld-elf/ehdr_start.s	27 Jun 2012 00:45:40 -0000
@@ -3,7 +3,7 @@
 _start:
 	.space 16
 
-	.section .rodata,"a",%progbits
+	.data
 	.globl foo
 foo:
 	.weak __ehdr_start

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-27  1:19               ` Alan Modra
@ 2012-06-27  1:30                 ` Roland McGrath
  2012-06-27  2:32                   ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: Roland McGrath @ 2012-06-27  1:30 UTC (permalink / raw)
  To: binutils, mcgrathr

On Tue, Jun 26, 2012 at 6:19 PM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Jun 26, 2012 at 06:21:43AM +0200, Hans-Peter Nilsson wrote:
> > cris-axis-linux-gnu.  Instead only targets expected to hold
> > phdrs in the address space should be included.  Other such tests
> > use "#target: *-*-linux* *-*-gnu*", which seems appropriate here
> > too.

I had thought that the original canonical ELF layouts with headers in text
were the norm and there would be only a few outliers.  I guess there are
many.  It's certainly far from being only *-*-linux* and *-*-gnu* that have this
layout, though.

> gas/testsuite/
>        * gas/i386/rep-ret.s: Zero pad section.
>        * gas/i386/rep-ret.d: Update.
> ld/testsuite/
>        * ld-elf/ehdr_start.s: Use data rather than rodata.
>        * ld-elf/ehdr_start.d: Run on linux and gnu targets only.

I added *-*-nacl* too, since that's the target whose needs motivated
the feature.

> The above fixes the failures, except for the frv-linux one, which now
> fails with
>
> ./ld-new: BFD (GNU Binutils) 2.22.52.20120627 assertion fail /src/binutils-current/bfd/elf32-frv.c:1333
> ./ld-new: BFD (GNU Binutils) 2.22.52.20120627 assertion fail /src/binutils-current/bfd/elf32-frv.c:1324
> LINKER BUG: .rofixup section size mismatch
> ./ld-new: final link failed: Nonrepresentable section on output

No idea what that might be about.  The test is quite simple.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ld: provide __ehdr_start magic symbol
  2012-06-27  1:30                 ` Roland McGrath
@ 2012-06-27  2:32                   ` Alan Modra
  0 siblings, 0 replies; 14+ messages in thread
From: Alan Modra @ 2012-06-27  2:32 UTC (permalink / raw)
  To: Roland McGrath; +Cc: binutils, aoliva, brolley

On Tue, Jun 26, 2012 at 06:30:15PM -0700, Roland McGrath wrote:
> I added *-*-nacl* too, since that's the target whose needs motivated
> the feature.

Oops, sorry, didn't mean to take off the main target of interest.

> > The above fixes the failures, except for the frv-linux one, which now
> > fails with
> >
> > ./ld-new: BFD (GNU Binutils) 2.22.52.20120627 assertion fail /src/binutils-current/bfd/elf32-frv.c:1333
> > ./ld-new: BFD (GNU Binutils) 2.22.52.20120627 assertion fail /src/binutils-current/bfd/elf32-frv.c:1324
> > LINKER BUG: .rofixup section size mismatch
> > ./ld-new: final link failed: Nonrepresentable section on output
> 
> No idea what that might be about.  The test is quite simple.

Yeah, that's a target problem of some sort.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-06-27  2:32 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-19 21:29 [PATCH] ld: provide __ehdr_start magic symbol Roland McGrath
2012-06-20  2:30 ` Alan Modra
2012-06-20 17:04   ` Roland McGrath
2012-06-21  2:12     ` Alan Modra
2012-06-21 16:49       ` Roland McGrath
2012-06-22  2:24         ` Alan Modra
2012-06-22 16:53           ` Roland McGrath
2012-06-24 22:03         ` Hans-Peter Nilsson
2012-06-25 22:36           ` Roland McGrath
2012-06-26  4:22             ` Hans-Peter Nilsson
2012-06-27  1:19               ` Alan Modra
2012-06-27  1:30                 ` Roland McGrath
2012-06-27  2:32                   ` Alan Modra
2012-06-21  3:34     ` Alan Modra

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).