public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Improve building with LTO
@ 2021-02-14 22:57 Alexander Miller
  2021-02-17 20:22 ` Mark Wielaard
  2021-02-18  2:38 ` [PATCH v2] " Alexander Miller
  0 siblings, 2 replies; 17+ messages in thread
From: Alexander Miller @ 2021-02-14 22:57 UTC (permalink / raw)
  To: elfutils-devel

Here's my attempt to use gcc-10's new symver attribute to avoid global
asm statements that cause trouble with LTO. That requires converting
from triple @ syntax to double @ syntax. To satisfy those picky linkers,
the unversioned name of the affected symbols has to be changed.
The NEW_VERSION macros have to be moved before the function definitions.

Also tried to improve the situation for older gcc versions. Although
gcc-5 added a no_reorder attribute that's supposed to help here, it
doesn't work reliably (but improves the situation substantially).

I've tested the patch with different compilers (gcc-10, gcc-9, gcc-6,
clang-11), linkers (bfd, gold, lld), 32/64 bit (x86), with/without LTO.
Of course you need to cheat a bit to build elfutils with clang, and
lld can't be used with every combination. The workaround for older gcc
was enough for gcc-9 and 32bit gcc-6, but 64bit gcc-6 builds needed
-flto-partition=none, so this seems to depend on the version and options
used.

Symbol versioning worked as expected in all cases, at least the list
of dynamic symbols of the libs looked good to me.

The test suite seems brittle, though. It fails on 32bit builds, with
gold or lld, and with lto builds using clang (unknown object format)
or gcc-6 (debug info not found). But that's not related to this patch.
For 64bit builds with gcc-{9,10} and bfd, the test suite succeeds even
with lto enabled.

Additional notes:
* The asm names for the compat versions seem unnecessary, but I've
  kept them.
* Commenting out old versions in the .map file may not be needed.
  It's mostly a leftover from an earlier attempt, but I didn't want to
  re-run all test and I actually prefer it like this, so I left it in.
* See commit message below.

------------------------ 8< ------------------------
Use symver attribute for symbol versioning instead of .symver
assembler directive when available. Convert to use double @ syntax
for default version in all cases (required when using the attribute).

Add the attributes externally_visible, no_reorder if available when
using assembler directives to improve the situation for < gcc-10.
This is not 100% reliable, though; -flto-partition=none may still be
needed in some cases.

Note that -Wno-error=stack-usage= is still needed to build with LTO.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24498
Signed-off-by: Alexander Miller <alex.miller@gmx.de>
---
 lib/ChangeLog                  | 13 +++++++
 lib/eu-config.h                | 63 +++++++++++++++++++++++++++-------
 libdw/ChangeLog                | 13 +++++++
 libdw/dwarf_aggregate_size.c   |  4 +--
 libdw/dwarf_arrayorder.c       |  2 +-
 libdw/dwarf_bitoffset.c        |  2 +-
 libdw/dwarf_bitsize.c          |  2 +-
 libdw/dwarf_bytesize.c         |  2 +-
 libdw/dwarf_decl_column.c      |  2 +-
 libdw/dwarf_decl_file.c        |  2 +-
 libdw/dwarf_decl_line.c        |  2 +-
 libdw/dwarf_srclang.c          |  4 +--
 libdw/libdw.map                | 41 +++++++++++-----------
 libdwelf/ChangeLog             |  5 +++
 libdwelf/dwelf_elf_begin.c     |  2 +-
 libdwfl/ChangeLog              |  7 ++++
 libdwfl/core-file.c            |  4 +--
 libdwfl/dwfl_module_build_id.c |  4 +--
 libdwfl/dwfl_report_elf.c      |  4 +--
 19 files changed, 128 insertions(+), 50 deletions(-)

diff --git a/lib/ChangeLog b/lib/ChangeLog
index 371e213..1f4cd62 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,3 +1,16 @@
+2021-02-14  Alexander Miller  <alex.miller@gmx.de>
+
+	* eu-config.h (used_in_asm): New macro.
+	(NEW_INTDEF): New macro.
+	(NEW_VERSION): Mark symbol as used_in_asm.  Use @@ symver and change
+	asm name instead.  New variant using symver attribute if available.
+	(OLD_VERSION): Update new symbol name. Indent asm directives.  New
+	variant using symver attribute.
+	(COMPAT_VERSION_NEWPROTO): Mark symbol as used_in_asm.  Reorder
+	lines.  Replace asm with __asm__ in declaration.  New variant using
+	symver attribute.
+	(COMPAT_VERSION): Likewise.
+
 2021-02-05  Mark Wielaard  <mark@klomp.org>
 
 	* printversion.c (print_version): Update copyright year.
