Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

[Mark Wielaard]

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

On Sat, 2014-05-03 at 00:40 +0200, Mark Wielaard wrote:
> All patches are on the mjw/dwelf branch and I'll post them as follow-ups
> to this message. The first 6 have already been discussed a couple of
> times on the list. Let me know if there are any concerns with the last
> 3 or with pushing the whole branch to master.

Since nobody seemed too concerned by these I have pushed them to master
now.

I think with this we are finally ready for the elfutils 0.159 release.
I'll go over the mailinglist and see if we missed something urgent. And
run some extra tests. I hope to propose a 0.159 candidate end of the
week.

Cheers,

Mark

Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

[Mark Wielaard]

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

> On Thu, 2014-04-24 at 14:23 +0200, Florian Weimer wrote:
> > Do you want me to resubmit the final part concerning libdwfl?
>
> No, thanks. I am experimenting with that patch and changed it to use the
> Dwfl find_debuginfo callback to locate the altlink file so that the user
> can override the search strategy (and put all resources in the Dwfl so
> the free_alt flag in libdw is unnecessary).
> 
> I'll post the modified patch when I get it working properly and have it
> rebased against your work again.

Sorry, that took longer than anticipated. What I ended up with is your
original patch to Move .gnu_debugaltlink handling from libdw to libdwfl.
And then I added 3 additional patches on top of it:
- libdwelf: Add dwelf_elf_gnu_build_id.
- libdwfl: Move dwz alt multi file searching to find_debuginfo.
- Remove --enable-dwz. dwz alt debug is no longer experimental.

That should make it easier to see what changes were really made.
The real change is the "libdwfl: Move dwz alt multi file searching to
find_debuginfo." one. That makes it so that the standard callbacks
dwfl_build_id_find_debuginfo and dwfl_standard_find_debuginfo will
now try to resolve and set the alternative debug file depending on
the given debuginfo_path. When using the build-id it works exactly as
finding the separate debug file since an alt file is exactly like that.
When using a path based lookup with the default path it will look in the
directory of the main or debug file and under /usr/lib/debug/.dwz. But
the user can of course adjust the debuginfo_path or provide its own
find_debuginfo callback. It was tested against systemtap which has a
non-trivial find_debuginfo callback and it works completely unchanged.

Finally I made --enable-dwz the default and removed the EXPERIMENTAL
marker.

All patches are on the mjw/dwelf branch and I'll post them as follow-ups
to this message. The first 6 have already been discussed a couple of
times on the list. Let me know if there are any concerns with the last
3 or with pushing the whole branch to master.

Thanks,

Mark

Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

[Mark Wielaard]

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

On Thu, 2014-04-24 at 14:23 +0200, Florian Weimer wrote:
> On 04/23/2014 02:44 PM, Mark Wielaard wrote:
> 
> >> I'm not sure if this addresses this completely addresses the issue.  A
> >> return value of 0 could mean "no debugaltlink data" or "the build ID has
> >> zero length".
> >
> > But a zero length build ID isn't valid, so then you would just return -1
> > and set DWARF_E_INVALID_ELF.
> 
> Okay, I did this in the attached patches.

Thanks. I added them both to the mjw/dwelf branch.

> Do you want me to resubmit the final part concerning libdwfl?

No, thanks. I am experimenting with that patch and changed it to use the
Dwfl find_debuginfo callback to locate the altlink file so that the user
can override the search strategy (and put all resources in the Dwfl so
the free_alt flag in libdw is unnecessary).

I'll post the modified patch when I get it working properly and have it
rebased against your work again.

Thanks,

Mark

Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

[Florian Weimer]

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

On 04/23/2014 02:44 PM, Mark Wielaard wrote:

>> I'm not sure if this addresses this completely addresses the issue.  A
>> return value of 0 could mean "no debugaltlink data" or "the build ID has
>> zero length".
>
> But a zero length build ID isn't valid, so then you would just return -1
> and set DWARF_E_INVALID_ELF.

Okay, I did this in the attached patches.

Do you want me to resubmit the final part concerning libdwfl?

-- 
Florian Weimer / Red Hat Product Security Team

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-libdwelf-Add-dwelf_dwarf_gnu_debugaltlink.patch --]
[-- Type: text/x-patch, Size: 12334 bytes --]

>From 3d5a11e8da77a1fd3589caec8ba852c8a1d87aa1 Mon Sep 17 00:00:00 2001
From: Florian Weimer <fweimer@redhat.com>
Date: Thu, 24 Apr 2014 14:06:43 +0200
Subject: [PATCH 1/4] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

Signed-off-by: Florian Weimer <fweimer@redhat.com>
---
 libdw/ChangeLog                         |  4 ++
 libdw/libdw.map                         |  1 +
 libdwelf/ChangeLog                      |  8 ++++
 libdwelf/Makefile.am                    |  2 +-
 libdwelf/dwelf_dwarf_gnu_debugaltlink.c | 62 ++++++++++++++++++++++++
 libdwelf/libdwelf.h                     | 10 ++++
 libdwelf/libdwelfP.h                    |  1 +
 tests/ChangeLog                         |  7 +++
 tests/Makefile.am                       |  7 +--
 tests/debugaltlink.c                    | 83 +++++++++++++++++++++++++++++++++
 tests/run-debugaltlink.sh               | 34 ++++++++++++++
 11 files changed, 215 insertions(+), 4 deletions(-)
 create mode 100644 libdwelf/dwelf_dwarf_gnu_debugaltlink.c
 create mode 100644 tests/debugaltlink.c
 create mode 100755 tests/run-debugaltlink.sh

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 698e346..ee2054e 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,7 @@
+2014-04-24  Florian Weimer  <fweimer@redhat.com>
+
+	* libdw.map (ELFUTILS_0.159): Export dwelf_dwarf_gnu_debugaltlink.
+
 2014-04-22  Florian Weimer  <fweimer@redhat.com>
 
 	* dwarf_getalt.c, dwarf_setalt.c: New files.
