public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [gold commit] Fix handling of __ehdr_start
@ 2014-05-02 23:30 Cary Coutant
  2014-05-06 21:51 ` Cary Coutant
  2014-05-20  6:22 ` Alan Modra
  0 siblings, 2 replies; 9+ messages in thread
From: Cary Coutant @ 2014-05-02 23:30 UTC (permalink / raw)
  To: binutils

This patch fixes gold's treatment of the special __ehdr_start
symbol to better match Gnu ld. The description of the problem
and subsequent discussion can be found here:

  https://sourceware.org/ml/binutils/2014-03/msg00309.html

and continuing into April:

  https://sourceware.org/ml/binutils/2014-04/msg00005.html

-cary


commit bd7babc4f78060d51442b5e2da82786a80f98ee5
Author: Cary Coutant <ccoutant@google.com>
Date:   Wed Apr 2 14:21:14 2014 -0700

    Fix handling of __ehdr_start when it cannot be defined.
    
    2014-05-02  Cary Coutant  <ccoutant@google.com>
    
    	* defstd.cc (in_segment): Define __ehdr_start here...
    	* layout.cc (Layout::finalize): ...Instead of here.  Set the
    	output segment when known.
    	* resolve.cc (Symbol::override_base_with_special): Remember
    	the original binding.
    	* symtab.cc (Symbol::set_output_segment): New function.
    	(Symbol::set_undefined): New function.
    	* symtab.h (Symbol::is_weak_undefined): Check original undef
    	binding.
    	(Symbol::is_strong_undefined): New function.
    	(Symbol::set_output_segment): New function.
    	(Symbol::set_undefined): New function.
    	* target-reloc.h (is_strong_undefined): Remove.
    	(issue_undefined_symbol_error): Call Symbol::is_weak_undefined.
    	Check for hidden undefs.
    	(relocate_section): Call Symbol::is_strong_undefined.
    
    	* testsuite/Makefile.am (ehdr_start_test_1)
    	(ehdr_start_test_2, ehdr_start_test_3)
    	(ehdr_start_test_4, ehdr_start_test_5): New test cases.
    	* testsuite/Makefile.in: Regenerate.
    	* testsuite/ehdr_start_def.cc: New source file.
    	* testsuite/ehdr_start_test.cc: New source file.
    	* testsuite/ehdr_start_test.t: New linker script.
    	* testsuite/ehdr_start_test_4.sh: New shell script.

