From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by sourceware.org (Postfix) with ESMTPS id CF3353858D28 for ; Fri, 19 Aug 2022 15:31:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CF3353858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=seketeli.org Received: (Authenticated sender: dodji@seketeli.org) by mail.gandi.net (Postfix) with ESMTPSA id 95EA01C000A; Fri, 19 Aug 2022 15:31:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=seketeli.org; s=gm1; t=1660923061; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xME/NtE1XGXVWrX/LgkEW39zRhM3xrgopHsZwjfUedo=; b=ZL7X9qyfJkkQas0RdNQZ8XDowv/t3PKp0pP7lPqkFE9zVhbrL35EqT7eiy4wQ9sD8ZFYcd YeMtl6sj77MvJDV4otbXKE7CEUYnfPrOrs05H0ehhM3gW+09cR7LuxEY/4PAG1m7dewWbz kZ3AD2aPSvx/fx72wfhLcsgDKBltm/xQvBVFySlvcoY1nEtVJogUPmj8uxlZI2CkPbl1xO VqK0yBRQWjqTfL1i25N2g5jQCh8RlcP8kCrpuGuPYzC0zrQC2hwFut68M1H+RPjFwejfcp RDgM8aU+CS6eJ3LzDNJc0XohdREV0uyL2mGbYWUHpP/Sx/LZuIfvynVXQ4WxbQ== Received: by localhost (Postfix, from userid 1000) id DD5CF5802BD; Fri, 19 Aug 2022 17:30:59 +0200 (CEST) From: Dodji Seketeli To: Ben Woodard Cc: Giuliano Procida , libabigail@sourceware.org, Matthias =?utf-8?Q?M=C3=A4nnich?= Subject: Re: [PATCH 3/3] ir: Consider integral types of same kind and size as equivalent Organization: Me, myself and I References: <877d35ms6k.fsf@seketeli.org> <1022329F-61A4-4E74-B449-D582A929515F@redhat.com> X-Operating-System: Fedora 38 X-URL: http://www.seketeli.net/~dodji Date: Fri, 19 Aug 2022 17:30:59 +0200 In-Reply-To: <1022329F-61A4-4E74-B449-D582A929515F@redhat.com> (Ben Woodard's message of "Thu, 18 Aug 2022 10:52:27 -0700") Message-ID: <87y1vkl07g.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, JMQ_SPF_NEUTRAL, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Aug 2022 15:31:07 -0000 Hello, Ben Woodard a =C3=A9crit: > I do not believe applying the fundamental type folding only to self > comparison will meet our needs. The real reason that self comparison > is important to us is we need to throw away the binary and then save > the abixml. Then later on we can compare the abixml to a new build to > find out if something happened that broke ABI. Thus for our needs, we > will not be in a self comparison operation at the time when we come > across this in a non-testing environment. I believe that the ordering > of the TUs that make up the binary could make this problem appear, > thus I do not think it will serve our purposes. OK. > I know that C doesn=E2=80=99t have a ODR but I=E2=80=99m fairly certain t= hat code > along the lines of what is in btrfs-progs will have portability > problems between architectures. Therefore, I think that programs like > this should be fixed. I=E2=80=99m start a discussion with the btrfs guys = and > explain the problem. I can imagine a possible reason for this problem > in btrfs-progs though, it could be caused by the need to read a FS > created on one arch on a machine of another arch. I ran into that a > few times many years ago and worked with upstream to fix them. (Ia64 > era IIRC) > > However, the fact that libabigail catches this problem is far too > late. At the very least I=E2=80=99d say the compiler should warn about is= sues > like this. We should talk to the compiler guys and see if we can have > them create a warning about this situation. This is a well known problem in the compiler space. G++ now has the -Wno-odr warning to detect ODR violation and emit a warning. But that's for C++. For C however, I am not sure. > Dodji could you start a discussion with our compiler team on this and > I will weigh in on it. Yeah, I'll see what I can do. > As for libabigail, this problem is too subtle for a normal user to > understand given the current output and so we need to do > something. When reading a corpus of a binary when you come across a > case where two different typedefs from two different TUs have > different types but the ABI of the fundamental type is compatible, you > should emit a warning saying something like: =E2=80=9Ctwo typedefs types = from > different TUs have conflicting types. ABI comparisons cannot be > reliably done. file1.c: and file2.c:=E2=80=9C. So basic= ally > at the place where you currently fold the fundamental integral types > in this patch if they are ABI compatible, print a warning > instead. That will let me know what the problem is when I do a test > run and I will report it to the upstream package rather than you. I=E2=80= =99ll > try to fix the world and it won=E2=80=99t be your problem. Emitting a comment that makes sense would be nice, indeed. But I'll need to tackle this separately because it's not exactly a one liner. There can be a lot of types being in that case, just because they happen to use a type that exhibits the problem. We need to emit the comment just for the type that is at the root cause of the problem. So it takes a little bit of thinking, I think. Also, I might have a way to handle this in a way that enable us to represent those ODR-violating types regardless. I don't like how libabigail is fragile here. But I need to play with the idea a little. And I might be wrong. I think this topic needs some more hashing out. > Maybe have a big comment near the logic in the DWARF reader or where this= is detected that explains the decision for the next guy.=20 > > So in summary:=20 > 1) Revert or replace this patch and add warning to libabigail. That shoul= d satisfy both me and Giuliano.=20 OK, I have just done this. The patch is https://sourceware.org/pipermail/l= ibabigail/2022q3/004666.html. > 2) Start a discussion with the GCC folk about adding a warning. I=E2=80= =99ll follow up. > 3) I=E2=80=99ll use the GCC warning discussion to bring this up with upst= ream btrfs people and possibly suggest a patch to fix their stuff. As I said above, I need to think about this a little bit more. > 4) I=E2=80=99ll do the same with any other packages that have a similar p= roblem. Yeah. The problem though is that it needs very careful debugging to understand the problem. That is where your point about libabigail emitting a warning makes sense. Thank you for your handling this. [...] Cheers, --=20 Dodji