diff --git a/libdw/libdw.map b/libdw/libdw.map
index 71247e3..3529980 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -297,5 +297,6 @@ ELFUTILS_0.159 {
   global:
     dwarf_getalt;
     dwarf_setalt;
+    dwelf_dwarf_gnu_debugaltlink;
     dwelf_elf_gnu_debuglink;
 } ELFUTILS_0.158;
diff --git a/libdwelf/ChangeLog b/libdwelf/ChangeLog
index caf1c5d..aa29132 100644
--- a/libdwelf/ChangeLog
+++ b/libdwelf/ChangeLog
@@ -1,3 +1,11 @@
+2014-04-24  Florian Weimer  <fweimer@redhat.com>
+
+	* dwelf_dwarf_gnu_debugaltlink.c: New file.
+	* Makefile.am (libdwelf_a_SOURCES): Add it.
+	* libdwelf.h (dwelf_dwarf_gnu_debugaltlink): Declare new function.
+	* libdwelfP.h (dwelf_dwarf_gnu_debugaltlink): Add internal
+	declaration.
+
 2014-04-11  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am: New file.
diff --git a/libdwelf/Makefile.am b/libdwelf/Makefile.am
index 0f684d4..1ca3a5c 100644
--- a/libdwelf/Makefile.am
+++ b/libdwelf/Makefile.am
@@ -38,7 +38,7 @@ noinst_LIBRARIES = libdwelf.a libdwelf_pic.a
 pkginclude_HEADERS = libdwelf.h
 noinst_HEADERS = libdwelfP.h
 
-libdwelf_a_SOURCES = dwelf_elf_gnu_debuglink.c
+libdwelf_a_SOURCES = dwelf_elf_gnu_debuglink.c dwelf_dwarf_gnu_debugaltlink.c
 
 libdwelf = $(libdw)
 
diff --git a/libdwelf/dwelf_dwarf_gnu_debugaltlink.c b/libdwelf/dwelf_dwarf_gnu_debugaltlink.c
new file mode 100644
index 0000000..b8285d0
--- /dev/null
+++ b/libdwelf/dwelf_dwarf_gnu_debugaltlink.c
@@ -0,0 +1,62 @@
+/* Returns the file name and build ID stored in the .gnu_altdebuglink if found.
+   Copyright (C) 2014 Red Hat, Inc.
+   This file is part of elfutils.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of either
+
+     * the GNU Lesser General Public License as published by the Free
+       Software Foundation; either version 3 of the License, or (at
+       your option) any later version
+
+   or
+
+     * the GNU General Public License as published by the Free
+       Software Foundation; either version 2 of the License, or (at
+       your option) any later version
+
+   or both in parallel, as here.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received copies of the GNU General Public License and
+   the GNU Lesser General Public License along with this program.  If
+   not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include "libdwelfP.h"
+
+ssize_t
+dwelf_dwarf_gnu_debugaltlink (Dwarf *dwarf,
+			      const char **name_p,
+			      const void **build_idp)
+{
+  Elf_Data *data = dwarf->sectiondata[IDX_gnu_debugaltlink];
+  if (data == NULL)
+    {
+      return 0;
+    }
+
+  const void *ptr = memchr (data->d_buf, '\0', data->d_size);
+  if (ptr == NULL)
+    {
+      __libdw_seterrno (DWARF_E_INVALID_ELF);
+      return -1;
+    }
+  size_t build_id_len = data->d_size - (ptr - data->d_buf + 1);
+  if (build_id_len == 0 || (size_t) (ssize_t) build_id_len != build_id_len)
+    {
+      __libdw_seterrno (DWARF_E_INVALID_ELF);
+      return -1;
+    }
+  *name_p = data->d_buf;
+  *build_idp = ptr + 1;
+  return build_id_len;
+}
+INTDEF(dwelf_dwarf_gnu_debugaltlink)
diff --git a/libdwelf/libdwelf.h b/libdwelf/libdwelf.h
index 5d636d1..6f1dbd3 100644
--- a/libdwelf/libdwelf.h
+++ b/libdwelf/libdwelf.h
@@ -47,6 +47,16 @@ extern "C" {
    section or some other error occured.  */
 extern const char *dwelf_elf_gnu_debuglink (Elf *elf, GElf_Word *crc);
 
+/* Returns the name and build ID from the .gnu_debugaltlink section if
+   found in the ELF.  On success, pointers to the name and build ID
+   are written to *NAMEP and *BUILDID_P, and the positive length of
+   the build ID is returned.  Returns 0 if the ELF lacks a
+   .gnu_debugaltlink section.  Returns -1 in case of malformed data or
+   other errors.  */
+extern ssize_t dwelf_dwarf_gnu_debugaltlink (Dwarf *dwarf,
+					     const char **namep,
+					     const void **build_idp);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libdwelf/libdwelfP.h b/libdwelf/libdwelfP.h
index bdadc8b..c00a834 100644
--- a/libdwelf/libdwelfP.h
+++ b/libdwelf/libdwelfP.h
@@ -36,5 +36,6 @@
 
 /* Avoid PLT entries.  */
 INTDECL (dwelf_elf_gnu_debuglink)