diff --git a/gold/defstd.cc b/gold/defstd.cc
index a50e75d..cee68a0 100644
--- a/gold/defstd.cc
+++ b/gold/defstd.cc
@@ -141,6 +141,20 @@ const Define_symbol_in_segment in_segment[] =
     true			// only_if_ref
   },
   {
+    "__ehdr_start",		// name
+    elfcpp::PT_LOAD,		// segment_type
+    elfcpp::PF(0),		// segment_flags_set
+    elfcpp::PF(0),		// segment_flags_clear
+    0,				// value
+    0,				// size
+    elfcpp::STT_NOTYPE,		// type
+    elfcpp::STB_GLOBAL,		// binding
+    elfcpp::STV_HIDDEN,		// visibility
+    0,				// nonvis
+    Symbol::SEGMENT_START,	// offset_from_base
+    true			// only_if_ref
+  },
+  {
     "etext",			// name
     elfcpp::PT_LOAD,		// segment_type
     elfcpp::PF_X,		// segment_flags_set
diff --git a/gold/layout.cc b/gold/layout.cc
index 02f691e..147f740 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -2749,12 +2749,14 @@ Layout::finalize(const Input_objects* input_objects, Symbol_table* symtab,
   // If there is a load segment that contains the file and program headers,
   // provide a symbol __ehdr_start pointing there.
   // A program can use this to examine itself robustly.
-  if (load_seg != NULL)
-    symtab->define_in_output_segment("__ehdr_start", NULL,
-				     Symbol_table::PREDEFINED, load_seg, 0, 0,
-				     elfcpp::STT_NOTYPE, elfcpp::STB_GLOBAL,
-				     elfcpp::STV_HIDDEN, 0,
-				     Symbol::SEGMENT_START, true);
+  Symbol *ehdr_start = symtab->lookup("__ehdr_start");
+  if (ehdr_start != NULL && ehdr_start->is_predefined())
+    {
+      if (load_seg != NULL)
+	ehdr_start->set_output_segment(load_seg, Symbol::SEGMENT_START);
+      else
+        ehdr_start->set_undefined();
+    }
 
   // Set the file offsets of all the non-data sections we've seen so
   // far which don't have to wait for the input sections.  We need
diff --git a/gold/resolve.cc b/gold/resolve.cc
index 9b442e2..8cc637a 100644
--- a/gold/resolve.cc
+++ b/gold/resolve.cc
@@ -915,6 +915,10 @@ Symbol::override_base_with_special(const Symbol* from)
   bool same_name = this->name_ == from->name_;
   gold_assert(same_name || this->has_alias());
 
+  // If we are overriding an undef, remember the original binding.
+  if (this->is_undefined())
+    this->set_undef_binding(this->binding_);
+
   this->source_ = from->source_;
   switch (from->source_)
     {
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 1a69f5b..4e8afb1 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -527,6 +527,31 @@ Symbol::set_output_section(Output_section* os)
     }
 }
 
+// Set the symbol's output segment.  This is used for pre-defined
+// symbols whose segments aren't known until after layout is done
+// (e.g., __ehdr_start).
+
+void
+Symbol::set_output_segment(Output_segment* os, Segment_offset_base base)
+{
+  gold_assert(this->is_predefined_);
+  this->source_ = IN_OUTPUT_SEGMENT;
+  this->u_.in_output_segment.output_segment = os;
+  this->u_.in_output_segment.offset_base = base;
+}
+
+// Set the symbol to undefined.  This is used for pre-defined
+// symbols whose segments aren't known until after layout is done
+// (e.g., __ehdr_start).
+
+void
+Symbol::set_undefined()
+{
+  gold_assert(this->is_predefined_);
+  this->source_ = IS_UNDEFINED;
+  this->is_predefined_ = false;
+}
+
 // Class Symbol_table.
 
 Symbol_table::Symbol_table(unsigned int count,
diff --git a/gold/symtab.h b/gold/symtab.h
index b06c7b4..b6366d4 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -238,7 +238,7 @@ class Symbol
   override_visibility(elfcpp::STV);
 
   // Set whether the symbol was originally a weak undef or a regular undef
-  // when resolved by a dynamic def.
+  // when resolved by a dynamic def or by a special symbol.
   inline void
   set_undef_binding(elfcpp::STB bind)
   {
@@ -249,7 +249,8 @@ class Symbol
       }
   }
 
-  // Return TRUE if a weak undef was resolved by a dynamic def.
+  // Return TRUE if a weak undef was resolved by a dynamic def or
+  // by a special symbol.
   inline bool
   is_undef_binding_weak() const
   { return this->undef_binding_weak_; }
@@ -517,7 +518,20 @@ class Symbol
   // Return whether this is a weak undefined symbol.
   bool
   is_weak_undefined() const
-  { return this->is_undefined() && this->binding() == elfcpp::STB_WEAK; }
+  {
+    return (this->is_undefined()
+	    && (this->binding() == elfcpp::STB_WEAK
+		|| this->is_undef_binding_weak()));
+  }
+
+  // Return whether this is a strong undefined symbol.
+  bool
+  is_strong_undefined() const
+  {
+    return (this->is_undefined()
+	    && this->binding() != elfcpp::STB_WEAK
+	    && !this->is_undef_binding_weak());
+  }
 
   // Return whether this is an absolute symbol.
   bool
@@ -782,6 +796,18 @@ class Symbol
   void
   set_output_section(Output_section*);
 
+  // Set the symbol's output segment.  This is used for pre-defined
+  // symbols whose segments aren't known until after layout is done
+  // (e.g., __ehdr_start).
+  void
+  set_output_segment(Output_segment*, Segment_offset_base);
+
+  // Set the symbol to undefined.  This is used for pre-defined
+  // symbols whose segments aren't known until after layout is done
+  // (e.g., __ehdr_start).
+  void
+  set_undefined();
+
   // Return whether there should be a warning for references to this
   // symbol.
   bool
@@ -1030,7 +1056,7 @@ class Symbol
   // True if UNDEF_BINDING_WEAK_ has been set (bit 32).
   bool undef_binding_set_ : 1;
   // True if this symbol was a weak undef resolved by a dynamic def
-  // (bit 33).
+  // or by a special symbol (bit 33).
   bool undef_binding_weak_ : 1;
   // True if this symbol is a predefined linker symbol (bit 34).
   bool is_predefined_ : 1;
diff --git a/gold/target-reloc.h b/gold/target-reloc.h
index f49020a..e44519b 100644
--- a/gold/target-reloc.h
+++ b/gold/target-reloc.h
@@ -143,12 +143,6 @@ class Default_comdat_behavior
   }
 };
 
