public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [patch] Fix handling of common symbols with plugins
@ 2014-09-09 23:42 Rafael Espíndola
  2014-09-16 15:33 ` Rafael Espíndola
  2014-09-17 17:17 ` Cary Coutant
  0 siblings, 2 replies; 15+ messages in thread
From: Rafael Espíndola @ 2014-09-09 23:42 UTC (permalink / raw)
  To: binutils; +Cc: Cary Coutant

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

Currently we don't produce the correct result if a common symbol shows
up in both ELF and IR. In a previous patch I tried to pass the size
information back to the plugin, but unfortunately there is no field in
d_plugin_symbol to pass the alignment.

What I have implemented now is:

* The plugin is expected to handle all common symbols in IR. It can do
it by merging them or by passing multiple objects to the linker with
each symbol.

* The linker will not blindly override a common symbol during the
replacement phase. Instead, the regular logic is applied so that we
don't discard information in case an ELF file had the largest size or
alignment.

In addition to "make check" I tested this with a small script and two
LLVM IR files. The script test all combinations of input order and
file type (IR or ELF) and checks that we get the same alignment and
size.

Cheers,
Rafael


gold
2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>

        * plugin.cc (Sized_pluginobj::do_add_symbols): Ignore isym->size.
        * resolve.cc (Symbol_table::resolve): Don't override common symbols
        during the replacement phase.

include
2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>

        * plugin-api.h (ld_plugin_symbol): Note that size is ignored.

ld
2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>

        * plugin.c (asymbol_from_plugin_symbol): Ignore ldsym->size.
        * testplug.c (parse_symdefstr): Ignore sym->size.