+INTDECL (dwelf_dwarf_gnu_debugaltlink)
 
 #endif	/* libdwelfP.h */
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 5a2850e..f3f947b 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,10 @@
+2014-04-24  Florian Weimer  <fweimer@redhat.com>
+
+	* debugaltlink.c, run-debugaltlink.sh: New files.
+	* Makefile.am (check_PROGRAMS): Add debugaltlink.
+	(TESTS): Add run-debugaltlink.sh.
+	(debugaltlink_LDADD): New variable.
+
 2014-04-11  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (AM_CPPFLAGS): Add -I libdwelf.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 67c7fbc..33803fc 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -49,7 +49,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \
 		  alldts md5-sha1-test typeiter typeiter2 low_high_pc \
 		  test-elf_cntl_gelf_getshdr dwflsyms dwfllines \
 		  dwfl-report-elf-align varlocs backtrace backtrace-child \
-		  backtrace-data backtrace-dwarf debuglink
+		  backtrace-data backtrace-dwarf debuglink debugaltlink
 asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \
 	    asm-tst6 asm-tst7 asm-tst8 asm-tst9
 
@@ -86,7 +86,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
 	run-readelf-macro.sh run-readelf-loc.sh \
 	run-readelf-aranges.sh run-readelf-line.sh \
 	run-native-test.sh run-bug1-test.sh \
-	run-debuglink.sh \
+	run-debuglink.sh run-debugaltlink.sh \
 	dwfl-bug-addr-overflow run-addrname-test.sh \
 	dwfl-bug-fd-leak dwfl-bug-report \
 	run-dwfl-bug-offline-rel.sh run-dwfl-addr-sect.sh \
@@ -176,7 +176,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \
 	     run-readelf-test1.sh run-readelf-test2.sh run-readelf-test3.sh \
 	     run-readelf-test4.sh run-readelf-twofiles.sh \
 	     run-bug1-test.sh testfile28.bz2 testfile28.rdwr.bz2 \
-	     run-debuglink.sh \
+	     run-debuglink.sh run-debugaltlink.sh \
 	     testfile29.bz2 testfile29.rdwr.bz2 \
 	     testfile30.bz2 testfile31.bz2 testfile32.bz2 testfile33.bz2 \
 	     testfile34.bz2 testfile35.bz2 testfile35.debug.bz2 \
@@ -405,6 +405,7 @@ backtrace_data_LDADD = $(libdw) $(libelf)
 backtrace_dwarf_CFLAGS = -Wno-unused-parameter
 backtrace_dwarf_LDADD = $(libdw) $(libelf)
 debuglink_LDADD = $(libdw) $(libelf)
+debugaltlink_LDADD = $(libdw) $(libelf)
 
 if GCOV
 check: check-am coverage
