* libabigail 2.1 trunk testing where are we? @ 2022-07-29 18:28 Ben Woodard 2022-07-29 20:57 ` Mark Wielaard 0 siblings, 1 reply; 6+ messages in thread From: Ben Woodard @ 2022-07-29 18:28 UTC (permalink / raw) To: Ben Woodard via Libabigail Yesterday I completed a massive test of libabigail against all of Fedora 36. This is some of the most comprehensive testing ever done and so it of course uncovered many new problems. Both Dodji and I want to get 2.1 out the door and so we both agree that we should release it with bugs and then clean things up in subsequent z-stream releases like 2.1.1. After testing nearly 25000 packages in Fedora, we currently have: 42 self-compare bugs - https://sourceware.org/bugzilla/show_bug.cgi?id=29413 <https://sourceware.org/bugzilla/show_bug.cgi?id=29413> These seem to fall into 4-5 groups. Dodji is working on a fix that he hopes will address at least a few of these. https://sourceware.org/bugzilla/show_bug.cgi?id=29299 <https://sourceware.org/bugzilla/show_bug.cgi?id=29299> 7 unique asserts - two of which are probably the same but hit on different lines. https://sourceware.org/bugzilla/show_bug.cgi?id=29412 <https://sourceware.org/bugzilla/show_bug.cgi?id=29412> 1 crash caused by an infinite loop. This appears in about a dozen packages https://sourceware.org/bugzilla/show_bug.cgi?id=29347 <https://sourceware.org/bugzilla/show_bug.cgi?id=29347> 1 crash due to incorrect ELF in that shows up in a small number of packages https://sourceware.org/bugzilla/show_bug.cgi?id=29346 <https://sourceware.org/bugzilla/show_bug.cgi?id=29346> At lest one performance problem with C++ codes, this inhibits testing of about 170 packages. https://sourceware.org/bugzilla/show_bug.cgi?id=29303 <https://sourceware.org/bugzilla/show_bug.cgi?id=29303> My plans: 2.1.x continue monitoring for regressions as Dodji grinds away at those bugs listed above. 2.2 and after Start folding in changes to the APIs to make them more generally usable i.e. suitable to my other work. Add deep library inspection recursively compare libraries in abicompat <https://sourceware.org/bugzilla/show_bug.cgi?id=27514> Add quick terminating binary result comparisons - is this library compatible? terminate as soon as you find a problem Add weak symbol replacement abicompat doesn't consider weak symbol replacement <https://sourceware.org/bugzilla/show_bug.cgi?id=29013> Add support for underlinked symbols abicompat doesn't consider functions (or variables) whose symbol type is NOTYPE <https://sourceware.org/bugzilla/show_bug.cgi?id=29008> Check exceptions for type compatibility exceptions are not checked for ABI compatibility. <https://sourceware.org/bugzilla/show_bug.cgi?id=28025> continue work at supressing DWARF idioms that prevent comparison between toolchaings. Add support for user-supplied koji config to fedabipkgdiff. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: libabigail 2.1 trunk testing where are we? 2022-07-29 18:28 libabigail 2.1 trunk testing where are we? Ben Woodard @ 2022-07-29 20:57 ` Mark Wielaard 2022-07-29 22:48 ` Ben Woodard 2022-09-20 8:47 ` Dodji Seketeli 0 siblings, 2 replies; 6+ messages in thread From: Mark Wielaard @ 2022-07-29 20:57 UTC (permalink / raw) To: Ben Woodard; +Cc: Ben Woodard via Libabigail [-- Attachment #1: Type: text/plain, Size: 440 bytes --] On Fri, Jul 29, 2022 at 11:28:18AM -0700, Ben Woodard via Libabigail wrote: > 1 crash due to incorrect ELF in that shows up in a small number of > packages https://sourceware.org/bugzilla/show_bug.cgi?id=29346 I have a patch that should work around that on: https://code.wildebeest.org/git/user/mjw/libabigail/commit/?h=pr29346 Also attached. Maybe someone with commit access could push it to a users try branch for testing? Thanks, Mark [-- Attachment #2: 0001-Handle-zero-sh_entsize-in-get_soname_of_elf_file.patch --] [-- Type: text/x-diff, Size: 1651 bytes --] From 05c53fcbdbccd8b08dfabd22caa9c7b4625596e8 Mon Sep 17 00:00:00 2001 From: Mark Wielaard <mark@klomp.org> Date: Wed, 20 Jul 2022 01:01:14 +0200 Subject: [PATCH] Handle zero sh_entsize in get_soname_of_elf_file Apparently guile produced ELF files don't set sh_entsize for the dynamic section. Which would cause a divide by zero. Luckily we do know how big an dynamic entry should be. So use gelf_fsize for ELF_T_DYN if sh_entsize is zero. * src/abg-dwarf-reader.cc (get_soname_of_elf_file): Make sure entsize is non-zero before use. https://sourceware.org/bugzilla/show_bug.cgi?id=29346 Signed-off-by: Mark Wielaard <mark@klomp.org> --- src/abg-dwarf-reader.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index e5159c89..288a56b8 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -16629,8 +16629,11 @@ get_soname_of_elf_file(const string& path, string &soname) Elf_Scn* scn = gelf_offscn (elf, phdr->p_offset); GElf_Shdr shdr_mem; GElf_Shdr* shdr = gelf_getshdr (scn, &shdr_mem); + size_t entsize = (shdr != NULL && shdr->sh_entsize != 0 + ? shdr->sh_entsize + : gelf_fsize (elf, ELF_T_DYN, 1, EV_CURRENT)); int maxcnt = (shdr != NULL - ? shdr->sh_size / shdr->sh_entsize : INT_MAX); + ? shdr->sh_size / entsize : INT_MAX); ABG_ASSERT (shdr == NULL || shdr->sh_type == SHT_DYNAMIC); Elf_Data* data = elf_getdata (scn, NULL); if (data == NULL) -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: libabigail 2.1 trunk testing where are we? 2022-07-29 20:57 ` Mark Wielaard @ 2022-07-29 22:48 ` Ben Woodard 2022-09-20 8:47 ` Dodji Seketeli 1 sibling, 0 replies; 6+ messages in thread From: Ben Woodard @ 2022-07-29 22:48 UTC (permalink / raw) To: Mark Wielaard; +Cc: Ben Woodard via Libabigail Thanks for the patch. I put it into my personal develop tree and it relatively immediately fixed 4/6 of the packages that I had identified as having that problem. The tests on the other two packages are still running and have been running for over half an hour and so something is working. We will see if they complete before they timeout. I may have to move those two to the takes to long to compete group https://sourceware.org/bugzilla/show_bug.cgi?id=29303 So this at least fixed: guile22 guile30 gnucash aisleriot I don’t yet have data on: gdb-headless sagemath -ben > On Jul 29, 2022, at 1:57 PM, Mark Wielaard <mark@klomp.org> wrote: > > On Fri, Jul 29, 2022 at 11:28:18AM -0700, Ben Woodard via Libabigail wrote: >> 1 crash due to incorrect ELF in that shows up in a small number of >> packages https://sourceware.org/bugzilla/show_bug.cgi?id=29346 > > I have a patch that should work around that on: > https://code.wildebeest.org/git/user/mjw/libabigail/commit/?h=pr29346 > Also attached. Maybe someone with commit access could push it to a > users try branch for testing? > > Thanks, > > Mark<0001-Handle-zero-sh_entsize-in-get_soname_of_elf_file.patch> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: libabigail 2.1 trunk testing where are we? 2022-07-29 20:57 ` Mark Wielaard 2022-07-29 22:48 ` Ben Woodard @ 2022-09-20 8:47 ` Dodji Seketeli 2022-09-20 15:05 ` Mark Wielaard 1 sibling, 1 reply; 6+ messages in thread From: Dodji Seketeli @ 2022-09-20 8:47 UTC (permalink / raw) To: Mark Wielaard; +Cc: Ben Woodard, Ben Woodard via Libabigail Hello fine fellows! Mark Wielaard <mark@klomp.org> a écrit: > On Fri, Jul 29, 2022 at 11:28:18AM -0700, Ben Woodard via Libabigail wrote: >> 1 crash due to incorrect ELF in that shows up in a small number of >> packages https://sourceware.org/bugzilla/show_bug.cgi?id=29346 > > I have a patch that should work around that on: > https://code.wildebeest.org/git/user/mjw/libabigail/commit/?h=pr29346 > Also attached. Maybe someone with commit access could push it to a > users try branch for testing? Man, thank you! [...] Ben Woodard via Libabigail <libabigail@sourceware.org> a écrit: > Thanks for the patch. > > I put it into my personal develop tree and it relatively immediately > fixed 4/6 of the packages that I had identified as having that > problem. The tests on the other two packages are still running and > have been running for over half an hour and so something is > working. We will see if they complete before they timeout. I may have > to move those two to the takes to long to compete group > https://sourceware.org/bugzilla/show_bug.cgi?id=29303 > > So this at least fixed: > guile22 > guile30 > gnucash > aisleriot Whoah! Thank you Ben! You guys rock. Okay, so I have just applied this to master. Mark, by the, way, just for my own education, would it have been ok to just use gelf_getshdr all the time, rather than using looking at the sh_entsize property of the section header that can be wrong sometimes? I am guessing the reason why you chose to keep looking at the later has to do with potential performance concerns? Anyway, either way, I am fine. Here is what got applied exactly: From f3b889a2cb94f8bb8372db14520d235dda7fdc3b Mon Sep 17 00:00:00 2001 From: Mark Wielaard <mark@klomp.org> Date: Wed, 20 Jul 2022 01:01:14 +0200 Subject: [PATCH] Handle zero sh_entsize in get_soname_of_elf_file Apparently guile produced ELF files don't set sh_entsize for the dynamic section. Which would cause a divide by zero. Luckily we do know how big an dynamic entry should be. So use gelf_fsize for ELF_T_DYN if sh_entsize is zero. * src/abg-dwarf-reader.cc (get_soname_of_elf_file): Make sure entsize is non-zero before use. https://sourceware.org/bugzilla/show_bug.cgi?id=29346 Signed-off-by: Mark Wielaard <mark@klomp.org> Signed-off-by: Dodji Seketeli <dodji@redhat.com> --- src/abg-dwarf-reader.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index 56909540..695683ed 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -16425,8 +16425,11 @@ get_soname_of_elf_file(const string& path, string &soname) Elf_Scn* scn = gelf_offscn (elf, phdr->p_offset); GElf_Shdr shdr_mem; GElf_Shdr* shdr = gelf_getshdr (scn, &shdr_mem); + size_t entsize = (shdr != NULL && shdr->sh_entsize != 0 + ? shdr->sh_entsize + : gelf_fsize (elf, ELF_T_DYN, 1, EV_CURRENT)); int maxcnt = (shdr != NULL - ? shdr->sh_size / shdr->sh_entsize : INT_MAX); + ? shdr->sh_size / entsize : INT_MAX); ABG_ASSERT (shdr == NULL || shdr->sh_type == SHT_DYNAMIC); Elf_Data* data = elf_getdata (scn, NULL); if (data == NULL) -- 2.37.2 [...] Cheers, -- Dodji ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: libabigail 2.1 trunk testing where are we? 2022-09-20 8:47 ` Dodji Seketeli @ 2022-09-20 15:05 ` Mark Wielaard 2022-09-25 7:06 ` Dodji Seketeli 0 siblings, 1 reply; 6+ messages in thread From: Mark Wielaard @ 2022-09-20 15:05 UTC (permalink / raw) To: Dodji Seketeli; +Cc: Ben Woodard, Ben Woodard via Libabigail Hi Dodji, On Tue, 2022-09-20 at 10:47 +0200, Dodji Seketeli wrote: > Okay, so I have just applied this to master. Thanks! > Mark, by the, way, just for my own education, would it have been ok to > just use gelf_getshdr all the time, rather than using looking at the > sh_entsize property of the section header that can be wrong sometimes? > > I am guessing the reason why you chose to keep looking at the later has > to do with potential performance concerns? I am sure I fully understand your question. Do you mean if we could have used gelf_fsize (elf, ELF_T_DYN, 1, EV_CURRENT) all the time? Yes, we could have simply done that. I didn't do that in the patch because that would have changed the existing code more. But maybe I should have done that and ignored the shdr completely to simplify things. Cheers, Mark ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: libabigail 2.1 trunk testing where are we? 2022-09-20 15:05 ` Mark Wielaard @ 2022-09-25 7:06 ` Dodji Seketeli 0 siblings, 0 replies; 6+ messages in thread From: Dodji Seketeli @ 2022-09-25 7:06 UTC (permalink / raw) To: Mark Wielaard; +Cc: Ben Woodard, Ben Woodard via Libabigail Hey Mark, Mark Wielaard <mark@klomp.org> a écrit: [...] >> I am guessing the reason why you chose to keep looking at the later has >> to do with potential performance concerns? > > I am sure I fully understand your question. > > Do you mean if we could have used > gelf_fsize (elf, ELF_T_DYN, 1, EV_CURRENT) > all the time? Yes. > Yes, we could have simply done that. I didn't do that in the patch > because that would have changed the existing code more. But maybe I > should have done that and ignored the shdr completely to simplify > things. OK, that makes sense to me, thanks. Cheers, -- Dodji ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-25 7:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-29 18:28 libabigail 2.1 trunk testing where are we? Ben Woodard 2022-07-29 20:57 ` Mark Wielaard 2022-07-29 22:48 ` Ben Woodard 2022-09-20 8:47 ` Dodji Seketeli 2022-09-20 15:05 ` Mark Wielaard 2022-09-25 7:06 ` Dodji Seketeli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).