[-- Attachment #2: common.patch --]
[-- Type: text/x-patch, Size: 2863 bytes --]

diff --git a/gold/plugin.cc b/gold/plugin.cc
index 6519732..0339d42 100644
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -1053,8 +1053,6 @@ Sized_pluginobj<size, big_endian>::do_add_symbols(Symbol_table* symtab,
   elfcpp::Sym<size, big_endian> sym(symbuf);
   elfcpp::Sym_write<size, big_endian> osym(symbuf);
 
-  typedef typename elfcpp::Elf_types<size>::Elf_WXword Elf_size_type;
-
   this->symbols_.resize(this->nsyms_);
 
   for (int i = 0; i < this->nsyms_; ++i)
@@ -1125,7 +1123,7 @@ Sized_pluginobj<size, big_endian>::do_add_symbols(Symbol_table* symtab,
 
       osym.put_st_name(0);
       osym.put_st_value(0);
-      osym.put_st_size(static_cast<Elf_size_type>(isym->size));
+      osym.put_st_size(0);
       osym.put_st_info(bind, elfcpp::STT_NOTYPE);
       osym.put_st_other(vis, 0);
       osym.put_st_shndx(shndx);
diff --git a/gold/resolve.cc b/gold/resolve.cc
index 8cc637a..abb5d90 100644
--- a/gold/resolve.cc
+++ b/gold/resolve.cc
@@ -303,11 +303,14 @@ Symbol_table::resolve(Sized_symbol<size>* to,
 
   // If we're processing replacement files, allow new symbols to override
   // the placeholders from the plugin objects.
+  // Treat common symbols specially since it is possible that an ELF
+  // file increased the size of the alignment.
   if (to->source() == Symbol::FROM_OBJECT)
     {
       Pluginobj* obj = to->object()->pluginobj();
       if (obj != NULL
-          && parameters->options().plugins()->in_replacement_phase())
+          && parameters->options().plugins()->in_replacement_phase()
+          && !to->is_common())
         {
           this->override(to, sym, st_shndx, is_ordinary, object, version);
           return;
diff --git a/include/plugin-api.h b/include/plugin-api.h
index 5797d4d..dfcd1f2 100644
--- a/include/plugin-api.h
+++ b/include/plugin-api.h
@@ -89,7 +89,7 @@ struct ld_plugin_symbol
   char *version;
   int def;
   int visibility;
-  uint64_t size;
+  uint64_t size; /* ignored */
   char *comdat_key;
   int resolution;
 };
diff --git a/ld/plugin.c b/ld/plugin.c
index f02a97f..871c714 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -332,7 +332,7 @@ asymbol_from_plugin_symbol (bfd *abfd, asymbol *asym,
     case LDPK_COMMON:
       flags = BSF_GLOBAL;
       section = bfd_com_section_ptr;
-      asym->value = ldsym->size;
+      asym->value = 0;
       /* For ELF targets, set alignment of common symbol to 1.  */
       if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
 	{
diff --git a/ld/testplug.c b/ld/testplug.c
index 4dedf95..e6954c1 100644
--- a/ld/testplug.c
+++ b/ld/testplug.c
@@ -217,7 +217,6 @@ parse_symdefstr (const char *str, struct ld_plugin_symbol *sym)
     return LDPS_ERR;
 
   /* Parsed successfully, so allocate strings and fill out fields.  */
-  sym->size = size;
   sym->resolution = LDPR_UNKNOWN;
   sym->name = malloc (colon1 - str + 1);
   if (!sym->name)

[-- Attachment #3: run.sh --]
[-- Type: application/x-sh, Size: 702 bytes --]

[-- Attachment #4: test.ll --]
[-- Type: application/octet-stream, Size: 34 bytes --]

@a = common global i8 0, align 8


[-- Attachment #5: test2.ll --]
[-- Type: application/octet-stream, Size: 35 bytes --]

@a = common global i16 0, align 4


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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-09 23:42 [patch] Fix handling of common symbols with plugins Rafael Espíndola
@ 2014-09-16 15:33 ` Rafael Espíndola
  2014-09-17 17:17 ` Cary Coutant
  1 sibling, 0 replies; 15+ messages in thread
From: Rafael Espíndola @ 2014-09-16 15:33 UTC (permalink / raw)
  To: Binutils; +Cc: Cary Coutant

ping.

On 9 September 2014 19:41, Rafael Espíndola <rafael.espindola@gmail.com> wrote:
> Currently we don't produce the correct result if a common symbol shows
> up in both ELF and IR. In a previous patch I tried to pass the size
> information back to the plugin, but unfortunately there is no field in
> d_plugin_symbol to pass the alignment.
>
> What I have implemented now is:
>
> * The plugin is expected to handle all common symbols in IR. It can do
> it by merging them or by passing multiple objects to the linker with
> each symbol.
>
> * The linker will not blindly override a common symbol during the
> replacement phase. Instead, the regular logic is applied so that we
> don't discard information in case an ELF file had the largest size or
> alignment.
>
> In addition to "make check" I tested this with a small script and two
> LLVM IR files. The script test all combinations of input order and
> file type (IR or ELF) and checks that we get the same alignment and
> size.
>
> Cheers,
> Rafael
>
>
> gold
> 2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>
>
>         * plugin.cc (Sized_pluginobj::do_add_symbols): Ignore isym->size.
>         * resolve.cc (Symbol_table::resolve): Don't override common symbols
>         during the replacement phase.
>
> include
> 2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>
>
>         * plugin-api.h (ld_plugin_symbol): Note that size is ignored.
>
> ld
> 2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>
>
>         * plugin.c (asymbol_from_plugin_symbol): Ignore ldsym->size.
>         * testplug.c (parse_symdefstr): Ignore sym->size.

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-09 23:42 [patch] Fix handling of common symbols with plugins Rafael Espíndola
  2014-09-16 15:33 ` Rafael Espíndola
@ 2014-09-17 17:17 ` Cary Coutant
  2014-09-17 17:20   ` H.J. Lu
  1 sibling, 1 reply; 15+ messages in thread
From: Cary Coutant @ 2014-09-17 17:17 UTC (permalink / raw)
  To: Rafael Espíndola, H.J. Lu; +Cc: Binutils

> gold
> 2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>
>
>         * plugin.cc (Sized_pluginobj::do_add_symbols): Ignore isym->size.
>         * resolve.cc (Symbol_table::resolve): Don't override common symbols
>         during the replacement phase.

The gold patch looks good to me. Thanks!

> include
> 2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>
>
>         * plugin-api.h (ld_plugin_symbol): Note that size is ignored.

This is OK if the ld patch is also approved.

> ld
> 2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>
>
>         * plugin.c (asymbol_from_plugin_symbol): Ignore ldsym->size.
>         * testplug.c (parse_symdefstr): Ignore sym->size.

Maybe HJ should look at this?

-cary

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-17 17:17 ` Cary Coutant
@ 2014-09-17 17:20   ` H.J. Lu
  2014-09-17 22:36     ` Rafael Espíndola
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2014-09-17 17:20 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Rafael Espíndola, Binutils

On Wed, Sep 17, 2014 at 10:17 AM, Cary Coutant <ccoutant@google.com> wrote:
>> gold
>> 2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>
>>
>>         * plugin.cc (Sized_pluginobj::do_add_symbols): Ignore isym->size.
>>         * resolve.cc (Symbol_table::resolve): Don't override common symbols
>>         during the replacement phase.
>
> The gold patch looks good to me. Thanks!
>
>> include
>> 2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>
>>
>>         * plugin-api.h (ld_plugin_symbol): Note that size is ignored.
>
> This is OK if the ld patch is also approved.
>
>> ld
>> 2014-09-09  Rafael Ávila de Espíndola <respindola@mozilla.com>
>>
>>         * plugin.c (asymbol_from_plugin_symbol): Ignore ldsym->size.
>>         * testplug.c (parse_symdefstr): Ignore sym->size.
>
> Maybe HJ should look at this?
>

Can you add an ld testcase to ld/testsuite/ld-plugin/lto.exp?


-- 
H.J.

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-17 17:20   ` H.J. Lu
@ 2014-09-17 22:36     ` Rafael Espíndola
  2014-09-17 22:46       ` Cary Coutant
  2014-09-18 14:43       ` H.J. Lu
  0 siblings, 2 replies; 15+ messages in thread
From: Rafael Espíndola @ 2014-09-17 22:36 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Cary Coutant, Binutils

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

>> The gold patch looks good to me. Thanks!

Pushed.

> Can you add an ld testcase to ld/testsuite/ld-plugin/lto.exp?

I don't think my patch changes the behavior of bfd-l, it is just a
cleanup. I applied Hal Finkel's patch to let ld work with llvm IR and
it passes the run.sh that I emailed earlier with or without my patch.

Having said that, while the size field is not useful for common
symbols, it might be just what is needed for coff's
COMDAT_SELECT_LARGEST. Do you know if that is implemented?

Thanks for mentioning tests. I decided to try to write one for gold. I
managed to write one that passes now but fails with my patch reverted,
but it uses readelf to check the alignment of .bss, is that OK?

Cheers,
Rafael

2014-09-17  Rafael Ávila de Espíndola <rafael.espindola@gmail.com>

        * testsuite/Makefile.am (plugin_test_10): New test.
        * testsuite/Makefile.in: Regenerate
        * testsuite/plugin_common_test_2.c (c1): Align to 8.
        * testsuite/plugin_test_10.sh: New file.

[-- Attachment #2: test.patch --]
[-- Type: text/x-patch, Size: 11927 bytes --]

diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 5e1cd0d..325ce92 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -1657,6 +1657,15 @@ MOSTLYCLEANFILES += two_file_test_1c.o
 two_file_test_1c.o: two_file_test_1.o
 	cp two_file_test_1.o $@
 
+check_PROGRAMS += plugin_test_10
+check_SCRIPTS += plugin_test_10.sh
+check_DATA += plugin_test_10.err
+MOSTLYCLEANFILES += plugin_test_10.err
+plugin_test_10: plugin_common_test_1.syms plugin_common_test_2.o  gcctestdir/ld plugin_test.so
+	$(CXXLINK) -Bgcctestdir/ -Wl,--no-demangle,--plugin,"./plugin_test.so" plugin_common_test_1.syms plugin_common_test_2.o 2>plugin_test_10.err
+plugin_test_10.err: plugin_test_10
+	@touch plugin_test_10.err
+
 plugin_test.so: plugin_test.o
 	$(LINK) -Bgcctestdir/ -shared plugin_test.o
 plugin_test.o: plugin_test.c
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index 7d0f78b..b446d1a 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -350,14 +350,16 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_5 \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6 \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7 \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_8
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_8 \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_34 =  \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_1.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_2.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_3.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_4.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6.sh \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.sh
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10.sh
 
 # Test that symbols known in the IR file but not in the replacement file
 # produce an unresolved symbol error.
@@ -369,7 +371,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.syms \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_9.err
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_9.err \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10.err
 # Make a copy of two_file_test_1.o, which does not define the symbol _Z4t16av.
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_36 =  \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_1.err \
@@ -380,7 +383,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_9.err \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	two_file_test_1c.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	two_file_test_1c.o \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10.err
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@@TLS_TRUE@am__append_37 = plugin_test_tls
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@@TLS_TRUE@am__append_38 = plugin_test_tls.sh
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@@TLS_TRUE@am__append_39 = plugin_test_tls.err
@@ -874,7 +878,8 @@ libgoldtest_a_OBJECTS = $(am_libgoldtest_a_OBJECTS)
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_5$(EXEEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6$(EXEEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7$(EXEEXT) \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_8$(EXEEXT)
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_8$(EXEEXT) \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10$(EXEEXT)
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@@TLS_TRUE@am__EXEEXT_24 = plugin_test_tls$(EXEEXT)
 @GCC_TRUE@@NATIVE_LINKER_TRUE@am__EXEEXT_25 =  \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	exclude_libs_test$(EXEEXT) \
@@ -1425,6 +1430,12 @@ plugin_test_1_LDADD = $(LDADD)
 plugin_test_1_DEPENDENCIES = libgoldtest.a ../libgold.a \
 	../../libiberty/libiberty.a $(am__DEPENDENCIES_1) \
 	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1)
+plugin_test_10_SOURCES = plugin_test_10.c
+plugin_test_10_OBJECTS = plugin_test_10.$(OBJEXT)
+plugin_test_10_LDADD = $(LDADD)
+plugin_test_10_DEPENDENCIES = libgoldtest.a ../libgold.a \
+	../../libiberty/libiberty.a $(am__DEPENDENCIES_1) \
+	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1)
 plugin_test_2_SOURCES = plugin_test_2.c
 plugin_test_2_OBJECTS = plugin_test_2.$(OBJEXT)
 plugin_test_2_LDADD = $(LDADD)
@@ -1888,14 +1899,14 @@ SOURCES = $(libgoldtest_a_SOURCES) basic_pic_test.c basic_pie_test.c \
 	local_labels_test.c many_sections_r_test.c \
 	$(many_sections_test_SOURCES) $(object_unittest_SOURCES) \
 	permission_test.c $(pie_copyrelocs_test_SOURCES) \
-	plugin_test_1.c plugin_test_2.c plugin_test_3.c \
-	plugin_test_4.c plugin_test_5.c plugin_test_6.c \
-	plugin_test_7.c plugin_test_8.c plugin_test_tls.c \
-	$(protected_1_SOURCES) $(protected_2_SOURCES) \
-	$(relro_now_test_SOURCES) $(relro_script_test_SOURCES) \
-	$(relro_strip_test_SOURCES) $(relro_test_SOURCES) \
-	$(script_test_1_SOURCES) script_test_11.c \
-	$(script_test_2_SOURCES) script_test_3.c \
+	plugin_test_1.c plugin_test_10.c plugin_test_2.c \
+	plugin_test_3.c plugin_test_4.c plugin_test_5.c \
+	plugin_test_6.c plugin_test_7.c plugin_test_8.c \
+	plugin_test_tls.c $(protected_1_SOURCES) \
+	$(protected_2_SOURCES) $(relro_now_test_SOURCES) \
+	$(relro_script_test_SOURCES) $(relro_strip_test_SOURCES) \
+	$(relro_test_SOURCES) $(script_test_1_SOURCES) \
+	script_test_11.c $(script_test_2_SOURCES) script_test_3.c \
 	$(searched_file_test_SOURCES) start_lib_test.c \
 	$(thin_archive_test_1_SOURCES) $(thin_archive_test_2_SOURCES) \
 	$(tls_phdrs_script_test_SOURCES) $(tls_pic_test_SOURCES) \
@@ -3274,6 +3285,15 @@ pie_copyrelocs_test$(EXEEXT): $(pie_copyrelocs_test_OBJECTS) $(pie_copyrelocs_te
 @PLUGINS_FALSE@plugin_test_1$(EXEEXT): $(plugin_test_1_OBJECTS) $(plugin_test_1_DEPENDENCIES) 
 @PLUGINS_FALSE@	@rm -f plugin_test_1$(EXEEXT)
 @PLUGINS_FALSE@	$(LINK) $(plugin_test_1_OBJECTS) $(plugin_test_1_LDADD) $(LIBS)
+@GCC_FALSE@plugin_test_10$(EXEEXT): $(plugin_test_10_OBJECTS) $(plugin_test_10_DEPENDENCIES) 
+@GCC_FALSE@	@rm -f plugin_test_10$(EXEEXT)
+@GCC_FALSE@	$(LINK) $(plugin_test_10_OBJECTS) $(plugin_test_10_LDADD) $(LIBS)
+@NATIVE_LINKER_FALSE@plugin_test_10$(EXEEXT): $(plugin_test_10_OBJECTS) $(plugin_test_10_DEPENDENCIES) 
+@NATIVE_LINKER_FALSE@	@rm -f plugin_test_10$(EXEEXT)
+@NATIVE_LINKER_FALSE@	$(LINK) $(plugin_test_10_OBJECTS) $(plugin_test_10_LDADD) $(LIBS)
+@PLUGINS_FALSE@plugin_test_10$(EXEEXT): $(plugin_test_10_OBJECTS) $(plugin_test_10_DEPENDENCIES) 
+@PLUGINS_FALSE@	@rm -f plugin_test_10$(EXEEXT)
+@PLUGINS_FALSE@	$(LINK) $(plugin_test_10_OBJECTS) $(plugin_test_10_LDADD) $(LIBS)
 @GCC_FALSE@plugin_test_2$(EXEEXT): $(plugin_test_2_OBJECTS) $(plugin_test_2_DEPENDENCIES) 
 @GCC_FALSE@	@rm -f plugin_test_2$(EXEEXT)
 @GCC_FALSE@	$(LINK) $(plugin_test_2_OBJECTS) $(plugin_test_2_LDADD) $(LIBS)
@@ -3657,6 +3677,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/permission_test.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/pie_copyrelocs_test.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_1.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_10.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_2.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_3.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_4.Po@am__quote@
@@ -4107,6 +4128,8 @@ plugin_test_6.sh.log: plugin_test_6.sh
 	@p='plugin_test_6.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_test_7.sh.log: plugin_test_7.sh
 	@p='plugin_test_7.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+plugin_test_10.sh.log: plugin_test_10.sh
+	@p='plugin_test_10.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_test_tls.sh.log: plugin_test_tls.sh
 	@p='plugin_test_tls.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_final_layout.sh.log: plugin_final_layout.sh
@@ -4387,6 +4410,8 @@ plugin_test_7.log: plugin_test_7$(EXEEXT)
 	@p='plugin_test_7$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_test_8.log: plugin_test_8$(EXEEXT)
 	@p='plugin_test_8$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+plugin_test_10.log: plugin_test_10$(EXEEXT)
+	@p='plugin_test_10$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_test_tls.log: plugin_test_tls$(EXEEXT)
 	@p='plugin_test_tls$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 exclude_libs_test.log: exclude_libs_test$(EXEEXT)
@@ -5324,6 +5349,10 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	mv -f $@.tmp $@
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@two_file_test_1c.o: two_file_test_1.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	cp two_file_test_1.o $@
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test_10: plugin_common_test_1.syms plugin_common_test_2.o  gcctestdir/ld plugin_test.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	$(CXXLINK) -Bgcctestdir/ -Wl,--no-demangle,--plugin,"./plugin_test.so" plugin_common_test_1.syms plugin_common_test_2.o 2>plugin_test_10.err
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test_10.err: plugin_test_10
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	@touch plugin_test_10.err
 
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test.so: plugin_test.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	$(LINK) -Bgcctestdir/ -shared plugin_test.o
diff --git a/gold/testsuite/plugin_common_test_2.c b/gold/testsuite/plugin_common_test_2.c
index c0c934c..df9f7f1 100644
--- a/gold/testsuite/plugin_common_test_2.c
+++ b/gold/testsuite/plugin_common_test_2.c
@@ -26,7 +26,7 @@
 
 #include <assert.h>
 
-int c1;
+int c1 __attribute__((aligned (8)));
 extern int c2;
 int c3;
 int c4 = 40;
diff --git a/gold/testsuite/plugin_test_10.sh b/gold/testsuite/plugin_test_10.sh
new file mode 100755
index 0000000..b4a2e77
--- /dev/null
+++ b/gold/testsuite/plugin_test_10.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+# plugin_test_6.sh -- a test case for the plugin API.
+
+# Copyright (C) 2010-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.
+
+# This file goes with plugin_common_test_1.c and plugin_common_test_2.c.
+# The first file is claimed by the plugin, the second one is not. We test
+# the bigger alignment in plugin_common_test_2.c is used.
+
+set -e
+
+readelf -SW plugin_test_10 > plugin_test_10.sections
+grep ".bss.* 8$" plugin_test_10.sections > /dev/null
+
+exit 0

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-17 22:36     ` Rafael Espíndola
@ 2014-09-17 22:46       ` Cary Coutant
  2014-09-18 16:53         ` Rafael Espíndola
  2014-09-18 14:43       ` H.J. Lu
  1 sibling, 1 reply; 15+ messages in thread
From: Cary Coutant @ 2014-09-17 22:46 UTC (permalink / raw)
  To: Rafael Espíndola; +Cc: H.J. Lu, Binutils

> Thanks for mentioning tests. I decided to try to write one for gold. I
> managed to write one that passes now but fails with my patch reverted,
> but it uses readelf to check the alignment of .bss, is that OK?

+# plugin_test_6.sh -- a test case for the plugin API.
+
+# Copyright (C) 2010-2014 Free Software Foundation, Inc.
+# Written by Cary Coutant <ccoutant@google.com>.

Update these lines.

+readelf -SW plugin_test_10 > plugin_test_10.sections
+grep ".bss.* 8$" plugin_test_10.sections > /dev/null
+
+exit 0

The readelf command should be run from the Makefile itself. Create a
new rule for plugin_test_10.sections, and use $(TEST_READELF). There
are several examples of this in testsuite/Makefile.am.

Then your test script only needs to run grep, but use grep's "-q"
option instead of "> /dev/null".

-cary

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-17 22:36     ` Rafael Espíndola
  2014-09-17 22:46       ` Cary Coutant
@ 2014-09-18 14:43       ` H.J. Lu
  1 sibling, 0 replies; 15+ messages in thread
From: H.J. Lu @ 2014-09-18 14:43 UTC (permalink / raw)
  To: Rafael Espíndola; +Cc: Cary Coutant, Binutils

On Wed, Sep 17, 2014 at 3:36 PM, Rafael Espíndola
<rafael.espindola@gmail.com> wrote:
>>> The gold patch looks good to me. Thanks!
>
> Pushed.
>
>> Can you add an ld testcase to ld/testsuite/ld-plugin/lto.exp?
>
> I don't think my patch changes the behavior of bfd-l, it is just a
> cleanup. I applied Hal Finkel's patch to let ld work with llvm IR and
> it passes the run.sh that I emailed earlier with or without my patch.

If it has no impact on your testcase, I prefer to leave ld alone.

> Having said that, while the size field is not useful for common
> symbols, it might be just what is needed for coff's
> COMDAT_SELECT_LARGEST. Do you know if that is implemented?
>

The ELF ld will always use the largest alignment and sizes for
the common symbol.

-- 
H.J.

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-17 22:46       ` Cary Coutant
@ 2014-09-18 16:53         ` Rafael Espíndola
  2014-09-18 16:54           ` Rafael Espíndola
  0 siblings, 1 reply; 15+ messages in thread
From: Rafael Espíndola @ 2014-09-18 16:53 UTC (permalink / raw)
  To: Cary Coutant; +Cc: H.J. Lu, Binutils

[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]

> +# plugin_test_6.sh -- a test case for the plugin API.
> +
> +# Copyright (C) 2010-2014 Free Software Foundation, Inc.
> +# Written by Cary Coutant <ccoutant@google.com>.
>
> Update these lines.

Done.

> +readelf -SW plugin_test_10 > plugin_test_10.sections
> +grep ".bss.* 8$" plugin_test_10.sections > /dev/null
> +
> +exit 0
>
> The readelf command should be run from the Makefile itself. Create a
> new rule for plugin_test_10.sections, and use $(TEST_READELF). There
> are several examples of this in testsuite/Makefile.am.
>
> Then your test script only needs to run grep, but use grep's "-q"
> option instead of "> /dev/null".

Done.

New patch attached.

Cheers,
Rafael

2014-09-18  Rafael Ávila de Espíndola <rafael.espindola@gmail.com>

        * testsuite/Makefile.am (plugin_test_10): New test.
        * testsuite/Makefile.in: Regenerate
        * testsuite/plugin_common_test_2.c (c1): Align to 8.
        * testsuite/plugin_test_10.sh: New file.

[-- Attachment #2: t.patch --]
[-- Type: text/x-patch, Size: 1254 bytes --]

diff --git a/gold/Makefile.in b/gold/Makefile.in
index d404666..39fa5e6 100644
--- a/gold/Makefile.in
+++ b/gold/Makefile.in
@@ -70,8 +70,8 @@ subdir = .
 DIST_COMMON = NEWS README ChangeLog $(srcdir)/Makefile.in \
 	$(srcdir)/Makefile.am $(top_srcdir)/configure \
 	$(am__configure_deps) $(srcdir)/config.in \
-	$(srcdir)/../mkinstalldirs $(top_srcdir)/po/Make-in ffsll.c \
-	ftruncate.c pread.c mremap.c yyscript.h yyscript.c \
+	$(srcdir)/../mkinstalldirs $(top_srcdir)/po/Make-in \
+	ftruncate.c pread.c ffsll.c mremap.c yyscript.h yyscript.c \
 	$(srcdir)/../depcomp $(srcdir)/../ylwrap
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
 am__aclocal_m4_deps = $(top_srcdir)/../config/depstand.m4 \
@@ -763,6 +763,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@$(DEPDIR)/ftruncate.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@$(DEPDIR)/mremap.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@$(DEPDIR)/pread.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/aarch64-reloc-property.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/aarch64.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/archive.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/arm-reloc-property.Po@am__quote@

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-18 16:53         ` Rafael Espíndola
@ 2014-09-18 16:54           ` Rafael Espíndola
  2014-09-18 17:09             ` Cary Coutant
  0 siblings, 1 reply; 15+ messages in thread
From: Rafael Espíndola @ 2014-09-18 16:54 UTC (permalink / raw)
  To: Cary Coutant; +Cc: H.J. Lu, Binutils

[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]

Sorry, now with the correct patch.

On 18 September 2014 12:53, Rafael Espíndola <rafael.espindola@gmail.com> wrote:
>> +# plugin_test_6.sh -- a test case for the plugin API.
>> +
>> +# Copyright (C) 2010-2014 Free Software Foundation, Inc.
>> +# Written by Cary Coutant <ccoutant@google.com>.
>>
>> Update these lines.
>
> Done.
>
>> +readelf -SW plugin_test_10 > plugin_test_10.sections
>> +grep ".bss.* 8$" plugin_test_10.sections > /dev/null
>> +
>> +exit 0
>>
>> The readelf command should be run from the Makefile itself. Create a
>> new rule for plugin_test_10.sections, and use $(TEST_READELF). There
>> are several examples of this in testsuite/Makefile.am.
>>
>> Then your test script only needs to run grep, but use grep's "-q"
>> option instead of "> /dev/null".
>
> Done.
>
> New patch attached.
>
> Cheers,
> Rafael
>
> 2014-09-18  Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
>
>         * testsuite/Makefile.am (plugin_test_10): New test.
>         * testsuite/Makefile.in: Regenerate
>         * testsuite/plugin_common_test_2.c (c1): Align to 8.
>         * testsuite/plugin_test_10.sh: New file.

[-- Attachment #2: t.patch --]
[-- Type: text/x-patch, Size: 11904 bytes --]

diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 5e1cd0d..dd06d5f 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -1657,6 +1657,17 @@ MOSTLYCLEANFILES += two_file_test_1c.o
 two_file_test_1c.o: two_file_test_1.o
 	cp two_file_test_1.o $@
 
+check_PROGRAMS += plugin_test_10
+check_SCRIPTS += plugin_test_10.sh
+check_DATA += plugin_test_10.sections
+MOSTLYCLEANFILES += plugin_test_10.sections
+plugin_test_10: plugin_common_test_1.syms plugin_common_test_2.o  gcctestdir/ld plugin_test.so
+	$(CXXLINK) -Bgcctestdir/ -Wl,--no-demangle,--plugin,"./plugin_test.so" plugin_common_test_1.syms plugin_common_test_2.o
+plugin_test_10.sections: plugin_test_10
+	$(TEST_READELF) -SW $< >$@ 2>/dev/null
+
+
+
 plugin_test.so: plugin_test.o
 	$(LINK) -Bgcctestdir/ -shared plugin_test.o
 plugin_test.o: plugin_test.c
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index 7d0f78b..fed610f 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -350,14 +350,16 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_5 \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6 \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7 \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_8
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_8 \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_34 =  \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_1.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_2.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_3.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_4.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6.sh \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.sh
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10.sh
 
 # Test that symbols known in the IR file but not in the replacement file
 # produce an unresolved symbol error.
@@ -369,7 +371,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.syms \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_9.err
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_9.err \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10.sections
 # Make a copy of two_file_test_1.o, which does not define the symbol _Z4t16av.
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_36 =  \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_1.err \
@@ -380,7 +383,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_9.err \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	two_file_test_1c.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	two_file_test_1c.o \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10.sections
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@@TLS_TRUE@am__append_37 = plugin_test_tls
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@@TLS_TRUE@am__append_38 = plugin_test_tls.sh
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@@TLS_TRUE@am__append_39 = plugin_test_tls.err
@@ -874,7 +878,8 @@ libgoldtest_a_OBJECTS = $(am_libgoldtest_a_OBJECTS)
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_5$(EXEEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_6$(EXEEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_7$(EXEEXT) \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_8$(EXEEXT)
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_8$(EXEEXT) \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	plugin_test_10$(EXEEXT)
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@@TLS_TRUE@am__EXEEXT_24 = plugin_test_tls$(EXEEXT)
 @GCC_TRUE@@NATIVE_LINKER_TRUE@am__EXEEXT_25 =  \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	exclude_libs_test$(EXEEXT) \
@@ -1425,6 +1430,12 @@ plugin_test_1_LDADD = $(LDADD)
 plugin_test_1_DEPENDENCIES = libgoldtest.a ../libgold.a \
 	../../libiberty/libiberty.a $(am__DEPENDENCIES_1) \
 	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1)
+plugin_test_10_SOURCES = plugin_test_10.c
+plugin_test_10_OBJECTS = plugin_test_10.$(OBJEXT)
+plugin_test_10_LDADD = $(LDADD)
+plugin_test_10_DEPENDENCIES = libgoldtest.a ../libgold.a \
+	../../libiberty/libiberty.a $(am__DEPENDENCIES_1) \
+	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1)
 plugin_test_2_SOURCES = plugin_test_2.c
 plugin_test_2_OBJECTS = plugin_test_2.$(OBJEXT)
 plugin_test_2_LDADD = $(LDADD)
@@ -1888,14 +1899,14 @@ SOURCES = $(libgoldtest_a_SOURCES) basic_pic_test.c basic_pie_test.c \
 	local_labels_test.c many_sections_r_test.c \
 	$(many_sections_test_SOURCES) $(object_unittest_SOURCES) \
 	permission_test.c $(pie_copyrelocs_test_SOURCES) \
-	plugin_test_1.c plugin_test_2.c plugin_test_3.c \
-	plugin_test_4.c plugin_test_5.c plugin_test_6.c \
-	plugin_test_7.c plugin_test_8.c plugin_test_tls.c \
-	$(protected_1_SOURCES) $(protected_2_SOURCES) \
-	$(relro_now_test_SOURCES) $(relro_script_test_SOURCES) \
-	$(relro_strip_test_SOURCES) $(relro_test_SOURCES) \
-	$(script_test_1_SOURCES) script_test_11.c \
-	$(script_test_2_SOURCES) script_test_3.c \
+	plugin_test_1.c plugin_test_10.c plugin_test_2.c \
+	plugin_test_3.c plugin_test_4.c plugin_test_5.c \
+	plugin_test_6.c plugin_test_7.c plugin_test_8.c \
+	plugin_test_tls.c $(protected_1_SOURCES) \
+	$(protected_2_SOURCES) $(relro_now_test_SOURCES) \
+	$(relro_script_test_SOURCES) $(relro_strip_test_SOURCES) \
+	$(relro_test_SOURCES) $(script_test_1_SOURCES) \
+	script_test_11.c $(script_test_2_SOURCES) script_test_3.c \
 	$(searched_file_test_SOURCES) start_lib_test.c \
 	$(thin_archive_test_1_SOURCES) $(thin_archive_test_2_SOURCES) \
 	$(tls_phdrs_script_test_SOURCES) $(tls_pic_test_SOURCES) \
@@ -3274,6 +3285,15 @@ pie_copyrelocs_test$(EXEEXT): $(pie_copyrelocs_test_OBJECTS) $(pie_copyrelocs_te
 @PLUGINS_FALSE@plugin_test_1$(EXEEXT): $(plugin_test_1_OBJECTS) $(plugin_test_1_DEPENDENCIES) 
 @PLUGINS_FALSE@	@rm -f plugin_test_1$(EXEEXT)
 @PLUGINS_FALSE@	$(LINK) $(plugin_test_1_OBJECTS) $(plugin_test_1_LDADD) $(LIBS)
+@GCC_FALSE@plugin_test_10$(EXEEXT): $(plugin_test_10_OBJECTS) $(plugin_test_10_DEPENDENCIES) 
+@GCC_FALSE@	@rm -f plugin_test_10$(EXEEXT)
+@GCC_FALSE@	$(LINK) $(plugin_test_10_OBJECTS) $(plugin_test_10_LDADD) $(LIBS)
+@NATIVE_LINKER_FALSE@plugin_test_10$(EXEEXT): $(plugin_test_10_OBJECTS) $(plugin_test_10_DEPENDENCIES) 
+@NATIVE_LINKER_FALSE@	@rm -f plugin_test_10$(EXEEXT)
+@NATIVE_LINKER_FALSE@	$(LINK) $(plugin_test_10_OBJECTS) $(plugin_test_10_LDADD) $(LIBS)
+@PLUGINS_FALSE@plugin_test_10$(EXEEXT): $(plugin_test_10_OBJECTS) $(plugin_test_10_DEPENDENCIES) 
+@PLUGINS_FALSE@	@rm -f plugin_test_10$(EXEEXT)
+@PLUGINS_FALSE@	$(LINK) $(plugin_test_10_OBJECTS) $(plugin_test_10_LDADD) $(LIBS)
 @GCC_FALSE@plugin_test_2$(EXEEXT): $(plugin_test_2_OBJECTS) $(plugin_test_2_DEPENDENCIES) 
 @GCC_FALSE@	@rm -f plugin_test_2$(EXEEXT)
 @GCC_FALSE@	$(LINK) $(plugin_test_2_OBJECTS) $(plugin_test_2_LDADD) $(LIBS)
@@ -3657,6 +3677,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/permission_test.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/pie_copyrelocs_test.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_1.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_10.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_2.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_3.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/plugin_test_4.Po@am__quote@
@@ -4107,6 +4128,8 @@ plugin_test_6.sh.log: plugin_test_6.sh
 	@p='plugin_test_6.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_test_7.sh.log: plugin_test_7.sh
 	@p='plugin_test_7.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+plugin_test_10.sh.log: plugin_test_10.sh
+	@p='plugin_test_10.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_test_tls.sh.log: plugin_test_tls.sh
 	@p='plugin_test_tls.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_final_layout.sh.log: plugin_final_layout.sh
@@ -4387,6 +4410,8 @@ plugin_test_7.log: plugin_test_7$(EXEEXT)
 	@p='plugin_test_7$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_test_8.log: plugin_test_8$(EXEEXT)
 	@p='plugin_test_8$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+plugin_test_10.log: plugin_test_10$(EXEEXT)
+	@p='plugin_test_10$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_test_tls.log: plugin_test_tls$(EXEEXT)
 	@p='plugin_test_tls$(EXEEXT)'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 exclude_libs_test.log: exclude_libs_test$(EXEEXT)
@@ -5324,6 +5349,10 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	mv -f $@.tmp $@
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@two_file_test_1c.o: two_file_test_1.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	cp two_file_test_1.o $@
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test_10: plugin_common_test_1.syms plugin_common_test_2.o  gcctestdir/ld plugin_test.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	$(CXXLINK) -Bgcctestdir/ -Wl,--no-demangle,--plugin,"./plugin_test.so" plugin_common_test_1.syms plugin_common_test_2.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test_10.sections: plugin_test_10
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	$(TEST_READELF) -SW $< >$@ 2>/dev/null
 
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test.so: plugin_test.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@	$(LINK) -Bgcctestdir/ -shared plugin_test.o
diff --git a/gold/testsuite/plugin_common_test_2.c b/gold/testsuite/plugin_common_test_2.c
index c0c934c..df9f7f1 100644
--- a/gold/testsuite/plugin_common_test_2.c
+++ b/gold/testsuite/plugin_common_test_2.c
@@ -26,7 +26,7 @@
 
 #include <assert.h>
 
-int c1;
+int c1 __attribute__((aligned (8)));
 extern int c2;
 int c3;
 int c4 = 40;
diff --git a/gold/testsuite/plugin_test_10.sh b/gold/testsuite/plugin_test_10.sh
new file mode 100755
index 0000000..80b9f15
--- /dev/null
+++ b/gold/testsuite/plugin_test_10.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+# plugin_test_10.sh -- a test case for the plugin API.
+
+# Copyright (C) 2010-2014 Free Software Foundation, Inc.
+# Written by Rafael Ávila de Espíndola <rafael.espindola@gmail.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.
+
+# This file goes with plugin_common_test_1.c and plugin_common_test_2.c.
+# The first file is claimed by the plugin, the second one is not. We test
+# the bigger alignment in plugin_common_test_2.c is used.
+
+set -e
+
+grep -q ".bss.* 8$" plugin_test_10.sections
+
+exit 0

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-18 16:54           ` Rafael Espíndola
@ 2014-09-18 17:09             ` Cary Coutant
  2014-09-27  2:53               ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: Cary Coutant @ 2014-09-18 17:09 UTC (permalink / raw)
  To: Rafael Espíndola; +Cc: H.J. Lu, Binutils

>> 2014-09-18  Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
>>
>>         * testsuite/Makefile.am (plugin_test_10): New test.
>>         * testsuite/Makefile.in: Regenerate
>>         * testsuite/plugin_common_test_2.c (c1): Align to 8.
>>         * testsuite/plugin_test_10.sh: New file.

This is OK. Thanks!

-cary

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-18 17:09             ` Cary Coutant
@ 2014-09-27  2:53               ` Alan Modra
  2014-09-29 17:21                 ` Cary Coutant
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2014-09-27  2:53 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Rafael Espíndola, H.J. Lu, Binutils

On Thu, Sep 18, 2014 at 10:09:06AM -0700, Cary Coutant wrote:
> >> 2014-09-18  Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
> >>
> >>         * testsuite/Makefile.am (plugin_test_10): New test.
> >>         * testsuite/Makefile.in: Regenerate
> >>         * testsuite/plugin_common_test_2.c (c1): Align to 8.
> >>         * testsuite/plugin_test_10.sh: New file.
> 
> This is OK. Thanks!

Fails on x86_64 when using mainline gcc.  c1 is aligned (value = 8) in
the relocatable object file
    20: 0000000000000008     4 OBJECT  GLOBAL DEFAULT  COM c1

plugin_common_test_1.syms:
    23: 0000000000000004     4 OBJECT  GLOBAL DEFAULT  COM c1

Final object file:
    39: 0000000000401b7c     4 OBJECT  GLOBAL DEFAULT   25 c1
and .bss only has an alignment of 4.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-27  2:53               ` Alan Modra
@ 2014-09-29 17:21                 ` Cary Coutant
  2014-09-30  2:22                   ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: Cary Coutant @ 2014-09-29 17:21 UTC (permalink / raw)
  To: Cary Coutant, Rafael Espíndola, H.J. Lu, Binutils

Is this still failing even after my patch last Thursday for PR
gold/17432? Or did it only start failing after that patch?

I see 8 in the value field in the .syms file, and an alignment of 8 on
.bss in the executable.

> Fails on x86_64 when using mainline gcc.  c1 is aligned (value = 8) in
> the relocatable object file
>     20: 0000000000000008     4 OBJECT  GLOBAL DEFAULT  COM c1

That looks like a dump of plugin_common_test_2.o.

> plugin_common_test_1.syms:
>     23: 0000000000000004     4 OBJECT  GLOBAL DEFAULT  COM c1

But this is from plugin_common_test_1.o. I expect
plugin_common_test_1.syms to have an alignment of 4.

> Final object file:
>     39: 0000000000401b7c     4 OBJECT  GLOBAL DEFAULT   25 c1
> and .bss only has an alignment of 4.

Is this from plugin_test_10?

Here's what I see:

$ grep c1 plugin_common_test_2.syms
    17: 0000000000000008     4 OBJECT  GLOBAL DEFAULT  COM c1

$ readelf -SW plugin_test_10 | grep \\.bss
  [26] .bss              NOBITS          0000000000401c30 000c30
00001c 00  WA  0   0  8

$ readelf -sW plugin_test_10 | grep c1
    51: 0000000000401c40     4 OBJECT  GLOBAL DEFAULT   26 c1

-cary

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-29 17:21                 ` Cary Coutant
@ 2014-09-30  2:22                   ` Alan Modra
  2014-09-30  4:14                     ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2014-09-30  2:22 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Rafael Espíndola, H.J. Lu, Binutils

On Mon, Sep 29, 2014 at 10:21:44AM -0700, Cary Coutant wrote:
> Is this still failing even after my patch last Thursday for PR
> gold/17432? Or did it only start failing after that patch?
> 
> I see 8 in the value field in the .syms file, and an alignment of 8 on
> .bss in the executable.
> 
> > Fails on x86_64 when using mainline gcc.  c1 is aligned (value = 8) in
> > the relocatable object file
> >     20: 0000000000000008     4 OBJECT  GLOBAL DEFAULT  COM c1
> 
> That looks like a dump of plugin_common_test_2.o.

Yes.

> > plugin_common_test_1.syms:
> >     23: 0000000000000004     4 OBJECT  GLOBAL DEFAULT  COM c1
> 
> But this is from plugin_common_test_1.o. I expect
> plugin_common_test_1.syms to have an alignment of 4.

Right.

> > Final object file:
> >     39: 0000000000401b7c     4 OBJECT  GLOBAL DEFAULT   25 c1
> > and .bss only has an alignment of 4.
> 
> Is this from plugin_test_10?

Yes.  The breakage appears to be due to 5efeedf6.  Prior to that the
test passes.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-30  2:22                   ` Alan Modra
@ 2014-09-30  4:14                     ` Alan Modra
  2014-09-30 23:01                       ` Cary Coutant
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2014-09-30  4:14 UTC (permalink / raw)
  To: Cary Coutant, Rafael Espíndola, H.J. Lu, Binutils

On Tue, Sep 30, 2014 at 11:50:30AM +0930, Alan Modra wrote:
> On Mon, Sep 29, 2014 at 10:21:44AM -0700, Cary Coutant wrote:
> > Is this still failing even after my patch last Thursday for PR
> > gold/17432? Or did it only start failing after that patch?
> > 
> > I see 8 in the value field in the .syms file, and an alignment of 8 on
> > .bss in the executable.
> > 
> > > Fails on x86_64 when using mainline gcc.  c1 is aligned (value = 8) in
> > > the relocatable object file
> > >     20: 0000000000000008     4 OBJECT  GLOBAL DEFAULT  COM c1
> > 
> > That looks like a dump of plugin_common_test_2.o.
> 
> Yes.
> 
> > > plugin_common_test_1.syms:
> > >     23: 0000000000000004     4 OBJECT  GLOBAL DEFAULT  COM c1
> > 
> > But this is from plugin_common_test_1.o. I expect
> > plugin_common_test_1.syms to have an alignment of 4.
> 
> Right.
> 
> > > Final object file:
> > >     39: 0000000000401b7c     4 OBJECT  GLOBAL DEFAULT   25 c1
> > > and .bss only has an alignment of 4.
> > 
> > Is this from plugin_test_10?
> 
> Yes.  The breakage appears to be due to 5efeedf6.  Prior to that the
> test passes.

Oops, wrong commit.  The patch that causes the failure is the one you
suspected, 1707f183, your PR 17432 change.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [patch] Fix handling of common symbols with plugins
  2014-09-30  4:14                     ` Alan Modra
@ 2014-09-30 23:01                       ` Cary Coutant
  0 siblings, 0 replies; 15+ messages in thread
From: Cary Coutant @ 2014-09-30 23:01 UTC (permalink / raw)
  To: Cary Coutant, Rafael Espíndola, H.J. Lu, Binutils

> Oops, wrong commit.  The patch that causes the failure is the one you
> suspected, 1707f183, your PR 17432 change.

It was a cut-and-paste error on my part, sorry! This should fix it...

-cary


diff --git a/gold/resolve.cc b/gold/resolve.cc
index 52dae8b..07dff4a 100644
--- a/gold/resolve.cc
+++ b/gold/resolve.cc
@@ -317,8 +317,8 @@ Symbol_table::resolve(Sized_symbol<size>* to,
          if (to->is_common() && !is_ordinary && st_shndx == elfcpp::SHN_COMMON)
            {
              adjust_common = true;
-             typename Sized_symbol<size>::Size_type tosize = to->symsize();
-             typename Sized_symbol<size>::Value_type tovalue = to->value();
+             tosize = to->symsize();
+             tovalue = to->value();
            }
          this->override(to, sym, st_shndx, is_ordinary, object, version);
          if (adjust_common)

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

end of thread, other threads:[~2014-09-30 23:01 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-09 23:42 [patch] Fix handling of common symbols with plugins Rafael Espíndola
2014-09-16 15:33 ` Rafael Espíndola
2014-09-17 17:17 ` Cary Coutant
2014-09-17 17:20   ` H.J. Lu
2014-09-17 22:36     ` Rafael Espíndola
2014-09-17 22:46       ` Cary Coutant
2014-09-18 16:53         ` Rafael Espíndola
2014-09-18 16:54           ` Rafael Espíndola
2014-09-18 17:09             ` Cary Coutant
2014-09-27  2:53               ` Alan Modra
2014-09-29 17:21                 ` Cary Coutant
2014-09-30  2:22                   ` Alan Modra
2014-09-30  4:14                     ` Alan Modra
2014-09-30 23:01                       ` Cary Coutant
2014-09-18 14:43       ` H.J. Lu

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