-inline bool
-is_strong_undefined(const Symbol* sym)
-{
-  return sym->is_undefined() && sym->binding() != elfcpp::STB_WEAK;
-}
-
 // Give an error for a symbol with non-default visibility which is not
 // defined locally.
 
@@ -190,7 +184,7 @@ issue_undefined_symbol_error(const Symbol* sym)
     return false;
 
   // We don't report weak symbols.
-  if (sym->binding() == elfcpp::STB_WEAK)
+  if (sym->is_weak_undefined())
     return false;
 
   // We don't report symbols defined in discarded sections.
@@ -216,6 +210,10 @@ issue_undefined_symbol_error(const Symbol* sym)
 	return false;
     }
 
+  // If the symbol is hidden, report it.
+  if (sym->visibility() == elfcpp::STV_HIDDEN)
+    return true;
+
   // When creating a shared library, only report unresolved symbols if
   // -z defs was used.
   if (parameters->options().shared() && !parameters->options().defs())
@@ -419,7 +417,7 @@ relocate_section(
 	gold_undefined_symbol_at_location(sym, relinfo, i, offset);
       else if (sym != NULL
 	       && sym->visibility() != elfcpp::STV_DEFAULT
-	       && (is_strong_undefined(sym) || sym->is_from_dynobj()))
+	       && (sym->is_strong_undefined() || sym->is_from_dynobj()))
 	visibility_error(sym);
 
       if (sym != NULL && sym->has_warning())
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 52cc05e..1f275b0 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -2199,6 +2199,50 @@ gdb_index_test_4.stdout: gdb_index_test_4
 
 endif HAVE_PUBNAMES
 
+# Test that __ehdr_start is defined correctly.
+check_PROGRAMS += ehdr_start_test_1
+ehdr_start_test_1_SOURCES = ehdr_start_test.cc
+ehdr_start_test_1_DEPENDENCIES = gcctestdir/ld
+ehdr_start_test_1_CXXFLAGS =
+ehdr_start_test_1_LDFLAGS = -Bgcctestdir/
+ehdr_start_test_1_LDADD =
+
+# Test that __ehdr_start is defined correctly with a weak reference.
+check_PROGRAMS += ehdr_start_test_2
+ehdr_start_test_2_SOURCES = ehdr_start_test.cc
+ehdr_start_test_2_DEPENDENCIES = gcctestdir/ld
+ehdr_start_test_2_CXXFLAGS = -DEHDR_START_WEAK
+ehdr_start_test_2_LDFLAGS = -Bgcctestdir/
+ehdr_start_test_2_LDADD =
+
+# Test that __ehdr_start is defined correctly when used with a linker script.
+check_PROGRAMS += ehdr_start_test_3
+ehdr_start_test_3_SOURCES = ehdr_start_test.cc
+ehdr_start_test_3_DEPENDENCIES = gcctestdir/ld $(srcdir)/ehdr_start_test.t
+ehdr_start_test_3_CXXFLAGS = -DEHDR_START_WEAK
+ehdr_start_test_3_LDFLAGS = -Bgcctestdir/ -Wl,-T,$(srcdir)/ehdr_start_test.t
+ehdr_start_test_3_LDADD =
+
+# Test that __ehdr_start is left undefined when the text segment is not
+# appropriately aligned.
+check_SCRIPTS += ehdr_start_test_4.sh
+check_DATA += ehdr_start_test_4.syms
+MOSTLYCLEANFILES += ehdr_start_test_4
+ehdr_start_test_4.syms: ehdr_start_test_4
+	$(TEST_NM) ehdr_start_test_4 > $@
+ehdr_start_test_4: ehdr_start_test_4.o gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ld/ -Wl,-Ttext=0x100100 $<
+ehdr_start_test_4.o: ehdr_start_test.cc
+	$(CXXCOMPILE) -c -DEHDR_START_WEAK -o $@ $<
+
+# Test that __ehdr_start is not overridden when supplied by the user.
+check_PROGRAMS += ehdr_start_test_5
+ehdr_start_test_5_SOURCES = ehdr_start_test.cc ehdr_start_def.cc
+ehdr_start_test_5_DEPENDENCIES = gcctestdir/ld
+ehdr_start_test_5_CXXFLAGS = -DEHDR_START_USER_DEF
+ehdr_start_test_5_LDFLAGS = -Bgcctestdir/
+ehdr_start_test_5_LDADD =
+
 # End-to-end incremental linking tests.
 # Incremental linking is currently supported only on the x86_64 target.
 
