From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (wildebeest.demon.nl [212.238.236.112]) by sourceware.org (Postfix) with ESMTPS id 564D3386F816 for ; Sat, 4 Jul 2020 22:33:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 564D3386F816 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mark@klomp.org Received: from tarox.wildebeest.org (tarox.wildebeest.org [172.31.17.39]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 02D8630002CA; Sun, 5 Jul 2020 00:33:44 +0200 (CEST) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id B68364000C6A; Sun, 5 Jul 2020 00:33:44 +0200 (CEST) From: Mark Wielaard To: elfutils-devel@sourceware.org Cc: Mark Wielaard Subject: [PATCH] libdwfl, src: Replace some asserts with proper check or error messages. Date: Sun, 5 Jul 2020 00:33:38 +0200 Message-Id: <20200704223338.26805-1-mark@klomp.org> X-Mailer: git-send-email 2.18.4 X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 04 Jul 2020 22:33:48 -0000 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 --- 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 + + * 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 * 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) ®s_offset, &nregloc, ®locs, &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 + + * 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 * 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