Re: [PATCH 5/5] Add frame pointer unwinding for aarch64

2017-04-27 Thread Ulf Hermann

Maybe something like the attached patch?


Well that's actually the original patch (as opposed to V2) with relaxed 
test conditions. You can write that a bit nicer by setting the new PC 
directly after retrieving LR and returning early if it doesn't work. See 
"[PATCH 2/3] Add frame pointer unwinding as fallback on arm" from 
February 16th. That's the original algorithm; for aarch64 I just added a 
few defines and included arm_unwind.c.


It's in fact a bit annoying for my use case as the non-CFI stack 
sections are mostly in between CFI-enabled stack sections here. However, 
I can accept this.


cheers,
Ulf


Re: [PATCH 5/5] Add frame pointer unwinding for aarch64

2017-04-27 Thread Mark Wielaard
On Thu, Apr 27, 2017 at 10:31:55AM +0200, Ulf Hermann wrote:
> > Maybe something like the attached patch?
> 
> Well that's actually the original patch (as opposed to V2) with relaxed test
> conditions. You can write that a bit nicer by setting the new PC directly
> after retrieving LR and returning early if it doesn't work. See "[PATCH 2/3]
> Add frame pointer unwinding as fallback on arm" from February 16th. That's
> the original algorithm; for aarch64 I just added a few defines and included
> arm_unwind.c.

I think the simplier implementation with relaxed test is better.
I'll adjust the patch to look more like your original.

> It's in fact a bit annoying for my use case as the non-CFI stack sections
> are mostly in between CFI-enabled stack sections here. However, I can accept
> this.

Does every fp-only frame gets duplicated after a DWARF CFI frame?
I'll look if I can better understand why that is.

Cheers,

Mark


Re: [PATCH 5/5] Add frame pointer unwinding for aarch64

2017-04-27 Thread Ulf Hermann

Does every fp-only frame gets duplicated after a DWARF CFI frame?
I'll look if I can better understand why that is.


The last thing I've tested on an actual aarch64 setup is what I'm 
removing in this change:


https://codereview.qt-project.org/#/c/191650/5/3rdparty/elfutils/backends/aarch64_unwind.c

As you can see, I'm setting PC to the thing I read from memory, not the 
value of the LR register. So I must have had the same problem when I 
wrote that.


Ulf


[PATCH] Fix nesting of braces

2017-04-27 Thread Ulf Hermann

The way it was before it didn't actually test if elf_update failed, but
rather did something random. !!() is a boolean and boolean
true can be represented as anything non-0, including negative numbers.

Signed-off-by: Ulf Hermann 
---
 libasm/ChangeLog | 4 
 libasm/asm_end.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/libasm/ChangeLog b/libasm/ChangeLog
index 1656842..d2bc408 100644
--- a/libasm/ChangeLog
+++ b/libasm/ChangeLog
@@ -1,3 +1,7 @@
+2017-04-27  Ulf Hermann  
+
+   * asm_end.c (binary_end): Fix nesting of braces.
+
 2017-02-12  Mark Wielaard  
* asm_newsym.c (asm_newsym): Increase TEMPSYMLEN to 13.
diff --git a/libasm/asm_end.c b/libasm/asm_end.c
index 191a535..ced24f5 100644
--- a/libasm/asm_end.c
+++ b/libasm/asm_end.c
@@ -464,7 +464,7 @@ binary_end (AsmCtx_t *ctx)
   gelf_update_ehdr (ctx->out.elf, ehdr);
/* Write out the ELF file.  */
-  if (unlikely (elf_update (ctx->out.elf, ELF_C_WRITE_MMAP)) < 0)
+  if (unlikely (elf_update (ctx->out.elf, ELF_C_WRITE_MMAP) < 0))
 {
   __libasm_seterrno (ASM_E_LIBELF);
   result = -1;
--
2.8.1.windows.1



[PATCH] Drop handrolled or #ifdef'ed libc replacements

2017-04-27 Thread Ulf Hermann
mempcpy, memrchr, rawmemchr, and argp are provided by gnulib now. We
don't need to define them locally and we don't need to search for an
external libargp.

Signed-off-by: Ulf Hermann 
---
 ChangeLog  |  4 
 configure.ac   | 31 ---
 lib/ChangeLog  |  5 +
 lib/system.h   |  5 -
 lib/xstrndup.c |  2 +-
 libasm/ChangeLog   |  4 
 libasm/disasm_str.c|  2 +-
 libdw/ChangeLog|  4 
 libdw/Makefile.am  |  2 +-
 libdwfl/ChangeLog  |  4 
 libdwfl/linux-kernel-modules.c |  1 -
 libebl/ChangeLog   |  5 +
 libebl/eblmachineflagname.c|  1 -
 libebl/eblopenbackend.c|  1 -
 libelf/ChangeLog   |  5 +
 libelf/elf_getarsym.c  |  8 
 libelf/elf_strptr.c| 11 ---
 src/ChangeLog  |  4 
 src/Makefile.am| 30 +++---
 tests/ChangeLog|  5 +
 tests/Makefile.am  | 32 
 tests/elfstrmerge.c|  1 -
 22 files changed, 74 insertions(+), 93 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a2a0f63..6fe525c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2017-04-27  Ulf Hermann  
+
+   * configure.ac: Drop checks for memrchr, rawmemchr, mempcpy, argp.
+
 2017-04-21  Ulf Hermann  
 
* .gitignore: Add generated gnulib headers.
diff --git a/configure.ac b/configure.ac
index a5aea7f..a48b13e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -290,13 +290,7 @@ zip_LIBS="$LIBS"
 LIBS="$save_LIBS"
 AC_SUBST([zip_LIBS])
 
-AC_CHECK_DECLS([memrchr, rawmemchr],[],[],
-   [#define _GNU_SOURCE
-#include ])
 AC_CHECK_DECLS([powerof2],[],[],[#include ])
-AC_CHECK_DECLS([mempcpy],[],[],
-   [#define _GNU_SOURCE
-#include ])
 
 AC_CHECK_LIB([stdc++], [__cxa_demangle], [dnl
 AC_DEFINE([USE_DEMANGLE], [1], [Defined if demangling is enabled])])
@@ -369,31 +363,6 @@ CFLAGS="$old_CFLAGS"])
 AM_CONDITIONAL(HAVE_IMPLICIT_FALLTHROUGH_WARNING,
   [test "x$ac_cv_implicit_fallthrough" != "xno"])
 
-dnl Check if we have argp available from our libc
-AC_LINK_IFELSE(
-   [AC_LANG_PROGRAM(
-   [#include ],
-   [int argc=1; char *argv[]={"test"}; 
argp_parse(0,argc,&argv,0,0,0); return 0;]
-   )],
-   [libc_has_argp="true"],
-   [libc_has_argp="false"]
-)
-
-dnl If our libc doesn't provide argp, then test for libargp
-if test "$libc_has_argp" = "false" ; then
-   AC_MSG_WARN("libc does not have argp")
-   AC_CHECK_LIB([argp], [argp_parse], [have_argp="true"], 
[have_argp="false"])
-
-   if test "$have_argp" = "false"; then
-   AC_MSG_ERROR("no libargp found")
-   else
-   argp_LDADD="-largp"
-   fi
-else
-   argp_LDADD=""
-fi
-AC_SUBST([argp_LDADD])
-
 dnl Check if we have  for EM_BPF disassembly.
 AC_CHECK_HEADERS(linux/bpf.h)
 AM_CONDITIONAL(HAVE_LINUX_BPF_H, [test "x$ac_cv_header_linux_bpf_h" = "xyes"])
diff --git a/lib/ChangeLog b/lib/ChangeLog
index 8cac7af..82c009e 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,3 +1,8 @@
+2017-04-27  Ulf Hermann  
+
+   * system.h: Drop mempcpy replacement.
+   * xstrndup.c: Don't include system.h.
+
 2017-04-20  Ulf Hermann  
 
* crc32.c: include config.h.
diff --git a/lib/system.h b/lib/system.h
index 9203335..ffa2bc7 100644
--- a/lib/system.h
+++ b/lib/system.h
@@ -63,11 +63,6 @@
 #define powerof2(x) (((x) & ((x) - 1)) == 0)
 #endif
 
-#if !HAVE_DECL_MEMPCPY
-#define mempcpy(dest, src, n) \
-((void *) ((char *) memcpy (dest, src, n) + (size_t) n))
-#endif
-
 /* A special gettext function we use if the strings are too short.  */
 #define sgettext(Str) \
   ({ const char *__res = strrchr (gettext (Str), '|');   \
diff --git a/lib/xstrndup.c b/lib/xstrndup.c
index a257aa9..d43e3b9 100644
--- a/lib/xstrndup.c
+++ b/lib/xstrndup.c
@@ -33,7 +33,7 @@
 #include 
 #include 
 #include "libeu.h"
-#include "system.h"
+
 
 /* Return a newly allocated copy of STRING.  */
 char *
diff --git a/libasm/ChangeLog b/libasm/ChangeLog
index ef183a6..80aa7de 100644
--- a/libasm/ChangeLog
+++ b/libasm/ChangeLog
@@ -1,3 +1,7 @@
+2017-02-27  Ulf Hermann  
+
+   * disasm_str.c: Don't include system.h
+
 2017-02-21  Ulf Hermann  
 
* Makefile.am: Link libasm agaist libgnu.a if requested.
diff --git a/libasm/disasm_str.c b/libasm/disasm_str.c
index c14e6d5..5b0bb29 100644
--- a/libasm/disasm_str.c
+++ b/libasm/disasm_str.c
@@ -31,7 +31,7 @@
 #endif
 
 #include 
-#include 
+
 #include "libasmP.h"
 
 
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 8052b5b..6e11181 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,7 @@
+2017-02-27  Ulf Hermann  
+
+   * Makefile.am: Remove argp_LDADD.
+
 2017-02

[PATCH] Check for -z,defs, -z,relro, -fPIC, -fPIE before using them

2017-04-27 Thread Ulf Hermann
On windows those aren't needed because the link results are no ELF
files and all code is position independent anyway. gcc then complains
about them, which is in turn caught by -Werror.

Signed-off-by: Ulf Hermann 
---
 ChangeLog|  5 +
 backends/ChangeLog   |  4 
 backends/Makefile.am |  4 ++--
 config/ChangeLog |  4 
 config/eu.am |  4 ++--
 configure.ac | 56 ++--
 lib/ChangeLog|  4 
 lib/Makefile.am  |  2 +-
 libasm/ChangeLog |  4 
 libasm/Makefile.am   |  2 +-
 libcpu/ChangeLog |  4 
 libcpu/Makefile.am   |  2 +-
 libdw/ChangeLog  |  4 
 libdw/Makefile.am|  4 ++--
 libebl/ChangeLog |  4 
 libebl/Makefile.am   |  2 +-
 libelf/ChangeLog |  4 
 libelf/Makefile.am   |  6 +++---
 libgnu/Makefile.am   |  2 +-
 tests/ChangeLog  |  4 
 tests/Makefile.am|  4 ++--
 21 files changed, 111 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6fe525c..36c3cc7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-27  Ulf Hermann  
 
+   * configure.ac: Check if -fPIC, -fPIE, -Wl,-z,defs,
+   and -Wl,-z,relro are supported by the compiler.
+
+2017-04-27  Ulf Hermann  
+
* configure.ac: Drop checks for memrchr, rawmemchr, mempcpy, argp.
 
 2017-04-21  Ulf Hermann  
diff --git a/backends/ChangeLog b/backends/ChangeLog
index 594aa98..baeb7b9 100644
--- a/backends/ChangeLog
+++ b/backends/ChangeLog
@@ -1,3 +1,7 @@
+2017-04-27  Ulf Hermann 
+
+   * Makefile.am: Use dso_LDFLAGS.
+
 2017-04-21  Ulf Hermann 
 
* Makefile.am: Link backends against libgnu.a if requested.
diff --git a/backends/Makefile.am b/backends/Makefile.am
index 5dcb3e1..3e1992e 100644
--- a/backends/Makefile.am
+++ b/backends/Makefile.am
@@ -138,10 +138,10 @@ libebl_%.so libebl_%.map: libebl_%_pic.a $(libelf) 
$(libdw) $(libgnu)
@rm -f $(@:.so=.map)
$(AM_V_at)echo 'ELFUTILS_$(PACKAGE_VERSION) { global: $*_init; local: 
*; };' \
  > $(@:.so=.map)
-   $(AM_V_CCLD)$(LINK) -shared -o $(@:.map=.so) \
+   $(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $(@:.map=.so) \
-Wl,--whole-archive $< $(cpu_$*) -Wl,--no-whole-archive \
-Wl,--version-script,$(@:.so=.map) \
-   -Wl,-z,defs -Wl,--as-needed $(libelf) $(libdw) $(libgnu)
+   -Wl,--as-needed $(libelf) $(libdw) $(libgnu)
@$(textrel_check)
 
 libebl_i386.so: $(cpu_i386)
diff --git a/config/ChangeLog b/config/ChangeLog
index 756bab6..59569e3 100644
--- a/config/ChangeLog
+++ b/config/ChangeLog
@@ -1,3 +1,7 @@
+2017-04-27  Ulf Hermann  
+
+   * eu.am: Use fpic_CFLAGS.
+
 2017-04-21  Ulf Hermann  
 
* eu.am: Add $(top_srcdir)libgnu and $(top_builddir)/libgnu to -I if 
requested.
diff --git a/config/eu.am b/config/eu.am
index 11c2fec..bc8c767 100644
--- a/config/eu.am
+++ b/config/eu.am
@@ -89,14 +89,14 @@ endif
 
 %.os: %.c %.o
 if AMDEP
-   $(AM_V_CC)if $(COMPILE.os) -c -o $@ -fPIC $(DEFS.os) -MT $@ -MD -MP \
+   $(AM_V_CC)if $(COMPILE.os) -c -o $@ $(fpic_CFLAGS) $(DEFS.os) -MT $@ 
-MD -MP \
  -MF "$(DEPDIR)/$*.Tpo" `test -f '$<' || echo '$(srcdir)/'`$<; \
then cat "$(DEPDIR)/$*.Tpo" >> "$(DEPDIR)/$*.Po"; \
 rm -f "$(DEPDIR)/$*.Tpo"; \
else rm -f "$(DEPDIR)/$*.Tpo"; exit 1; \
fi
 else
-   $(AM_V_CC)$(COMPILE.os) -c -o $@ -fPIC $(DEFS.os) $<
+   $(AM_V_CC)$(COMPILE.os) -c -o $@ $(fpic_CFLAGS) $(DEFS.os) $<
 endif
 
 CLEANFILES = *.gcno *.gcda
diff --git a/configure.ac b/configure.ac
index a48b13e..107762f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -136,13 +136,65 @@ CFLAGS="$old_CFLAGS"])
 AS_IF([test "x$ac_cv_c99" != xyes],
   AC_MSG_ERROR([gcc with GNU99 support required]))
 
+AC_CACHE_CHECK([whether gcc supports -fPIC], ac_cv_fpic, [dnl
+save_CFLAGS="$CFLAGS"
+CFLAGS="$save_CFLAGS -fPIC -Werror"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE()], ac_cv_fpic=yes, ac_cv_fpic=no)
+CFLAGS="$save_CFLAGS"
+])
+if test "$ac_cv_fpic" = "yes"; then
+   fpic_CFLAGS="-fPIC"
+else
+   fpic_CFLAGS=""
+fi
+AC_SUBST([fpic_CFLAGS])
+
+AC_CACHE_CHECK([whether gcc supports -fPIE], ac_cv_fpie, [dnl
+save_CFLAGS="$CFLAGS"
+CFLAGS="$save_CFLAGS -fPIE -Werror"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE()], ac_cv_fpie=yes, ac_cv_fpie=no)
+CFLAGS="$save_CFLAGS"
+])
+if test "$ac_cv_fpie" = "yes"; then
+   fpie_CFLAGS="-fPIE"
+else
+   fpie_CFLAGS=""
+fi
+AC_SUBST([fpie_CFLAGS])
+
+dso_LDFLAGS="-shared"
+
+ZDEFS_LDFLAGS="-Wl,-z,defs"
+AC_CACHE_CHECK([whether gcc supports $ZDEFS_LDFLAGS], ac_cv_zdefs, [dnl
+save_LDFLAGS="$LDFLAGS"
+LDFLAGS="$ZDEFS_LDFLAGS $save_LDFLAGS"
+AC_LINK_IFELSE([AC_LANG_PROGRAM()], ac_cv_zdefs=yes, ac_cv_zdefs=no)
+LDFLAGS="$save_LDFLAGS"
+])
+if test "$ac_cv_zdefs" = "yes"; then
+   dso_LDFLAGS="$dso_LDFLAGS $ZDEFS_LDFLAGS"
+fi
+
+ZRELRO_LDFLAGS="-Wl,-z,relro"
+AC_CACHE_CHECK([whether gcc supports $ZRELRO_LDFLAGS], ac_cv_zrelro, [dnl
+

[PATCH v2] Check for -z,defs, -z,relro, -fPIC, -fPIE before using them

2017-04-27 Thread Ulf Hermann
On windows those aren't needed because the link results are no ELF
files and all code is position independent anyway. gcc then complains
about them, which is in turn caught by -Werror.

Signed-off-by: Ulf Hermann 
---
 ChangeLog|  5 +
 backends/ChangeLog   |  4 
 backends/Makefile.am |  4 ++--
 config/ChangeLog |  4 
 config/eu.am |  4 ++--
 configure.ac | 56 ++--
 lib/ChangeLog|  4 
 lib/Makefile.am  |  2 +-
 libasm/ChangeLog |  4 
 libasm/Makefile.am   |  2 +-
 libcpu/ChangeLog |  4 
 libcpu/Makefile.am   |  2 +-
 libdw/ChangeLog  |  4 
 libdw/Makefile.am|  4 ++--
 libebl/ChangeLog |  4 
 libebl/Makefile.am   |  2 +-
 libelf/ChangeLog |  4 
 libelf/Makefile.am   |  6 +++---
 libgnu/Makefile.am   |  2 +-
 tests/ChangeLog  |  4 
 tests/Makefile.am|  4 ++--
 21 files changed, 111 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6fe525c..36c3cc7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-27  Ulf Hermann  
 
+   * configure.ac: Check if -fPIC, -fPIE, -Wl,-z,defs,
+   and -Wl,-z,relro are supported by the compiler.
+
+2017-04-27  Ulf Hermann  
+
* configure.ac: Drop checks for memrchr, rawmemchr, mempcpy, argp.
 
 2017-04-21  Ulf Hermann  
diff --git a/backends/ChangeLog b/backends/ChangeLog
index 594aa98..baeb7b9 100644
--- a/backends/ChangeLog
+++ b/backends/ChangeLog
@@ -1,3 +1,7 @@
+2017-04-27  Ulf Hermann 
+
+   * Makefile.am: Use dso_LDFLAGS.
+
 2017-04-21  Ulf Hermann 
 
* Makefile.am: Link backends against libgnu.a if requested.
diff --git a/backends/Makefile.am b/backends/Makefile.am
index 5dcb3e1..3e1992e 100644
--- a/backends/Makefile.am
+++ b/backends/Makefile.am
@@ -138,10 +138,10 @@ libebl_%.so libebl_%.map: libebl_%_pic.a $(libelf) 
$(libdw) $(libgnu)
@rm -f $(@:.so=.map)
$(AM_V_at)echo 'ELFUTILS_$(PACKAGE_VERSION) { global: $*_init; local: 
*; };' \
  > $(@:.so=.map)
-   $(AM_V_CCLD)$(LINK) -shared -o $(@:.map=.so) \
+   $(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $(@:.map=.so) \
-Wl,--whole-archive $< $(cpu_$*) -Wl,--no-whole-archive \
-Wl,--version-script,$(@:.so=.map) \
-   -Wl,-z,defs -Wl,--as-needed $(libelf) $(libdw) $(libgnu)
+   -Wl,--as-needed $(libelf) $(libdw) $(libgnu)
@$(textrel_check)
 
 libebl_i386.so: $(cpu_i386)
diff --git a/config/ChangeLog b/config/ChangeLog
index 756bab6..59569e3 100644
--- a/config/ChangeLog
+++ b/config/ChangeLog
@@ -1,3 +1,7 @@
+2017-04-27  Ulf Hermann  
+
+   * eu.am: Use fpic_CFLAGS.
+
 2017-04-21  Ulf Hermann  
 
* eu.am: Add $(top_srcdir)libgnu and $(top_builddir)/libgnu to -I if 
requested.
diff --git a/config/eu.am b/config/eu.am
index 11c2fec..bc8c767 100644
--- a/config/eu.am
+++ b/config/eu.am
@@ -89,14 +89,14 @@ endif
 
 %.os: %.c %.o
 if AMDEP
-   $(AM_V_CC)if $(COMPILE.os) -c -o $@ -fPIC $(DEFS.os) -MT $@ -MD -MP \
+   $(AM_V_CC)if $(COMPILE.os) -c -o $@ $(fpic_CFLAGS) $(DEFS.os) -MT $@ 
-MD -MP \
  -MF "$(DEPDIR)/$*.Tpo" `test -f '$<' || echo '$(srcdir)/'`$<; \
then cat "$(DEPDIR)/$*.Tpo" >> "$(DEPDIR)/$*.Po"; \
 rm -f "$(DEPDIR)/$*.Tpo"; \
else rm -f "$(DEPDIR)/$*.Tpo"; exit 1; \
fi
 else
-   $(AM_V_CC)$(COMPILE.os) -c -o $@ -fPIC $(DEFS.os) $<
+   $(AM_V_CC)$(COMPILE.os) -c -o $@ $(fpic_CFLAGS) $(DEFS.os) $<
 endif
 
 CLEANFILES = *.gcno *.gcda
diff --git a/configure.ac b/configure.ac
index a48b13e..107762f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -136,13 +136,65 @@ CFLAGS="$old_CFLAGS"])
 AS_IF([test "x$ac_cv_c99" != xyes],
   AC_MSG_ERROR([gcc with GNU99 support required]))
 
+AC_CACHE_CHECK([whether gcc supports -fPIC], ac_cv_fpic, [dnl
+save_CFLAGS="$CFLAGS"
+CFLAGS="$save_CFLAGS -fPIC -Werror"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE()], ac_cv_fpic=yes, ac_cv_fpic=no)
+CFLAGS="$save_CFLAGS"
+])
+if test "$ac_cv_fpic" = "yes"; then
+   fpic_CFLAGS="-fPIC"
+else
+   fpic_CFLAGS=""
+fi
+AC_SUBST([fpic_CFLAGS])
+
+AC_CACHE_CHECK([whether gcc supports -fPIE], ac_cv_fpie, [dnl
+save_CFLAGS="$CFLAGS"
+CFLAGS="$save_CFLAGS -fPIE -Werror"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE()], ac_cv_fpie=yes, ac_cv_fpie=no)
+CFLAGS="$save_CFLAGS"
+])
+if test "$ac_cv_fpie" = "yes"; then
+   fpie_CFLAGS="-fPIE"
+else
+   fpie_CFLAGS=""
+fi
+AC_SUBST([fpie_CFLAGS])
+
+dso_LDFLAGS="-shared"
+
+ZDEFS_LDFLAGS="-Wl,-z,defs"
+AC_CACHE_CHECK([whether gcc supports $ZDEFS_LDFLAGS], ac_cv_zdefs, [dnl
+save_LDFLAGS="$LDFLAGS"
+LDFLAGS="$ZDEFS_LDFLAGS $save_LDFLAGS"
+AC_LINK_IFELSE([AC_LANG_PROGRAM()], ac_cv_zdefs=yes, ac_cv_zdefs=no)
+LDFLAGS="$save_LDFLAGS"
+])
+if test "$ac_cv_zdefs" = "yes"; then
+   dso_LDFLAGS="$dso_LDFLAGS $ZDEFS_LDFLAGS"
+fi
+
+ZRELRO_LDFLAGS="-Wl,-z,relro"
+AC_CACHE_CHECK([whether gcc supports $ZRELRO_LDFLAGS], ac_cv_zrelro, [dnl
+

[PATCH] Check if gcc complains about __attribute__ (visibility(..))

2017-04-27 Thread Ulf Hermann
If so, define attribute_hidden to be empty. Also, use attribute_hidden
in all places where we hide symbols.

Change-Id: I37353459710dbbd1c6c6c46110514fc18515c814
Signed-off-by: Ulf Hermann 
---
 ChangeLog   |  5 +
 configure.ac| 16 
 lib/ChangeLog   |  5 +
 lib/eu-config.h |  4 
 libdw/ChangeLog |  5 +
 libdw/libdwP.h  |  2 +-
 libdw/libdw_alloc.c |  2 +-
 libelf/ChangeLog|  4 
 libelf/libelfP.h|  2 +-
 9 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 36c3cc7..01f88f3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-27  Ulf Hermann  
 
+   * configure.ac: Check if the compiler supports
+   __attribute__((visibility(...))).
+
+2017-04-27  Ulf Hermann  
+
* configure.ac: Check if -fPIC, -fPIE, -Wl,-z,defs,
and -Wl,-z,relro are supported by the compiler.
 
diff --git a/configure.ac b/configure.ac
index 107762f..165149d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -136,6 +136,22 @@ CFLAGS="$old_CFLAGS"])
 AS_IF([test "x$ac_cv_c99" != xyes],
   AC_MSG_ERROR([gcc with GNU99 support required]))
 
