232226 - AIX shared library build configuration shouldn't use -G

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Jim Dunn]

When building shared libs on AIX, if one uses -G, then certain undersirable
build parameters are used.  

  the option -G is equivalent to
  -berok -brtl -bnortllib -bnosymbolic -bnoautoexp -bM:SRE

What we really want to prevent is -berok, which may hide build issues
(instead of finding them at build time, they would appear at runtime).

So working with Phil we think the correct thing to do is 
dis-assemble the -G and only use what we need/want, instead
of specifying -G and then overriding options later in the
build command

Comment 1

[Jim Dunn]

In response to Phil's direct question of using -bsymbolic.  What I found 
was the following http://compilers.iecc.com/comparch/article/01-08-052
Not sure if this is necessary useful, but where we would use -bsymbolic
is in rules.mk under the IS_COMPONENT section.  However in that section
we are specifying -bE:export_file so that I believe is handling the
internal .vs. external mappings.  Not sure if adding -bsymbolic will
make things faster.

Comment 2

[Philip K. Warren]

Attachment: Patch v1

This patch does the following:

1) Removes -G -bnoentry from DSO_LDOPTS. The -G option contains the following:
  -berok - We do not want this behavior. We would like undefined symbols to
halt the build process instead of simply cause crashes at the end of the build.

  -brtl - This is already in the LDFLAGS variable passed when creating all
shared libraries and binaries.
  -bnortllib - This didn't have any effect. Since rules.mk passes $(LDFLAGS)
after $(DSO_LDOPTS), the -bnortllib was being overwritten by -brtl in
$(LDFLAGS) (-brtl turns on -brtllib and -bsymbolic).
  -bnoautoexp - This didn't have any effect on the program. Components are
exported with a custom export file for AIX in rules.mk. For non-component 
shared libraries, an export list is created with the -qmkshrobj option to the
compiler.
  -bM:SRE - This is already passed to the linker when the -qmkshrobj compiler
option is specified. This can be confirmed by running the link step with -V to
show how the linker is being invoked.

For -bnoentry, this is also passed to the linker when using -qmkshrobj.

2) Modified LDFLAGS to pass -brtl instead of -Wl,-brtl. Both are valid.

3) Added support for CC_VERSION and CXX_VERSION when using the VisualAge C++
compiler. There is no '-v' like command for finding the compiler version, so I
determine it by finding the version number of the installed compiler fileset.
If there is any error, it just uses a blank value (which is what is being done
now).

4) Added -bnortllib to EXTRA_DSO_LDOPTS in rules.mk. Shared libraries should
not include a reference to the run time linker library - only programs which
use runtime linking need this. Here is a quote from the IBM Redbook "AIX
Linking and Loading Mechanisms" which states this point:

"The module built with -G option (hence containing the -bnortllib option) will
be enabled for run-time linking, but a reference to the run-time linker library
itself will be removed. Only the main program needs to have a reference to the
run-time linker libraries. Therefore, linking the shared libraries with -G and
the main module with -brtl enables run-time linking for the application."

I have also added the -bnortllib to the DSO_LDOPTS variables for NSPR and LDAP.
I will make the same change for NSS in a new patch to Bug 231018.

Comment 3

[Wan-Teh Chang]

If a shared library calls dlopen to load other shared
libraries, does that shared library need to be linked with
-brtllib?  For example, libnspr4.so contains the
PR_LoadLibrary function, which calls dlopen.  I am wondering
if we need to link libnspr4.so with -brtllib.

Please run the NSPR tests (see bug 228899 comment 7) after
you apply this patch.

Comment 4

[Philip K. Warren]

Attachment: NSPR Test Results

Here are the NSPR test results with this patch.

Comment 5

[Wan-Teh Chang]

Comment on attachment 139930 [details] [diff] [review]
Patch v1

Thank you for the NSPR test results.  r=wtc on the
NSPR change in this patch.

Comment 6

[Wan-Teh Chang]

Comment on attachment 139930 [details] [diff] [review]
Patch v1

I've checked in the NSPR change in this patch
on the NSPR trunk (NSPR 4.5.1) and the
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.7a).

Comment 7

[Philip K. Warren]

Comment on attachment 139930 [details] [diff] [review]
Patch v1

Need to rethink this a bit. Currently, binaries are created with -brtl
-bnortllib -Wl,-brtl (from gtk-config) but for some reason the last -brtl is
not being passed to the linker.

Comment 8

[Philip K. Warren]

This seems like a bug in the compiler:

pkw@ut:~$ cc -v test.c -o test -brtl -bnortllib -brtl
... 
exec:
/bin/ld(/bin/ld,-b32,/lib/crt0.o,-bpT:0x10000000,-bpD:0x20000000,test.o,-otest,
-brtl,-bnortllib,-brtl,-L/usr/lpp/xlopt,-lxlopt,-lc,NULL) 
...

pkw@ut:~$ cc -v test.c -o test -brtl -bnortllib -Wl,-brtl
...
exec: /bin/ld(/bin/ld,-brtl,-b32,/lib/crt0.o,-bpT:0x10000000,-bpD:0x20000000,test.o,
-otest,-brtl,-bnortllib,-L/usr/lpp/xlopt,-lxlopt,-lc,NULL)

In the first command, the order of arguments passed to the linker is preserved
(see -brtl,-bnortllib,-brtl). In the second command (which uses -Wl,-brtl), the
order is different (-brtl,-brtl,-bnortllib). So files compiled in the order of
the second command will not include a reference to the run-time linker library.

Comment 9

[Philip K. Warren]

Attachment: Patch v2

- This leaves out the -bnortllib change for Mozilla libraries until the problem
reported in comment 8 is resolved.
- Adds support for CC_VERSION and CXX_VERSION in about:buildconfig when using
the AIX VisualAge C++ compiler.

Comment 10

[Wan-Teh Chang]

Philip, I've already checked in the -bnortllib changes
for NSPR and NSS.  Should I back them out?

Comment 11

[Philip K. Warren]

No they are still valid there (and are working properly). We cannot turn it on 
for other Mozilla libraries at this time because of the problem in comment 8. 
This occurs because the output of `gtk-config --libs` includes -Wl,-brtl 
instead of -brtl.

Comment 12

[Philip K. Warren]

Fixed.

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1321; previous revision: 1.1320
done