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 unwind

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

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_unwi

[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 ch

[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 ---

[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/Makefi

[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/Makefi

[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

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

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

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

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 address

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

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

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 allow