Closed Bug 271630 Opened 21 years ago Closed 21 years ago

XPCOM glue broken on Linux/PPC [monkeypox orange]

Categories

(Core :: XPCOM, defect)

PowerPC
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.8beta1

People

(Reporter: darin.moz, Assigned: darin.moz)

References

Details

(Keywords: crash, regression)

Attachments

(1 file)

XPCOM glue broken on Linux/PPC The recent change to NS_GetFrozenFunctions causes trouble for applications using the XPCOM glue. The problem occurs with the resolution of the non-"_P" symbols. For example, NS_CStringContainerInit is defined both in the XPCOM glue library and in the XPCOM stub library. As a result, when we call NS_GetFrozenFunctions, we return the address of the NS_CStringContainerInit found in the XPCOM glue library. This results in infinite recursion when NS_CStringContainerInit is called. This explains why monkeypox is orange. Since monkeypox is using GCC 2.95.2, there is no way to specify hidden visibility. So, if we want to continue supporting that target, I think the only solution is to avoid using the same name for these functions in both libraries. Perhaps the best solution would be to move nsStringAPI.cpp into xpcom_core and use a similar _P naming trick. We'd probably want to do this as part of LibXUL anyways so we don't have to export the string classes.
Blocks: 267767
Severity: normal → major
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta
Attached patch v1 patchSplinter Review
Comment on attachment 166967 [details] [diff] [review] v1 patch contrary to what the diff may indicate, i'm actually just resurrecting xpcom/build/nsStringAPI.cpp ;-) I also intend to eventually mark all of the nsXPCOM.h functions as hidden when XPCOM_GLUE is defined. This patch makes that change for nsStringAPI.h, but not the others. I'll do the rest as a followup patch in another bug.
Attachment #166967 - Flags: review?(bsmedberg)
Comment on attachment 166967 [details] [diff] [review] v1 patch I don't like it. The extra level of call indirection is going to hurt future performance visibly. Instead, let's rename the *glue* functions using a define: #ifdef XPCOM_GLUE #define NS_CStringContainer_Init NS_CStringContainer_Init_Glue etc... This solves the problem with non-hidden symbols conflicting, without forcing the extra layer of indirection.
Attachment #166967 - Flags: review?(bsmedberg) → review-
bsmedberg: we can't change the names of symbols that already exist in people's programs. moreover, i thought you wanted to make libxul only export frozen symbols? as is, it will have to export portions of the internal string library.
Are we going to export the string API from libxul as frozen exports? What are the symbol names going to be? I want firefox, by version 1.5, to use only the frozen string API. The patch in bug 270110 is a great step toward this, because it doesn't require drastic code changes. However, it is an unacceptable performance penalty for every string call to go through NS_StringFunc -> NS_StringFunc_P. Since libxpcom.so depends on libxpcom_core.so, why can't we put all the frozen exports in libxpcom_core.so, except for the one required by glue (NS_GetFrozenFunctions)? Is there any previous code which we need to support that dynamically links (using dlopen, dlsym) to other funtions in libxpcom.so ?
> Are we going to export the string API from libxul as frozen exports? What are > the symbol names going to be? I'm not sure how we are going to deal with this. Obviously, with libxul we are starting with a clean slate. The only requirement is that we maintain the frozen set of entry points into libxpcom.so. Existing embeddings and components should not be broken. > I want firefox, by version 1.5, to use only the frozen string API. The patch > in bug 270110 is a great step toward this, because it doesn't require drastic > code changes. However, it is an unacceptable performance penalty for every > string call to go through NS_StringFunc -> NS_StringFunc_P. Then perhaps Firefox should link directly to libxul... hmm... > Since libxpcom.so depends on libxpcom_core.so, why can't we put all the > frozen exports in libxpcom_core.so, except for the one required by glue > (NS_GetFrozenFunctions)? Is there any previous code which we need to support > that dynamically links (using dlopen, dlsym) to other funtions in libxpcom.so > ? There are existing components and embedding applications that depend on the frozen set of entry points into libxpcom.so. We cannot mess with those symbols. If an embedding application or component links to non-frozen symbols in libxpcom.so (from Moz 1.7), then it is already broken by the creation of libxpcom_core.so. To make things simple, I'd just start with having libxul be a private, implementation detail. I'd expose libxpcom as the interface into libxul. If we find that the overhead of thunking through libxpcom is unacceptable then we find a way to optimize it. I don't think it will be a big problem.
bsmedberg: I took a look at the code MSVC 6 generates for NS_GetComponentManager in XPCOM.dll today, and it consists of a single jmp statement: 100011A1: FF 25 24 20 00 10 jmp dword ptr ds:[10002024h] The jump-to address corresponds to the relocation table for xpcom_core.dll. I would be surprised if we see this register on the performance meter. IIRC, GCC employs a similar optimization.
Comment on attachment 166967 [details] [diff] [review] v1 patch re-requesting review.
Attachment #166967 - Flags: review- → review?(bsmedberg)
Attachment #166967 - Flags: review?(bsmedberg) → review+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
reopening since monkeypox is still orange :( investigating...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
ah, monkeypox had not been updated yet in accordance with the new build rules. so, it was not pulling SeamonkeyAll :-( i went ahead and added MOZ_CO_PROJECT=suite and --enable-application=suite bsmedberg: have you verified that all the rest of the tinderboxen are up-to-date? i mark this bug fixed once i see monkeypox cycle green.
marking FIXED again since monkeypox finally did turn green as expected.
Status: REOPENED → RESOLVED
Closed: 21 years ago21 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: