[PATCH] libdwfl, src: Replace some asserts with proper check or error messages.

[PATCH] libdwfl, src: Replace some asserts with proper check or error messages.

[Mark Wielaard]

library code really shouldn't assert and for utilities a proper
error message is better than crashing with an assert.

https://sourceware.org/bugzilla/show_bug.cgi?id=26176

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libdwfl/ChangeLog                | 14 ++++++++++++++
 libdwfl/argp-std.c               |  3 ++-
 libdwfl/dwfl_build_id_find_elf.c |  7 +++++--
 libdwfl/frame_unwind.c           |  3 ++-
 libdwfl/linux-core-attach.c      |  7 ++++---
 libdwfl/linux-pid-attach.c       |  6 +++++-
 src/ChangeLog                    |  7 +++++++
 src/stack.c                      |  7 ++++++-
 src/unstrip.c                    |  9 +++++----
 9 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 5a3d566f..e59efd77 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,17 @@
+2020-07-05  Mark Wielaard  <mark@klomp.org>
+
+	* argp-std.c (parse_opt): Don't assert, but call fail when
+	dwfl_report_end fails.
+	* dwfl_build_id_find_elf.c (__libdwfl_open_by_build_id): Don't
+	assert, but goto bad_id when snprintf fails.
+	* frame_unwind.c (__libdwfl_frame_unwind): Don't assert, but
+	return when dwfl_frame_pc fails.
+	* linux-core-attach.c (core_set_initial_registers): Don't assert,
+	but return false when gelf_getnote fails or the core note is not
+	as expected.
+	* linux-pid-attach.c (dwfl_linux_proc_attach): Don't assert, but
+	goto fail when snprintf fails.
+
 2020-06-16  Mark Wielaard  <mark@klomp.org>
 
 	* frame_unwind.c (handle_cfi): Flag an error if
diff --git a/libdwfl/argp-std.c b/libdwfl/argp-std.c
index 8ee91587..2aa1b5e0 100644
--- a/libdwfl/argp-std.c
+++ b/libdwfl/argp-std.c
@@ -342,7 +342,8 @@ parse_opt (int key, char *arg, struct argp_state *state)
 	   argp_parse.  */
 
 	int result = INTUSE(dwfl_report_end) (dwfl, NULL, NULL);
-	assert (result == 0);
+	if (result != 0)
+	  return fail (dwfl, -1, arg, state);
 
 	/* Update the input all along, so a parent parser can see it.
 	   As we free OPT the update below will be no longer active.  */
