Closed
Bug 154206
Opened 22 years ago
Closed 22 years ago
hack to make existing linux flashplayer & real binaries work with gcc 3.1 builds
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.1beta
People
(Reporter: dmosedale, Assigned: dmosedale)
References
Details
Attachments
(2 files, 4 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
dmosedale
:
review+
dmosedale
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
On x86 linux, the current flashplayer plugin (version 5.0r48) expects
a couple of builtin symbols from libgcc which were available in some older
versions of gcc. However, they're _NOT_ available in gcc 3.1, so if we
want that plugin to work with a gcc-3.1 built binary, we need to provide these
symbols.
It turns out that all the rest of the C++ goop in flashplayer is linked
statically, so there are no ABI issues here.
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.1beta
Assignee | ||
Comment 1•22 years ago
|
||
First cut at a patch. In some sense, this code should really live in
nsPluginsDirUnix.cpp. However, when I put it there, the symbols show up in the
.so as expected (according to nm), but ld.so doesn't link them to
flashplayer.so for whatever reason. Putting it in nsAppRunner.cpp, like this
patch does, works.
Comments? Does anyone object too strenuously to a hack of this sort being
checked in?
Comment 2•22 years ago
|
||
*** Bug 154205 has been marked as a duplicate of this bug. ***
Comment 3•22 years ago
|
||
Comment on attachment 89120 [details] [diff] [review]
patch, v1
>+ if (aPtr)
>+ free(aPtr);
Can you use something that doesn't make this a mismatch with your
__builtin_vec_new?
Assignee | ||
Comment 4•22 years ago
|
||
dbaron: that's actually the exact way that __builtin_vec_new in the old libgcc2
does it.
Assignee | ||
Comment 5•22 years ago
|
||
Comment 6•22 years ago
|
||
I think that components-version-script is redefining those __builtin_vec symbols
to be local to the plugin dll and that's why the plugin can't find them.
Can you try disabling that script (or adding those symbols to the script) and
placing the code snippet back in nsPluginsDirUnix.cpp?
I think we need a better configure name than old-plugins-hack but I can't think
of one atm.
Assignee | ||
Comment 7•22 years ago
|
||
I actually built with --enable-debug (in addition to --enable-optimize), so
components-version-script is not even getting run. Reading the linker man page
sure made it sound promising, though, so I tried adding -Wl,-E to the link line,
but that didn't help either.
Next I turned on ld.so debugging, and it appears that the dynamic linker only
attempts to bind symbols in flashplayer.so to (the transitive closure of) the
libraries directly linked to the final executable (mozilla-bin). I played with
a bunch of linker switches, and was unable to find any that would change this
behavior. Rather than sinking more time into this, I'm inclined to leave the
hack in nsAppRunner.cpp.
I've been thinking over the mismatched allocator thing. On the one hand, it
makes me pretty uncomfortable, because I wonder if the reason it happens to work
in egcs is because of some underlying guarantees that I don't know about due to
the fact that it lives in a compiler-internal place. Or maybe it never worked.
On the other hand, I'm a little leery of changing the semantics by making the
allocators match. Any opinions on this one?
As far as autoconf feature names, I'm happy to take suggestions, but I haven't
been able to think of one that's both succinct enough and high-level enough.
Comment 8•22 years ago
|
||
Oh.
There was a very large thread on this late last month on the gcc mailing list. See
http://gcc.gnu.org/ml/gcc/2002-05, specifically the threads 'duplicate data
objects in shared libraries', 'Treat RTLD_LOCAL like Solaris', 'Minimal
GCC/Linux shared lib + EH bug example', and a few others. The most important
ones from this POV were probably on
htp://sources.redhat.com/ml/libc-alpha/2002-05/ too, but the backround is
interesting too. I don't know what conclusion (if any) was reached, though.
However, (assuming that the behaviour in libc stays as is) what if the
mozilla-bin binary wasn't linked against libstdc++? Ie, instead of this patch,
compile with CXX=gcc, or try '-nostdlib', and add -lgcc -lc to the link flags.
You may need to add -static-libgcc in either or both cases.
This would then require the old libstdc++ library to be present on the user's
system for flash to work - I don't think that that is an issue, though.
Assignee | ||
Comment 9•22 years ago
|
||
bbaetz: I didn't read all those threads, but I think I read enough to conclude
that they're discussing a problem that is very closely-related to what I'm
seeing, but not exactly the same. In particular, Jason Merrill's summary at
http://gcc.gnu.org/ml/gcc/2002-05/msg01970.html seems to support this. For one
thing, I suspect that our component loader isn't causing RTLD_NOW to be used,
anyhow. But it has given me some ideas about other things to try. It would be
nice to have this in the plugins library, because then embeddors (eg Galeon)
could get this hack automagically as well.
As far as the linking without libstdc++ stuff goes, I've been under the
impression that our portability man (scc) is interested in seeing Mozilla move
towards using libstdc++ more in the app, rather than less. So I'm not convinced
this is worth spending time on.
See also http://hints.linuxfromscratch.org/hints/mozilla.txt as well as bug
124006 (which this appears to be a DUP of).
Comment 10•22 years ago
|
||
This is a dupe of several bugs...
libstdc++ has explicitly refused to freeze the binary API yet, so I suspect that
that would be a losing idea.
If this is in the plugin api, though, what about out code which calls
__builtin_vector_new? Won't we pick up that definition? thers not even a
guarantee that the memory allocators are similar, although since libstdc++ uses
libc's, that is true on linux, but I don't think its true on all unix platforms.
Assignee | ||
Comment 11•22 years ago
|
||
> libstdc++ has explicitly refused to freeze the binary API yet, so I suspect
> that that would be a losing idea.
Well, we'd be no worse off than we are today.
Additionally, we could statically link about libstdc++ (since we think that's
finally allowable from a license point of view), and hope that by the time
there's enough usage in Mozilla to add a significant amount of bloat, libstdc++
will finally have frozen.
In any case, building without libstdc++ is hardly a supported configuration
today, so I'm inclined to not worry about this now. If we really intend to make
this a support configuration, let's file a separate bug for that.
> If this is in the plugin api, though, what about out code which calls
> __builtin_vector_new? Won't we pick up that definition?
gcc 3 doesn't have __builtin_vec_new and friends. Also note the configure test
which explicitly prevents this from happening.
> thers not even a guarantee that the memory allocators are similar, although
> since libstdc++ uses libc's, that is true on linux, but I don't think its true
> on all unix platforms.
This hack is only enabled by default on linux anyway, so I don't think that's
likely to be a problem.
Assignee | ||
Comment 12•22 years ago
|
||
Various minor tweaks, and changed the option name from "old-plugins-hack" to
"compat-mem-wrappers". The idea here is that with the old-plugins-hack name,
folks might have beeen tempted to turn it on other platforms without actually
knowing what it does ("yeah, i want old plugins to work"). Anyone here care to
r / sr ?
Attachment #89120 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Summary: hack to make existing linux flashplayer binary work with gcc 3.1 builds → hack to make existing linux flashplayer & real binaries work with gcc 3.1 builds
Assignee | ||
Comment 13•22 years ago
|
||
This version now includes a few more symbols to make the RealPlayer 8 plugin
happy as well. Renamed the configure option yet again to accomodate these new
symbols.
Attachment #89866 -
Attachment is obsolete: true
Comment 14•22 years ago
|
||
Comment on attachment 90140 [details] [diff] [review]
patch, v3
r=bryner
Attachment #90140 -
Flags: review+
Assignee | ||
Comment 15•22 years ago
|
||
Fix syntax error introduced in previous patch rev.
Attachment #90140 -
Attachment is obsolete: true
Comment 16•22 years ago
|
||
Comment on attachment 90443 [details] [diff] [review]
patch, v4
r=serge
Attachment #90443 -
Flags: review+
Comment 17•22 years ago
|
||
Comment on attachment 90443 [details] [diff] [review]
patch, v4
Is this on by default? My reading is that it is.
Assignee | ||
Comment 18•22 years ago
|
||
blizzard: it's on by default for x86 linux only. and then only if the relevant
symbols are missing at compile time.
Assignee | ||
Comment 19•22 years ago
|
||
*** Bug 124006 has been marked as a duplicate of this bug. ***
Comment 20•22 years ago
|
||
Comment on attachment 90443 [details] [diff] [review]
patch, v4
sr=blizzard
Attachment #90443 -
Flags: superreview+
Assignee | ||
Comment 21•22 years ago
|
||
I misunderstood the way autoconf creates HAVE_* #defines, so the last patch
failed to link under egcs 1.1.2. This version corrects that, and seems to do
the right thing on egcs 1.1.2, gcc 2.96, and gcc 3.1.
Assignee | ||
Updated•22 years ago
|
Attachment #90443 -
Attachment is obsolete: true
Assignee | ||
Comment 22•22 years ago
|
||
Comment on attachment 91154 [details] [diff] [review]
patch, v5
The only change between the last rev and this one was the symbol names in the
#ifndef statements. Carrying forward r= and sr=.
Attachment #91154 -
Flags: superreview+
Attachment #91154 -
Flags: review+
Comment 23•22 years ago
|
||
Comment on attachment 91154 [details] [diff] [review]
patch, v5
a=asa (on behalf of drivers) for checkin to the 1.1 trunk.
Attachment #91154 -
Flags: approval+
Assignee | ||
Comment 24•22 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 25•22 years ago
|
||
*** Bug 91470 has been marked as a duplicate of this bug. ***
Comment 27•22 years ago
|
||
I just noticed that the Java Sun people have FINALLY (March 13, '03) realized
this is a problem and are going to begin distributing a GCC 3.2 JRE.
See here:
http://developer.java.sun.com/developer/bugParade/bugs/4687814.html
Yay!
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•