+AC_CACHE_CHECK([whether gcc supports __attribute__((visibility()))],
+   ac_cv_visibility, [dnl
+save_CFLAGS="$CFLAGS"
+CFLAGS="$save_CFLAGS -Werror"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([dnl
+int __attribute__((visibility("hidden")))
+foo (int a)
+{
+  return a;
+}])], ac_cv_visibility=yes, ac_cv_visibility=no)
+CFLAGS="$save_CFLAGS"])
+if test "$ac_cv_visibility" = "yes"; then
+   AC_DEFINE([HAVE_VISIBILITY], [1],
+ [Defined if __attribute__((visibility())) is supported])
+fi
+
 AC_CACHE_CHECK([whether gcc supports -fPIC], ac_cv_fpic, [dnl
 save_CFLAGS="$CFLAGS"
 CFLAGS="$save_CFLAGS -fPIC -Werror"
diff --git a/lib/ChangeLog b/lib/ChangeLog
index 605b9b9..ecc6179 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-27  Ulf Hermann  
 
+   * eu-config.h: Define attribute_hidden to be empty if the compiler
+   doesn't support it.
+
+2017-04-27  Ulf Hermann  
+
* Makefile.am: Use fpic_CFLAGS.
 
 2017-04-27  Ulf Hermann  
diff --git a/lib/eu-config.h b/lib/eu-config.h
index 400cdc6..0709828 100644
--- a/lib/eu-config.h
+++ b/lib/eu-config.h
@@ -68,8 +68,12 @@
 #define internal_strong_alias(name, aliasname) \
   extern __typeof (name) aliasname __attribute__ ((alias (#name))) 
internal_function;
 
+#ifdef HAVE_VISIBILITY
 #define attribute_hidden \
   __attribute__ ((visibility ("hidden")))
+#else
+#define attribute_hidden /* empty */
+#endif
 
 /* Define ALLOW_UNALIGNED if the architecture allows operations on
unaligned memory locations.  */
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index d15c861..79c3898 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,5 +1,10 @@
 2017-02-27  Ulf Hermann  
 
+   * libdwP.h: Use attribute_hidden.
+   * libdw_alloc.c: Likewise.
+
+2017-02-27  Ulf Hermann  
+
* Makefile.am: Use fpic_CFLAGS and dso_LDFLAGS.
 
 2017-02-27  Ulf Hermann  
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index 5d095a7..cefcafd 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -433,7 +433,7 @@ extern void *__libdw_allocate (Dwarf *dbg, size_t minsize, 
size_t align)
  __attribute__ ((__malloc__)) __nonnull_attribute__ (1);
 
 /* Default OOM handler.  */
-extern void __libdw_oom (void) __attribute ((noreturn, visibility ("hidden")));
+extern void __libdw_oom (void) __attribute ((noreturn)) attribute_hidden;
 
 /* Allocate the internal data for a unit not seen before.  */
 extern struct Dwarf_CU *__libdw_intern_next_unit (Dwarf *dbg, bool debug_types)
diff --git a/libdw/libdw_alloc.c b/libdw/libdw_alloc.c
index 28a8cf6..d6af23a 100644
--- a/libdw/libdw_alloc.c
+++ b/libdw/libdw_alloc.c
@@ -70,7 +70,7 @@ dwarf_new_oom_handler (Dwarf *dbg, Dwarf_OOM handler)
 
 
 void
-__attribute ((noreturn, visibility ("hidden")))
+__attribute ((noreturn)) attribute_hidden
 __libdw_oom (void)
 {
   while (1)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 1c6cce2..fd58ed3 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,5 +1,9 @@
 2017-04-27  Ulf Hermann  
 
+   * libelfP.h: Use attribute_hidden.
+
+2017-04-27  Ulf Hermann  
+
* Makefile.am: Use fpic_CFLAGS and dso_LDFLAGS.
 
 2017-04-27  Ulf Hermann  
diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index 7ee6625..a4a0a3a 100644
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -578,7 +578,7 @@ extern Elf_Data *__elf64_xlatetof_internal (Elf_Data 
*__dest,
 extern unsigned int __elf_version_internal (unsigned int __version)
  attribute_hidden;
 extern unsigned long int __elf_hash_internal (const char *__string)
-   __attribute__ ((__pure__, visibility ("hidden")));
+   __attribute__ ((__pure__)) attribute_hidden;
 extern long int __elf32_checksum_internal (Elf *__elf) attribute_hidden;
 extern long int __elf64_checksum_internal (Elf *__elf) attribute_hidden;
 
-- 
2.1.4



Re: [PATCH] Protect against integer overflow on shnum

2017-04-27 Thread Mark Wielaard
On Thu, Apr 20, 2017 at 04:04:54PM +0200, Ulf Hermann wrote:
> If shnum is 0, the many "shnum - 1" would result in an overflow. Check it
> for 0, and only subtract once, rather than on every usage.

Since in both cases this is for the prelink undo section which skips
the zero header this is a more natural way to express shnum.

Applied to master.

Thanks,

Mark


Re: [PATCH] Don't look for kernel version if not running on linux

2017-04-27 Thread Mark Wielaard
On Thu, Apr 20, 2017 at 04:08:48PM +0200, Ulf Hermann wrote:
> We don't want to use it, even if it exists.

I am not sure this is really the right place to patch.
The value is retrieved through uname () which is POSIX and so should
be available even on non-GNU/Linux systems. When kernel_release ()
returns NULL the callers expect errno to be set so they can use it
to return a failure code.

Cheers,

Mark


Re: [PATCH] Include strings.h to make ffs available

2017-04-27 Thread Mark Wielaard
On Thu, Apr 20, 2017 at 04:33:28PM +0200, Ulf Hermann wrote:
> We cannot rely on it to be available from any of the other headers.

You are right. I never realized there was both string.h and strings.h.
Applied to master.

Thanks,

Mark


Re: [PATCH] Avoid signed/unsigned comparison

2017-04-27 Thread Mark Wielaard
On Thu, Apr 20, 2017 at 04:40:30PM +0200, Ulf Hermann wrote:
> Some compilers implicitly cast the result of uint_fast16_t *
> uint_fast16_t to something signed and then complain about the
> comparison to (unsigned) size_t.

Really? That is allowed? Using a signed type for uint_fast16_t?

> Casting phnum to size_t is a good idea anyway as 16bit multiplication
> can easily overflow and we are not checking for this.

OK, that seems an ok enough reason.
Applied to master.

Thanks,

Mark


Re: [PATCH] Make elf section sorting more deterministic

2017-04-27 Thread Mark Wielaard
On Thu, Apr 20, 2017 at 04:54:26PM +0200, Ulf Hermann wrote:
> At least one test (dwfl-addr-sect) depends on the order of elf sections
> with equal addresses. This is not guaranteed by the code. Compare also
> by end address and name to tell entries apart.

O, interesting find. If the start addresses match the order depends on
the specific qsort algorithm. So you need a real tie breaker.

I think it is simpler and more predictable if we just take the section
number into account. It seem to have the added benefit that it provide
the same ordering as before with the glibc qsort, so no testcases need
to be adjusted. Does the following work for you?

diff --git a/libdwfl/derelocate.c b/libdwfl/derelocate.c
index 439a24e..0d10672 100644
--- a/libdwfl/derelocate.c
+++ b/libdwfl/derelocate.c
@@ -63,7 +63,10 @@ compare_secrefs (const void *a, const void *b)
   if ((*p1)->start > (*p2)->start)
 return 1;
 
-  return 0;
+  /* Same start address, then just compare which section came first.  */
+  size_t n1 = elf_ndxscn ((*p1)->scn);
+  size_t n2 = elf_ndxscn ((*p2)->scn);
+  return n1 - n2;
 }
 
 static int


Re: [PATCH] On elf_update, remember when we mmap()

2017-04-27 Thread Mark Wielaard
On Thu, Apr 20, 2017 at 04:57:41PM +0200, Ulf Hermann wrote:
> Otherwise we skip the munmap() later. This leaks resources.

Oops. Good find. Applied to master.

When configured --with-valgrind the tests are run under valgrind
and memory leaks will fail the tests. But since this is mmap
valgrind won't report it. How did you find it?

Thanks,

Mark


Re: [PATCH] Fix nesting of braces

2017-04-27 Thread Mark Wielaard
On Thu, Apr 27, 2017 at 04:35:23PM +0200, Ulf Hermann wrote:
> The way it was before it didn't actually test if elf_update failed, but
> rather did something random. !!() is a boolean and boolean
> true can be represented as anything non-0, including negative numbers.

Urgh. Another good find.
Thanks. Applied to master.


Re: [PATCH] Avoid signed/unsigned comparison

2017-04-27 Thread Josh Stone
On 04/27/2017 11:24 AM, Mark Wielaard wrote:
> On Thu, Apr 20, 2017 at 04:40:30PM +0200, Ulf Hermann wrote:
>> Some compilers implicitly cast the result of uint_fast16_t *
>> uint_fast16_t to something signed and then complain about the
>> comparison to (unsigned) size_t.
> 
> Really? That is allowed? Using a signed type for uint_fast16_t?

I think integer promotion (which happens before the operation) may use a
signed int.  It has to preserve the sign of the value itself, but I
think not necessarily the signedness of the resulting type.

Glibc uses "unsigned int"/"unsigned long int" for uint_fast16_t on
32/64-bit platforms, which means you won't get integer promotion.