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)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.1beta

People

(Reporter: dmosedale, Assigned: dmosedale)

References

Details

Attachments

(2 files, 4 obsolete files)

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.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.1beta
Attached patch patch, v1 (obsolete) (deleted) — Splinter Review
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?
*** Bug 154205 has been marked as a duplicate of this bug. ***
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?
dbaron: that's actually the exact way that __builtin_vec_new in the old libgcc2 does it.
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.
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.
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.
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).
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.
> 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.
Attached patch patch, v2 (obsolete) (deleted) — Splinter Review
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
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
Attached patch patch, v3 (obsolete) (deleted) — Splinter Review
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 on attachment 90140 [details] [diff] [review] patch, v3 r=bryner
Attachment #90140 - Flags: review+
Attached patch patch, v4 (obsolete) (deleted) — Splinter Review
Fix syntax error introduced in previous patch rev.
Attachment #90140 - Attachment is obsolete: true
Comment on attachment 90443 [details] [diff] [review] patch, v4 r=serge
Attachment #90443 - Flags: review+
Comment on attachment 90443 [details] [diff] [review] patch, v4 Is this on by default? My reading is that it is.
blizzard: it's on by default for x86 linux only. and then only if the relevant symbols are missing at compile time.
*** Bug 124006 has been marked as a duplicate of this bug. ***
Comment on attachment 90443 [details] [diff] [review] patch, v4 sr=blizzard
Attachment #90443 - Flags: superreview+
Attached patch patch, v5 (deleted) — Splinter Review
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.
Attachment #90443 - Attachment is obsolete: true
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 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+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
*** Bug 91470 has been marked as a duplicate of this bug. ***
.
Status: RESOLVED → VERIFIED
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!
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: