>From 33d436aefb6b63a159943bd24eb432b8cfb8b369 Mon Sep 17 00:00:00 2001 From: Di Chen <dic...@redhat.com> Date: Sun, 24 Dec 2023 11:44:48 +0800 Subject: [PATCH] libdwfl: Remove asserts from library code
It would be better for elfutils library functions to return an error code instead of aborting because of a failed assert. This commit works on removing the asserts under libdwfl. There are two ways handling the removing: 1) Replace the asserts with if statements. 2) Replace the asserts with eu_static_assert. Asserts are heavily used across all elfutils libraries, and it's impossibe to implement the removing in one commit. So let's gradually remove the asserts in the later coming commits. https://sourceware.org/bugzilla/show_bug.cgi?id=31027 Signed-off-by: Di Chen <dic...@redhat.com> --- libdwfl/dwfl_frame.c | 14 +++++++++----- libdwfl/dwfl_frame_pc.c | 3 ++- libdwfl/dwfl_frame_regs.c | 7 +++++-- libdwfl/dwfl_module_build_id.c | 3 ++- libdwfl/dwfl_module_getdwarf.c | 3 ++- libdwfl/dwfl_module_getsrc_file.c | 3 ++- libdwfl/dwfl_module_register_names.c | 3 ++- libdwfl/dwfl_segment_report_module.c | 2 +- 8 files changed, 25 insertions(+), 13 deletions(-) diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c index 5ee71dd4..18150e2a 100644 --- a/libdwfl/dwfl_frame.c +++ b/libdwfl/dwfl_frame.c @@ -85,12 +85,14 @@ free_states (Dwfl_Frame *state) static Dwfl_Frame * state_alloc (Dwfl_Thread *thread) { - assert (thread->unwound == NULL); + if (thread->unwound != NULL) + return NULL; Ebl *ebl = thread->process->ebl; size_t nregs = ebl_frame_nregs (ebl); if (nregs == 0) return NULL; - assert (nregs < sizeof (((Dwfl_Frame *) NULL)->regs_set) * 8); + if (nregs >= sizeof (((Dwfl_Frame *) NULL)->regs_set) * 8) + return NULL; Dwfl_Frame *state = malloc (sizeof (*state) + sizeof (*state->regs) * nregs); if (state == NULL) return NULL; @@ -283,8 +285,9 @@ dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg), } int err = callback (&thread, arg); if (err != DWARF_CB_OK) - return err; - assert (thread.unwound == NULL); + return err; + if (thread.unwound != NULL) + return -1; } /* NOTREACHED */ } @@ -450,7 +453,8 @@ dwfl_thread_getframes (Dwfl_Thread *thread, __libdwfl_seterrno (err); return -1; } - assert (state->pc_state == DWFL_FRAME_STATE_PC_UNDEFINED); + if (state->pc_state != DWFL_FRAME_STATE_PC_UNDEFINED) + return -1; free_states (state); return 0; } diff --git a/libdwfl/dwfl_frame_pc.c b/libdwfl/dwfl_frame_pc.c index 296c815b..b9991e9a 100644 --- a/libdwfl/dwfl_frame_pc.c +++ b/libdwfl/dwfl_frame_pc.c @@ -35,7 +35,8 @@ bool dwfl_frame_pc (Dwfl_Frame *state, Dwarf_Addr *pc, bool *isactivation) { - assert (state->pc_state == DWFL_FRAME_STATE_PC_SET); + if (state->pc_state != DWFL_FRAME_STATE_PC_SET) + return false; *pc = state->pc; ebl_normalize_pc (state->thread->process->ebl, pc); if (isactivation) diff --git a/libdwfl/dwfl_frame_regs.c b/libdwfl/dwfl_frame_regs.c index a4bd3884..064f5e8a 100644 --- a/libdwfl/dwfl_frame_regs.c +++ b/libdwfl/dwfl_frame_regs.c @@ -37,8 +37,11 @@ dwfl_thread_state_registers (Dwfl_Thread *thread, int firstreg, unsigned nregs, const Dwarf_Word *regs) { Dwfl_Frame *state = thread->unwound; - assert (state && state->unwound == NULL); - assert (state->initial_frame); + if (state == NULL || state->unwound != NULL || !state->initial_frame) + { + __libdwfl_seterrno (DWFL_E_INVALID_REGISTER); + return false; + } for (unsigned regno = firstreg; regno < firstreg + nregs; regno++) if (! __libdwfl_frame_reg_set (state, regno, regs[regno - firstreg])) { diff --git a/libdwfl/dwfl_module_build_id.c b/libdwfl/dwfl_module_build_id.c index 0c198f23..4dcc046e 100644 --- a/libdwfl/dwfl_module_build_id.c +++ b/libdwfl/dwfl_module_build_id.c @@ -65,7 +65,8 @@ __libdwfl_find_build_id (Dwfl_Module *mod, bool set, Elf *elf) int build_id_len; /* For mod == NULL use dwelf_elf_gnu_build_id directly. */ - assert (mod != NULL); + if (mod == NULL) + return -1; int result = __libdwfl_find_elf_build_id (mod, elf, &build_id_bits, &build_id_elfaddr, &build_id_len); diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c index 6f98c02b..596de51e 100644 --- a/libdwfl/dwfl_module_getdwarf.c +++ b/libdwfl/dwfl_module_getdwarf.c @@ -161,7 +161,8 @@ open_elf (Dwfl_Module *mod, struct dwfl_file *file) mod->e_type = ET_DYN; } else - assert (mod->main.elf != NULL); + if (mod->main.elf == NULL) + return DWFL_E (LIBELF, elf_errno ()); return DWFL_E_NOERROR; } diff --git a/libdwfl/dwfl_module_getsrc_file.c b/libdwfl/dwfl_module_getsrc_file.c index fc144225..4456dc58 100644 --- a/libdwfl/dwfl_module_getsrc_file.c +++ b/libdwfl/dwfl_module_getsrc_file.c @@ -165,7 +165,8 @@ dwfl_module_getsrc_file (Dwfl_Module *mod, if (cur_match > 0) { - assert (*nsrcs == 0 || *srcsp == match); + if (*nsrcs != 0 && *srcsp != match) + return -1; *nsrcs = cur_match; *srcsp = match; diff --git a/libdwfl/dwfl_module_register_names.c b/libdwfl/dwfl_module_register_names.c index 9ea09371..6fb4195b 100644 --- a/libdwfl/dwfl_module_register_names.c +++ b/libdwfl/dwfl_module_register_names.c @@ -73,7 +73,8 @@ dwfl_module_register_names (Dwfl_Module *mod, } if (likely (len > 0)) { - assert (len > 1); /* Backend should never yield "". */ + if (len <= 1) /* Backend should never yield "". */ + return -1; result = (*func) (arg, regno, setname, prefix, name, bits, type); } } diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c index dc34e0ae..39e27e22 100644 --- a/libdwfl/dwfl_segment_report_module.c +++ b/libdwfl/dwfl_segment_report_module.c @@ -530,7 +530,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name, if (filesz > SIZE_MAX / sizeof (Elf32_Nhdr)) continue; - assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr)); + eu_static_assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr)); void *notes; if (ei_data == MY_ELFDATA -- 2.41.0
From 33d436aefb6b63a159943bd24eb432b8cfb8b369 Mon Sep 17 00:00:00 2001 From: Di Chen <dic...@redhat.com> Date: Sun, 24 Dec 2023 11:44:48 +0800 Subject: [PATCH] libdwfl: Remove asserts from library code It would be better for elfutils library functions to return an error code instead of aborting because of a failed assert. This commit works on removing the asserts under libdwfl. There are two ways handling the removing: 1) Replace the asserts with if statements. 2) Replace the asserts with eu_static_assert. Asserts are heavily used across all elfutils libraries, and it's impossibe to implement the removing in one commit. So let's gradually remove the asserts in the later coming commits. https://sourceware.org/bugzilla/show_bug.cgi?id=31027 Signed-off-by: Di Chen <dic...@redhat.com> --- libdwfl/dwfl_frame.c | 14 +++++++++----- libdwfl/dwfl_frame_pc.c | 3 ++- libdwfl/dwfl_frame_regs.c | 7 +++++-- libdwfl/dwfl_module_build_id.c | 3 ++- libdwfl/dwfl_module_getdwarf.c | 3 ++- libdwfl/dwfl_module_getsrc_file.c | 3 ++- libdwfl/dwfl_module_register_names.c | 3 ++- libdwfl/dwfl_segment_report_module.c | 2 +- 8 files changed, 25 insertions(+), 13 deletions(-) diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c index 5ee71dd4..18150e2a 100644 --- a/libdwfl/dwfl_frame.c +++ b/libdwfl/dwfl_frame.c @@ -85,12 +85,14 @@ free_states (Dwfl_Frame *state) static Dwfl_Frame * state_alloc (Dwfl_Thread *thread) { - assert (thread->unwound == NULL); + if (thread->unwound != NULL) + return NULL; Ebl *ebl = thread->process->ebl; size_t nregs = ebl_frame_nregs (ebl); if (nregs == 0) return NULL; - assert (nregs < sizeof (((Dwfl_Frame *) NULL)->regs_set) * 8); + if (nregs >= sizeof (((Dwfl_Frame *) NULL)->regs_set) * 8) + return NULL; Dwfl_Frame *state = malloc (sizeof (*state) + sizeof (*state->regs) * nregs); if (state == NULL) return NULL; @@ -283,8 +285,9 @@ dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg), } int err = callback (&thread, arg); if (err != DWARF_CB_OK) - return err; - assert (thread.unwound == NULL); + return err; + if (thread.unwound != NULL) + return -1; } /* NOTREACHED */ } @@ -450,7 +453,8 @@ dwfl_thread_getframes (Dwfl_Thread *thread, __libdwfl_seterrno (err); return -1; } - assert (state->pc_state == DWFL_FRAME_STATE_PC_UNDEFINED); + if (state->pc_state != DWFL_FRAME_STATE_PC_UNDEFINED) + return -1; free_states (state); return 0; } diff --git a/libdwfl/dwfl_frame_pc.c b/libdwfl/dwfl_frame_pc.c index 296c815b..b9991e9a 100644 --- a/libdwfl/dwfl_frame_pc.c +++ b/libdwfl/dwfl_frame_pc.c @@ -35,7 +35,8 @@ bool dwfl_frame_pc (Dwfl_Frame *state, Dwarf_Addr *pc, bool *isactivation) { - assert (state->pc_state == DWFL_FRAME_STATE_PC_SET); + if (state->pc_state != DWFL_FRAME_STATE_PC_SET) + return false; *pc = state->pc; ebl_normalize_pc (state->thread->process->ebl, pc); if (isactivation) diff --git a/libdwfl/dwfl_frame_regs.c b/libdwfl/dwfl_frame_regs.c index a4bd3884..064f5e8a 100644 --- a/libdwfl/dwfl_frame_regs.c +++ b/libdwfl/dwfl_frame_regs.c @@ -37,8 +37,11 @@ dwfl_thread_state_registers (Dwfl_Thread *thread, int firstreg, unsigned nregs, const Dwarf_Word *regs) { Dwfl_Frame *state = thread->unwound; - assert (state && state->unwound == NULL); - assert (state->initial_frame); + if (state == NULL || state->unwound != NULL || !state->initial_frame) + { + __libdwfl_seterrno (DWFL_E_INVALID_REGISTER); + return false; + } for (unsigned regno = firstreg; regno < firstreg + nregs; regno++) if (! __libdwfl_frame_reg_set (state, regno, regs[regno - firstreg])) { diff --git a/libdwfl/dwfl_module_build_id.c b/libdwfl/dwfl_module_build_id.c index 0c198f23..4dcc046e 100644 --- a/libdwfl/dwfl_module_build_id.c +++ b/libdwfl/dwfl_module_build_id.c @@ -65,7 +65,8 @@ __libdwfl_find_build_id (Dwfl_Module *mod, bool set, Elf *elf) int build_id_len; /* For mod == NULL use dwelf_elf_gnu_build_id directly. */ - assert (mod != NULL); + if (mod == NULL) + return -1; int result = __libdwfl_find_elf_build_id (mod, elf, &build_id_bits, &build_id_elfaddr, &build_id_len); diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c index 6f98c02b..596de51e 100644 --- a/libdwfl/dwfl_module_getdwarf.c +++ b/libdwfl/dwfl_module_getdwarf.c @@ -161,7 +161,8 @@ open_elf (Dwfl_Module *mod, struct dwfl_file *file) mod->e_type = ET_DYN; } else - assert (mod->main.elf != NULL); + if (mod->main.elf == NULL) + return DWFL_E (LIBELF, elf_errno ()); return DWFL_E_NOERROR; } diff --git a/libdwfl/dwfl_module_getsrc_file.c b/libdwfl/dwfl_module_getsrc_file.c index fc144225..4456dc58 100644 --- a/libdwfl/dwfl_module_getsrc_file.c +++ b/libdwfl/dwfl_module_getsrc_file.c @@ -165,7 +165,8 @@ dwfl_module_getsrc_file (Dwfl_Module *mod, if (cur_match > 0) { - assert (*nsrcs == 0 || *srcsp == match); + if (*nsrcs != 0 && *srcsp != match) + return -1; *nsrcs = cur_match; *srcsp = match; diff --git a/libdwfl/dwfl_module_register_names.c b/libdwfl/dwfl_module_register_names.c index 9ea09371..6fb4195b 100644 --- a/libdwfl/dwfl_module_register_names.c +++ b/libdwfl/dwfl_module_register_names.c @@ -73,7 +73,8 @@ dwfl_module_register_names (Dwfl_Module *mod, } if (likely (len > 0)) { - assert (len > 1); /* Backend should never yield "". */ + if (len <= 1) /* Backend should never yield "". */ + return -1; result = (*func) (arg, regno, setname, prefix, name, bits, type); } } diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c index dc34e0ae..39e27e22 100644 --- a/libdwfl/dwfl_segment_report_module.c +++ b/libdwfl/dwfl_segment_report_module.c @@ -530,7 +530,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name, if (filesz > SIZE_MAX / sizeof (Elf32_Nhdr)) continue; - assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr)); + eu_static_assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr)); void *notes; if (ei_data == MY_ELFDATA -- 2.41.0