diff --git a/tests/debugaltlink.c b/tests/debugaltlink.c
new file mode 100644
index 0000000..6d97d50
--- /dev/null
+++ b/tests/debugaltlink.c
@@ -0,0 +1,83 @@
+/* Test program for dwelf_dwarf_gnu_debugaltlink, print name and build ID.
+   Copyright (C) 2014 Red Hat, Inc.
+   This file is part of elfutils.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+#include <assert.h>
+#include <inttypes.h>
+#include <err.h>
+#include <errno.h>
+#include ELFUTILS_HEADER(dw)
+#include ELFUTILS_HEADER(dwelf)
+#include <stdio.h>
+#include <error.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+int
+main (int argc, char *argv[])
+{
+  if (argc < 2)
+    error (EXIT_FAILURE, 0, "No input file given");
+
+  elf_version (EV_CURRENT);
+
+  for (int i = 1; i < argc; i++)
+    {
+      const char *file = argv[i];
+      int fd = open (file, O_RDONLY);
+      if (fd < 0)
+	error (EXIT_FAILURE, errno, "couldn't open file '%s'", file);
+
+      Dwarf *dwarf = dwarf_begin (fd, DWARF_C_READ);
+      if (dwarf == NULL)
+	{
+	  printf("%s: dwarf_begin failed: %s\n", file, dwarf_errmsg (-1));
+	  close (fd);
+	  continue;
+	}
+
+      const char *name;
+      const void *build_id;
+      ssize_t ret = dwelf_dwarf_gnu_debugaltlink
+	(dwarf, &name, &build_id);
+      switch (ret)
+	{
+	case 0:
+	  printf ("%s: <no .gnu_debugaltlink section>\n", file);
+	  break;
+	case -1:
+	  errx (1, "dwelf_dwarf_gnu_debugaltlink (%s): %s",
+		file, dwarf_errmsg (-1));
+	default:
+	  printf ("%s: %s, build ID: ", file, name);
+	  const unsigned char *p = build_id;
+	  const unsigned char *end = p + ret;
+	  while (p < end)
+	      printf("%02x", (unsigned)*p++);
+	  putchar('\n');
+	}
+
+      dwarf_end (dwarf);
+      close (fd);
+    }
+
+  return 0;
+}
diff --git a/tests/run-debugaltlink.sh b/tests/run-debugaltlink.sh
new file mode 100755
index 0000000..fa7dd26
--- /dev/null
+++ b/tests/run-debugaltlink.sh
@@ -0,0 +1,34 @@
+#! /bin/sh
+# Copyright (C) 2014 Red Hat, Inc.
+# This file is part of elfutils.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. $srcdir/test-subr.sh
+
+# Just some random testfiles, four with, one without .gnu_debugaltlink
+testfiles testfile42 testfile_multi_main testfile-dwzstr \
+    test-offset-loop libtestfile_multi_shared.so
+
+testrun_compare  ${abs_builddir}/debugaltlink testfile42 \
+    testfile_multi_main testfile-dwzstr \
+    test-offset-loop libtestfile_multi_shared.so <<\EOF
+testfile42: <no .gnu_debugaltlink section>
+testfile_multi_main: testfile_multi.dwz, build ID: a0d6c06e0d912d74033b6fe2808753cae8f6f594
+testfile-dwzstr: testfile-dwzstr.multi, build ID: 6da22627dae55c1d62cf9122827c665e240a056b
+test-offset-loop: test-offset-loop.alt, build ID: 066bbf1a7bc5676f5015ee1966a088f23bdb83ae
+libtestfile_multi_shared.so: testfile_multi.dwz, build ID: a0d6c06e0d912d74033b6fe2808753cae8f6f594
+EOF
+
+exit 0
-- 
1.9.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-tests-allfcts.c-Install-alternate-debug-information.patch --]
[-- Type: text/x-patch, Size: 3171 bytes --]

>From 8d4aa6c149674211c28d96729774b7c18b19f6bd Mon Sep 17 00:00:00 2001
From: Florian Weimer <fweimer@redhat.com>
Date: Tue, 15 Apr 2014 16:58:39 +0200
Subject: [PATCH 2/4] tests/allfcts.c: Install alternate debug information

This change also adds more error checking and reporting.

Signed-off-by: Florian Weimer <fweimer@redhat.com>
---
 tests/ChangeLog |  6 ++++++
 tests/allfcts.c | 35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index f3f947b..450b286 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,5 +1,11 @@
 2014-04-24  Florian Weimer  <fweimer@redhat.com>
 
+	* allfcts.c (setup_alt): New function.
+	(main): Call it.  Implementation additional error checking and
+	reporting.
+
+2014-04-24  Florian Weimer  <fweimer@redhat.com>
+
 	* debugaltlink.c, run-debugaltlink.sh: New files.
 	* Makefile.am (check_PROGRAMS): Add debugaltlink.
 	(TESTS): Add run-debugaltlink.sh.
diff --git a/tests/allfcts.c b/tests/allfcts.c
index 10e0f07..d3c8d26 100644
--- a/tests/allfcts.c
+++ b/tests/allfcts.c
@@ -18,8 +18,10 @@
 # include <config.h>
 #endif
 
+#include <err.h>
 #include <fcntl.h>
 #include ELFUTILS_HEADER(dw)
+#include ELFUTILS_HEADER(dwelf)
 #include <stdio.h>
 #include <unistd.h>
 
@@ -37,6 +39,28 @@ cb (Dwarf_Die *func, void *arg __attribute__ ((unused)))
   return DWARF_CB_ABORT;
 }
 
+static Dwarf *
+setup_alt (Dwarf *main)
+{
+  const char *alt_name;
+  const void *build_id;
+  ssize_t ret = dwelf_dwarf_gnu_debugaltlink (main, &alt_name, &build_id);
+  if (ret == 0)
+    return NULL;
+  if (ret == -1)
+    errx (1, "dwelf_dwarf_gnu_debugaltlink: %s", dwarf_errmsg (-1));
+  int fd = open (alt_name, O_RDONLY);
+  if (fd < 0)
+    err (1, "open (%s)", alt_name);
+  Dwarf *dbg_alt = dwarf_begin (fd, DWARF_C_READ);
+  if (dbg_alt == NULL)
+    errx (1, "dwarf_begin (%s): %s", alt_name, dwarf_errmsg (-1));
+  if (elf_cntl (dwarf_getelf (dbg_alt), ELF_C_FDREAD) != 0)
+    errx (1, "elf_cntl (%s, ELF_C_FDREAD): %s", alt_name, elf_errmsg (-1));
+  close (fd);
+  dwarf_setalt (main, dbg_alt);
+  return dbg_alt;
+}
 
 int
 main (int argc, char *argv[])
@@ -44,6 +68,8 @@ main (int argc, char *argv[])
   for (int i = 1; i < argc; ++i)
     {
       int fd = open (argv[i], O_RDONLY);
+      if (fd < 0)
+	err (1, "open (%s)", argv[i]);
 
       Dwarf *dbg = dwarf_begin (fd, DWARF_C_READ);
       if (dbg != NULL)
@@ -51,6 +77,7 @@ main (int argc, char *argv[])
 	  Dwarf_Off off = 0;
 	  size_t cuhl;
 	  Dwarf_Off noff;
+	  Dwarf *dbg_alt = setup_alt (dbg);
 
 	  while (dwarf_nextcu (dbg, off, &noff, &cuhl, NULL, NULL, NULL) == 0)
 	    {
@@ -62,14 +89,20 @@ main (int argc, char *argv[])
 	      do
 		{
 		  doff = dwarf_getfuncs (die, cb, NULL, doff);
+		  if (dwarf_errno () != 0)
+		    errx (1, "dwarf_getfuncs (%s): %s",
+			  argv[i], dwarf_errmsg (-1));
 		}
-	      while (doff != 0 && dwarf_errno () == 0);
+	      while (doff != 0);
 
 	      off = noff;
 	    }
 
+	  dwarf_end (dbg_alt);
 	  dwarf_end (dbg);
 	}
+      else
+	errx (1, "dwarf_begin (%s): %s", argv[i], dwarf_errmsg (-1));
 
       close (fd);
     }
-- 
1.9.0

Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

[Mark Wielaard]

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

On Tue, 2014-04-22 at 09:33 +0200, Florian Weimer wrote:
> On 04/18/2014 01:02 PM, Mark Wielaard wrote:
> > On Tue, 2014-04-15 at 14:49 +0200, Florian Weimer wrote:
> >> +2014-04-15  Florian Weimer  <fweimer@redhat.com>
> >> +
> >> +	* dwelf_dwarf_gnu_debugaltlink.c: New file.
> >> +	* Makefile.am (libdwelf_a_SOURCES): Add it.
> >> +	* libdwelf.h (dwelf_dwarf_gnu_debugaltlink): Declare new function.
> >> +	* libdwelfP.h (dwelf_dwarf_gnu_debugaltlink): Add internal
> >> +	declaration.
> >
> > The code looks fine. But I still like the "tri-state" return more to
> > indicate why/how things went. Precisely because then you don't have to
> > do:
> >
> >> +  Elf_Data *data = dwarf->sectiondata[IDX_gnu_debugaltlink];
> >> +  if (data == NULL)
> >> +    {
> >> +      /* Missing data is not actually an error. */
> >> +      __libdw_seterrno (0);
> >> +      return NULL;
> >> +    }
> >> [...]
> >> +/* Returns the name of the alternate debug information file from the
> >> +   .gnu_debugaltlink section if found in the ELF.  On success, writes
> >> +   a pointer to the build ID to *BUILD_IDP, and its length to
> >> +   *BUILD_ID_LENP.  Returns NULL if the ELF file didn't have a
> >> +   .gnu_debuglink section (and sets the dwarf_errcode error code to
> >> +   0), had malformed data in the section or some other error occured.
> >> +   The returned pointers are valid while the ELF handle is valid.  */
> >> +extern const char *dwelf_dwarf_gnu_debugaltlink (Dwarf *dwarf,
> >> +						 const void **build_idp,
> >> +						 size_t *build_id_lenp);
> 
> I'm not sure if this addresses this completely addresses the issue.  A 
> return value of 0 could mean "no debugaltlink data" or "the build ID has 
> zero length".

