public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
* 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).