powerpc64: only sync icache if we relocated in executable segment

[kib]

Summary

and only after all relocations for the segment are processed.

[kib]

Not even compiled.

[adrian]

ok, I cleaned this up and have tested it in a chroot constantly doing buildkernel. It didn't trigger any make failures so far.

https://people.freebsd.org/~adrian/powerpc64/20260125-D54246-fixup-1.diff

[kib]

Fixes by adrian.

[adrian]

libexec/rtld-elf/powerpc64/reloc.c

415–416

we dont need this loop anymore, no?

[kib]

Remove nop loop.

[adrian]

libexec/rtld-elf/powerpc64/reloc.c

352

oops I didn't convert this from c-- to c++, please update

[kib]

c++

[adrian]

This works for me on freebsd-16 on power8, I'm doing test buildworld -j32 on it right now!

[jrtc27]

In D54246#1254603, @adrian wrote:

This works for me on freebsd-16 on power8, I'm doing test buildworld -j32 on it right now!

But does the isync code ever get hit? This doesn't make sense to me. It's a whole bunch of complexity for something that we are missing a crucial part of to actually be useful, namely handling the relocations actually used for instructions rather than data.

[jrtc27]

libexec/rtld-elf/powerpc64/reloc.c

335–336

I believe this should never end up being printed. Please test that? If it does, I would be extremely interested in what the object in question is.

[adrian]

ok, but on insistence from jrtc27 i added some debug logging here and it's never actually invoking syncicache in my tests.

ivy, what were you running make on? just buildworld/buildkernel or something else?

[kib]

In D54246#1254643, @adrian wrote:

ok, but on insistence from jrtc27 i added some debug logging here and it's never actually invoking syncicache in my tests.

This does not mean much. It only shows that the current binaries do not have this issue. But old binaries might have, then removing the hack breaks them.

This is really about Tier 2 arch, where the project officially does not care about ABI and ability to run all existing binaries.

[jrtc27]

In D54246#1254644, @kib wrote:

In D54246#1254643, @adrian wrote:

ok, but on insistence from jrtc27 i added some debug logging here and it's never actually invoking syncicache in my tests.

This does not mean much. It only shows that the current binaries do not have this issue. But old binaries might have, then removing the hack breaks them.

Please, read what I've said many times. There is no world in which flushing the icache is helpful for the set of dynamic relocations that rtld supports, and none of them are relocations that glibc will ever flush the icache for. Therefore it is utterly pointless to have code to do so.

This is really about Tier 2 arch, where the project officially does not care about ABI and ability to run all existing binaries.

No, I would say the exact same thing if it were Tier 1. This is useless code that only serves to slow down program loading.

[adrian]

ok so I had a chat with jessica, and after some more digging to try and understand what's going on here, I see both views:

• jessica says we shouldn't /see/ instruction relocation because we don't have any of the relocated bits here
• kib says we should support the relocation bits in case someone finds/found a silly way to use it in the text segment.

I've tested building some ports and kernel/world, I haven't seen it fire once on an executable segment.

So, I don't necessarily like deleting it without having some runtime checks to know that someone's not doing anything daft.
So how about the worst of both worlds? This diff, but in the call to syncicache we actually put a _rtld_fdprintf(STDERR_FILENO, ...) and abort() call for -16.

That way we can catch if something happens that's unexpected hopefully by said error print/abort rather than weird occasional
stuff happening.