diff --git a/gold/testsuite/ehdr_start_def.cc b/gold/testsuite/ehdr_start_def.cc
new file mode 100644
index 0000000..f102a78
--- /dev/null
+++ b/gold/testsuite/ehdr_start_def.cc
@@ -0,0 +1,26 @@
+// ehdr_start_def.cc -- test for __ehdr_start linker-defined symbol.
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+// Written by Cary Coutant <ccoutant@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// We provide a user-defined __ehdr_start, to make sure that the
+// linker does not override this with the linker-defined symbol.
+
+char __ehdr_start[] = { 'a', 'b', 'c', 'd' };
diff --git a/gold/testsuite/ehdr_start_test.cc b/gold/testsuite/ehdr_start_test.cc
new file mode 100644
index 0000000..a119b5e
--- /dev/null
+++ b/gold/testsuite/ehdr_start_test.cc
@@ -0,0 +1,67 @@
+// ehdr_start_test.cc -- test for __ehdr_start linker-defined symbol.
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+// Written by Cary Coutant <ccoutant@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to produce as many different types of
+// relocations as we can in a stand-alone program that does not use
+// TLS.  This program is compiled without optimization.
+
+#include "config.h"
+
+#include <cassert>
+#include <cstdio>
+
+#include "elfcpp.h"
+
+#ifdef EHDR_START_WEAK
+#define WEAK_ATTR __attribute__ ((weak))
+#else
+#define WEAK_ATTR
+#endif
+
+extern char __ehdr_start[] WEAK_ATTR;
+
+int
+main() {
+  printf("&__ehdr_start = %p\n", &__ehdr_start);
+
+#ifdef EHDR_START_UNDEF
+  assert(&__ehdr_start == 0);
+#else
+  assert(&__ehdr_start != NULL);
+
+  printf("ELF header: \\x%02x%c%c%c\n", __ehdr_start[0], __ehdr_start[1],
+	 __ehdr_start[2], __ehdr_start[3]);
+#ifdef EHDR_START_USER_DEF
+  assert(__ehdr_start[0] == 'a'
+	 && __ehdr_start[1] == 'b'
+	 && __ehdr_start[2] == 'c'
+	 && __ehdr_start[3] == 'd');
+#else
+  assert(__ehdr_start[elfcpp::EI_MAG0] == elfcpp::ELFMAG0
+	 && __ehdr_start[elfcpp::EI_MAG1] == elfcpp::ELFMAG1
+	 && __ehdr_start[elfcpp::EI_MAG2] == elfcpp::ELFMAG2
+	 && __ehdr_start[elfcpp::EI_MAG3] == elfcpp::ELFMAG3);
+#endif
+#endif
+
+  return 0;
+}
diff --git a/gold/testsuite/ehdr_start_test.t b/gold/testsuite/ehdr_start_test.t
new file mode 100644
index 0000000..50daa64
--- /dev/null
+++ b/gold/testsuite/ehdr_start_test.t
@@ -0,0 +1,42 @@
+/* ehdr_start_test.t -- __ehdr_start test for gold
+
+   Copyright (C) 2008-2014 Free Software Foundation, Inc.
+   Written by Ian Lance Taylor <iant@google.com>.
+
+   This file is part of gold.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+/* With luck this will work on all platforms.  */
+
+SECTIONS
+{
+  /* Set the text segment to start on a non-page boundary.  */
+  . = 0x10000040;
+
+  .text : { *(.text) }
+  . += 0x100000;
+  . = ALIGN(0x100);
+
+  .tdata : { *(.tdata .tdata.* .gnu.linkonce.td.*) }
+  .tbss : { *(.tbss .tbss.* .gnu.linkonce.tb.*) *(.tcommon) }
+  .data.rel.ro : { *(.data.rel.ro.local* .gnu.linkonce.d.rel.ro.local.*)
+		   *(.data.rel.ro* .gnu.linkonce.d.rel.ro.*) }
+  .dynamic : { *(.dynamic) }
+  .got : { *(.got) }
+  .got.plt : { *(.got.plt) }
+  .data : { *(.data .data.* .gnu.linkonce.d.*) }
+}
diff --git a/gold/testsuite/ehdr_start_test_4.sh b/gold/testsuite/ehdr_start_test_4.sh
new file mode 100755
index 0000000..ad0a5a7
--- /dev/null
+++ b/gold/testsuite/ehdr_start_test_4.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+# ehdr_start_test_4.sh -- test that __ehdr_start symbol is undefined.
+
+# Copyright (C) 2014 Free Software Foundation, Inc.
+# Written by Cary Coutant <ccoutant@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+check()
+{
+    if ! grep -q "$2" "$1"
+    then
+	echo "Did not find expected symbol in $1:"
+	echo "   $2"
+	echo ""
+	echo "Actual output below:"
+	cat "$1"
+	exit 1
+    fi
+}
+
+check ehdr_start_test_4.syms "w __ehdr_start"
+
+exit 0

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

