Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use -G to build shared object on Solaris #757

Closed
noloader opened this issue Feb 1, 2020 · 7 comments
Closed

Don't use -G to build shared object on Solaris #757

noloader opened this issue Feb 1, 2020 · 7 comments
Assignees
Milestone

Comments

@noloader
Copy link

noloader commented Feb 1, 2020

Please don't use -G to build the shared object on Solaris. Instead, use -shared.

...
Generating hiredis.pc for pkgconfig...
gcc -G -o libhiredis.so -h libhiredis.so.0.14 -L/usr/local/lib -m64 -Wl,-R,'$ORIGIN/../lib' -Wl,-R,/usr/local/lib net.o hiredis.o sds.o async.o read.o
ar rcs libhiredis.a net.o hiredis.o sds.o async.o read.o
gcc -O3 -fPIC -g2 -O2 -m64 -march=native -fPIC -pthread -Wall -W -Wstrict-prototypes -Wwrite-strings -g -ggdb -o hiredis-test -L/usr/local/lib -m64 -Wl,-R,'$ORIGIN/../lib' -Wl,-R,/usr/local/lib -ldl -lnsl -lsocket test.o libhiredis.a

Here's the result of using -G:

ld.so.1: unittest: fatal: relocation error: R_AMD64_PC32: file /usr/local/lib/libhiredis.so.0.14: symbol _init: value 0x7f004e1390bd does not fit
gmake: *** [test] Killed

The failed self-test showed up when testing Unbound.

@michael-grunder michael-grunder self-assigned this Feb 3, 2020
@michael-grunder
Copy link
Collaborator

Having done a bit of research it looks like you're right about this, but I don't have solaris boxes where I could test it.

The only thing I'm concerned about is whether the change might cause a regression in very old/obscure versions of Solaris. In other words, can we simply change -G to -shared within our SunOS uname branch, or do we need to be more specific about it.

Do you happen to know?

@noloader
Copy link
Author

noloader commented Feb 3, 2020

Hi Michael,

The only thing I'm concerned about ... very old/obscure versions of Solaris

Ah, OK. Just my 2 cents here... I help maintain some other libraries. My rule of thumb is, don't break the common case for the obscure case. Make the obscure case the special case so things "just work" for the common case. In this context, the common case is modern Solaris like 11.3 or 11.4, and the obscure case is Solaris 9.

As far as testing, I can give you SSH access to my test machines. For Solaris I use 11.3 i86pc (x86_64). It is a Core i5 4th gen, so you have AES, RDRAND and AVX. I need your authorized_keys file, if interested. Email me at noloader, gmail account.

OpenCSW provides free access to Solaris 9 through Solaris 11 on both Sparc and i86pc. See Upstream Maintainers to request an account.

I believe the GCC Compile Farm (Cfarm) provides access to the same machines via OpenCSW. gcc210 is Solaris 10/Sparc, and gcc211 is Solaris 11/Sparc. Accounts are free to FOSS developers. See Request an Account for the Cfarm.

I advise against running Solaris in a VM. Use real hardware instead. Solaris' virtual memory manager is different than Linux. The Solaris memory manager does not oversubscribe memory, and allocations are always backed by RAM. So a Solaris guest is a hog that causes the host's memory manager to work extra hard when Solaris allocates big chunks of RAM. The host's disk i/o goes wild when the guest is Solaris.

Do you happen to know?

Sorry, I'm not sure.

And I should probably say, use -shared when using GCC on Solaris. I have a sneaky suspicion -G may work as expected when using SunC compiler.

@noloader
Copy link
Author

noloader commented Feb 3, 2020

For what it is worth, here is how I patch the hiredis recipe in my build scripts:

IS_SUNC=$("$CC" -V 2>&1 | grep -i -c -E 'sun|studio')
IS_SOLARIS=$(uname -s | grep -i -c 'sunos')
...

# Awful Solaris 64-bit hack. Use -G for SunC, and -shared for GCC
if [[ "$IS_SOLARIS" -ne 0 && "$IS_SUNC" -eq 0 ]]; then
    sed 's/ -G / -shared /g' Makefile > Makefile.fixed
    mv Makefile.fixed Makefile; chmod +x Makefile
fi

@michael-grunder
Copy link
Collaborator

OK. Just my 2 cents here... I help maintain some other libraries. My rule of thumb is, don't break the common case for the obscure case

Yeah we're on the same page, and it certainly seems like -G is the obscure case now (especially given the line was last touched in 2011 via 608e29b).

Ideally we can make the change without breaking builds that are currently compiling although we have some leeway here since we're still pre 1.0.0.

@michael-grunder michael-grunder added this to the 1.0.0 milestone Apr 12, 2020
michael-grunder added a commit to michael-grunder/hiredis that referenced this issue Apr 17, 2020
michael-grunder added a commit to michael-grunder/hiredis that referenced this issue Apr 17, 2020
michael-grunder added a commit to michael-grunder/hiredis that referenced this issue Apr 17, 2020
@michael-grunder
Copy link
Collaborator

What do you think about the change in 23e3bd4.

I can't use grep -E because apparently that doesn't exist in Solaris? Or at least it threw an error in my Solaris VM.

I'm somewhat concerned about the portability of egrep but as far as I can tell this exists everywhere.

@michael-grunder
Copy link
Collaborator

@noloader Unless you have an objection to 23e3bd4 I'm going to get it merged. I tested building on Solaris both with SunStudio 12.6 and gcc and both worked for me.

Interestingly SunStudio does not like -ggdb but it didn't prevent building.

@michael-grunder
Copy link
Collaborator

Closing via #796

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants