76710 - Explore "-Bsymbolic" linker flag for Linux

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect, P2)

Description

[Phil Peterson]

During a recent discussion with someone embedding mozilla on Linux, the issue
arose that we allow the linker to pick up symbols from any shared library. This
turned out to be a problem since there were some duplicate symbols in the
embedder's codebase. They'd like us to use the linker flag "-b symbolic" (sorry
if this syntax is wrong), which would force the linker to look internally for
symbols to resolve.

Comment 1

[chris hofmann]

would rfg's, global symbol clean up work also help to 
fix this problem?

Comment 2

[cls]

       -Bsymbolic
              When creating a shared library, bind references  to
              global  symbols to the definition within the shared
              library, if any.  Normally, it is  possible  for  a
              program linked against a shared library to override
              the definition within the shared library.  This op­
              tion is only meaningful on ELF platforms which sup­
              port shared libraries.

How about we get rid of the global symbols instead? :)

Comment 3

[Phil Peterson]

How would you ever load any code with no global symbols? :-)  Seriously, I think
the problem is that the embedding customer is using different versions of some
of the same code we use (e.g. expat)

Comment 4

[TVL]

this can be a problem if the two libraries in question are different versions 
(libjpeg for example), or if they are built differently (say someone else uses 
expat, but compiled in utf8 mode instead of ucs2 mode, or if someone needs zlib, 
but doesn't pull in zip file support, so mozilla ends up using some symbols from 
it's copy and some from the apps copy.

as to chris's comments about global symbol cleanup, if i follow it, the problem 
is when the conflicting symbols are linked into the embeddors app itself.  
there's no way to keep gcc from exporting the symbols from there.

Comment 5

[Jon Granrose]

anyone have any updates on this?

phil - is this a 0.9.1 bug or can it be moved out to 0.9.3 or 1.0?

Comment 6

[Phil Peterson]

granrose, 0.9.2 would be ok. cls, what's ahead of this in line?

Comment 7

[cls]

Attachment: Add -Wl,-Bsymbolic to MKSHLIB

Comment 8

[cls]

The only thing ahead of this is me figuring out why I can no longer run mozilla
on my box after a kernel panic induced reinstall.  I've attached the patch that
I can't verify atm.  It includes changes to nspr & coreconf as well.

Comment 9

[cls]

Attachment: Fix xmlterm & ldap

Comment 10

[Jon Granrose]

and the stopping console spammage set of patches.

Comment 11

[Wan-Teh Chang]

The -Bsymbolic linker flag is not necessary for the NSPR shared
libraries.

The -Bsymbolic linker flag should be useful for libnss3.so, which
is linked with libdbm.a (Berkeley DB 1.85).  That's the only third
party static library we use.  It is not necessary for the other
NSS shared libraries.

Comment 12

[cls]

I didn't think the prereq for triggering bug was that the library had to link
against a 3rd library.  Unless I'm reading it wrong, the bug can be triggered by
an app using the same global symbol as defined in one of our libs.  Therefore,
all of our libs will need this flag.

Since LDAP is using NSPR's build system (bug 75028), the change needs to be made
to the NSPR build system as well or increase the priority of fixing bug 75028.

The changes to mozilla/configure.in and the xmlterm & ldap Makefile.ins has been
checked in.

Comment 13

[cls]

Reassigning to wtc to check in the necessary NSPR & NSS changes.

Comment 14

[Wan-Teh Chang]

Using -Bsymbolic on all shared libraries is wrong.  For example,
if libnspr4.so were linked with -Bsymbolic, it would be
impossible to override PR_Malloc and PR_Free with a better
malloc package as NSPR itself would be calling its own PR_Malloc
and PR_Free while the rest of the application would be calling the
new PR_Malloc and PR_Free.

Sometimes, two versions of the same library can coexist
happily (for example, expat).  Sometimes, the whole application
must use the same version (for example, malloc and free).
You really need to decide this on a case by case basis.

I don't think any of the NSPR and NSS shared libraries need
-Bsymbolic.

Comment 15

[cls]

Ok, good point.  This may be causing problems with our --enable-boehm builds as
well.  Still investigating but if it is, this change will have to be backed out.

Comment 16

[Wan-Teh Chang]

cls wrote:
> This may be causing problems with our --enable-boehm builds as
> well.

Only if libc.so on Linux was built with -Bsymbolic, which is unlikely.
(By the way, libc on AIX 4.1 or 4.2 was built that way, which
makes it challenging to override malloc and free.)

Again, my point is that we should only use -Bsymbolic where it
is needed.  Although -Bsymbolic can avoid some problems, it may
introduce other problems.

Comment 17

[cls]

wtc wrote:
> cls wrote:
> > This may be causing problems with our --enable-boehm builds as
> > well.

> Only if libc.so on Linux was built with -Bsymbolic, which is unlikely.

Dbaron verified that removing -Bsymbolic from the boehm build fixed it.

Comment 18

[jim_nance]

The -Bsymbolic flag breaks jprof too.  I dont quite understand whats going on
but removing the flag makes things work.  Its bug 81881 if you want more info.

Comment 19

[Jim Dunn]

FYI this change (which has since been pulled by cls) also broke
the linux talkback builds (i.e. talkback didn't work on linux).
However, you can explore doing this ONLY for the cases where
IS_COMPONENT=1.  I have done this for hp-ux in config/rules.mk
See config/rules.mk line 386 & 407.  I know I can't set this
for ALL .so's (.sl's) on hpux, but I had to do it for some/all 
the .so's (.sl's) in the components dir.

