From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 07BEE22F34 for ; Wed, 28 Feb 2024 21:37:25 +0100 (CET) Received: (qmail 5214 invoked by uid 550); 28 Feb 2024 20:33:45 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 5173 invoked from network); 28 Feb 2024 20:33:44 -0000 Date: Wed, 28 Feb 2024 15:37:29 -0500 From: Rich Felker To: Max Filippov Cc: musl@lists.openwall.com Message-ID: <20240228203729.GS4163@brightrain.aerifal.cx> References: <20240227232430.GM4163@brightrain.aerifal.cx> <20240228001303.GN4163@brightrain.aerifal.cx> <20240228183032.GO4163@brightrain.aerifal.cx> <20240228183700.GP4163@brightrain.aerifal.cx> <20240228201412.GQ4163@brightrain.aerifal.cx> <20240228202644.GR4163@brightrain.aerifal.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240228202644.GR4163@brightrain.aerifal.cx> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] Initial xtensa/fdpic port review On Wed, Feb 28, 2024 at 03:26:45PM -0500, Rich Felker wrote: > On Wed, Feb 28, 2024 at 03:14:12PM -0500, Rich Felker wrote: > > On Wed, Feb 28, 2024 at 11:41:30AM -0800, Max Filippov wrote: > > > On Wed, Feb 28, 2024 at 11:34 AM Max Filippov wrote: > > > > > > > > On Wed, Feb 28, 2024 at 10:36 AM Rich Felker wrote: > > > > > On Wed, Feb 28, 2024 at 01:30:32PM -0500, Rich Felker wrote: > > > > > > On Wed, Feb 28, 2024 at 09:20:33AM -0800, Max Filippov wrote: > > > > > > > > > > > > > > > > p->relocated = 1; > > > > > > > > > } > > > > > > > > > @@ -1485,7 +1487,7 @@ void __libc_exit_fini() > > > > > > > > > if (dyn[0] & (1< > > > > > > > > size_t n = dyn[DT_FINI_ARRAYSZ]/sizeof(size_t); > > > > > > > > > size_t *fn = (size_t *)laddr(p, dyn[DT_FINI_ARRAY])+n; > > > > > > > > > - while (n--) ((void (*)(void))*--fn)(); > > > > > > > > > + while (n--) fpaddr(p, *--fn)(); > > > > > > > > > > > > > > > > If this is fixable on the tooling side it really should be fixed > > > > > > > > there. init/fini arrays should have actual language-level function > > > > > > > > addresses (descriptor addresses on fdpic), not instruction addresses. > > > > > > > > > > > > > > I read libgcc code at > > > > > > > https://github.com/jcmvbkbc/gcc-xtensa/blob/xtensa-14-8789-fdpic/libgcc/crtstuff.c#L498-L503 > > > > > > > and the way it's written suggests that this was done on purpose. > > > > > > > I put it into the WIP pile to figure out later what the purpose was.. > > > > > > > I thought that SH might not have this issue because it just didn't > > > > > > > use the .array_init/.array_fini. > > > > > > > > > > > > I'm pretty sure we're using it -- musl-cross-make always forces it on > > > > > > via the gcc configure command line -- but it's possible there's some > > > > > > override disabling it for sh. I'll try some test cases and confirm > > > > > > whether sh is doing it right. Maybe the arm folks will have input on > > > > > > this too..? > > > > > > > > > > Confirmed both that it works, and that it's working via init_array. > > > > > GCC emits: > > > > > > > > > > .section .init_array,"aw" > > > > > .align 2 > > > > > .long foo@FUNCDESC > > > > > > > > > > for > > > > > > > > > > __attribute__((__constructor__)) > > > > > void foo() { ... } > > > > > > > > > > > > > Oh, no doubt that that C code generates a function descriptor, it > > > > works for xtensa too. But the piece of libgcc quoted above specifically > > > > puts a pointer to an object, not to a function into the .init_array. > > > > > > It was introduced to gcc by the ARM FDPIC series: > > > https://github.com/jcmvbkbc/gcc-xtensa/commit/11189793b6ef60645d5d1126d0bd9d0dd83e6583 > > > > > > This is the second change that I find made by the ARM FDPIC > > > series that appears to be not right for other FDPIC ports, first > > > being this change to the C++ unwinding code: > > > https://github.com/jcmvbkbc/gcc-xtensa/commit/67b0605494f32811364e25328d3522467aaf0638 > > > > OK, so the arm folks put explicitly wrong/broken code here. That needs > > to be reverted, and they can work out the mess they created on glibc. > > > > There is probably wrong arm target code too whereby gcc is generating > > instruction addresses for __attribute__((__constructor__)) rather than > > function addresses. > > > > If they have compat to worry about with glibc binaries, that's going > > to be a mess for them to fix, but we can just patch it out for musl > > target regardless of what they do since we have no existing broken > > binaries. > > OK, really good news! They didn't actually botch it. This test on > godbolt shows GCC is doing the right thing: > > https://godbolt.org/z/b53ExYoPf > > (GCC 13.1.0 ARM, -O2 -mfdpic, following code) > > #include > __attribute__((__constructor__)) > void foo() > { > printf("hello "); > } > > int main() > { > printf("world\n"); > } > > Rather, someone just made the crtstuff gratuitously do the wrong > thing, then later hard-coded disabling initfiniarray on fdpic because > it was doing the wrong thing: > > https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9c560cf23996271ee26dfc4a1d8484b85173cd12;hp=6bcbf80c6e2bd8a60d88bbcac3d70ffb67f4888f > > So all that's needed is to revert the wrong patch to crtstuff and then > xtensa and arm initfiniarray will work as they should. > > Anyone want to open a GCC bug tracker item for this? Filed as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114158