diff --git a/libdwfl/dwfl_build_id_find_elf.c b/libdwfl/dwfl_build_id_find_elf.c
index 4e56143f..f685c979 100644
--- a/libdwfl/dwfl_build_id_find_elf.c
+++ b/libdwfl/dwfl_build_id_find_elf.c
@@ -48,6 +48,7 @@ __libdwfl_open_by_build_id (Dwfl_Module *mod, bool debug, char **file_name,
 #define MAX_BUILD_ID_BYTES 64
   if (id_len < MIN_BUILD_ID_BYTES || id_len > MAX_BUILD_ID_BYTES)
     {
+    bad_id:
       __libdwfl_seterrno (DWFL_E_WRONG_ID_ELF);
       return -1;
     }
@@ -59,12 +60,14 @@ __libdwfl_open_by_build_id (Dwfl_Module *mod, bool debug, char **file_name,
   strcpy (id_name, "/.build-id/");
   int n = snprintf (&id_name[sizeof "/.build-id/" - 1],
 		    4, "%02" PRIx8 "/", (uint8_t) id[0]);
-  assert (n == 3);
+  if (n != 3)
+    goto bad_id;;
   for (size_t i = 1; i < id_len; ++i)
     {
       n = snprintf (&id_name[sizeof "/.build-id/" - 1 + 3 + (i - 1) * 2],
 		    3, "%02" PRIx8, (uint8_t) id[i]);
-      assert (n == 2);
+      if (n != 2)
+	goto bad_id;
     }
   if (debug)
     strcpy (&id_name[sizeof "/.build-id/" - 1 + 3 + (id_len - 1) * 2],
diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
index bdceeb3e..9ac33833 100644
--- a/libdwfl/frame_unwind.c
+++ b/libdwfl/frame_unwind.c
@@ -723,7 +723,8 @@ __libdwfl_frame_unwind (Dwfl_Frame *state)
      which would deadlock us.  */
   Dwarf_Addr pc;
   bool ok = INTUSE(dwfl_frame_pc) (state, &pc, NULL);
-  assert (ok);
+  if (!ok)
+    return;
   /* Check whether this is the initial frame or a signal frame.
      Then we need to unwind from the original, unadjusted PC.  */
   if (! state->initial_frame && ! state->signal_frame)
diff --git a/libdwfl/linux-core-attach.c b/libdwfl/linux-core-attach.c
index c0f1b0d0..f68062f0 100644
--- a/libdwfl/linux-core-attach.c
+++ b/libdwfl/linux-core-attach.c
@@ -181,7 +181,8 @@ core_set_initial_registers (Dwfl_Thread *thread, void *thread_arg_voidp)
   size_t getnote_err = gelf_getnote (note_data, offset, &nhdr, &name_offset,
 				     &desc_offset);
   /* __libdwfl_attach_state_for_core already verified the note is there.  */
-  assert (getnote_err != 0);
+  if (getnote_err == 0)
+    return false;
   /* Do not check NAME for now, help broken Linux kernels.  */
   const char *name = (nhdr.n_namesz == 0
 		      ? "" : note_data->d_buf + name_offset);
@@ -195,8 +196,8 @@ core_set_initial_registers (Dwfl_Thread *thread, void *thread_arg_voidp)
 				     &regs_offset, &nregloc, &reglocs,
 				     &nitems, &items);
   /* __libdwfl_attach_state_for_core already verified the note is there.  */
-  assert (core_note_err != 0);
-  assert (nhdr.n_type == NT_PRSTATUS);
+  if (core_note_err == 0 || nhdr.n_type != NT_PRSTATUS)
+    return false;
   const Ebl_Core_Item *item;
   for (item = items; item < items + nitems; item++)
     if (strcmp (item->name, "pid") == 0)
diff --git a/libdwfl/linux-pid-attach.c b/libdwfl/linux-pid-attach.c
index f19e9b49..3a6af997 100644
--- a/libdwfl/linux-pid-attach.c
+++ b/libdwfl/linux-pid-attach.c
@@ -422,7 +422,11 @@ dwfl_linux_proc_attach (Dwfl *dwfl, pid_t pid, bool assume_ptrace_stopped)
 
   char name[64];
   int i = snprintf (name, sizeof (name), "/proc/%ld/task", (long) pid);
-  assert (i > 0 && i < (ssize_t) sizeof (name) - 1);
+  if (i <= 0 || i >= (ssize_t) sizeof (name) - 1)
+    {
+      errno = -ENOMEM;
+      goto fail;
+    }
   DIR *dir = opendir (name);
   if (dir == NULL)
     {
diff --git a/src/ChangeLog b/src/ChangeLog
index 0129b8bf..f65246f1 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,10 @@
+2020-07-05  Mark Wielaard  <mark@klomp.org>
+
+	* stack.c (module_callback): Don't assert if dwfl_module_info fails.
+	* unstrip.c (adjust_relocs): Produce a proper error when HAS
+	section has inconsistent size or entsize.
+	(match_module): Don't assert if dwfl_module_info fails.
+
 2020-06-16  Mark Wielaard  <mark@klomp.org>
 
 	* ar.c (do_oper_extract): Split large if statement. Call fchown
diff --git a/src/stack.c b/src/stack.c
index 4daabce7..2ec7c972 100644
--- a/src/stack.c
+++ b/src/stack.c
@@ -143,7 +143,12 @@ module_callback (Dwfl_Module *mod, void **userdata __attribute__((unused)),
   const char *debugfile;
   const char *modname = dwfl_module_info (mod, NULL, NULL, &end, NULL,
                                           NULL, &mainfile, &debugfile);
-  assert (strcmp (modname, name) == 0);
+  if (modname == NULL || strcmp (modname, name) != 0)
+    {
+      end = start + 1;
+      mainfile = NULL;
+      debugfile = NULL;
+    }
 
   int width = get_addr_width (mod);
   printf ("0x%0*" PRIx64 "-0x%0*" PRIx64 " %s\n",
diff --git a/src/unstrip.c b/src/unstrip.c
index 9b8c09a1..a855038a 100644
--- a/src/unstrip.c
+++ b/src/unstrip.c
@@ -500,7 +500,8 @@ adjust_relocs (Elf_Scn *outscn, Elf_Scn *inscn, const GElf_Shdr *shdr,
 	  error (EXIT_FAILURE, 0, "Symbol table cannot have zero sh_entsize");
 	const size_t nsym = symshdr->sh_size / symshdr->sh_entsize;
 	const size_t onent = shdr->sh_size / shdr->sh_entsize;
-	assert (data->d_size == shdr->sh_size);
+	if (data->d_size != shdr->sh_size)
+	  error (EXIT_FAILURE, 0, "HASH section has inconsistent size");
 
 #define CONVERT_HASH(Hash_Word)						      \
 	{								      \
@@ -509,7 +510,8 @@ adjust_relocs (Elf_Scn *outscn, Elf_Scn *inscn, const GElf_Shdr *shdr,
 	  const size_t nchain = old_hash[1];				      \
 	  const Hash_Word *const old_bucket = &old_hash[2];		      \
 	  const Hash_Word *const old_chain = &old_bucket[nbucket];	      \
-	  assert (onent == 2 + nbucket + nchain);			      \
+	  if (onent != 2 + nbucket + nchain)				      \
+	    error (EXIT_FAILURE, 0, "HASH section has inconsistent entsize"); \
 									      \
 	  const size_t nent = 2 + nbucket + nsym;			      \
 	  Hash_Word *const new_hash = xcalloc (nent, sizeof new_hash[0]);     \
@@ -2469,8 +2471,7 @@ match_module (Dwfl_Module *mod,
       const char *file;
       const char *check = dwfl_module_info (mod, NULL, NULL, NULL,
 					    NULL, NULL, &file, NULL);
-      assert (check == name);
-      if (file == NULL)
+      if (check == NULL || strcmp (check, name) != 0 || file == NULL)
 	return DWARF_CB_OK;
 
       name = file;
-- 
2.18.4

Re: [PATCH] libdwfl, src: Replace some asserts with proper check or error messages.

[Mark Wielaard]

On Sun, Jul 05, 2020 at 12:33:38AM +0200, Mark Wielaard wrote:
> library code really shouldn't assert and for utilities a proper
> error message is better than crashing with an assert.

Pushed to master.