Because of these lines in rules.mk we have been forced to 
adopt 1 of the 2 following Makefile.in implmentations.

1)
 EXTRA_DSO_LDOPTS must have a "=+" and not just "=" so that
 we don't overwrite EXTRA_DSO_LDOPTS = -Bsymbolic
OR
2)
 if we go with EXTRA_DSO_LDOPTS = syntax then we have to make sure
 that we include rules.mk AFTER we set EXTRA_DSO_LDOPTS, so that
 we -Bsymbolic will be added to EXTRA_DSO_LDOPTS

Both examples are used extensively throughout mozilla

NOTE: I have not tried this on Linux, but you might want to and if
you do, you might want to verify that it doesn't break talkback builds
before checking it in.

Comment 20

[jim_nance]

Let me second Jim Dunns suggestion.  I played around with -Bsymbolic for just
the components a few weeks ago and it worked well.  I believe we get the same
affect if we enable stripping of the component libs, but I would like to see the
-Bsymbolic in there as well.  It makes things more consistent between debug and
optimized builds.

Comment 21

[cls]

Attachment: Add -Wl,-Bsymbolic to EXTRA_DSO_LDOPTS for Linux components only

Comment 22

[cls]

Bumping out because I *still* can't get this option to play nice with talkback.

Comment 23

[hacker formerly known as [e-mail]]

I tested this patch again and I was able to get a working talkback build when
using -Bsymbolic.  No further mucking with EXTRA_DSO_LDOPTS was needed.

Comment 24

[Shiva Thirumazhusai]

Comment on attachment 46632 [details] [diff] [review]
Add -Wl,-Bsymbolic to EXTRA_DSO_LDOPTS for Linux components only

Step 1 :Applied this patch into my local build (optimized and non-debug but
with debug information )
Step 2: Generated talkback configuration file
Step 3: Copied rest of the talkback files from trunk installations
Step 4: Uploaded the symbols to talkback server 
Step 5: Artificially created a crash and  submitted the crash 
Step 6 :Incident showed up in talkback database with proper symbols resolution.
Step 7: I guess it is working.

Comment 25

[hacker formerly known as [e-mail]]

The patch has been checked in.

Comment 26

[hacker formerly known as [e-mail]]

Hrm.  We *really* need to upgrade our base compiler.  Egcs and/or binutils from
RH 6.2 require that symbols be resolved at link time and this is breaking in a
few places through the tree like smoketest blocker bug 125530 .

Comment 27

[hacker formerly known as [e-mail]]

Problems appear to be resolved.