But a zero length build ID isn't valid, so then you would just return -1
and set DWARF_E_INVALID_ELF.

> > Thanks for adding the testcase. run-debugaltlink.sh should also be added
> > to EXTRA_DIST (I also forgot that in my dwelf_elf_gnu_debuglink patch
> > (fixed on the mjw/dwelf branch).
> 
> Can't we handle this in a better way with make variables, so that the 
> file needs to be listed in just one place?

Yeah we probably should, I have forgotten more than once.

Cheers,

Mark

Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

[Florian Weimer]

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

On 04/18/2014 01:02 PM, Mark Wielaard wrote:
> On Tue, 2014-04-15 at 14:49 +0200, Florian Weimer wrote:
>> +2014-04-15  Florian Weimer  <fweimer@redhat.com>
>> +
>> +	* dwelf_dwarf_gnu_debugaltlink.c: New file.
>> +	* Makefile.am (libdwelf_a_SOURCES): Add it.
>> +	* libdwelf.h (dwelf_dwarf_gnu_debugaltlink): Declare new function.
>> +	* libdwelfP.h (dwelf_dwarf_gnu_debugaltlink): Add internal
>> +	declaration.
>
> The code looks fine. But I still like the "tri-state" return more to
> indicate why/how things went. Precisely because then you don't have to
> do:
>
>> +  Elf_Data *data = dwarf->sectiondata[IDX_gnu_debugaltlink];
>> +  if (data == NULL)
>> +    {
>> +      /* Missing data is not actually an error. */
>> +      __libdw_seterrno (0);
>> +      return NULL;
>> +    }
>> [...]
>> +/* Returns the name of the alternate debug information file from the
>> +   .gnu_debugaltlink section if found in the ELF.  On success, writes
>> +   a pointer to the build ID to *BUILD_IDP, and its length to
>> +   *BUILD_ID_LENP.  Returns NULL if the ELF file didn't have a
>> +   .gnu_debuglink section (and sets the dwarf_errcode error code to
>> +   0), had malformed data in the section or some other error occured.
>> +   The returned pointers are valid while the ELF handle is valid.  */
>> +extern const char *dwelf_dwarf_gnu_debugaltlink (Dwarf *dwarf,
>> +						 const void **build_idp,
>> +						 size_t *build_id_lenp);

I'm not sure if this addresses this completely addresses the issue.  A 
return value of 0 could mean "no debugaltlink data" or "the build ID has 
zero length".

I don't like setting the error code to zero, but as far as I can tell, 
the error codes are not exposed, and a caller would have to resort to 
string comparisons to check if it's the expected "missing section" 
error, or something about lack of resources or malformed input data.

>> +2014-04-15  Florian Weimer  <fweimer@redhat.com>
>> +
>> +	* debugaltlink.c, run-debugaltlink.sh: New files.
>> +	* Makefile.am (check_PROGRAMS): Add debugaltlink.
>> +	(TESTS): Add run-debugaltlink.sh.
>> +	(debugaltlink_LDADD): New variable.
>
> Thanks for adding the testcase. run-debugaltlink.sh should also be added
> to EXTRA_DIST (I also forgot that in my dwelf_elf_gnu_debuglink patch
> (fixed on the mjw/dwelf branch).

Can't we handle this in a better way with make variables, so that the 
file needs to be listed in just one place?

Anyway, I've fixed it in my copy.

-- 
Florian Weimer / Red Hat Product Security Team

Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

[Mark Wielaard]

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

On Tue, 2014-04-15 at 14:49 +0200, Florian Weimer wrote:
> +2014-04-15  Florian Weimer  <fweimer@redhat.com>
> +
> +	* dwelf_dwarf_gnu_debugaltlink.c: New file.
> +	* Makefile.am (libdwelf_a_SOURCES): Add it.
> +	* libdwelf.h (dwelf_dwarf_gnu_debugaltlink): Declare new function.
> +	* libdwelfP.h (dwelf_dwarf_gnu_debugaltlink): Add internal
> +	declaration.

The code looks fine. But I still like the "tri-state" return more to
indicate why/how things went. Precisely because then you don't have to
do:

> +  Elf_Data *data = dwarf->sectiondata[IDX_gnu_debugaltlink];
> +  if (data == NULL)
> +    {
> +      /* Missing data is not actually an error. */
> +      __libdw_seterrno (0);
> +      return NULL;
> +    }
> [...]
> +/* Returns the name of the alternate debug information file from the
> +   .gnu_debugaltlink section if found in the ELF.  On success, writes
> +   a pointer to the build ID to *BUILD_IDP, and its length to
> +   *BUILD_ID_LENP.  Returns NULL if the ELF file didn't have a
> +   .gnu_debuglink section (and sets the dwarf_errcode error code to
> +   0), had malformed data in the section or some other error occured.
> +   The returned pointers are valid while the ELF handle is valid.  */
> +extern const char *dwelf_dwarf_gnu_debugaltlink (Dwarf *dwarf,
> +						 const void **build_idp,
> +						 size_t *build_id_lenp);

So I would write this as:

/* Returns the name and build_id of the alternate debug information file
   of the given DWARF if found through the .gnu_debugaltlink section. On
   success, writes a pointer to the name to *NAMEP and a pointer to the
   build ID to *BUILD_IDP and returns the length of the build_id.
   Returns zero if the DWARF didn't have an alternate debug information
   file in its ELF file .gnu_debuglink section. Returns -1 for errors
   and sets dwarf_errno.  The returned pointers are valid while the
   DWARF handle is valid.  */
extern int dwelf_dwarf_gnu_debugaltlink (Dwarf *dwarf,
                                         const char **namep,
                                         const void **build_idp);

> +2014-04-15  Florian Weimer  <fweimer@redhat.com>
> +
> +	* debugaltlink.c, run-debugaltlink.sh: New files.
> +	* Makefile.am (check_PROGRAMS): Add debugaltlink.
> +	(TESTS): Add run-debugaltlink.sh.
> +	(debugaltlink_LDADD): New variable.

Thanks for adding the testcase. run-debugaltlink.sh should also be added
to EXTRA_DIST (I also forgot that in my dwelf_elf_gnu_debuglink patch
(fixed on the mjw/dwelf branch).

Cheers,

Mark

[PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink

[Florian Weimer]

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

Signed-off-by: Florian Weimer <fweimer@redhat.com>
---
 libdw/ChangeLog                         |  2 +
 libdw/libdw.map                         |  1 +
 libdwelf/ChangeLog                      |  8 ++++
 libdwelf/Makefile.am                    |  2 +-
 libdwelf/dwelf_dwarf_gnu_debugaltlink.c | 57 ++++++++++++++++++++++++
 libdwelf/libdwelf.h                     | 11 +++++
 libdwelf/libdwelfP.h                    |  1 +
 tests/ChangeLog                         |  7 +++
 tests/Makefile.am                       |  5 ++-
 tests/debugaltlink.c                    | 77 +++++++++++++++++++++++++++++++++
 tests/run-debugaltlink.sh               | 34 +++++++++++++++
 11 files changed, 202 insertions(+), 3 deletions(-)
 create mode 100644 libdwelf/dwelf_dwarf_gnu_debugaltlink.c
 create mode 100644 tests/debugaltlink.c
 create mode 100755 tests/run-debugaltlink.sh

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 1ecdb02..3097880 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -13,6 +13,8 @@
 	declarations.
 	* libdw.map (ELFUTILS_0.159): Export the two new functions.
 
+	* libdw.map (ELFUTILS_0.159): Export dwelf_dwarf_gnu_debugaltlink.
+
 2014-04-11  Mark Wielaard  <mjw@redhat.com>
 
 	* libdw.map (ELFUTILS_0.159): New. Add dwelf_elf_gnu_debuglink.
diff --git a/libdw/libdw.map b/libdw/libdw.map
index 71247e3..3529980 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -297,5 +297,6 @@ ELFUTILS_0.159 {
   global:
     dwarf_getalt;
     dwarf_setalt;
+    dwelf_dwarf_gnu_debugaltlink;
     dwelf_elf_gnu_debuglink;
 } ELFUTILS_0.158;
diff --git a/libdwelf/ChangeLog b/libdwelf/ChangeLog
index caf1c5d..7a3cd03 100644
--- a/libdwelf/ChangeLog
+++ b/libdwelf/ChangeLog
@@ -1,3 +1,11 @@
+2014-04-15  Florian Weimer  <fweimer@redhat.com>
+
+	* dwelf_dwarf_gnu_debugaltlink.c: New file.
+	* Makefile.am (libdwelf_a_SOURCES): Add it.
+	* libdwelf.h (dwelf_dwarf_gnu_debugaltlink): Declare new function.
+	* libdwelfP.h (dwelf_dwarf_gnu_debugaltlink): Add internal
+	declaration.
+
 2014-04-11  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am: New file.
diff --git a/libdwelf/Makefile.am b/libdwelf/Makefile.am
index 0f684d4..1ca3a5c 100644
--- a/libdwelf/Makefile.am
+++ b/libdwelf/Makefile.am
@@ -38,7 +38,7 @@ noinst_LIBRARIES = libdwelf.a libdwelf_pic.a
 pkginclude_HEADERS = libdwelf.h
 noinst_HEADERS = libdwelfP.h
 
-libdwelf_a_SOURCES = dwelf_elf_gnu_debuglink.c
+libdwelf_a_SOURCES = dwelf_elf_gnu_debuglink.c dwelf_dwarf_gnu_debugaltlink.c
 
 libdwelf = $(libdw)
 
diff --git a/libdwelf/dwelf_dwarf_gnu_debugaltlink.c b/libdwelf/dwelf_dwarf_gnu_debugaltlink.c
new file mode 100644
index 0000000..fe473ab
--- /dev/null
+++ b/libdwelf/dwelf_dwarf_gnu_debugaltlink.c
@@ -0,0 +1,57 @@
+/* Returns the file name and build ID stored in the .gnu_altdebuglink if found.
+   Copyright (C) 2014 Red Hat, Inc.
+   This file is part of elfutils.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of either
+
+     * the GNU Lesser General Public License as published by the Free
+       Software Foundation; either version 3 of the License, or (at
+       your option) any later version
+
+   or
+
+     * the GNU General Public License as published by the Free
+       Software Foundation; either version 2 of the License, or (at
+       your option) any later version
+
+   or both in parallel, as here.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received copies of the GNU General Public License and
+   the GNU Lesser General Public License along with this program.  If
+   not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include "libdwelfP.h"
+
+const char *
+dwelf_dwarf_gnu_debugaltlink (Dwarf *dwarf, const void **build_idp,
+			      size_t *build_id_lenp)
+{
+  Elf_Data *data = dwarf->sectiondata[IDX_gnu_debugaltlink];
+  if (data == NULL)
+    {
+      /* Missing data is not actually an error. */
+      __libdw_seterrno (0);
+      return NULL;
+    }
+
+  const void *ptr = memchr (data->d_buf, '\0', data->d_size);
+  if (ptr == NULL)
+    {
+      __libdw_seterrno (DWARF_E_INVALID_ELF);
+      return NULL;
+    }
+  *build_idp = ptr + 1;
+  *build_id_lenp = data->d_size - (ptr - data->d_buf + 1);
+  return data->d_buf;
+}
+INTDEF(dwelf_dwarf_gnu_debugaltlink)
diff --git a/libdwelf/libdwelf.h b/libdwelf/libdwelf.h
index 5d636d1..fbf7f2e 100644
--- a/libdwelf/libdwelf.h
+++ b/libdwelf/libdwelf.h
@@ -47,6 +47,17 @@ extern "C" {
    section or some other error occured.  */
 extern const char *dwelf_elf_gnu_debuglink (Elf *elf, GElf_Word *crc);
 
+/* Returns the name of the alternate debug information file from the
+   .gnu_debugaltlink section if found in the ELF.  On success, writes
+   a pointer to the build ID to *BUILD_IDP, and its length to
+   *BUILD_ID_LENP.  Returns NULL if the ELF file didn't have a
+   .gnu_debuglink section (and sets the dwarf_errcode error code to
+   0), had malformed data in the section or some other error occured.
+   The returned pointers are valid while the ELF handle is valid.  */
+extern const char *dwelf_dwarf_gnu_debugaltlink (Dwarf *dwarf,
+						 const void **build_idp,
+						 size_t *build_id_lenp);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libdwelf/libdwelfP.h b/libdwelf/libdwelfP.h
index bdadc8b..c00a834 100644
--- a/libdwelf/libdwelfP.h
+++ b/libdwelf/libdwelfP.h
@@ -36,5 +36,6 @@
 
 /* Avoid PLT entries.  */
 INTDECL (dwelf_elf_gnu_debuglink)
+INTDECL (dwelf_dwarf_gnu_debugaltlink)
 
 #endif	/* libdwelfP.h */
diff --git a/tests/ChangeLog b/tests/ChangeLog
index f6d6fd6..a5fc945 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,10 @@
+2014-04-15  Florian Weimer  <fweimer@redhat.com>
+
+	* debugaltlink.c, run-debugaltlink.sh: New files.
+	* Makefile.am (check_PROGRAMS): Add debugaltlink.
+	(TESTS): Add run-debugaltlink.sh.
+	(debugaltlink_LDADD): New variable.
+
 2014-04-11  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (AM_CPPFLAGS): Add -I libdwelf.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 39dbcfc..f139c65 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -53,7 +53,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \
 		  alldts md5-sha1-test typeiter typeiter2 low_high_pc \
 		  test-elf_cntl_gelf_getshdr dwflsyms dwfllines \
 		  dwfl-report-elf-align varlocs backtrace backtrace-child \
-		  backtrace-data backtrace-dwarf debuglink
+		  backtrace-data backtrace-dwarf debuglink debugaltlink
 asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \
 	    asm-tst6 asm-tst7 asm-tst8 asm-tst9
 
@@ -90,7 +90,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
 	run-readelf-macro.sh run-readelf-loc.sh \
 	run-readelf-aranges.sh run-readelf-line.sh \
 	run-native-test.sh run-bug1-test.sh \
-	run-debuglink.sh \
+	run-debuglink.sh run-debugaltlink.sh \
 	dwfl-bug-addr-overflow run-addrname-test.sh \
 	dwfl-bug-fd-leak dwfl-bug-report \
 	run-dwfl-bug-offline-rel.sh run-dwfl-addr-sect.sh \
@@ -413,6 +413,7 @@ backtrace_data_LDADD = $(libdw) $(libelf) $(libmudflap)
 backtrace_dwarf_CFLAGS = -Wno-unused-parameter
 backtrace_dwarf_LDADD = $(libdw) $(libelf) $(libmudflap)
 debuglink_LDADD = $(libdw) $(libelf)
+debugaltlink_LDADD = $(libdw) $(libelf)
 
 if GCOV
 check: check-am coverage
diff --git a/tests/debugaltlink.c b/tests/debugaltlink.c
new file mode 100644
index 0000000..f75cc6e
--- /dev/null
+++ b/tests/debugaltlink.c
@@ -0,0 +1,77 @@
+/* Test program for dwelf_dwarf_gnu_debugaltlink, print name and build ID.
+   Copyright (C) 2014 Red Hat, Inc.
+   This file is part of elfutils.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+#include <assert.h>
+#include <inttypes.h>
+#include <errno.h>
+#include ELFUTILS_HEADER(dw)
+#include ELFUTILS_HEADER(dwelf)
+#include <stdio.h>
+#include <error.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+int
+main (int argc, char *argv[])
+{
+  if (argc < 2)
+    error (EXIT_FAILURE, 0, "No input file given");
+
+  elf_version (EV_CURRENT);
+
+  for (int i = 1; i < argc; i++)
+    {
+      const char *file = argv[i];
+      int fd = open (file, O_RDONLY);
+      if (fd < 0)
+	error (EXIT_FAILURE, errno, "couldn't open file '%s'", file);
+
+      Dwarf *dwarf = dwarf_begin (fd, DWARF_C_READ);
+      if (dwarf == NULL)
+	{
+	  printf("%s: dwarf_begin failed: %s\n", file, dwarf_errmsg (-1));
+	  close (fd);
+	  continue;
+	}
+
+      const void *build_id;
+      size_t build_id_len;
+      const char *name = dwelf_dwarf_gnu_debugaltlink
+	(dwarf, &build_id, &build_id_len);
+      if (name == NULL)
+	printf ("%s: <no .gnu_debugaltlink section>\n", file);
+      else
+	{
+	  printf ("%s: %s, build ID: ", file, name);
+	  const unsigned char *p = build_id;
+	  const unsigned char *end = p + build_id_len;
+	  while (p < end)
+	      printf("%02x", (unsigned)*p++);
+	  putchar('\n');
+	}
+
+      dwarf_end (dwarf);
+      close (fd);
+    }
+
+  return 0;
+}
diff --git a/tests/run-debugaltlink.sh b/tests/run-debugaltlink.sh
new file mode 100755
index 0000000..fa7dd26
--- /dev/null
+++ b/tests/run-debugaltlink.sh
@@ -0,0 +1,34 @@
+#! /bin/sh
+# Copyright (C) 2014 Red Hat, Inc.
+# This file is part of elfutils.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. $srcdir/test-subr.sh
+
+# Just some random testfiles, four with, one without .gnu_debugaltlink
+testfiles testfile42 testfile_multi_main testfile-dwzstr \
+    test-offset-loop libtestfile_multi_shared.so
+
+testrun_compare  ${abs_builddir}/debugaltlink testfile42 \
+    testfile_multi_main testfile-dwzstr \
+    test-offset-loop libtestfile_multi_shared.so <<\EOF
+testfile42: <no .gnu_debugaltlink section>
+testfile_multi_main: testfile_multi.dwz, build ID: a0d6c06e0d912d74033b6fe2808753cae8f6f594
+testfile-dwzstr: testfile-dwzstr.multi, build ID: 6da22627dae55c1d62cf9122827c665e240a056b
+test-offset-loop: test-offset-loop.alt, build ID: 066bbf1a7bc5676f5015ee1966a088f23bdb83ae
+libtestfile_multi_shared.so: testfile_multi.dwz, build ID: a0d6c06e0d912d74033b6fe2808753cae8f6f594
+EOF
+
+exit 0
-- 
1.9.0