* Re: [gold commit] Fix handling of __ehdr_start
  2014-05-02 23:30 [gold commit] Fix handling of __ehdr_start Cary Coutant
@ 2014-05-06 21:51 ` Cary Coutant
  2014-05-20  6:22 ` Alan Modra
  1 sibling, 0 replies; 9+ messages in thread
From: Cary Coutant @ 2014-05-06 21:51 UTC (permalink / raw)
  To: Binutils

I've backported this patch to the binutils-2.24 branch.

-cary

>     Fix handling of __ehdr_start when it cannot be defined.
>
>     2014-05-02  Cary Coutant  <ccoutant@google.com>
>
>         * defstd.cc (in_segment): Define __ehdr_start here...
>         * layout.cc (Layout::finalize): ...Instead of here.  Set the
>         output segment when known.
>         * resolve.cc (Symbol::override_base_with_special): Remember
>         the original binding.
>         * symtab.cc (Symbol::set_output_segment): New function.
>         (Symbol::set_undefined): New function.
>         * symtab.h (Symbol::is_weak_undefined): Check original undef
>         binding.
>         (Symbol::is_strong_undefined): New function.
>         (Symbol::set_output_segment): New function.
>         (Symbol::set_undefined): New function.
>         * target-reloc.h (is_strong_undefined): Remove.
>         (issue_undefined_symbol_error): Call Symbol::is_weak_undefined.
>         Check for hidden undefs.
>         (relocate_section): Call Symbol::is_strong_undefined.
>
>         * testsuite/Makefile.am (ehdr_start_test_1)
>         (ehdr_start_test_2, ehdr_start_test_3)
>         (ehdr_start_test_4, ehdr_start_test_5): New test cases.
>         * testsuite/Makefile.in: Regenerate.
>         * testsuite/ehdr_start_def.cc: New source file.
>         * testsuite/ehdr_start_test.cc: New source file.
>         * testsuite/ehdr_start_test.t: New linker script.
>         * testsuite/ehdr_start_test_4.sh: New shell script.

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

