[PATCH] GOLD: emit STT_FILE symbols when more than 64k sections

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] GOLD: emit STT_FILE symbols when more than 64k sections

Tristan Gingold-2
Hello,

a simple thinko in object.cc prevents from emitting STT_FILE symbols when --gc-sections is used and when the file contains more than 64K symbols.  See the testcase.

No regression on x86-64 linux.

Ok to commit ?

Tristan.

commit 42ecf78bdb139fcc7ed27f1e101f8b3544479892
Author: Tristan Gingold <[hidden email]>
Date:   Mon Dec 5 15:04:28 2016 +0100

    Always emit STT_FILE symbols.
   
    gold/
    * object.cc (do_count_local_symbols): Always emit STT_FILE symbols.

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 913ad08..bc459d7 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,11 @@
+2016-12-05  Tristan Gingold  <[hidden email]>
+
+ * object.cc (do_count_local_symbols): Always emit STT_FILE symbols.
+ * testsuite/Makefile.in: Regenerate.
+ * testsuite/Makefile.am: Add testcase.
+ * testsuite/file_in_many_sections.c: New file.
+ * testsuite/file_in_many_sections_test.sh: New file.
+
 2016-12-01  Cary Coutant  <[hidden email]>
 
  PR gold/20717
diff --git a/gold/object.cc b/gold/object.cc
index a631c99..2255a1a 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -2225,7 +2225,8 @@ Sized_relobj_file<size, big_endian>::do_count_local_symbols(Stringpool* pool,
 
       // Decide whether this symbol should go into the output file.
 
-      if ((shndx < shnum && out_sections[shndx] == NULL)
+      if ((sym.get_st_type() != elfcpp::STT_FILE
+   && (shndx < shnum && out_sections[shndx] == NULL))
   || shndx == this->discarded_eh_frame_shndx_)
  {
   lv.set_no_output_symtab_entry();
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 1575aa3..17f45d6 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -1339,6 +1339,16 @@ many_sections_r_test.o: many_sections_test.o gcctestdir/ld
 many_sections_r_test: many_sections_r_test.o gcctestdir/ld
  $(CXXLINK) -Bgcctestdir/ many_sections_r_test.o $(LIBS)
 
+check_SCRIPTS += file_in_many_sections_test.sh
+check_DATA += file_in_many_sections.stdout
+MOSTLYCLEANFILES += file_in_many_sections
+file_in_many_sections.o: file_in_many_sections.c many_sections_define.h
+ $(COMPILE) -c -fdata-sections -o $@ $(srcdir)/file_in_many_sections.c
+file_in_many_sections: file_in_many_sections.o gcctestdir/ld
+ $(LINK) -Bgcctestdir/ file_in_many_sections.o -Wl,--gc-sections
+file_in_many_sections.stdout: file_in_many_sections
+ $(TEST_READELF) -s $< > $@
+
 check_PROGRAMS += initpri1
 initpri1_SOURCES = initpri1.c
 initpri1_DEPENDENCIES = gcctestdir/ld
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index a9bea38..2b4c4ca 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -342,6 +342,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ many_sections_check.h
 @GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_41 = many_sections_define.h \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ many_sections_check.h \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ file_in_many_sections \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ debug_msg.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ missing_key_func.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ debug_msg_cdebug.err \
@@ -370,12 +371,6 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ alt/thin_archive_test_2.o \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ alt/thin_archive_test_4.o \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ alt/libthin2.a alt/libthin4.a
-@GCC_FALSE@initpri1_DEPENDENCIES =
-@NATIVE_LINKER_FALSE@initpri1_DEPENDENCIES =
-@GCC_FALSE@initpri2_DEPENDENCIES =
-@NATIVE_LINKER_FALSE@initpri2_DEPENDENCIES =
-@GCC_FALSE@initpri3a_DEPENDENCIES =
-@NATIVE_LINKER_FALSE@initpri3a_DEPENDENCIES =
 
 # This test fails on targets not using .ctors and .dtors sections (e.g. ARM
 # EABI). Given that gcc is moving towards using .init_array in all cases,
@@ -462,6 +457,12 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ script_test_9.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ script_test_14.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ dynamic_list.stdout
+@GCC_FALSE@initpri1_DEPENDENCIES =
+@NATIVE_LINKER_FALSE@initpri1_DEPENDENCIES =
+@GCC_FALSE@initpri2_DEPENDENCIES =
+@NATIVE_LINKER_FALSE@initpri2_DEPENDENCIES =
+@GCC_FALSE@initpri3a_DEPENDENCIES =
+@NATIVE_LINKER_FALSE@initpri3a_DEPENDENCIES =
 @GCC_FALSE@script_test_1_DEPENDENCIES =
 @NATIVE_LINKER_FALSE@script_test_1_DEPENDENCIES =
 @GCC_FALSE@script_test_2_DEPENDENCIES =
@@ -5053,6 +5054,8 @@ x32_overflow_pc32.sh.log: x32_overflow_pc32.sh
  @p='x32_overflow_pc32.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 i386_mov_to_lea.sh.log: i386_mov_to_lea.sh
  @p='i386_mov_to_lea.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+file_in_many_sections_test.sh.log: file_in_many_sections_test.sh
+ @p='file_in_many_sections_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 debug_msg.sh.log: debug_msg.sh
  @p='debug_msg.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 missing_key_func.sh.log: missing_key_func.sh
@@ -6278,6 +6281,12 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld -r -o $@ many_sections_test.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@many_sections_r_test: many_sections_r_test.o gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -Bgcctestdir/ many_sections_r_test.o $(LIBS)
+@GCC_TRUE@@[hidden email]: file_in_many_sections.c many_sections_define.h
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(COMPILE) -c -fdata-sections -o $@ $(srcdir)/file_in_many_sections.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@file_in_many_sections: file_in_many_sections.o gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(LINK) -Bgcctestdir/ file_in_many_sections.o -Wl,--gc-sections
+@GCC_TRUE@@[hidden email]: file_in_many_sections
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -s $< > $@
 @GCC_TRUE@@[hidden email]: debug_msg.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/debug_msg.cc
 @GCC_TRUE@@[hidden email]: odr_violation1.cc
diff --git a/gold/testsuite/file_in_many_sections.c b/gold/testsuite/file_in_many_sections.c
new file mode 100644
index 0000000..64146cf
--- /dev/null
+++ b/gold/testsuite/file_in_many_sections.c
@@ -0,0 +1,34 @@
+// file_in_many_sections.c -- test STT_FILE when more than 64k sections
+
+// Copyright (C) 2016 Free Software Foundation, Inc.
+// Written by Tristan Gingold <[hidden email]>
+
+// 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.
+
+// This program tests having many sections.  It uses a generated .h
+// files to define 70,000 variables, each in a different section.  It
+// uses another generated .h file to verify that they all have the
+// right value.
+
+#include "many_sections_define.h"
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gold/testsuite/file_in_many_sections_test.sh b/gold/testsuite/file_in_many_sections_test.sh
new file mode 100755
index 0000000..453bc67
--- /dev/null
+++ b/gold/testsuite/file_in_many_sections_test.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+# file_in_many_sections.sh -- test
+
+# Copyright (C) 2016 Free Software Foundation, Inc.
+# Written by Tristan Gingold <[hidden email]>
+
+# 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 verify if .text sections are grouped
+# according to prefix.  .text.unlikely, .text.startup and .text.hot should
+# be grouped and placed together.
+
+set -e
+
+grep file_in_many_sections.c file_in_many_sections.stdout

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] GOLD: emit STT_FILE symbols when more than 64k sections

Cary Coutant-3
> a simple thinko in object.cc prevents from emitting STT_FILE symbols when --gc-sections is used and when the file contains more than 64K symbols.  See the testcase.
>
> No regression on x86-64 linux.
>
> Ok to commit ?

Not quite. The real problem was that the is_ordinary flag wasn't being
checked before we use shndx, and the effect was that any SHN_ABS
symbol may have been incorrectly omitted. I've applied the attached
patch.

Thanks for finding the problem, and for providing a test case!

-cary


2016-12-05  Cary Coutant  <[hidden email]>
            Tristan Gingold  <[hidden email]>

gold/
        * object.cc (Sized_relobj_file::do_count_local_symbols): Check
        is_ordinary before using shndx.
        * testsuite/Makefile.am (file_in_many_sections_test.sh): New test case.
        * testsuite/Makefile.in: Regenerate.
        * testsuite/file_in_many_sections.c: New source file.
        * testsuite/file_in_many_sections_test.sh: New script.

file-sym.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] GOLD: emit STT_FILE symbols when more than 64k sections

Tristan Gingold-2

> On 05 Dec 2016, at 19:44, Cary Coutant <[hidden email]> wrote:
>
>> a simple thinko in object.cc prevents from emitting STT_FILE symbols when --gc-sections is used and when the file contains more than 64K symbols.  See the testcase.
>>
>> No regression on x86-64 linux.
>>
>> Ok to commit ?
>
> Not quite. The real problem was that the is_ordinary flag wasn't being
> checked before we use shndx, and the effect was that any SHN_ABS
> symbol may have been incorrectly omitted. I've applied the attached
> patch.
>
> Thanks for finding the problem, and for providing a test case!

Thanks for the correct fix.