Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.88k stars 531 forks source link

Perl on Solaris Sparc incorrectly adds -L/usr/lib/sparcv9 into ldflags and lddlflags #22246

Open vlmarek opened 1 month ago

vlmarek commented 1 month ago

Perl incorrectly adds /usr/lib/sparcv9 to list of library search paths. If you do you may not be able to override a library from that path with library of your own.

At the moment perl compiled on solaris sparc shows:

$ perl -V | grep usr/lib/sparcv9
    ldflags =' -m64 -fstack-protector-strong -L/usr/lib/sparcv9 -L/usr/gnu/lib '
    libpth=/lib/64 /usr/lib/64 /usr/gcc/13/lib /usr/lib /usr/lib/sparcv9 /usr/gnu/lib /usr/ccs/lib
    libc=/usr/lib/sparcv9/libc.so
    lddlflags=' -shared -m64 -L/usr/lib/sparcv9 -L/usr/gnu/lib -fstack-protector-strong'

Both ldflags and lddlflags incorrectly add -L/usr/lib/sparcv9 to link command.

This can happen:

gcc ... -L/usr/lib/sparcv9 ... -Lmy_workspace/lib -lsnmp

will use libsnmp.so from /usr/lib/sparcv9 instead from my_workspace/lib as intended.

Please note that this is solaris specific change but for sparc only. X86 does not include such a change.

This is a workaround/fix I am going to include in Solaris Perl package:

--- perl-5.38.2/hints/solaris_2.sh
+++ perl-5.38.2/hints/solaris_2.sh
@@ -607,7 +607,7 @@ EOM
                    ;;
                esac
                if test "$processor" = sparc; then
-                   loclibpth="/usr/lib/sparcv9 $loclibpth"
+                   #loclibpth="/usr/lib/sparcv9 $loclibpth"
                    ccflags="$ccflags -mcpu=v9"
                fi
                ccflags="$ccflags -m64"

Thank you

book commented 1 month ago

Is /usr/lib/sparcv9 needed anywhere?

In that case, would a patch like the following (swapping the paths) be more suitable?

--- perl-5.38.2/hints/solaris_2.sh
+++ perl-5.38.2/hints/solaris_2.sh
@@ -607,7 +607,7 @@ EOM
                    ;;
                esac
                if test "$processor" = sparc; then
-                   loclibpth="/usr/lib/sparcv9 $loclibpth"
+                   loclibpth="$loclibpth /usr/lib/sparcv9"
                    ccflags="$ccflags -mcpu=v9"
                fi
                ccflags="$ccflags -m64"
haarg commented 1 month ago

I'd like to get this change merged, but I'm a little hesitant given how close we are to a final stable release. It appears that adding the path was related to getting long doubles to work, but those changes are from 2000 and could easily not be relevant anymore.

There also seems to be an another place that is adding /usr/lib/sparcv9 to loclibpth, for non-gcc compilers. Should that be changed as well?

And does everything work properly with this change when using -Dusemorebits?

vlmarek commented 1 month ago

Hi,

Thanks for looking onto the issue.

There is no rush in getting the change in. I have placed the path into our builds of perl https://github.com/oracle/solaris-userland/blob/master/components/perl/perl538/patches/sparc_link_path.patch so it is being used. You make take it as a testing :)

We are using two compilers in Solaris. gcc and studio. Both look into /usr/lib/sparcv9 when linking 64bit binary (and into /usr/lib/amd64 on x86). During runtime the 64 bit binaries do look into /usr/lib/64 (which is symlink to /usr/lib/sparcv9) for libraries (as seen in crle -64 output). So the only reason to list -L/usr/lib/sparcv9 would be to change the order in which the library directories are scanned. If you do that you must have very good reason. But it is something I would strongly advise against.

We should not be placing /usr/lib/sparcv9 into list of search paths by default.

If there are other compilers which compile 64bit binaries on solaris and do not scan /usr/lib/sparcv9 by default, then I think that this is issue of that specific compiler. In which case the path could be added for that specific compiler (and even then I think it is not correct to do so).

As for the -Dusemorebits we use -Duse64bitall during compilation. I will try compile with -Dusemorebits too and run tests and report.

Thank you Vlad

vlmarek commented 1 month ago

make check did produce the same result with and without -Dusemorebits . If you have any specific testing in mind I'm happy to perform it. Thank you.