* Re: [gold commit] Fix handling of __ehdr_start
  2014-05-02 23:30 [gold commit] Fix handling of __ehdr_start Cary Coutant
  2014-05-06 21:51 ` Cary Coutant
@ 2014-05-20  6:22 ` Alan Modra
  2014-05-27 18:40   ` Cary Coutant
  1 sibling, 1 reply; 9+ messages in thread
From: Alan Modra @ 2014-05-20  6:22 UTC (permalink / raw)
  To: Cary Coutant; +Cc: binutils

On Fri, May 02, 2014 at 04:30:09PM -0700, Cary Coutant wrote:
>     	* testsuite/ehdr_start_test_4.sh: New shell script.

This test has been failing for me.  Shouldn't it be expecting an
undefined __ehdr_start?

	* testsuite/ehdr_start_test_4.sh: Expect undefined sym.

diff --git a/gold/testsuite/ehdr_start_test_4.sh b/gold/testsuite/ehdr_start_test_4.sh
index ad0a5a7..5e3d20f 100755
--- a/gold/testsuite/ehdr_start_test_4.sh
+++ b/gold/testsuite/ehdr_start_test_4.sh
@@ -35,6 +35,6 @@ check()
     fi
 }
 
-check ehdr_start_test_4.syms "w __ehdr_start"
+check ehdr_start_test_4.syms "U __ehdr_start"
 
 exit 0

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [gold commit] Fix handling of __ehdr_start
  2014-05-20  6:22 ` Alan Modra
@ 2014-05-27 18:40   ` Cary Coutant
  2014-05-28  1:55     ` Alan Modra
  0 siblings, 1 reply; 9+ messages in thread
From: Cary Coutant @ 2014-05-27 18:40 UTC (permalink / raw)
  To: Cary Coutant, Binutils

On Mon, May 19, 2014 at 11:22 PM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, May 02, 2014 at 04:30:09PM -0700, Cary Coutant wrote:
>>       * testsuite/ehdr_start_test_4.sh: New shell script.
>
> This test has been failing for me.  Shouldn't it be expecting an
> undefined __ehdr_start?
>
>         * testsuite/ehdr_start_test_4.sh: Expect undefined sym.
>
> diff --git a/gold/testsuite/ehdr_start_test_4.sh b/gold/testsuite/ehdr_start_test_4.sh
> index ad0a5a7..5e3d20f 100755
> --- a/gold/testsuite/ehdr_start_test_4.sh
> +++ b/gold/testsuite/ehdr_start_test_4.sh
> @@ -35,6 +35,6 @@ check()
>      fi
>  }
>
> -check ehdr_start_test_4.syms "w __ehdr_start"
> +check ehdr_start_test_4.syms "U __ehdr_start"
>
>  exit 0

It should show as a weak undef. What platform are you testing on?

Maybe I need to change the pattern to "[wU] __ehdr_start"?

-cary

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

* Re: [gold commit] Fix handling of __ehdr_start
  2014-05-27 18:40   ` Cary Coutant
@ 2014-05-28  1:55     ` Alan Modra
  2014-05-28  7:26       ` Alan Modra
  0 siblings, 1 reply; 9+ messages in thread
From: Alan Modra @ 2014-05-28  1:55 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Binutils

On Tue, May 27, 2014 at 11:40:28AM -0700, Cary Coutant wrote:
> It should show as a weak undef.

Hmm, yes.  I'm seeing this instead:
    52: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND __ehdr_start

> What platform are you testing on?

powerpc64-linux.  I'll see if I can track down the problem when I find
some time.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [gold commit] Fix handling of __ehdr_start
  2014-05-28  1:55     ` Alan Modra
@ 2014-05-28  7:26       ` Alan Modra
  2014-05-29 19:02         ` Cary Coutant
  0 siblings, 1 reply; 9+ messages in thread
From: Alan Modra @ 2014-05-28  7:26 UTC (permalink / raw)
  To: Cary Coutant, Binutils

On Wed, May 28, 2014 at 11:25:27AM +0930, Alan Modra wrote:
> On Tue, May 27, 2014 at 11:40:28AM -0700, Cary Coutant wrote:
> > It should show as a weak undef.
> 
> Hmm, yes.  I'm seeing this instead:
>     52: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND __ehdr_start
> 
> > What platform are you testing on?
> 
> powerpc64-linux.  I'll see if I can track down the problem when I find
> some time.

