Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-devel / 2015-10

patch for rtl/linux/aarch64/cprt0.as

11 messages · 4 people · –

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Edmund Grimley Evans

Clearly I don't really know what I'm doing with these start-up files... but I found I needed this patch to build the Debian package on arm64, so it's probably something for the AArch64 fixes branch.

I tried running the test suite with and without this patch, and it appeared that webtbs/tw28089 went into an infinite loop without the patch and passed with it, but I've never seen that test fail before, and I don't see any connection with the patch, so I suspect there was some kind of random race and this patch does not really make any difference for the test suite.

By the way, the failures on powerpc and ppc64 look rather as if they might be start-up-file-related:

https://buildd.debian.org/status/package.php?p=fpc&suite=experimental

Edmund

--- a/rtl/linux/aarch64/cprt0.as
+++ b/rtl/linux/aarch64/cprt0.as
@@ -41,23 +41,23 @@ _start:
init, fini, rtld_fini, stack_end) */
adrp x0,:got:PASCALMAIN
ldr x0,[x0,#:got_lo12:PASCALMAIN]
- adrp x3,:got:__libc_csu_init
- ldr x3,[x3,#:got_lo12:__libc_csu_init]
- adrp x4,:got:__libc_csu_fini
- ldr x4,[x4,#:got_lo12:__libc_csu_fini]
+ adrp x3,:got:_init_dummy
+ ldr x3,[x3,#:got_lo12:_init_dummy]
+ adrp x4,:got:_fini_dummy
+ ldr x4,[x4,#:got_lo12:_fini_dummy]
bl __libc_start_main

/* This should never happen */
b abort

- .globl _init
- .type _init,#function
-_init:
+ .globl _init_dummy
+ .type _init_dummy,#function
+_init_dummy:
ret

- .globl _fini
- .type _fini,#function
-_fini:
+ .globl _fini_dummy
+ .type _fini_dummy,#function
+_fini_dummy:
ret

.globl _haltproc

Sergei Gorelkin
to Edmund Grimley Evans

06.10.2015 1:08, Edmund Grimley Evans пишет:

Show quoted text 54 lines
Clearly I don't really know what I'm doing with these start-up files... but I found I needed this patch to build the Debian package on arm64, so it's probably something for the AArch64 fixes branch. I tried running the test suite with and without this patch, and it appeared that webtbs/tw28089 went into an infinite loop without the patch and passed with it, but I've never seen that test fail before, and I don't see any connection with the patch, so I suspect there was some kind of random race and this patch does not really make any difference for the test suite. By the way, the failures on powerpc and ppc64 look rather as if they might be start-up-file-related: https://buildd.debian.org/status/package.php?p=fpc&suite=experimental Edmund --- a/rtl/linux/aarch64/cprt0.as +++ b/rtl/linux/aarch64/cprt0.as @@ -41,23 +41,23 @@ _start: init, fini, rtld_fini, stack_end) */ adrp x0,:got:PASCALMAIN ldr x0,[x0,#:got_lo12:PASCALMAIN] - adrp x3,:got:__libc_csu_init - ldr x3,[x3,#:got_lo12:__libc_csu_init] - adrp x4,:got:__libc_csu_fini - ldr x4,[x4,#:got_lo12:__libc_csu_fini] + adrp x3,:got:_init_dummy + ldr x3,[x3,#:got_lo12:_init_dummy] + adrp x4,:got:_fini_dummy + ldr x4,[x4,#:got_lo12:_fini_dummy] bl __libc_start_main /* This should never happen */ b abort - .globl _init - .type _init,#function -_init: + .globl _init_dummy + .type _init_dummy,#function +_init_dummy: ret - .globl _fini - .type _fini,#function -_fini: + .globl _fini_dummy + .type _fini_dummy,#function +_fini_dummy: ret .globl _haltproc

This is absolutely wrong thing to do. Replacing __libc_csu_init with a dummy routine effectively defeats the entire libc/ELF startup. While doing so can make simple fpc-only programs linked with libc appear as working, any more complex programs will experience very obscure and hard to debug errors as soon as they touch the stuff that is normally inability to find symbol "_init" during linking is typically caused by missing gcc (_init comes from crtbegin.o which belongs to gcc package, while another startup file crti.o belongs to libc package, or vice versa - I don't remember exactly), or by improperly installed fpc (it cannot find the crtbegin.o and other startup files).
I don't know exactly why FPC had chosen such dirty hack to hide this issue early in its history, but once it was used for one target, it became copy-pasted to many other targets as well. But stopping this practice is the good thing to do.

Regards,
Sergei

Jonas Maebe
to Edmund Grimley Evans: Clearly I don't really know what I'm doing with these start-up files... but I found I…

Edmund Grimley Evans wrote on Tue, 06 Oct 2015:

Clearly I don't really know what I'm doing with these start-up files... but I found I needed this patch to build the Debian package on arm64, so it's probably something for the AArch64 fixes branch.

As Sergei mentioned, this is definitely not the right approach. The difficulty with finding these files automatically is that they're often in non-standard locations. Normally, fpcmkcfg (utils/fpcmkcfg/fpcmkcfg.pp, executed to create /etc/fpc.cfg) will ask GCC for the path to these object files by extracting the path from the output of "gcc --print-libgcc-file-name". It will then prepend -Fl to that path and add the result to fpc.cfg

Now, for some reason, it seems that right now on Linux we only do this for x86 and PowerPC (32 and 64 bit). It seems there's an "else result := '-Fl'+GetDefaultLibGCCDir(CPU, OS, S);" missing for Linux. That would explain why you have those problems on AArch64, but not for PowerPC. Unless those machines don't have the libc-devel package installed, or no gcc when the system fpc was installed? (although that's hard to imagine)

Jonas

Edmund Grimley Evans
to Jonas Maebe

Jonas:

Now, for some reason, it seems that right now on Linux we only do this for x86 and PowerPC (32 and 64 bit).

So x86_64 does this the wrong way. So perhaps I should try to fix this on x86_64 (amd64) first because then there will be more people who can test any proposed patch. But it's a bit late for 3.0.0, isn't it?

Edmund

Jonas Maebe
to Edmund Grimley Evans

Edmund Grimley Evans wrote on Tue, 06 Oct 2015:

Show quoted text 6 lines
Jonas: > Now, for some reason, it seems that right now on Linux we only do this for > x86 and PowerPC (32 and 64 bit). So x86_64 does this the wrong way.

No, with "x86" I meant "i386 and x86_64". Sorry for not being clear.

But it's a bit late for 3.0.0, isn't it?

Due to Apple's Xcode 7 release breaking a bunch of things and some late found bugs in the resource string support, we will have an RC2 before the final release. In addition, problems related to the generation of the configuration file are generally only discovered while building a release, so fixed for that are definitely good candidates to still include.

I think the current plan is to tag RC2 tomorrow night, so if possible it would be good to have your proposed changes before then.

Jonas

Edmund Grimley Evans
to Jonas Maebe

Jonas:

> So x86_64 does this the wrong way. No, with "x86" I meant "i386 and x86_64". Sorry for not being clear.

What you wrote was ambiguous, but I interpreted it as just 32-bit because my aarch64 patch, above, makes rtl/linux/aarch64/cprt0.as more similar to rtl/linux/x86_64/cprt0.as. Isn't the x86_64 file doing exactly what Sergei says is wrong?

Edmund

Jonas Maebe
to Edmund Grimley Evans

Edmund Grimley Evans wrote on Tue, 06 Oct 2015:

Show quoted text 10 lines
Jonas: >> So x86_64 does this the wrong way. > No, with "x86" I meant "i386 and x86_64". Sorry for not being clear. What you wrote was ambiguous, but I interpreted it as just 32-bit because my aarch64 patch, above, makes rtl/linux/aarch64/cprt0.as more similar to rtl/linux/x86_64/cprt0.as. Isn't the x86_64 file doing exactly what Sergei says is wrong?

Oh, I thought you were referring to the problem regarding the inability to find the crt* files, possibly due to failing to locate their directory (although I've just realised that while building FPC itself, the code in packages/fpmkunit/src/fpmkunit.pp is called directly by fpmake since the global configuration file is ignored in that case, so that shouldn't matter).

Anyway, you're right, the startup code for Linux/x86-64 does not pass the libc init/fini routines and hence is broken. And that's indeed not something to still change right before the release.

Jonas

Edmund Grimley Evans
to Jonas Maebe

Jonas:

Anyway, you're right, the startup code for Linux/x86-64 does not pass the libc init/fini routines and hence is broken. And that's indeed not something to still change right before the release.

But perhaps you'll want to consider fpc-x86_64-cprt0.patch (attached) for the trunk. I can confirm that it doesn't break Debian's build. Perhaps someone else can test it in some other way.

And here's an improved patch for AArch64: fpc-aarch64-cprt0.patch. AArch64 seems to need a corresponding version of the change that was needed for i386, so I've included both changes here. (So if you've already applied the patch from Debian bug #800811 there will be a conflict.) Apart from the i386 part, this patch could perhaps go into fixes_3_0_ios as it clearly (I hope) only affects AArch64-Linux so can't break anything that used to work.

Edmund

Joost van der Sluis
to Jonas Maebe: Edmund Grimley Evans wrote on Tue, 06 Oct 2015: As Sergei mentioned, this is definitely…

Op 06-10-15 om 08:47 schreef Jonas Maebe:

Show quoted text 8 lines
As Sergei mentioned, this is definitely not the right approach. The difficulty with finding these files automatically is that they're often in non-standard locations. Normally, fpcmkcfg (utils/fpcmkcfg/fpcmkcfg.pp, executed to create /etc/fpc.cfg) will ask GCC for the path to these object files by extracting the path from the output of "gcc --print-libgcc-file-name". It will then prepend -Fl to that path and add the result to fpc.cfg

There has been a patch to fix the extraction of the libgcc-path in trunk very recently. There was a problem with non-English versions of gcc. (using set language)

Joost.

Jonas Maebe
to Edmund Grimley Evans: Jonas: But perhaps you'll want to consider fpc-x86_64-cprt0.patch (attached) for the…
Edmund Grimley Evans wrote
But perhaps you'll want to consider fpc-x86_64-cprt0.patch (attached) for the trunk. I can confirm that it doesn't break Debian's build. Perhaps someone else can test it in some other way.

I've run the testsuite and didn't see any regressions on RHEL 5.5 either.

And here's an improved patch for AArch64: fpc-aarch64-cprt0.patch. AArch64 seems to need a corresponding version of the change that was needed for i386, so I've included both changes here.

Thanks, all committed.

Show quoted text 5 lines
(So if you've already applied the patch from Debian bug #800811 there will be a conflict.) Apart from the i386 part, this patch could perhaps go into fixes_3_0_ios as it clearly (I hope) only affects AArch64-Linux so can't break anything that used to work.

Yes, I'll do that.

Jonas

Jonas Maebe
to Jonas Maebe

Jonas Maebe wrote on Thu, 08 Oct 2015:

Show quoted text 9 lines
On 06/10/15 20:50, Edmund Grimley Evans wrote: > (So if you've > already applied the patch from Debian bug #800811 there will be a > conflict.) Apart from the i386 part, this patch could perhaps go into > fixes_3_0_ios as it clearly (I hope) only affects AArch64-Linux so > can't break anything that used to work. Yes, I'll do that.

I merged it in the fixes_3_0_ios branch last weekend.

Jonas