diff --git a/lib/eu-config.h b/lib/eu-config.h
index f0e3d07..7e9bd1e 100644
--- a/lib/eu-config.h
+++ b/lib/eu-config.h
@@ -176,27 +176,66 @@ asm (".section predict_data, \"aw\"; .previous\n"
 /* This macro is used by the tests conditionalize for standalone building.  */
 #define ELFUTILS_HEADER(name) <lib##name.h>
 
+/* Don't reorder with global asm blocks or optimize away. (Doesn't reliably
+   keep it in the same LTO partition, though; -flto-partition=none may be
+   still needed for some gcc versions < 10.) */
+#ifdef __has_attribute
+# if __has_attribute(no_reorder)
+#  define used_in_asm __attribute__ ((externally_visible, no_reorder))
+# endif
+#endif
+#ifndef used_in_asm
+# define used_in_asm /* empty */
+#endif
 
 #ifdef SYMBOL_VERSIONING
-# define OLD_VERSION(name, version) \
-  asm (".globl _compat." #version "." #name "\n" \
-       "_compat." #version "." #name " = " #name "\n" \
-       ".symver _compat." #version "." #name "," #name "@" #version);
-# define NEW_VERSION(name, version) \
-  asm (".symver " #name "," #name "@@@" #version);
-# define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
-  asm (".symver _compat." #version "." #name "," #name "@" #version); \
+# define NEW_INTDEF(name) __typeof (name) INTUSE(name) \
+  __attribute__ ((alias ("_new." #name))) attribute_hidden;
+# ifdef __has_attribute
+#  if __has_attribute(symver)
+#   define NEW_VERSION(name, version) \
+  __typeof (name) name __asm__ ("_new." #name) \
+    __attribute__ ((symver (#name "@@" #version)));
+#   define OLD_VERSION(name, version) \
+  __typeof (name) _compat_old##__COUNTER__##_##name \
+    __asm__ ("_compat." #version "." #name) \
+    __attribute__ ((alias ("_new." #name), symver (#name "@" #version)));
+#   define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
   __typeof (_compat_##prefix##_##name) _compat_##prefix##_##name \
-    asm ("_compat." #version "." #name);
-# define COMPAT_VERSION(name, version, prefix) \
+    __asm__ ("_compat." #version "." #name) \
+    __attribute__ ((symver (#name "@" #version)));
+#   define COMPAT_VERSION(name, version, prefix) \
   asm (".symver _compat." #version "." #name "," #name "@" #version); \
-  __typeof (name) _compat_##prefix##_##name asm ("_compat." #version "." #name);
+  __typeof (name) _compat_##prefix##_##name \
+    __asm__ ("_compat." #version "." #name) \
+    __attribute__ ((symver (#name "@" #version)));
+#  endif
+# endif
+# ifndef NEW_VERSION
+#  define OLD_VERSION(name, version) \
+  asm (".globl _compat." #version "." #name "\n\t" \
+       "_compat." #version "." #name " = _new." #name "\n\t" \
+       ".symver _compat." #version "." #name "," #name "@" #version);
+#  define NEW_VERSION(name, version) \
+  __typeof (name) name __asm__ ("_new." #name) used_in_asm; \
+  asm (".symver _new." #name ", " #name "@@" #version);
+#  define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
+  __typeof (_compat_##prefix##_##name) _compat_##prefix##_##name \
+    __asm__ ("_compat." #version "." #name) used_in_asm; \
+  asm (".symver _compat." #version "." #name ", " #name "@" #version);
+#  define COMPAT_VERSION(name, version, prefix) \
+  __typeof (name) _compat_##prefix##_##name \
+    __asm__ ("_compat." #version "." #name) used_in_asm; \
+  asm (".symver _compat." #version "." #name ", " #name "@" #version);
+# endif
 #else
+# define NEW_INTDEF(name) INTDEF(name)
 # define OLD_VERSION(name, version) /* Nothing for static linking.  */
 # define NEW_VERSION(name, version) /* Nothing for static linking.  */
 # define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
   error "should use #ifdef SYMBOL_VERSIONING"
-# define COMPAT_VERSION(name, version, prefix) error "should use #ifdef SYMBOL_VERSIONING"
+# define COMPAT_VERSION(name, version, prefix) \
+  error "should use #ifdef SYMBOL_VERSIONING"
 #endif
 
 #ifndef FALLTHROUGH
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index b8038f0..aa14f00 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,16 @@
+2021-02-14  Alexander Miller  <alex.miller@gmx.de>
+
+	* libdw.map: Move local symbols to node ELFUTILS_0.  Comment
+	out old symbol versions.
+	* dwarf_aggregate_size.c (dwarf_aggregate_size): Move NEW_VERSION
+	before definition.  Replace INTDEF with NEW_INTDEF.
+	* dwarf_srclang.c (dwarf_srclang): Likewise.
+	* dwarf_arrayorder.c (dwarf_arrayorder): Move NEW_VERSION.
+	* dwarf_bitoffset.c (dwarf_bitoffset): Likewise.
+	* dwarf_bitsize.c (dwarf_bitsize): Likewise.
+	* dwarf_bytesize.c (dwarf_bytesize): Likewise.
+	* dwarf_decl_column.c (dwarf_decl_column): Likewise.
+
 2020-12-20  Dmitry V. Levin  <ldv@altlinux.org>
 
 	* .gitignore: New file.
diff --git a/libdw/dwarf_aggregate_size.c b/libdw/dwarf_aggregate_size.c
index 75105e4..552f122 100644
--- a/libdw/dwarf_aggregate_size.c
+++ b/libdw/dwarf_aggregate_size.c
@@ -209,6 +209,7 @@ aggregate_size (Dwarf_Die *die, Dwarf_Word *size,
   return -1;
 }
 
+NEW_VERSION (dwarf_aggregate_size, ELFUTILS_0.161)
 int
 dwarf_aggregate_size (Dwarf_Die *die, Dwarf_Word *size)
 {
@@ -219,6 +220,5 @@ dwarf_aggregate_size (Dwarf_Die *die, Dwarf_Word *size)
 
   return aggregate_size (&die_mem, size, &type_mem, 0);
 }
-INTDEF (dwarf_aggregate_size)
+NEW_INTDEF (dwarf_aggregate_size)
 OLD_VERSION (dwarf_aggregate_size, ELFUTILS_0.144)
-NEW_VERSION (dwarf_aggregate_size, ELFUTILS_0.161)
diff --git a/libdw/dwarf_arrayorder.c b/libdw/dwarf_arrayorder.c
index da64f99..782e075 100644
--- a/libdw/dwarf_arrayorder.c
+++ b/libdw/dwarf_arrayorder.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_arrayorder, ELFUTILS_0.143)
 int
 dwarf_arrayorder (Dwarf_Die *die)
 {
@@ -46,4 +47,3 @@ dwarf_arrayorder (Dwarf_Die *die)
 				  &value) == 0 ? (int) value : -1;
 }
 OLD_VERSION (dwarf_arrayorder, ELFUTILS_0.122)
-NEW_VERSION (dwarf_arrayorder, ELFUTILS_0.143)
diff --git a/libdw/dwarf_bitoffset.c b/libdw/dwarf_bitoffset.c
index c1a3a34..61a0d59 100644
--- a/libdw/dwarf_bitoffset.c
+++ b/libdw/dwarf_bitoffset.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_bitoffset, ELFUTILS_0.143)
 int
 dwarf_bitoffset (Dwarf_Die *die)
 {
@@ -46,4 +47,3 @@ dwarf_bitoffset (Dwarf_Die *die)
 				  &value) == 0 ? (int) value : -1;
 }
 OLD_VERSION (dwarf_bitoffset, ELFUTILS_0.122)
-NEW_VERSION (dwarf_bitoffset, ELFUTILS_0.143)
diff --git a/libdw/dwarf_bitsize.c b/libdw/dwarf_bitsize.c
index 0ed9b71..35e8744 100644
--- a/libdw/dwarf_bitsize.c
+++ b/libdw/dwarf_bitsize.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_bitsize, ELFUTILS_0.143)
 int
 dwarf_bitsize (Dwarf_Die *die)
 {
@@ -46,4 +47,3 @@ dwarf_bitsize (Dwarf_Die *die)
 				  &value) == 0 ? (int) value : -1;
 }
 OLD_VERSION (dwarf_bitsize, ELFUTILS_0.122)
-NEW_VERSION (dwarf_bitsize, ELFUTILS_0.143)
diff --git a/libdw/dwarf_bytesize.c b/libdw/dwarf_bytesize.c
index 116cd32..6d1ff9a 100644
--- a/libdw/dwarf_bytesize.c
+++ b/libdw/dwarf_bytesize.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_bytesize, ELFUTILS_0.143)
 int
 dwarf_bytesize (Dwarf_Die *die)
 {
@@ -46,4 +47,3 @@ dwarf_bytesize (Dwarf_Die *die)
 				  &value) == 0 ? (int) value : -1;
 }
 OLD_VERSION (dwarf_bytesize, ELFUTILS_0.122)
-NEW_VERSION (dwarf_bytesize, ELFUTILS_0.143)
diff --git a/libdw/dwarf_decl_column.c b/libdw/dwarf_decl_column.c
index 08d36b8..3225fd1 100644
--- a/libdw/dwarf_decl_column.c
+++ b/libdw/dwarf_decl_column.c
@@ -35,10 +35,10 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_decl_column, ELFUTILS_0.143)
 int
 dwarf_decl_column (Dwarf_Die *decl, int *colp)
 {
   return __libdw_attr_intval (decl, colp, DW_AT_decl_column);
 }
 OLD_VERSION (dwarf_decl_column, ELFUTILS_0.122)
-NEW_VERSION (dwarf_decl_column, ELFUTILS_0.143)
diff --git a/libdw/dwarf_decl_file.c b/libdw/dwarf_decl_file.c
index d4aa0a1..75662a3 100644
--- a/libdw/dwarf_decl_file.c
+++ b/libdw/dwarf_decl_file.c
@@ -36,6 +36,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_decl_file, ELFUTILS_0.143)
 const char *
 dwarf_decl_file (Dwarf_Die *die)
 {
@@ -86,4 +87,3 @@ dwarf_decl_file (Dwarf_Die *die)
   return cu->files->info[idx].name;
 }
 OLD_VERSION (dwarf_decl_file, ELFUTILS_0.122)
-NEW_VERSION (dwarf_decl_file, ELFUTILS_0.143)
diff --git a/libdw/dwarf_decl_line.c b/libdw/dwarf_decl_line.c
index 80fae6c..6b31eeb 100644
--- a/libdw/dwarf_decl_line.c
+++ b/libdw/dwarf_decl_line.c
@@ -37,13 +37,13 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_decl_line, ELFUTILS_0.143)
 int
 dwarf_decl_line (Dwarf_Die *func, int *linep)
 {
   return __libdw_attr_intval (func, linep, DW_AT_decl_line);
 }
 OLD_VERSION (dwarf_decl_line, ELFUTILS_0.122)
-NEW_VERSION (dwarf_decl_line, ELFUTILS_0.143)
 
 
 int internal_function
diff --git a/libdw/dwarf_srclang.c b/libdw/dwarf_srclang.c
index f10e764..77bd58c 100644
--- a/libdw/dwarf_srclang.c
+++ b/libdw/dwarf_srclang.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_srclang, ELFUTILS_0.143)
 int
 dwarf_srclang (Dwarf_Die *die)
 {
@@ -45,6 +46,5 @@ dwarf_srclang (Dwarf_Die *die)
 				  (die, DW_AT_language, &attr_mem),
 				  &value) == 0 ? (int) value : -1;
 }
-INTDEF (dwarf_srclang)
+NEW_INTDEF (dwarf_srclang)
 OLD_VERSION (dwarf_srclang, ELFUTILS_0.122)
-NEW_VERSION (dwarf_srclang, ELFUTILS_0.143)
diff --git a/libdw/libdw.map b/libdw/libdw.map
index 8ab0a2a..356ae4c 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -1,21 +1,25 @@
-ELFUTILS_0 { };
+ELFUTILS_0 {
+  local:
+    *;
+};
+
 ELFUTILS_0.122 {
   global:
     dwarf_abbrevhaschildren;
     dwarf_addrdie;
-    dwarf_arrayorder;
+    #dwarf_arrayorder;
     dwarf_attr;
     dwarf_attr_integrate;
     dwarf_begin;
     dwarf_begin_elf;
-    dwarf_bitoffset;
-    dwarf_bitsize;
-    dwarf_bytesize;
+    #dwarf_bitoffset;
+    #dwarf_bitsize;
+    #dwarf_bytesize;
     dwarf_child;
     dwarf_cuoffset;
-    dwarf_decl_column;
-    dwarf_decl_file;
-    dwarf_decl_line;
+    #dwarf_decl_column;
+    #dwarf_decl_file;
+    #dwarf_decl_line;
     dwarf_diecu;
     dwarf_diename;
     dwarf_dieoffset;
@@ -84,7 +88,7 @@ ELFUTILS_0.122 {
     dwarf_onesrcline;
     dwarf_ranges;
     dwarf_siblingof;
-    dwarf_srclang;
+    #dwarf_srclang;
     dwarf_tag;
     dwarf_whatattr;
     dwarf_whatform;
@@ -133,16 +137,13 @@ ELFUTILS_0.122 {
     dwfl_offline_section_address;
     dwfl_onesrcline;
     dwfl_report_begin;
-    dwfl_report_elf;
+    #dwfl_report_elf;
     dwfl_report_end;
     dwfl_report_module;
     dwfl_report_offline;
     dwfl_standard_argp;
     dwfl_standard_find_debuginfo;
     dwfl_version;
-
-  local:
-    *;
 } ELFUTILS_0;
 
 ELFUTILS_0.126 {
@@ -165,7 +166,7 @@ ELFUTILS_0.130 {
   global:
     dwfl_build_id_find_elf;
     dwfl_build_id_find_debuginfo;
-    dwfl_module_build_id;
+    #dwfl_module_build_id;
     dwfl_module_report_build_id;
 
 } ELFUTILS_0.127;
@@ -218,13 +219,13 @@ ELFUTILS_0.143 {
 } ELFUTILS_0.142;
 
 ELFUTILS_0.144 {
-  global:
-    dwarf_aggregate_size;
+  #global:
+    #dwarf_aggregate_size;
 } ELFUTILS_0.143;
 
 ELFUTILS_0.146 {
-  global:
-    dwfl_core_file_report;
+  #global:
+     #dwfl_core_file_report;
 } ELFUTILS_0.144;
 
 ELFUTILS_0.148 {
@@ -348,8 +349,8 @@ ELFUTILS_0.173 {
 } ELFUTILS_0.171;
 
 ELFUTILS_0.175 {
-  global:
-    dwelf_elf_begin;
+  #global:
+    #dwelf_elf_begin;
 } ELFUTILS_0.173;
 
 ELFUTILS_0.177 {
diff --git a/libdwelf/ChangeLog b/libdwelf/ChangeLog
index a0ff9f4..cbfe1ec 100644
--- a/libdwelf/ChangeLog
+++ b/libdwelf/ChangeLog
@@ -1,3 +1,8 @@
+2021-02-14  Alexander Miller  <alex.miller@gmx.de>
+
+	* dwelf_elf_begin.c (dwelf_elf_begin): Move NEW_VERSION before
+	definition.
+
 2020-12-12  Dmitry V. Levin  <ldv@altlinux.org>
 
 	* libdwelf.h: Fix spelling typos in comments.
diff --git a/libdwelf/dwelf_elf_begin.c b/libdwelf/dwelf_elf_begin.c
index c7d63a1..c3cfe63 100644
--- a/libdwelf/dwelf_elf_begin.c
+++ b/libdwelf/dwelf_elf_begin.c
@@ -36,6 +36,7 @@
 
 #include <unistd.h>
 
+NEW_VERSION (dwelf_elf_begin, ELFUTILS_0.177)
 Elf *
 dwelf_elf_begin (int fd)
 {
@@ -61,4 +62,3 @@ dwelf_elf_begin (int fd)
   return NULL;
 }
 OLD_VERSION (dwelf_elf_begin, ELFUTILS_0.175)
-NEW_VERSION (dwelf_elf_begin, ELFUTILS_0.177)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index d107e78..c96c716 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,10 @@
+2021-02-14  Alexander Miller  <alex.miller@gmx.de>
+
+	* core-file.c (dwfl_core_file_report): Move NEW_VERSION before
+	definition.  Replace INTDEF with NEW_INTDEF.
+	* dwfl_module_build_id.c (dwfl_module_build_id): Likewise.
+	* dwfl_report_elf.c (dwfl_report_elf): Likewise.
+
 2021-02-01  Érico Nogueira  <ericonr@disroot.org>
 
 	* dwfl_error.c (strerror_r): Only use the GNU version when available.
diff --git a/libdwfl/core-file.c b/libdwfl/core-file.c
index a0ccc9b..4e4c9b3 100644
--- a/libdwfl/core-file.c
+++ b/libdwfl/core-file.c
@@ -440,6 +440,7 @@ __libdwfl_dynamic_vaddr_get (Elf *elf, GElf_Addr *vaddrp)
   return false;
 }
 
+NEW_VERSION (dwfl_core_file_report, ELFUTILS_0.158)
 int
 dwfl_core_file_report (Dwfl *dwfl, Elf *elf, const char *executable)
 {
@@ -625,8 +626,7 @@ dwfl_core_file_report (Dwfl *dwfl, Elf *elf, const char *executable)
      error rather than just nothing found.  */
   return listed > 0 ? listed : retval;
 }
-INTDEF (dwfl_core_file_report)
-NEW_VERSION (dwfl_core_file_report, ELFUTILS_0.158)
+NEW_INTDEF (dwfl_core_file_report)
 
 #ifdef SYMBOL_VERSIONING
 int _compat_without_executable_dwfl_core_file_report (Dwfl *dwfl, Elf *elf);
diff --git a/libdwfl/dwfl_module_build_id.c b/libdwfl/dwfl_module_build_id.c
index 6ca9376..0c198f2 100644
--- a/libdwfl/dwfl_module_build_id.c
+++ b/libdwfl/dwfl_module_build_id.c
@@ -77,6 +77,7 @@ __libdwfl_find_build_id (Dwfl_Module *mod, bool set, Elf *elf)
   return found_build_id (mod, set, build_id_bits, build_id_len, build_id_vaddr);
 }
 
+NEW_VERSION (dwfl_module_build_id, ELFUTILS_0.138)
 int
 dwfl_module_build_id (Dwfl_Module *mod,
 		      const unsigned char **bits, GElf_Addr *vaddr)
@@ -102,8 +103,7 @@ dwfl_module_build_id (Dwfl_Module *mod,
   *vaddr = mod->build_id_vaddr;
   return mod->build_id_len;
 }
-INTDEF (dwfl_module_build_id)
-NEW_VERSION (dwfl_module_build_id, ELFUTILS_0.138)
+NEW_INTDEF (dwfl_module_build_id)
 
 #ifdef SYMBOL_VERSIONING
 COMPAT_VERSION (dwfl_module_build_id, ELFUTILS_0.130, vaddr_at_end)
diff --git a/libdwfl/dwfl_report_elf.c b/libdwfl/dwfl_report_elf.c
index 9da8669..a5f0e5e 100644
--- a/libdwfl/dwfl_report_elf.c
+++ b/libdwfl/dwfl_report_elf.c
@@ -287,6 +287,7 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name,
   return m;
 }
 
+NEW_VERSION (dwfl_report_elf, ELFUTILS_0.156)
 Dwfl_Module *
 dwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name, int fd,
 		 GElf_Addr base, bool add_p_vaddr)
@@ -322,8 +323,7 @@ dwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name, int fd,
 
   return mod;
 }
-INTDEF (dwfl_report_elf)
-NEW_VERSION (dwfl_report_elf, ELFUTILS_0.156)
+NEW_INTDEF (dwfl_report_elf)
 
 #ifdef SYMBOL_VERSIONING
 Dwfl_Module *
-- 
2.26.2


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

* Re: [PATCH] Improve building with LTO
  2021-02-14 22:57 [PATCH] Improve building with LTO Alexander Miller
@ 2021-02-17 20:22 ` Mark Wielaard
  2021-02-18 14:27   ` Alexander Miller
  2021-02-18  2:38 ` [PATCH v2] " Alexander Miller
  1 sibling, 1 reply; 17+ messages in thread
From: Mark Wielaard @ 2021-02-17 20:22 UTC (permalink / raw)
  To: Alexander Miller, elfutils-devel

Hi Alexander,

First my apologies, your last commit had "Alexander Miller via
Elfutils-devel <elfutils-devel@sourceware.org>" as author. That is the
stupid mailinglist doing From mangling, bad mailinglist. But also bad
me for not catching that. We probably need a git hook to reject pushes
with such bogus authors. Could you add a From: Alexander Miller <
alex.miller@gmx.de> in the body (I believe git send-email --from=...
will do that for you). That way git am does the right thing.

On Sun, 2021-02-14 at 23:57 +0100, Alexander Miller via Elfutils-devel wrote:
> Here's my attempt to use gcc-10's new symver attribute to avoid global
> asm statements that cause trouble with LTO. That requires converting
> from triple @ syntax to double @ syntax. To satisfy those picky linkers,
> the unversioned name of the affected symbols has to be changed.
> The NEW_VERSION macros have to be moved before the function definitions.

This is really exciting! I had more or less given up on this. I
couldn't make it work because of the triple @ syntax not being
supported. But it looks we don't need that after all.

> Also tried to improve the situation for older gcc versions. Although
> gcc-5 added a no_reorder attribute that's supposed to help here, it
> doesn't work reliably (but improves the situation substantially).

So you rewrite the asm statements to not use @@@ just @ and @@, that
way they match the symver attribute usage. Smart, that way they are as
similar as possible.

> I've tested the patch with different compilers (gcc-10, gcc-9, gcc-6,
> clang-11), linkers (bfd, gold, lld), 32/64 bit (x86), with/without LTO.
> Of course you need to cheat a bit to build elfutils with clang, and
> lld can't be used with every combination. The workaround for older gcc
> was enough for gcc-9 and 32bit gcc-6, but 64bit gcc-6 builds needed
> -flto-partition=none, so this seems to depend on the version and options
> used.

Wow, you really went above and beyond. IMHO it would have been fine to
simply say that you need GCC10 and a recent binutils ld to get LTO
working. Supporting LTO with gcc-6 is really nice, but people stuck on
such an old compiler should probably first look at upgrading that than
trying to play with LTO. We should probably look at making an LTO build
part of the buildbot.

> Symbol versioning worked as expected in all cases, at least the list
> of dynamic symbols of the libs looked good to me.
> 
> The test suite seems brittle, though. It fails on 32bit builds, with
> gold or lld, and with lto builds using clang (unknown object format)
> or gcc-6 (debug info not found). But that's not related to this patch.
> For 64bit builds with gcc-{9,10} and bfd, the test suite succeeds even
> with lto enabled.

Do the 32bit builds with gcc10/binutils ld have a clean test suite?

> Additional notes:
> * The asm names for the compat versions seem unnecessary, but I've
>   kept them.
> * Commenting out old versions in the .map file may not be needed.
>   It's mostly a leftover from an earlier attempt, but I didn't want to
>   re-run all test and I actually prefer it like this, so I left it in.

I would like to make sure it isn't needed. Having to comment out old
versions is a bit of a pain. In my own attempts I actually tried it the
other way around. Keep the earliest version in the version script and
don't add (remove) new versions (those would come from the symver).
That is slightly easier for maintenance, so you don't need to remove
older versions, just add the new (.symver) version in the source.

> * See commit message below.
> 
> ------------------------ 8< ------------------------
> Use symver attribute for symbol versioning instead of .symver
> assembler directive when available. Convert to use double @ syntax
> for default version in all cases (required when using the attribute).
> 
> Add the attributes externally_visible, no_reorder if available when
> using assembler directives to improve the situation for < gcc-10.
> This is not 100% reliable, though; -flto-partition=none may still be
> needed in some cases.

How does the __COUNTER__ magic work?

> Note that -Wno-error=stack-usage= is still needed to build with LTO.

Yeah that is a pity. The issue is in just a few files, but with LTO
those get combined with all the others causing issues. I believe the
warnings are only in the tools now. It would be really good if we could
get rid of the stack-usage warnings in: readelf.c, nm.c, size.c,
strip.c, elflint.c, findtextrel.c, elfcmp.c, objdump.c, ranlib.c, ar.c,
unstrip.c. But that is for another day.

I have to stare at the marcos a bit to make sure I really understand
what is going on. But this looks really good.

Thanks,

Mark

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

* [PATCH v2] Improve building with LTO
  2021-02-14 22:57 [PATCH] Improve building with LTO Alexander Miller
  2021-02-17 20:22 ` Mark Wielaard
@ 2021-02-18  2:38 ` Alexander Miller
  2021-08-28  9:31   ` Dmitry V. Levin
  1 sibling, 1 reply; 17+ messages in thread
From: Alexander Miller @ 2021-02-18  2:38 UTC (permalink / raw)
  To: elfutils-devel

From: Alexander Miller <alex.miller@gmx.de>

Use symver attribute for symbol versioning instead of .symver
assembler directive when available. Convert to use double @ syntax
for default version in all cases (required when using the attribute).

Add the attributes externally_visible, no_reorder if available when
using assembler directives to improve the situation for < gcc-10.
This is not 100% reliable, though; -flto-partition=none may still be
needed in some cases.

Note that -Wno-error=stack-usage= is still needed to build with LTO.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24498
Signed-off-by: Alexander Miller <alex.miller@gmx.de>
---
 lib/ChangeLog                  | 13 +++++++
 lib/eu-config.h                | 65 +++++++++++++++++++++++++++-------
 libdw/ChangeLog                | 11 ++++++
 libdw/dwarf_aggregate_size.c   |  4 +--
 libdw/dwarf_arrayorder.c       |  2 +-
 libdw/dwarf_bitoffset.c        |  2 +-
 libdw/dwarf_bitsize.c          |  2 +-
 libdw/dwarf_bytesize.c         |  2 +-
 libdw/dwarf_decl_column.c      |  2 +-
 libdw/dwarf_decl_file.c        |  2 +-
 libdw/dwarf_decl_line.c        |  2 +-
 libdw/dwarf_srclang.c          |  4 +--
 libdwelf/ChangeLog             |  5 +++
 libdwelf/dwelf_elf_begin.c     |  2 +-
 libdwfl/ChangeLog              |  7 ++++
 libdwfl/core-file.c            |  4 +--
 libdwfl/dwfl_module_build_id.c |  4 +--
 libdwfl/dwfl_report_elf.c      |  4 +--
 18 files changed, 107 insertions(+), 30 deletions(-)

diff --git a/lib/ChangeLog b/lib/ChangeLog
index 371e213..1f4cd62 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,3 +1,16 @@
+2021-02-14  Alexander Miller  <alex.miller@gmx.de>
+
+	* eu-config.h (used_in_asm): New macro.
+	(NEW_INTDEF): New macro.
+	(NEW_VERSION): Mark symbol as used_in_asm.  Use @@ symver and change
+	asm name instead.  New variant using symver attribute if available.
+	(OLD_VERSION): Update new symbol name. Indent asm directives.  New
+	variant using symver attribute.
+	(COMPAT_VERSION_NEWPROTO): Mark symbol as used_in_asm.  Reorder
+	lines.  Replace asm with __asm__ in declaration.  New variant using
+	symver attribute.
+	(COMPAT_VERSION): Likewise.
+
 2021-02-05  Mark Wielaard  <mark@klomp.org>
 
 	* printversion.c (print_version): Update copyright year.
diff --git a/lib/eu-config.h b/lib/eu-config.h
index f0e3d07..c7d7cbb 100644
--- a/lib/eu-config.h
+++ b/lib/eu-config.h
@@ -176,27 +176,68 @@ asm (".section predict_data, \"aw\"; .previous\n"
 /* This macro is used by the tests conditionalize for standalone building.  */
 #define ELFUTILS_HEADER(name) <lib##name.h>
 
+/* Don't reorder with global asm blocks or optimize away. (Doesn't reliably
+   keep it in the same LTO partition, though; -flto-partition=none may be
+   still needed for some gcc versions < 10.) */
+#ifdef __has_attribute
+# if __has_attribute(no_reorder)
+#  define used_in_asm __attribute__ ((externally_visible, no_reorder))
+# endif
+#endif
+#ifndef used_in_asm
+# define used_in_asm /* empty */
+#endif
 
 #ifdef SYMBOL_VERSIONING
-# define OLD_VERSION(name, version) \
-  asm (".globl _compat." #version "." #name "\n" \
-       "_compat." #version "." #name " = " #name "\n" \
-       ".symver _compat." #version "." #name "," #name "@" #version);
-# define NEW_VERSION(name, version) \
-  asm (".symver " #name "," #name "@@@" #version);
-# define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
-  asm (".symver _compat." #version "." #name "," #name "@" #version); \
+# define NEW_INTDEF(name) __typeof (name) INTUSE(name) \
+  __attribute__ ((alias ("_new." #name))) attribute_hidden;
+# ifdef __has_attribute
+#  if __has_attribute(symver)
+#   define NEW_VERSION(name, version) \
+  __typeof (name) name __asm__ ("_new." #name) \
+    __attribute__ ((symver (#name "@@" #version)));
+#   define OLD_VERSION(name, version) _OLD_VERSION1(name, __COUNTER__, version)
+#   define _OLD_VERSION1(name, num, version) _OLD_VERSION2(name, num, version)
+#   define _OLD_VERSION2(name, num, version) \
+  __typeof (name) _compat_old##num##_##name \
+    __asm__ ("_compat." #version "." #name) \
+    __attribute__ ((alias ("_new." #name), symver (#name "@" #version)));
+#   define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
   __typeof (_compat_##prefix##_##name) _compat_##prefix##_##name \
-    asm ("_compat." #version "." #name);
-# define COMPAT_VERSION(name, version, prefix) \
+    __asm__ ("_compat." #version "." #name) \
+    __attribute__ ((symver (#name "@" #version)));
+#   define COMPAT_VERSION(name, version, prefix) \
   asm (".symver _compat." #version "." #name "," #name "@" #version); \
-  __typeof (name) _compat_##prefix##_##name asm ("_compat." #version "." #name);
+  __typeof (name) _compat_##prefix##_##name \
+    __asm__ ("_compat." #version "." #name) \
+    __attribute__ ((symver (#name "@" #version)));
+#  endif
+# endif
+# ifndef NEW_VERSION
+#  define OLD_VERSION(name, version) \
+  asm (".globl _compat." #version "." #name "\n\t" \
+       "_compat." #version "." #name " = _new." #name "\n\t" \
+       ".symver _compat." #version "." #name "," #name "@" #version);
+#  define NEW_VERSION(name, version) \
+  __typeof (name) name __asm__ ("_new." #name) used_in_asm; \
+  asm (".symver _new." #name ", " #name "@@" #version);
+#  define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
+  __typeof (_compat_##prefix##_##name) _compat_##prefix##_##name \
+    __asm__ ("_compat." #version "." #name) used_in_asm; \
+  asm (".symver _compat." #version "." #name ", " #name "@" #version);
+#  define COMPAT_VERSION(name, version, prefix) \
+  __typeof (name) _compat_##prefix##_##name \
+    __asm__ ("_compat." #version "." #name) used_in_asm; \
+  asm (".symver _compat." #version "." #name ", " #name "@" #version);
+# endif
 #else
+# define NEW_INTDEF(name) INTDEF(name)
 # define OLD_VERSION(name, version) /* Nothing for static linking.  */
 # define NEW_VERSION(name, version) /* Nothing for static linking.  */
 # define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
   error "should use #ifdef SYMBOL_VERSIONING"
-# define COMPAT_VERSION(name, version, prefix) error "should use #ifdef SYMBOL_VERSIONING"
+# define COMPAT_VERSION(name, version, prefix) \
+  error "should use #ifdef SYMBOL_VERSIONING"
 #endif
 
 #ifndef FALLTHROUGH
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index b8038f0..597f54b 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,14 @@
+2021-02-14  Alexander Miller  <alex.miller@gmx.de>
+
+	* dwarf_aggregate_size.c (dwarf_aggregate_size): Move NEW_VERSION
+	before definition.  Replace INTDEF with NEW_INTDEF.
+	* dwarf_srclang.c (dwarf_srclang): Likewise.
+	* dwarf_arrayorder.c (dwarf_arrayorder): Move NEW_VERSION.
+	* dwarf_bitoffset.c (dwarf_bitoffset): Likewise.
+	* dwarf_bitsize.c (dwarf_bitsize): Likewise.
+	* dwarf_bytesize.c (dwarf_bytesize): Likewise.
+	* dwarf_decl_column.c (dwarf_decl_column): Likewise.
+
 2020-12-20  Dmitry V. Levin  <ldv@altlinux.org>
 
 	* .gitignore: New file.
diff --git a/libdw/dwarf_aggregate_size.c b/libdw/dwarf_aggregate_size.c
index 75105e4..552f122 100644
--- a/libdw/dwarf_aggregate_size.c
+++ b/libdw/dwarf_aggregate_size.c
@@ -209,6 +209,7 @@ aggregate_size (Dwarf_Die *die, Dwarf_Word *size,
   return -1;
 }
 
+NEW_VERSION (dwarf_aggregate_size, ELFUTILS_0.161)
 int
 dwarf_aggregate_size (Dwarf_Die *die, Dwarf_Word *size)
 {
@@ -219,6 +220,5 @@ dwarf_aggregate_size (Dwarf_Die *die, Dwarf_Word *size)
 
   return aggregate_size (&die_mem, size, &type_mem, 0);
 }
-INTDEF (dwarf_aggregate_size)
+NEW_INTDEF (dwarf_aggregate_size)
 OLD_VERSION (dwarf_aggregate_size, ELFUTILS_0.144)
-NEW_VERSION (dwarf_aggregate_size, ELFUTILS_0.161)
diff --git a/libdw/dwarf_arrayorder.c b/libdw/dwarf_arrayorder.c
index da64f99..782e075 100644
--- a/libdw/dwarf_arrayorder.c
+++ b/libdw/dwarf_arrayorder.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_arrayorder, ELFUTILS_0.143)
 int
 dwarf_arrayorder (Dwarf_Die *die)
 {
@@ -46,4 +47,3 @@ dwarf_arrayorder (Dwarf_Die *die)
 				  &value) == 0 ? (int) value : -1;
 }
 OLD_VERSION (dwarf_arrayorder, ELFUTILS_0.122)
-NEW_VERSION (dwarf_arrayorder, ELFUTILS_0.143)
diff --git a/libdw/dwarf_bitoffset.c b/libdw/dwarf_bitoffset.c
index c1a3a34..61a0d59 100644
--- a/libdw/dwarf_bitoffset.c
+++ b/libdw/dwarf_bitoffset.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_bitoffset, ELFUTILS_0.143)
 int
 dwarf_bitoffset (Dwarf_Die *die)
 {
@@ -46,4 +47,3 @@ dwarf_bitoffset (Dwarf_Die *die)
 				  &value) == 0 ? (int) value : -1;
 }
 OLD_VERSION (dwarf_bitoffset, ELFUTILS_0.122)
-NEW_VERSION (dwarf_bitoffset, ELFUTILS_0.143)
diff --git a/libdw/dwarf_bitsize.c b/libdw/dwarf_bitsize.c
index 0ed9b71..35e8744 100644
--- a/libdw/dwarf_bitsize.c
+++ b/libdw/dwarf_bitsize.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_bitsize, ELFUTILS_0.143)
 int
 dwarf_bitsize (Dwarf_Die *die)
 {
@@ -46,4 +47,3 @@ dwarf_bitsize (Dwarf_Die *die)
 				  &value) == 0 ? (int) value : -1;
 }
 OLD_VERSION (dwarf_bitsize, ELFUTILS_0.122)
-NEW_VERSION (dwarf_bitsize, ELFUTILS_0.143)
diff --git a/libdw/dwarf_bytesize.c b/libdw/dwarf_bytesize.c
index 116cd32..6d1ff9a 100644
--- a/libdw/dwarf_bytesize.c
+++ b/libdw/dwarf_bytesize.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_bytesize, ELFUTILS_0.143)
 int
 dwarf_bytesize (Dwarf_Die *die)
 {
@@ -46,4 +47,3 @@ dwarf_bytesize (Dwarf_Die *die)
 				  &value) == 0 ? (int) value : -1;
 }
 OLD_VERSION (dwarf_bytesize, ELFUTILS_0.122)
-NEW_VERSION (dwarf_bytesize, ELFUTILS_0.143)
diff --git a/libdw/dwarf_decl_column.c b/libdw/dwarf_decl_column.c
index 08d36b8..3225fd1 100644
--- a/libdw/dwarf_decl_column.c
+++ b/libdw/dwarf_decl_column.c
@@ -35,10 +35,10 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_decl_column, ELFUTILS_0.143)
 int
 dwarf_decl_column (Dwarf_Die *decl, int *colp)
 {
   return __libdw_attr_intval (decl, colp, DW_AT_decl_column);
 }
 OLD_VERSION (dwarf_decl_column, ELFUTILS_0.122)
-NEW_VERSION (dwarf_decl_column, ELFUTILS_0.143)
diff --git a/libdw/dwarf_decl_file.c b/libdw/dwarf_decl_file.c
index d4aa0a1..75662a3 100644
--- a/libdw/dwarf_decl_file.c
+++ b/libdw/dwarf_decl_file.c
@@ -36,6 +36,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_decl_file, ELFUTILS_0.143)
 const char *
 dwarf_decl_file (Dwarf_Die *die)
 {
@@ -86,4 +87,3 @@ dwarf_decl_file (Dwarf_Die *die)
   return cu->files->info[idx].name;
 }
 OLD_VERSION (dwarf_decl_file, ELFUTILS_0.122)
-NEW_VERSION (dwarf_decl_file, ELFUTILS_0.143)
diff --git a/libdw/dwarf_decl_line.c b/libdw/dwarf_decl_line.c
index 80fae6c..6b31eeb 100644
--- a/libdw/dwarf_decl_line.c
+++ b/libdw/dwarf_decl_line.c
@@ -37,13 +37,13 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_decl_line, ELFUTILS_0.143)
 int
 dwarf_decl_line (Dwarf_Die *func, int *linep)
 {
   return __libdw_attr_intval (func, linep, DW_AT_decl_line);
 }
 OLD_VERSION (dwarf_decl_line, ELFUTILS_0.122)
-NEW_VERSION (dwarf_decl_line, ELFUTILS_0.143)
 
 
 int internal_function
diff --git a/libdw/dwarf_srclang.c b/libdw/dwarf_srclang.c
index f10e764..77bd58c 100644
--- a/libdw/dwarf_srclang.c
+++ b/libdw/dwarf_srclang.c
@@ -35,6 +35,7 @@
 #include "libdwP.h"
 
 
+NEW_VERSION (dwarf_srclang, ELFUTILS_0.143)
 int
 dwarf_srclang (Dwarf_Die *die)
 {
@@ -45,6 +46,5 @@ dwarf_srclang (Dwarf_Die *die)
 				  (die, DW_AT_language, &attr_mem),
 				  &value) == 0 ? (int) value : -1;
 }
-INTDEF (dwarf_srclang)
+NEW_INTDEF (dwarf_srclang)
 OLD_VERSION (dwarf_srclang, ELFUTILS_0.122)
-NEW_VERSION (dwarf_srclang, ELFUTILS_0.143)
diff --git a/libdwelf/ChangeLog b/libdwelf/ChangeLog
index a0ff9f4..cbfe1ec 100644
--- a/libdwelf/ChangeLog
+++ b/libdwelf/ChangeLog
@@ -1,3 +1,8 @@
+2021-02-14  Alexander Miller  <alex.miller@gmx.de>
+
+	* dwelf_elf_begin.c (dwelf_elf_begin): Move NEW_VERSION before
+	definition.
+
 2020-12-12  Dmitry V. Levin  <ldv@altlinux.org>
 
 	* libdwelf.h: Fix spelling typos in comments.
diff --git a/libdwelf/dwelf_elf_begin.c b/libdwelf/dwelf_elf_begin.c
index c7d63a1..c3cfe63 100644
--- a/libdwelf/dwelf_elf_begin.c
+++ b/libdwelf/dwelf_elf_begin.c
@@ -36,6 +36,7 @@
 
 #include <unistd.h>
 
+NEW_VERSION (dwelf_elf_begin, ELFUTILS_0.177)
 Elf *
 dwelf_elf_begin (int fd)
 {
@@ -61,4 +62,3 @@ dwelf_elf_begin (int fd)
   return NULL;
 }
 OLD_VERSION (dwelf_elf_begin, ELFUTILS_0.175)
-NEW_VERSION (dwelf_elf_begin, ELFUTILS_0.177)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index d107e78..c96c716 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,10 @@
+2021-02-14  Alexander Miller  <alex.miller@gmx.de>
+
+	* core-file.c (dwfl_core_file_report): Move NEW_VERSION before
+	definition.  Replace INTDEF with NEW_INTDEF.
+	* dwfl_module_build_id.c (dwfl_module_build_id): Likewise.
+	* dwfl_report_elf.c (dwfl_report_elf): Likewise.
+
 2021-02-01  Érico Nogueira  <ericonr@disroot.org>
 
 	* dwfl_error.c (strerror_r): Only use the GNU version when available.
diff --git a/libdwfl/core-file.c b/libdwfl/core-file.c
index a0ccc9b..4e4c9b3 100644
--- a/libdwfl/core-file.c
+++ b/libdwfl/core-file.c
@@ -440,6 +440,7 @@ __libdwfl_dynamic_vaddr_get (Elf *elf, GElf_Addr *vaddrp)
   return false;
 }
 
+NEW_VERSION (dwfl_core_file_report, ELFUTILS_0.158)
 int
 dwfl_core_file_report (Dwfl *dwfl, Elf *elf, const char *executable)
 {
@@ -625,8 +626,7 @@ dwfl_core_file_report (Dwfl *dwfl, Elf *elf, const char *executable)
      error rather than just nothing found.  */
   return listed > 0 ? listed : retval;
 }
-INTDEF (dwfl_core_file_report)
-NEW_VERSION (dwfl_core_file_report, ELFUTILS_0.158)
+NEW_INTDEF (dwfl_core_file_report)
 
 #ifdef SYMBOL_VERSIONING
 int _compat_without_executable_dwfl_core_file_report (Dwfl *dwfl, Elf *elf);
diff --git a/libdwfl/dwfl_module_build_id.c b/libdwfl/dwfl_module_build_id.c
index 6ca9376..0c198f2 100644
--- a/libdwfl/dwfl_module_build_id.c
+++ b/libdwfl/dwfl_module_build_id.c
@@ -77,6 +77,7 @@ __libdwfl_find_build_id (Dwfl_Module *mod, bool set, Elf *elf)
   return found_build_id (mod, set, build_id_bits, build_id_len, build_id_vaddr);
 }
 
+NEW_VERSION (dwfl_module_build_id, ELFUTILS_0.138)
 int
 dwfl_module_build_id (Dwfl_Module *mod,
 		      const unsigned char **bits, GElf_Addr *vaddr)
@@ -102,8 +103,7 @@ dwfl_module_build_id (Dwfl_Module *mod,
   *vaddr = mod->build_id_vaddr;
   return mod->build_id_len;
 }
-INTDEF (dwfl_module_build_id)
-NEW_VERSION (dwfl_module_build_id, ELFUTILS_0.138)
+NEW_INTDEF (dwfl_module_build_id)
 
 #ifdef SYMBOL_VERSIONING
 COMPAT_VERSION (dwfl_module_build_id, ELFUTILS_0.130, vaddr_at_end)
diff --git a/libdwfl/dwfl_report_elf.c b/libdwfl/dwfl_report_elf.c
index 9da8669..a5f0e5e 100644
--- a/libdwfl/dwfl_report_elf.c
+++ b/libdwfl/dwfl_report_elf.c
@@ -287,6 +287,7 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name,
   return m;
 }
 
+NEW_VERSION (dwfl_report_elf, ELFUTILS_0.156)
 Dwfl_Module *
 dwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name, int fd,
 		 GElf_Addr base, bool add_p_vaddr)
@@ -322,8 +323,7 @@ dwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name, int fd,
 
   return mod;
 }
-INTDEF (dwfl_report_elf)
-NEW_VERSION (dwfl_report_elf, ELFUTILS_0.156)
+NEW_INTDEF (dwfl_report_elf)
 
 #ifdef SYMBOL_VERSIONING
 Dwfl_Module *
-- 
2.26.2


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

* Re: [PATCH] Improve building with LTO
  2021-02-17 20:22 ` Mark Wielaard
@ 2021-02-18 14:27   ` Alexander Miller
  0 siblings, 0 replies; 17+ messages in thread
From: Alexander Miller @ 2021-02-18 14:27 UTC (permalink / raw)
  To: Mark Wielaard, elfutils-devel

[Re-sending mail because the list has been omitted by mistake.]

On Wed, 17 Feb 2021 21:22:15 +0100
Mark Wielaard <mark@klomp.org> wrote:

> So you rewrite the asm statements to not use @@@ just @ and @@, that
> way they match the symver attribute usage. Smart, that way they are as
> similar as possible.

Yes, I wanted symbol versioning to work the same way in both cases.

> > I've tested the patch with different compilers (gcc-10, gcc-9, gcc-6,
> > clang-11), linkers (bfd, gold, lld), 32/64 bit (x86), with/without LTO.
> > Of course you need to cheat a bit to build elfutils with clang, and
> > lld can't be used with every combination. The workaround for older gcc
> > was enough for gcc-9 and 32bit gcc-6, but 64bit gcc-6 builds needed
> > -flto-partition=none, so this seems to depend on the version and options
> > used.
>
> Wow, you really went above and beyond. IMHO it would have been fine to
> simply say that you need GCC10 and a recent binutils ld to get LTO
> working. Supporting LTO with gcc-6 is really nice, but people stuck on
> such an old compiler should probably first look at upgrading that than
> trying to play with LTO. We should probably look at making an LTO build
> part of the buildbot.

Of course gcc-6 is really old (I just happened to have it still available).
But getting LTO to work with gcc-9 would be nice. That worked in my tests,
but I'm not sure if it's reliable. And it's only a small addition (and I
didn't add anything more for gcc-6).

But most importantly I didn't want to paint myself into a corner and change
symbol versioning in a way that will only ever work with one particular
toolchain.

> > The test suite seems brittle, though. It fails on 32bit builds, with
> > gold or lld, and with lto builds using clang (unknown object format)
> > or gcc-6 (debug info not found). But that's not related to this patch.
> > For 64bit builds with gcc-{9,10} and bfd, the test suite succeeds even
> > with lto enabled.
>
> Do the 32bit builds with gcc10/binutils ld have a clean test suite?

Only the 64bit builds with bfd passed. For all 32bit builds (64bit system,
but built with -m32) the test case "run-backtrace-native-biarch.sh" fails:
|
| 0x556a53cef000  0x556a53cf4000  .../tests/backtrace-child-biarch
| 0x7fadd8118000  0x7fadd828d000  /lib64/libc-2.32.so
| 0x7fadd8298000  0x7fadd82b4000  /lib64/libpthread-2.32.so
| 0x7fadd82f0000  0x7fadd831d000  /lib64/ld-2.32.so
| 0x7ffdf13fc000  0x7ffdf13fd000  [vdso: 6783]
| TID 6783:
| .../tests/backtrace: dwfl_thread_getframes: no error
| backtrace: linux-pid-attach.c:326: pid_set_initial_registers: Assertion `pid_arg->tid_attached == 0' failed.
| ./test-subr.sh: line 84:  6782 Aborted                 LD_LIBRARY_PATH="${built_library_path}${LD_LIBRARY_PATH:+:}$LD_LIBRARY_PATH" $VALGRIND_CMD "$@"
| backtrace-child-biarch: no main
| FAIL run-backtrace-native-biarch.sh (exit status: 1)

But that isn't related to my patch. It was the same before.

> > * Commenting out old versions in the .map file may not be needed.
> >   It's mostly a leftover from an earlier attempt, but I didn't want to
> >   re-run all test and I actually prefer it like this, so I left it in.
>
> I would like to make sure it isn't needed. Having to comment out old
> versions is a bit of a pain.

I've run a few tests and it works without that change. I'll skip that
in the next version of the patch.

(Gold has trouble when a an object defines an unversioned symbol and
the version script contains the old version; that isn't the case in my
solution because the "new" functions get renamed.)

> How does the __COUNTER__ magic work?

It doesn't.

Sorry. I'm adding two levels of indirection to make it work. I didn't
notice since it's not needed at the moment. In the future, somebody
might want to add multiple old versions for a symbol, and then they
need different names.

> I have to stare at the marcos a bit to make sure I really understand
> what is going on. But this looks really good.

Don't hesitate to ask if something isn't clear.

I'll post an updated patch later.


Cheers,

Alex

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

* Re: [PATCH v2] Improve building with LTO
  2021-02-18  2:38 ` [PATCH v2] " Alexander Miller
@ 2021-08-28  9:31   ` Dmitry V. Levin
  2021-11-04 11:23     ` Dmitry V. Levin
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry V. Levin @ 2021-08-28  9:31 UTC (permalink / raw)
  To: elfutils-devel

On Thu, Feb 18, 2021 at 03:38:56AM +0100, Alexander Miller via Elfutils-devel wrote:
> From: Alexander Miller <alex.miller@gmx.de>
> 
> Use symver attribute for symbol versioning instead of .symver
> assembler directive when available. Convert to use double @ syntax
> for default version in all cases (required when using the attribute).
> 
> Add the attributes externally_visible, no_reorder if available when
> using assembler directives to improve the situation for < gcc-10.
> This is not 100% reliable, though; -flto-partition=none may still be
> needed in some cases.
> 
> Note that -Wno-error=stack-usage= is still needed to build with LTO.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24498
> Signed-off-by: Alexander Miller <alex.miller@gmx.de>
> ---
>  lib/ChangeLog                  | 13 +++++++
>  lib/eu-config.h                | 65 +++++++++++++++++++++++++++-------
>  libdw/ChangeLog                | 11 ++++++
>  libdw/dwarf_aggregate_size.c   |  4 +--
>  libdw/dwarf_arrayorder.c       |  2 +-
>  libdw/dwarf_bitoffset.c        |  2 +-
>  libdw/dwarf_bitsize.c          |  2 +-
>  libdw/dwarf_bytesize.c         |  2 +-
>  libdw/dwarf_decl_column.c      |  2 +-
>  libdw/dwarf_decl_file.c        |  2 +-
>  libdw/dwarf_decl_line.c        |  2 +-
>  libdw/dwarf_srclang.c          |  4 +--
>  libdwelf/ChangeLog             |  5 +++
>  libdwelf/dwelf_elf_begin.c     |  2 +-
>  libdwfl/ChangeLog              |  7 ++++
>  libdwfl/core-file.c            |  4 +--
>  libdwfl/dwfl_module_build_id.c |  4 +--
>  libdwfl/dwfl_report_elf.c      |  4 +--
>  18 files changed, 107 insertions(+), 30 deletions(-)

ping?


-- 
ldv

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

* Re: [PATCH v2] Improve building with LTO
  2021-08-28  9:31   ` Dmitry V. Levin
@ 2021-11-04 11:23     ` Dmitry V. Levin
  2021-11-04 12:12       ` Mark Wielaard
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry V. Levin @ 2021-11-04 11:23 UTC (permalink / raw)
  To: elfutils-devel

On Sat, Aug 28, 2021 at 12:31:43PM +0300, Dmitry V. Levin wrote:
> On Thu, Feb 18, 2021 at 03:38:56AM +0100, Alexander Miller via Elfutils-devel wrote:
> > From: Alexander Miller <alex.miller@gmx.de>
> > 
> > Use symver attribute for symbol versioning instead of .symver
> > assembler directive when available. Convert to use double @ syntax
> > for default version in all cases (required when using the attribute).
> > 
> > Add the attributes externally_visible, no_reorder if available when
> > using assembler directives to improve the situation for < gcc-10.
> > This is not 100% reliable, though; -flto-partition=none may still be
> > needed in some cases.
> > 
> > Note that -Wno-error=stack-usage= is still needed to build with LTO.
> > 
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24498
> > Signed-off-by: Alexander Miller <alex.miller@gmx.de>
> > ---
> >  lib/ChangeLog                  | 13 +++++++
> >  lib/eu-config.h                | 65 +++++++++++++++++++++++++++-------
> >  libdw/ChangeLog                | 11 ++++++
> >  libdw/dwarf_aggregate_size.c   |  4 +--
> >  libdw/dwarf_arrayorder.c       |  2 +-
> >  libdw/dwarf_bitoffset.c        |  2 +-
> >  libdw/dwarf_bitsize.c          |  2 +-
> >  libdw/dwarf_bytesize.c         |  2 +-
> >  libdw/dwarf_decl_column.c      |  2 +-
> >  libdw/dwarf_decl_file.c        |  2 +-
> >  libdw/dwarf_decl_line.c        |  2 +-
> >  libdw/dwarf_srclang.c          |  4 +--
> >  libdwelf/ChangeLog             |  5 +++
> >  libdwelf/dwelf_elf_begin.c     |  2 +-
> >  libdwfl/ChangeLog              |  7 ++++
> >  libdwfl/core-file.c            |  4 +--
> >  libdwfl/dwfl_module_build_id.c |  4 +--
> >  libdwfl/dwfl_report_elf.c      |  4 +--
> >  18 files changed, 107 insertions(+), 30 deletions(-)
> 
> ping?

FWiW, I applied this patch in ALT's elfutils package about 2 months ago.
Thanks to -Wstack-usage fixes merged, it no longer requires
-Wno-error=stack-usage= to build with LTO.


-- 
ldv

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

* Re: [PATCH v2] Improve building with LTO
  2021-11-04 11:23     ` Dmitry V. Levin
@ 2021-11-04 12:12       ` Mark Wielaard
  2021-11-08 10:02         ` Dmitry V. Levin
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Wielaard @ 2021-11-04 12:12 UTC (permalink / raw)
  To: Dmitry V. Levin, elfutils-devel

Hi Dmitry,

On Thu, 2021-11-04 at 14:23 +0300, Dmitry V. Levin wrote:
> On Sat, Aug 28, 2021 at 12:31:43PM +0300, Dmitry V. Levin wrote:
> > On Thu, Feb 18, 2021 at 03:38:56AM +0100, Alexander Miller via
> > Elfutils-devel wrote:
> > > From: Alexander Miller <alex.miller@gmx.de>
> > > 
> > > Use symver attribute for symbol versioning instead of .symver
> > > assembler directive when available. Convert to use double @
> > > syntax
> > > for default version in all cases (required when using the
> > > attribute).
> > > 
> > > Add the attributes externally_visible, no_reorder if available
> > > when
> > > using assembler directives to improve the situation for < gcc-10.
> > > This is not 100% reliable, though; -flto-partition=none may still
> > > be
> > > needed in some cases.
> > > 
> > > Note that -Wno-error=stack-usage= is still needed to build with
> > > LTO.
> > > 
> > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24498
> > > Signed-off-by: Alexander Miller <alex.miller@gmx.de>
> > > ---
> > >  lib/ChangeLog                  | 13 +++++++
> > >  lib/eu-config.h                | 65 +++++++++++++++++++++++++++-
> > > ------
> > >  libdw/ChangeLog                | 11 ++++++
> > >  libdw/dwarf_aggregate_size.c   |  4 +--
> > >  libdw/dwarf_arrayorder.c       |  2 +-
> > >  libdw/dwarf_bitoffset.c        |  2 +-
> > >  libdw/dwarf_bitsize.c          |  2 +-
> > >  libdw/dwarf_bytesize.c         |  2 +-
> > >  libdw/dwarf_decl_column.c      |  2 +-
> > >  libdw/dwarf_decl_file.c        |  2 +-
> > >  libdw/dwarf_decl_line.c        |  2 +-
> > >  libdw/dwarf_srclang.c          |  4 +--
> > >  libdwelf/ChangeLog             |  5 +++
> > >  libdwelf/dwelf_elf_begin.c     |  2 +-
> > >  libdwfl/ChangeLog              |  7 ++++
> > >  libdwfl/core-file.c            |  4 +--
> > >  libdwfl/dwfl_module_build_id.c |  4 +--
> > >  libdwfl/dwfl_report_elf.c      |  4 +--
> > >  18 files changed, 107 insertions(+), 30 deletions(-)
> > 
> > ping?
> 
> FWiW, I applied this patch in ALT's elfutils package about 2 months
> ago.
> Thanks to -Wstack-usage fixes merged, it no longer requires
> -Wno-error=stack-usage= to build with LTO.

Thanks. This patch was indeed one reason I kept postponing the release,
because I didn't have have time to properly review it.

Which gcc versions have you tried this against (with/without -flto?)

I admit I am still a bit nervous about the switch away from @@@ to just
@ and @@. I was secretly hoping gcc would add @@@ support to the symver
attribute. But that doesn't seem to be happening, and even if it did,
it would be gcc 12+ only. So maybe we can include it for this release
and just tell people they may keep the pieces if they use -flto. But it
does also impact symbol versioning for non-lto builds, so I am still a
little hesitant. I'll try to do some tests to make sure things look ok
with different gcc versions.

Cheers,

Mark

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

* Re: [PATCH v2] Improve building with LTO
  2021-11-04 12:12       ` Mark Wielaard
@ 2021-11-08 10:02         ` Dmitry V. Levin
  2021-11-08 23:18           ` Mark Wielaard
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry V. Levin @ 2021-11-08 10:02 UTC (permalink / raw)
  To: elfutils-devel

Hi Mark,

On Thu, Nov 04, 2021 at 01:12:29PM +0100, Mark Wielaard wrote:
> Hi Dmitry,
> 
> On Thu, 2021-11-04 at 14:23 +0300, Dmitry V. Levin wrote:
> > On Sat, Aug 28, 2021 at 12:31:43PM +0300, Dmitry V. Levin wrote:
> > > On Thu, Feb 18, 2021 at 03:38:56AM +0100, Alexander Miller via
> > > Elfutils-devel wrote:
> > > > From: Alexander Miller <alex.miller@gmx.de>
> > > > 
> > > > Use symver attribute for symbol versioning instead of .symver
> > > > assembler directive when available. Convert to use double @
> > > > syntax
> > > > for default version in all cases (required when using the
> > > > attribute).
> > > > 
> > > > Add the attributes externally_visible, no_reorder if available
> > > > when
> > > > using assembler directives to improve the situation for < gcc-10.
> > > > This is not 100% reliable, though; -flto-partition=none may still
> > > > be
> > > > needed in some cases.
> > > > 
> > > > Note that -Wno-error=stack-usage= is still needed to build with
> > > > LTO.
> > > > 
> > > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24498
> > > > Signed-off-by: Alexander Miller <alex.miller@gmx.de>
> > > > ---
> > > >  lib/ChangeLog                  | 13 +++++++
> > > >  lib/eu-config.h                | 65 +++++++++++++++++++++++++++-
> > > > ------
> > > >  libdw/ChangeLog                | 11 ++++++
> > > >  libdw/dwarf_aggregate_size.c   |  4 +--
> > > >  libdw/dwarf_arrayorder.c       |  2 +-
> > > >  libdw/dwarf_bitoffset.c        |  2 +-
> > > >  libdw/dwarf_bitsize.c          |  2 +-
> > > >  libdw/dwarf_bytesize.c         |  2 +-
> > > >  libdw/dwarf_decl_column.c      |  2 +-
> > > >  libdw/dwarf_decl_file.c        |  2 +-
> > > >  libdw/dwarf_decl_line.c        |  2 +-
> > > >  libdw/dwarf_srclang.c          |  4 +--
> > > >  libdwelf/ChangeLog             |  5 +++
> > > >  libdwelf/dwelf_elf_begin.c     |  2 +-
> > > >  libdwfl/ChangeLog              |  7 ++++
> > > >  libdwfl/core-file.c            |  4 +--
> > > >  libdwfl/dwfl_module_build_id.c |  4 +--
> > > >  libdwfl/dwfl_report_elf.c      |  4 +--
> > > >  18 files changed, 107 insertions(+), 30 deletions(-)
> > > 
> > > ping?
> > 
> > FWiW, I applied this patch in ALT's elfutils package about 2 months
> > ago.
> > Thanks to -Wstack-usage fixes merged, it no longer requires
> > -Wno-error=stack-usage= to build with LTO.
> 
> Thanks. This patch was indeed one reason I kept postponing the release,
> because I didn't have have time to properly review it.
> 
> Which gcc versions have you tried this against (with/without -flto?)

I tested with gcc10 and gcc11.
I could try older versions, although I didn't feel that necessary.

> I admit I am still a bit nervous about the switch away from @@@ to just
> @ and @@. I was secretly hoping gcc would add @@@ support to the symver
> attribute. But that doesn't seem to be happening, and even if it did,
> it would be gcc 12+ only. So maybe we can include it for this release
> and just tell people they may keep the pieces if they use -flto. But it

https://sourceware.org/bugzilla/show_bug.cgi?id=27367 will likely strike
those who would build elfutils with -flto using gcc11+.

> does also impact symbol versioning for non-lto builds, so I am still a
> little hesitant. I'll try to do some tests to make sure things look ok
> with different gcc versions.

What do you mean by "it does also impact symbol versioning for non-lto
builds"?  The code for non-lto builds changes, but the versioning
should remain the same, shouldn't it?


-- 
ldv

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

* Re: [PATCH v2] Improve building with LTO
  2021-11-08 10:02         ` Dmitry V. Levin
@ 2021-11-08 23:18           ` Mark Wielaard
  2021-11-09  8:58             ` Dmitry V. Levin
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Wielaard @ 2021-11-08 23:18 UTC (permalink / raw)
  To: Dmitry V. Levin; +Cc: elfutils-devel

Hi Dmitry,

On Mon, Nov 08, 2021 at 01:02:28PM +0300, Dmitry V. Levin wrote:
> > Thanks. This patch was indeed one reason I kept postponing the release,
> > because I didn't have have time to properly review it.
> > 
> > Which gcc versions have you tried this against (with/without -flto?)
> 
> I tested with gcc10 and gcc11.
> I could try older versions, although I didn't feel that necessary.

I also did try with gcc 4.8 and gcc 8 (both without lto though).

> https://sourceware.org/bugzilla/show_bug.cgi?id=27367 will likely strike
> those who would build elfutils with -flto using gcc11+.

But only if they use -ffat-lto-objects (which isn't the default)?  Did
you see and try the patch I proposed? Do you think we should include
it? I would like someone else to check/try it.

> > does also impact symbol versioning for non-lto builds, so I am still a
> > little hesitant. I'll try to do some tests to make sure things look ok
> > with different gcc versions.
> 
> What do you mean by "it does also impact symbol versioning for non-lto
> builds"?  The code for non-lto builds changes, but the versioning
> should remain the same, shouldn't it?

I meant I am paranoid :) We are using slightly different asm or an
attribute to mark the symbol version than we did before. But I double
checked the exported versioned symbols with and without this patch on
different gcc versions and they look fine.

So I did just now push this patch.

Cheers,

Mark


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

* Re: [PATCH v2] Improve building with LTO
  2021-11-08 23:18           ` Mark Wielaard
@ 2021-11-09  8:58             ` Dmitry V. Levin
  2021-11-09  9:04               ` Martin Liška
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry V. Levin @ 2021-11-09  8:58 UTC (permalink / raw)
  To: elfutils-devel

Hi Mark,

On Tue, Nov 09, 2021 at 12:18:31AM +0100, Mark Wielaard wrote:
> Hi Dmitry,
> 
> On Mon, Nov 08, 2021 at 01:02:28PM +0300, Dmitry V. Levin wrote:
> > > Thanks. This patch was indeed one reason I kept postponing the release,
> > > because I didn't have have time to properly review it.
> > > 
> > > Which gcc versions have you tried this against (with/without -flto?)
> > 
> > I tested with gcc10 and gcc11.
> > I could try older versions, although I didn't feel that necessary.
> 
> I also did try with gcc 4.8 and gcc 8 (both without lto though).
> 
> > https://sourceware.org/bugzilla/show_bug.cgi?id=27367 will likely strike
> > those who would build elfutils with -flto using gcc11+.
> 
> But only if they use -ffat-lto-objects (which isn't the default)?

Yes, but those who build elfutils with -flto are likely using
-ffat-lto-objects if they build static libraries.

> Did you see and try the patch I proposed? Do you think we should include
> it? I would like someone else to check/try it.

Yes, the patch makes run-readelf-self.sh test pass, and causes no visible
regressions.  I'm not familiar with that code to review the patch, but
I'm happy to use it anyway.

> > > does also impact symbol versioning for non-lto builds, so I am still a
> > > little hesitant. I'll try to do some tests to make sure things look ok
> > > with different gcc versions.
> > 
> > What do you mean by "it does also impact symbol versioning for non-lto
> > builds"?  The code for non-lto builds changes, but the versioning
> > should remain the same, shouldn't it?
> 
> I meant I am paranoid :) We are using slightly different asm or an
> attribute to mark the symbol version than we did before. But I double
> checked the exported versioned symbols with and without this patch on
> different gcc versions and they look fine.
> 
> So I did just now push this patch.

Thanks!


-- 
ldv

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

* Re: [PATCH v2] Improve building with LTO
  2021-11-09  8:58             ` Dmitry V. Levin
@ 2021-11-09  9:04               ` Martin Liška
  2021-11-09  9:09                 ` Dmitry V. Levin
  2021-11-09 11:45                 ` Mark Wielaard
  0 siblings, 2 replies; 17+ messages in thread
From: Martin Liška @ 2021-11-09  9:04 UTC (permalink / raw)
  To: Dmitry V. Levin, elfutils-devel

On 11/9/21 09:58, Dmitry V. Levin wrote:
> Yes, but those who build elfutils with -flto are likely using
> -ffat-lto-objects if they build static libraries.

Yes, I can confirm that we do that as openSUSE, we actually build with:

-flto -flto-partition=none -Wno-error=stack-usage= -ffat-lto-objects

right now.

Cheers,
Martin

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

* Re: [PATCH v2] Improve building with LTO
  2021-11-09  9:04               ` Martin Liška
@ 2021-11-09  9:09                 ` Dmitry V. Levin
  2021-11-09  9:11                   ` Martin Liška
  2021-11-09 11:45                 ` Mark Wielaard
  1 sibling, 1 reply; 17+ messages in thread
From: Dmitry V. Levin @ 2021-11-09  9:09 UTC (permalink / raw)
  To: elfutils-devel

On Tue, Nov 09, 2021 at 10:04:57AM +0100, Martin Liška wrote:
> On 11/9/21 09:58, Dmitry V. Levin wrote:
> > Yes, but those who build elfutils with -flto are likely using
> > -ffat-lto-objects if they build static libraries.
> 
> Yes, I can confirm that we do that as openSUSE, we actually build with:
> 
> -flto -flto-partition=none -Wno-error=stack-usage= -ffat-lto-objects

After updating to commit elfutils-0.185-76-g039f427a you can safely drop
"-flto-partition=none" and "-Wno-error=stack-usage=" parts.


-- 
ldv

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

* Re: [PATCH v2] Improve building with LTO
  2021-11-09  9:09                 ` Dmitry V. Levin
@ 2021-11-09  9:11                   ` Martin Liška
  0 siblings, 0 replies; 17+ messages in thread
From: Martin Liška @ 2021-11-09  9:11 UTC (permalink / raw)
  To: Dmitry V. Levin, elfutils-devel

On 11/9/21 10:09, Dmitry V. Levin wrote:
> On Tue, Nov 09, 2021 at 10:04:57AM +0100, Martin Liška wrote:
>> On 11/9/21 09:58, Dmitry V. Levin wrote:
>>> Yes, but those who build elfutils with -flto are likely using
>>> -ffat-lto-objects if they build static libraries.
>>
>> Yes, I can confirm that we do that as openSUSE, we actually build with:
>>
>> -flto -flto-partition=none -Wno-error=stack-usage= -ffat-lto-objects
> 
> After updating to commit elfutils-0.185-76-g039f427a you can safely drop
> "-flto-partition=none" and "-Wno-error=stack-usage=" parts.

Great, good news!

Martin
  


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

* Re: [PATCH v2] Improve building with LTO
  2021-11-09  9:04               ` Martin Liška
  2021-11-09  9:09                 ` Dmitry V. Levin
@ 2021-11-09 11:45                 ` Mark Wielaard
  2021-11-09 13:31                   ` Martin Liška
  1 sibling, 1 reply; 17+ messages in thread
From: Mark Wielaard @ 2021-11-09 11:45 UTC (permalink / raw)
  To: Martin Liška, Dmitry V. Levin, elfutils-devel

Hi Martin,

On Tue, 2021-11-09 at 10:04 +0100, Martin Liška wrote:
> On 11/9/21 09:58, Dmitry V. Levin wrote:
> > Yes, but those who build elfutils with -flto are likely using
> > -ffat-lto-objects if they build static libraries.
> 
> Yes, I can confirm that we do that as openSUSE, we actually build
> with:
> 
> -flto -flto-partition=none -Wno-error=stack-usage= -ffat-lto-objects

As Dmitry pointed out you should now be able to drop both -flto-
partition=none an -Wno-error=stack-usage= (the linker will still warn,
but not error on the large stack-usage issues (we should probably still
fix them though).

What do the test results look like? Do they all PASS with -ffat-lto-
objects?

Could you try the proposed patch for
https://sourceware.org/bugzilla/show_bug.cgi?id=27367
https://sourceware.org/pipermail/elfutils-devel/2021q4/004314.html

Thanks,

Mark

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

* Re: [PATCH v2] Improve building with LTO
  2021-11-09 11:45                 ` Mark Wielaard
@ 2021-11-09 13:31                   ` Martin Liška
  2021-11-09 13:33                     ` Martin Liška
  2021-11-09 17:49                     ` Mark Wielaard
  0 siblings, 2 replies; 17+ messages in thread
From: Martin Liška @ 2021-11-09 13:31 UTC (permalink / raw)
  To: Mark Wielaard, Dmitry V. Levin, elfutils-devel

On 11/9/21 12:45, Mark Wielaard wrote:
> Hi Martin,
> 
> On Tue, 2021-11-09 at 10:04 +0100, Martin Liška wrote:
>> On 11/9/21 09:58, Dmitry V. Levin wrote:
>>> Yes, but those who build elfutils with -flto are likely using
>>> -ffat-lto-objects if they build static libraries.
>>
>> Yes, I can confirm that we do that as openSUSE, we actually build
>> with:
>>
>> -flto -flto-partition=none -Wno-error=stack-usage= -ffat-lto-objects
> 
> As Dmitry pointed out you should now be able to drop both -flto-
> partition=none an -Wno-error=stack-usage= (the linker will still warn,
> but not error on the large stack-usage issues (we should probably still
> fix them though).

Correct, I'm not using:
F="-O2 -g -Wall -flto=auto -ffat-lto-objects" && export CFLAGS="$F" && export CXXFLAGS="$F" && export LDFLAGS="$F" && ./configure


> 
> What do the test results look like? Do they all PASS with -ffat-lto-
> objects?
> 
> Could you try the proposed patch for
> https://sourceware.org/bugzilla/show_bug.cgi?id=27367
> https://sourceware.org/pipermail/elfutils-devel/2021q4/004314.html

With the suggested patch (and BFD) all is green and shiny:

============================================================================
Testsuite summary for elfutils 0.185
============================================================================
# TOTAL: 252
# PASS:  249
# SKIP:  3
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0

Cheers,
Martin

> 
> Thanks,
> 
> Mark
> 


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

* Re: [PATCH v2] Improve building with LTO
  2021-11-09 13:31                   ` Martin Liška
@ 2021-11-09 13:33                     ` Martin Liška
  2021-11-09 17:49                     ` Mark Wielaard
  1 sibling, 0 replies; 17+ messages in thread
From: Martin Liška @ 2021-11-09 13:33 UTC (permalink / raw)
  To: Mark Wielaard, Dmitry V. Levin, elfutils-devel

On 11/9/21 14:31, Martin Liška wrote:
> Correct, I'm not using:

s/not/now

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

* Re: [PATCH v2] Improve building with LTO
  2021-11-09 13:31                   ` Martin Liška
  2021-11-09 13:33                     ` Martin Liška
@ 2021-11-09 17:49                     ` Mark Wielaard
  1 sibling, 0 replies; 17+ messages in thread
From: Mark Wielaard @ 2021-11-09 17:49 UTC (permalink / raw)
  To: Martin Liška, Dmitry V. Levin, elfutils-devel

Hi Martin,

On Tue, 2021-11-09 at 14:31 +0100, Martin Liška wrote:
> > What do the test results look like? Do they all PASS with -ffat-
> > lto-objects?
> > 
> > Could you try the proposed patch for
> > https://sourceware.org/bugzilla/show_bug.cgi?id=27367
> > https://sourceware.org/pipermail/elfutils-devel/2021q4/004314.html
> 
> With the suggested patch (and BFD) all is green and shiny:

Thanks for the testing, I pushed that patch now.

Cheers,

Mark

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

end of thread, other threads:[~2021-11-09 17:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-14 22:57 [PATCH] Improve building with LTO Alexander Miller
2021-02-17 20:22 ` Mark Wielaard
2021-02-18 14:27   ` Alexander Miller
2021-02-18  2:38 ` [PATCH v2] " Alexander Miller
2021-08-28  9:31   ` Dmitry V. Levin
2021-11-04 11:23     ` Dmitry V. Levin
2021-11-04 12:12       ` Mark Wielaard
2021-11-08 10:02         ` Dmitry V. Levin
2021-11-08 23:18           ` Mark Wielaard
2021-11-09  8:58             ` Dmitry V. Levin
2021-11-09  9:04               ` Martin Liška
2021-11-09  9:09                 ` Dmitry V. Levin
2021-11-09  9:11                   ` Martin Liška
2021-11-09 11:45                 ` Mark Wielaard
2021-11-09 13:31                   ` Martin Liška
2021-11-09 13:33                     ` Martin Liška
2021-11-09 17:49                     ` Mark Wielaard

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