Also happens on x86_64-linux.  I'm surprised you haven't seen it.
__ehdr_start is STV_HIDDEN, Symbol_table::override_with_special makes
it forced_local, thus output from Symbol_table::sized_write_symbol
is STB_LOCAL.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [gold commit] Fix handling of __ehdr_start
  2014-05-28  7:26       ` Alan Modra
@ 2014-05-29 19:02         ` Cary Coutant
  2014-05-29 23:59           ` Alan Modra
  0 siblings, 1 reply; 9+ messages in thread
From: Cary Coutant @ 2014-05-29 19:02 UTC (permalink / raw)
  To: Cary Coutant, Binutils

>> Hmm, yes.  I'm seeing this instead:
>>     52: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND __ehdr_start
>>
>> > What platform are you testing on?
>>
>> powerpc64-linux.  I'll see if I can track down the problem when I find
>> some time.
>
> Also happens on x86_64-linux.  I'm surprised you haven't seen it.
> __ehdr_start is STV_HIDDEN, Symbol_table::override_with_special makes
> it forced_local, thus output from Symbol_table::sized_write_symbol
> is STB_LOCAL.

On x86_64 for me:

$ readelf -sW ehdr_start_test_4.o | grep __ehdr_start
    46: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __ehdr_start

$ readelf -sW ehdr_start_test_4 | grep __ehdr_start
    86: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __ehdr_start

You don't have some compiler wrapper that's setting -fvisibility=hidden, do you?

-cary

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

* Re: [gold commit] Fix handling of __ehdr_start
  2014-05-29 19:02         ` Cary Coutant
@ 2014-05-29 23:59           ` Alan Modra
  2014-05-30 17:13             ` Cary Coutant
  0 siblings, 1 reply; 9+ messages in thread
From: Alan Modra @ 2014-05-29 23:59 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Binutils

On Thu, May 29, 2014 at 12:02:51PM -0700, Cary Coutant wrote:
> >> Hmm, yes.  I'm seeing this instead:
> >>     52: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND __ehdr_start
> >>
> >> > What platform are you testing on?
> >>
> >> powerpc64-linux.  I'll see if I can track down the problem when I find
> >> some time.
> >
> > Also happens on x86_64-linux.  I'm surprised you haven't seen it.
> > __ehdr_start is STV_HIDDEN, Symbol_table::override_with_special makes
> > it forced_local, thus output from Symbol_table::sized_write_symbol
> > is STB_LOCAL.
> 
> On x86_64 for me:
> 
> $ readelf -sW ehdr_start_test_4.o | grep __ehdr_start
>     46: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __ehdr_start

I have the above.

> $ readelf -sW ehdr_start_test_4 | grep __ehdr_start
>     86: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __ehdr_start

But not this..

> You don't have some compiler wrapper that's setting -fvisibility=hidden, do you?

No.  The hidden visibility is coming from defstd.cc

  {
    "__ehdr_start",		// name
    elfcpp::PT_LOAD,		// segment_type
    elfcpp::PF(0),		// segment_flags_set
    elfcpp::PF(0),		// segment_flags_clear
    0,				// value
    0,				// size
    elfcpp::STT_NOTYPE,		// type
    elfcpp::STB_GLOBAL,		// binding
    elfcpp::STV_HIDDEN,		// visibility
    0,				// nonvis
    Symbol::SEGMENT_START,	// offset_from_base
    true			// only_if_ref
  },

Do you have some local change here?

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [gold commit] Fix handling of __ehdr_start
  2014-05-29 23:59           ` Alan Modra
@ 2014-05-30 17:13             ` Cary Coutant
  0 siblings, 0 replies; 9+ messages in thread
From: Cary Coutant @ 2014-05-30 17:13 UTC (permalink / raw)
  To: Cary Coutant, Binutils

> Do you have some local change here?

Arg. The rule for ehdr_start_test_4 had "-Bgcctestdir/ld/" instead of
"-Bgcctestdir/", so I was actually building that test with an older
version of gold that was in my PATH.

So, yes, the check script should be looking for "U" instead of "w".

Will commit a fix right away.

Sorry!

-cary

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

end of thread, other threads:[~2014-05-30 17:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-02 23:30 [gold commit] Fix handling of __ehdr_start Cary Coutant
2014-05-06 21:51 ` Cary Coutant
2014-05-20  6:22 ` Alan Modra
2014-05-27 18:40   ` Cary Coutant
2014-05-28  1:55     ` Alan Modra
2014-05-28  7:26       ` Alan Modra
2014-05-29 19:02         ` Cary Coutant
2014-05-29 23:59           ` Alan Modra
2014-05-30 17:13             ` Cary Coutant

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