Closed
Bug 621446
Opened 14 years ago
Closed 13 years ago
Crash [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify ]
Categories
(Core :: Networking: HTTP, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
Tracking | Status | |
---|---|---|
fennec | - | --- |
People
(Reporter: jdm, Assigned: mayhemer)
References
(Blocks 1 open bug)
Details
(Keywords: crash, Whiteboard: [mobile-crash])
Crash Data
Attachments
(6 files, 2 obsolete files)
4.99 KB,
patch
|
jduell.mcbugs
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta-
mayhemer
:
checkin+
|
Details | Diff | Splinter Review |
1.19 KB,
patch
|
jduell.mcbugs
:
review+
akeybl
:
approval-mozilla-aurora+
mayhemer
:
checkin+
|
Details | Diff | Splinter Review |
1.10 KB,
patch
|
jduell.mcbugs
:
review+
|
Details | Diff | Splinter Review |
1.21 KB,
patch
|
mayhemer
:
review+
|
Details | Diff | Splinter Review |
1.30 KB,
patch
|
glandium
:
review-
|
Details | Diff | Splinter Review |
3.13 KB,
patch
|
jduell.mcbugs
:
review+
mayhemer
:
checkin+
|
Details | Diff | Splinter Review |
Signature mozilla::net::HttpChannelParent::RecvRedirect2Verify
UUID 942948b3-19a1-467a-b718-ca65b2101226
Time 2010-12-26 04:52:25.734260
Uptime 29
Last Crash 45242 seconds (12.6 hours) before submission
Install Age 93314 seconds (1.1 days) since version was first installed.
Product Fennec
Version 4.0b3
Build ID 20101221205132
Branch 1.9
OS Linux
OS Version 0.0.0 Linux 2.6.32.9 #1 Mon Oct 25 17:39:35 KST 2010 armv7l
CPU arm
CPU Info
Crash Reason SIGSEGV
Crash Address 0x0
User Comments
App Notes samsung GT-I9000
samsung/GT-I9000/GT-I9000/GT-I9000:2.2/FROYO/XXJPO:user/release-keys
Processor Notes
EMCheckCompatibility False
Frame Module Signature [Expand] Source
0 libxul.so mozilla::net::HttpChannelParent::RecvRedirect2Verify netwerk/protocol/http/HttpChannelParent.cpp:346
1 libxul.so mozilla::net::PHttpChannelParent::OnMessageReceived PHttpChannelParent.cpp:623
2 libxul.so mozilla::dom::PContentParent::OnMessageReceived PContentParent.cpp:604
3 libxul.so mozilla::ipc::AsyncChannel::OnDispatchMessage ipc/glue/AsyncChannel.cpp:262
4 libxul.so mozilla::ipc::RPCChannel::OnMaybeDequeueOne ipc/glue/RPCChannel.cpp:440
5 libxul.so RunnableMethod<mozilla::ipc::RPCChannel, bool , Tuple0>::Run ipc/chromium/src/base/task.h:308
6 libxul.so mozilla::ipc::RPCChannel::DequeueTask::Run RPCChannel.h:475
7 libxul.so MessageLoop::RunTask ipc/chromium/src/base/message_loop.cc:344
8 libxul.so MessageLoop::DeferOrRunPendingTask ipc/chromium/src/base/message_loop.cc:354
9 libxul.so MessageLoop::DoWork ipc/chromium/src/base/message_loop.cc:451
10 libxul.so mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:115
11 libxul.so MessageLoop::RunInternal ipc/chromium/src/base/message_loop.cc:220
12 libxul.so MessageLoop::Run ipc/chromium/src/base/message_loop.cc:512
13 libxul.so nsBaseAppShell::Run widget/src/xpwidgets/nsBaseAppShell.cpp:198
14 libxul.so nsAppStartup::Run toolkit/components/startup/src/nsAppStartup.cpp:192
15 libxul.so XRE_main toolkit/xre/nsAppRunner.cpp:3693
16 libxul.so GeckoStart toolkit/xre/nsAndroidStartup.cpp:131
17 libc.so libc.so@0x111b3
18 libc.so libc.so@0x10ca3
Specifically, we're crashing here:
346 mRedirectCallback->OnRedirectVerifyCallback(result);
Assignee | ||
Comment 1•14 years ago
|
||
Josh, any STR or log from that session? Can you reproduce in any way and create a log? (best with attachment 496689 [details] [diff] [review] applied and NSPR_LOG_MODULES=timestamp,nsHttp:5,nsSocketTransport:5)
Could that be caused by a redirect to a protocol different from http(s) ?
Could you check mRedirectCallback value? I assume it is null, but just make sure it is.
Reporter | ||
Comment 3•14 years ago
|
||
Sorry, this isn't a crash that I received, it just came up in the daily crash reports I was looking at.
Reporter | ||
Comment 4•14 years ago
|
||
http://crash-stats.mozilla.com/report/list?product=Fennec&query_search=signature&query_type=exact&query=mozilla%3A%3Anet%3A%3AHttpChannelParent%3A%3ARecvRedirect2Verify&date=01%2F03%2F2011%2011%3A47%3A08&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=mozilla%3A%3Anet%3A%3AHttpChannelParent%3A%3ARecvRedirect2Verify shows 4 crashes in the past 4 weeks, so it's not a high priority item.
Assignee | ||
Comment 5•14 years ago
|
||
Thanks.
Assignee | ||
Comment 6•14 years ago
|
||
This is not that rare crash in Fennec. I think we should investigate.
tracking-fennec: --- → ?
Assignee | ||
Comment 7•14 years ago
|
||
All crashes I can find are happening on builds older then or including 2010-12-21. On 2010-11-23 I landed a fix for this crash.
What's strange is that crash-stats lead to a file revision that already contains the fix.
Changelog till the point crash-stats directs to:
http://hg.mozilla.org/mozilla-central/shortlog/59591
Changelog till my fix:
http://hg.mozilla.org/mozilla-central/shortlog/58111
Changelog the crashing build was made of could not be determined, the nightly build with reference to it has been deleted two days ago :(
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 8•14 years ago
|
||
https://crash-stats.mozilla.com/report/index/dfcb3858-e24c-4059-8f7c-dd4432110204
This crash has been seen in 4.0b4, so there's obviously some situation in which it can still occur.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee | ||
Comment 9•14 years ago
|
||
I didn't find this report. Thanks Josh, I'll try to figure it out.
Assignee: nobody → honzab.moz
Status: REOPENED → ASSIGNED
Updated•14 years ago
|
tracking-fennec: ? → 2.0+
Comment 10•14 years ago
|
||
Very low occurrences on crash-stats, but would like to get a fix in
tracking-fennec: 2.0+ → 2.0-
Updated•13 years ago
|
Crash Signature: [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify ]
6th in the top 10 crashes for aurora (9); more reports : https://crash-stats.mozilla.com/report/list?range_value=7&range_unit=days&date=2011-10-24%2012%3A00%3A00&signature=mozilla%3A%3Anet%3A%3AHttpChannelParent%3A%3ARecvRedirect2Verify&version=Fennec%3A9.0a2
Whiteboard: [mobile-crash]
Comment 12•13 years ago
|
||
Also appearing significantly on mobile trunk now. A better list of crashes is https://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=mozilla%3A%3Anet%3A%3AHttpChannelParent%3A%3ARecvRedirect2Verify - shows all crashes with this signature in the last 4 weeks.
Assignee | ||
Comment 13•13 years ago
|
||
This is a patch that before we would crash is trying to catch some states by PR_Abort'ing based on some added debug information.
Attachment #581409 -
Flags: review?(jduell.mcbugs)
Assignee | ||
Comment 14•13 years ago
|
||
Comment on attachment 581409 [details] [diff] [review]
investigation patch
https://tbpl.mozilla.org/?tree=Try&rev=a7e5ed5f73aa
Comment 15•13 years ago
|
||
Comment on attachment 581409 [details] [diff] [review]
investigation patch
Review of attachment 581409 [details] [diff] [review]:
-----------------------------------------------------------------
Is this crash still happening? I seem to get {0} results on the crash reports site when I search for mozilla::net::HttpChannelParent::RecvRedirect2Verify on fennec for the last 28 days. (But I've found it hard to actually locate crashes before).
Obviously we'd expect this to have dropped off given that we're only using e10s on tablets now, but I would have expected some # of crashes.
Anyway, if this is still happening, this is a reasonable way to proceed with debugging.
::: netwerk/protocol/http/HttpChannelParent.h
@@ +139,5 @@
> RequestHeaderTuples *mHeadersToSyncToChild;
> +
> + bool mSentRedirect1Begin : 1;
> + bool mSentRedirect1BeginFailed : 1;
> + bool mReceviedRedirect2Verify : 1;
misspelled "Received"
Attachment #581409 -
Flags: review?(jduell.mcbugs) → review+
Assignee | ||
Comment 16•13 years ago
|
||
Comment on attachment 581409 [details] [diff] [review]
investigation patch
https://hg.mozilla.org/integration/mozilla-inbound/rev/63c3d103610c
Attachment #581409 -
Flags: checkin+
Comment 17•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/63c3d103610c
https://hg.mozilla.org/mozilla-central/rev/b605399234fc
Status: ASSIGNED → RESOLVED
Closed: 14 years ago → 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Assignee | ||
Comment 18•13 years ago
|
||
This was not a fix. Just a debugging patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•13 years ago
|
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 19•13 years ago
|
||
Comment on attachment 581409 [details] [diff] [review]
investigation patch
[Approval Request Comment]
Regression caused by (bug #): None
User impact if declined: None
Testing completed (on m-c, etc.): None needed, see bellow
Risk to taking this patch (and alternatives if risky): None, see bellow
This patch is trying to figure out a cause of an existing crash. If the crash would occur anyway, the patch instead aborts on a certain line (with optimization disabled) to indicate what is the state of the object:
+ if (!mRedirectCallback) {
+ // Bug 621446 investigation (optimization turned off above)
+ if (mReceviedRedirect2Verify)
+ ::PR_Abort();
+ if (mSentRedirect1BeginFailed)
+ ::PR_Abort();
+ if (mSentRedirect1Begin && NS_FAILED(result))
+ ::PR_Abort();
+ if (mSentRedirect1Begin && NS_SUCCEEDED(result))
+ ::PR_Abort();
+ if (!mRedirectChannel)
+ ::PR_Abort();
+ }
+
+ mReceviedRedirect2Verify = true;
+
> mRedirectCallback->OnRedirectVerifyCallback(result);
On the > line the crash occurs.
The reason I would like to get this landed on Aurora or Beta is to get the results sooner. The patch has so far landed on m-c (Fennec 12). Since actively is used only Beta and Release where the crash gets recorded, it would help identify the cause a bit sooner then after some 13 weeks to get the patch to Beta.
Attachment #581409 -
Flags: approval-mozilla-beta?
Attachment #581409 -
Flags: approval-mozilla-aurora?
Updated•13 years ago
|
OS: Linux → Android
Hardware: x86 → ARM
Updated•13 years ago
|
Severity: normal → critical
Comment 20•13 years ago
|
||
Adding #pragma GCC optimize ("O0") breaks the possibility to build Firefox 12 on Ubuntu. See https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/918763
Comment 21•13 years ago
|
||
Comment on attachment 581409 [details] [diff] [review]
investigation patch
[Triage Comment]
Approving for Aurora to speed up the investigation, but we're not going to take a patch for investigation at this point in FF10.
Attachment #581409 -
Flags: approval-mozilla-beta?
Attachment #581409 -
Flags: approval-mozilla-beta-
Attachment #581409 -
Flags: approval-mozilla-aurora?
Attachment #581409 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 22•13 years ago
|
||
(In reply to Vivien Nicolas (:vingtetun) from comment #20)
> Adding #pragma GCC optimize ("O0") breaks the possibility to build Firefox
> 12 on Ubuntu. See
> https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/918763
Please see https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/918763/comments/9
Assignee | ||
Comment 23•13 years ago
|
||
Comment on attachment 581409 [details] [diff] [review]
investigation patch
https://hg.mozilla.org/releases/mozilla-aurora/rev/8aaa33c0d915
Assignee | ||
Comment 24•13 years ago
|
||
I've figured out that using PR_Abort() was not the right way to invoke a reported crash with the correct signature.
According Benjamin's advice I need to turn this to NS_RUNTIMEABORT.
Attachment #597971 -
Flags: review?(jduell.mcbugs)
Updated•13 years ago
|
Attachment #597971 -
Flags: review?(jduell.mcbugs) → review+
Assignee | ||
Comment 25•13 years ago
|
||
Comment on attachment 597971 [details] [diff] [review]
Investigation patch update
https://hg.mozilla.org/integration/mozilla-inbound/rev/5002199145a9
PLEASE LEAVE THE BUG OPEN, THIS IS ONLY FOR INVESTIGATION.
Attachment #597971 -
Flags: checkin+
Comment 26•13 years ago
|
||
Whiteboard: [mobile-crash] → [mobile-crash] [leave open]
Target Milestone: mozilla12 → mozilla13
Assignee | ||
Comment 27•13 years ago
|
||
Comment on attachment 597971 [details] [diff] [review]
Investigation patch update
[Approval Request Comment]
Please see comment 19 and comment 21 (request and approval). The reasons are the same for this update patch.
Attachment #597971 -
Flags: approval-mozilla-aurora?
Comment 28•13 years ago
|
||
Still broken. On lucid, putting "//" in front of the #pragma GCC optimize ("O0") was sufficient for my build to continue. Lucid is using gcc-4.1.5.
Comment 29•13 years ago
|
||
Comment on attachment 597971 [details] [diff] [review]
Investigation patch update
[Triage Comment]
Approved for Aurora 12.
Attachment #597971 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 30•13 years ago
|
||
Comment on attachment 597971 [details] [diff] [review]
Investigation patch update
https://hg.mozilla.org/releases/mozilla-aurora/rev/010814586207
Updated•13 years ago
|
Crash Signature: [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify ] → [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify]
[@ __libc_android_abort | PR_Abort]
Comment 31•13 years ago
|
||
This line, added in comment 23's cset...
> #pragma warning(disable : 4068)
... is MSVC-specific, and it introduces the following build warning on GCC 4.6:
> HttpChannelParent.cpp:336:0: warning: ignoring #pragma warning [-Wunknown-pragmas]
Elsewhere in the codebase, we wrap this sort of #pragma in "#ifdef _MSC_VER" so that other compilers won't see it / complain about it. The attached followup patch does that here, as well.
Attachment #605532 -
Flags: review?(jduell.mcbugs)
Updated•13 years ago
|
Attachment #605532 -
Flags: review?(jduell.mcbugs) → review+
Comment 32•13 years ago
|
||
Pushed followup to fix build warning:
http://hg.mozilla.org/integration/mozilla-inbound/rev/90f6b58ce0fa
Comment 33•13 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #32)
> Pushed followup to fix build warning:
> http://hg.mozilla.org/integration/mozilla-inbound/rev/90f6b58ce0fa
https://hg.mozilla.org/mozilla-central/rev/90f6b58ce0fa
Assignee | ||
Comment 34•13 years ago
|
||
Thanks, Daniel.
Comment 35•13 years ago
|
||
This patch do not use the #pragma GCC optimize("0O") directive if GCC <= 4.4. In practive this will not change anything for the investigation since people with such a version of GCC are commenting this line (or even removing it in the case of Ubuntu).
Attachment #606580 -
Flags: review?(jduell.mcbugs)
Comment 36•13 years ago
|
||
Comment on attachment 606580 [details] [diff] [review]
Patch
Review of attachment 606580 [details] [diff] [review]:
-----------------------------------------------------------------
We are able to compile B2G again in 10.04 with GCC 4.4.x with this patch
Comment 37•13 years ago
|
||
(In reply to Guillermo López (:willyaranda) from comment #36)
> Comment on attachment 606580 [details] [diff] [review]
> Patch
>
> Review of attachment 606580 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> We are able to compile B2G again in 10.04 with GCC 4.4.x with this patch
Doesn't work for me today with GCC 4.4.5. Could it be only for 64 bits?
Comment 38•13 years ago
|
||
(In reply to Vivien Nicolas (:vingtetun) from comment #37)
> (In reply to Guillermo López (:willyaranda) from comment #36)
> > Comment on attachment 606580 [details] [diff] [review]
> > Patch
> >
> > Review of attachment 606580 [details] [diff] [review]:
> > -----------------------------------------------------------------
> >
> > We are able to compile B2G again in 10.04 with GCC 4.4.x with this patch
>
> Doesn't work for me today with GCC 4.4.5. Could it be only for 64 bits?
We are compiling for 32bits, sorry :(
Comment 39•13 years ago
|
||
Comment on attachment 606580 [details] [diff] [review]
Patch
Review of attachment 606580 [details] [diff] [review]:
-----------------------------------------------------------------
Looks fine. Honza, you may want to have a look if you know more about which gcc versions you want this to be applied on.
Attachment #606580 -
Flags: review?(jduell.mcbugs) → review+
Updated•13 years ago
|
Version: unspecified → Trunk
Assignee | ||
Comment 40•13 years ago
|
||
Hm... I think the best would be to condition the investigation code to build just when building Fennec. This is not needed to build for desktop Firefox at all.
However, if all platforms are now OK, then let's don't change this any more.
Comment 41•13 years ago
|
||
(In reply to Guillermo López (:willyaranda) from comment #36)
> attachment 606580 [details] [diff] [review]
Thanks all!
This attachment fixes bug 741104 (internal compiler error on Ubuntu 10.04 64bit) for me.
Comment 42•13 years ago
|
||
This is the same patch as before except that it check for x86_64 since it looks like this is the only platform that fails.
Attachment #606580 -
Attachment is obsolete: true
Attachment #611700 -
Flags: review?(jduell.mcbugs)
Comment 43•13 years ago
|
||
Comment on attachment 611700 [details] [diff] [review]
Patch v0.2
Review of attachment 611700 [details] [diff] [review]:
-----------------------------------------------------------------
Looks fine to me, though I'm not an expert in compiler version macros. But I want Honza to sign off on this--he seemed to think we might want to instead limit usage only to fennec. I have no strong opinion on it.
Attachment #611700 -
Flags: review?(jduell.mcbugs) → review?(honzab.moz)
Assignee | ||
Comment 44•13 years ago
|
||
Guys, do we have a MOZ_something macro for build of Fennec for Android? It would be simpler to just condition the optimization (and maybe the whole assertion(false) too) code under that #ifdef then to figure out on what platforms are the compiler options correct or not. This will be some 5th patch in this area of code that will be gone ones anyway...
Comment 45•13 years ago
|
||
Hey Honza,
nsHostResolver used to have #ifdef ANDROID to change the values of some #defines for the Android build. AFAIK, it's all android, not just XUL or native builds.
Assignee | ||
Comment 46•13 years ago
|
||
Vivien, can you please use #ifdef ANDROID in your Patch v0.2?
People are seeing compiler errors on gcc 4.4 on desktop that are fixed by this patch, so I don't think we want to limit it to Android.
Comment 48•13 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #47)
> People are seeing compiler errors on gcc 4.4 on desktop that are fixed by
> this patch, so I don't think we want to limit it to Android.
AIUI, he's suggesting to use the pragma on Android only, instead of enabling it on all platforms. However, it seems to me Vivien and others are having the problem when building B2G, and I think that defines ANDROID too.
Assignee | ||
Comment 49•13 years ago
|
||
Comment on attachment 611700 [details] [diff] [review]
Patch v0.2
Review of attachment 611700 [details] [diff] [review]:
-----------------------------------------------------------------
I suggest to wrap this to #ifdef ANDROID.
Josh, I can do that my self if you are out of time. I messed this up.
Attachment #611700 -
Flags: review?(honzab.moz)
Comment 50•13 years ago
|
||
> I suggest to wrap this to #ifdef ANDROID.
Please don't. As said in comment 41, I run into this on desktop Ubuntu 64bit, too. The only way I could compile Firefox and Thunderbird was to apply that attachment.
Comment 51•13 years ago
|
||
Comment on attachment 611700 [details] [diff] [review]
Patch v0.2
see above
Attachment #611700 -
Flags: review?(honzab.moz)
Assignee | ||
Updated•13 years ago
|
Attachment #611700 -
Flags: review?(honzab.moz)
Assignee | ||
Comment 52•13 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #50)
> > I suggest to wrap this to #ifdef ANDROID.
>
> Please don't. As said in comment 41, I run into this on desktop Ubuntu
> 64bit, too.
desktop Ubuntu 64bit defines ANDROID ?
Assignee | ||
Comment 53•13 years ago
|
||
To explain again:
I want to wrap all my added experimental investigation code under #ifdef ANDROID since I search for the cause on Android devices only. Those ABORTS don't even need to be added on any other platform. At least, the optimization pragmas don't need.
Comment 54•13 years ago
|
||
> Review of attachment 611700 [details] [diff] [review] [diff] [review]:
> I suggest to wrap this to #ifdef ANDROID.
Honza, I guess I misunderstood your "this".
As long as
#pragma GCC optimize ("O0")
is not applied on gcc 4.4, we should be fine, yes. I would suggest that you make a comment that this crashes gcc 4.4, though. So:
+#ifdef ANDROID
+// Compiling with a version of GCC <= 4.4 fails with an internal compiler
+// error.
#pragma GCC optimize ("O0")
+#endif
would be fine with me, yes.
Comment 55•13 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #54)
> > Review of attachment 611700 [details] [diff] [review]:
> > I suggest to wrap this to #ifdef ANDROID.
>
> Honza, I guess I misunderstood your "this".
>
> As long as
> #pragma GCC optimize ("O0")
> is not applied on gcc 4.4, we should be fine, yes. I would suggest that you
> make a comment that this crashes gcc 4.4, though. So:
>
> +#ifdef ANDROID
> +// Compiling with a version of GCC <= 4.4 fails with an internal compiler
> +// error.
> #pragma GCC optimize ("O0")
> +#endif
>
> would be fine with me, yes.
That wouldn't be true, since android builds do use gcc 4.4.
Comment 56•13 years ago
|
||
Attachment #611700 -
Attachment is obsolete: true
Attachment #612491 -
Flags: review?(honzab.moz)
Comment 57•13 years ago
|
||
> Vivien, can you please use #ifdef ANDROID in your Patch v0.2?
I did that. Honza, could you please more clear what exactly you want?
Comment 58•13 years ago
|
||
Attachment #612496 -
Flags: review?(honzab.moz)
Comment 59•13 years ago
|
||
Comment on attachment 612496 [details] [diff] [review]
gcc 4.4 internal compiler error, all pragmas on ANDROID only - Fix v4
Review of attachment 612496 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/protocol/http/HttpChannelParent.cpp
@@ +343,3 @@
> #pragma GCC optimize ("O0")
> +#endif
> +#endif
This will effectively disable the pragma on the builds Mozilla produces for Android, since they use gcc 4.4. This is the direct opposite of what we want.
Attachment #612496 -
Flags: review?(honzab.moz) → review-
Comment 60•13 years ago
|
||
I don't know what you want. Can you please just make a patch that does what you want and fixes the compile on desktop? Thanks. All this "no, not that either" doesn't lead anywhere.
Comment 61•13 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #60)
> I don't know what you want. Can you please just make a patch that does what
> you want and fixes the compile on desktop? Thanks. All this "no, not that
> either" doesn't lead anywhere.
I want nothing. It's not even clear to me what is failing and what is not. Are Vivien and other having problems building B2G building for a phone or desktop? (cf. comment 48)
Comment 62•13 years ago
|
||
Well, you said:
> This is the direct opposite of what we want.
My interest is described in its entirety in comment 41.
Comment 63•13 years ago
|
||
Okay, re-reading my message, I understand the "I don't know what you want." better. So here is my take: the pragma is there to avoid PR_Aborts being merged by the compiler, so that problems happening on Android can be debugged more accurately (knowing which PR_Abort is triggered). Anything that will effectively disable the pragma on our Android builds will make that moot.
Now, until it is clear what fails and what doesn't with that pragma, it's hard to go further. Obviously, it's not "gcc 4.4", since Android builds with gcc 4.4.
Comment 64•13 years ago
|
||
On the other hand, if the investigation is only for Android, why is there a specific pragma for msvc ?
Comment 65•13 years ago
|
||
So, do I understand this correctly that you want to set
#pragma GCC optimize ("O0")
if and only if ANDROID or (!) gcc > 4.4.
I.e. if not ANDROID, don't do -O0. If gcc <= 4.4, don't do -O0
?
Comment 66•13 years ago
|
||
> I.e. if not ANDROID, don't do -O0. If gcc <= 4.4, don't do -O0
Ignore that, the last part is wrong. Is this what you want
"if (and only if) ANDROID or (!) gcc > 4.4, set #pragma GCC optimize ("O0")"
?
Comment 67•13 years ago
|
||
(In reply to Ben Bucksch (:BenB) from comment #66)
> > I.e. if not ANDROID, don't do -O0. If gcc <= 4.4, don't do -O0
>
> Ignore that, the last part is wrong. Is this what you want
> "if (and only if) ANDROID or (!) gcc > 4.4, set #pragma GCC optimize ("O0")"
> ?
Honza seems to want if ANDROID set pragma. What I'm saying in comment 48 is that I think B2G does have ANDROID when building for phones, and it's not clear whether that fails or not. If it does, then if ANDROID is not going to change anything for B2G. If it doesn't then fine.
Assignee | ||
Comment 68•13 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #67)
> Honza seems to want if ANDROID set pragma.
Yes.
> What I'm saying in comment 48 is
> that I think B2G does have ANDROID when building for phones, and it's not
> clear whether that fails or not. If it does, then if ANDROID is not going to
> change anything for B2G. If it doesn't then fine.
Do we have anyone that could simply (i.e. with low effort) figure this out?
Assignee | ||
Comment 69•13 years ago
|
||
Comment on attachment 612491 [details] [diff] [review]
gcc 4.4 internal compiler error - Fix v3
Review of attachment 612491 [details] [diff] [review]:
-----------------------------------------------------------------
That's it! Thank you.
::: netwerk/protocol/http/HttpChannelParent.cpp
@@ +335,4 @@
> // merged to a single address.
> #ifdef _MSC_VER
> #pragma warning(disable : 4068)
> #endif
You can probably remove this Windows specific part now.
Please push this to try, no tests needed. (I can do that my self if you can't).
Attachment #612491 -
Flags: review?(honzab.moz) → review+
Comment 70•13 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #68)
> (In reply to Mike Hommey [:glandium] from comment #67)
> > Honza seems to want if ANDROID set pragma.
>
> Yes.
>
> > What I'm saying in comment 48 is
> > that I think B2G does have ANDROID when building for phones, and it's not
> > clear whether that fails or not. If it does, then if ANDROID is not going to
> > change anything for B2G. If it doesn't then fine.
>
> Do we have anyone that could simply (i.e. with low effort) figure this out?
ANDROID is set: see http://mxr.mozilla.org/mozilla-central/source/configure.in#327
What you can use instead is MOZ_WIDGET_ANDROID.
Comment 71•13 years ago
|
||
Please help, i just downloaded the latest copy of firefox 12 and tried compiling on the Ubuntu 10.04 LTS (64-bit), and got the following errors:
http://pastebin.com/J7wgvRNK
Reading the source code's pragma leads me to this present bug thread, how can I continue to compile successfully?
Comment 72•13 years ago
|
||
Comment on attachment 612491 [details] [diff] [review]
gcc 4.4 internal compiler error - Fix v3
Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/c785549b6eb2
Comment 73•13 years ago
|
||
Assignee | ||
Comment 74•13 years ago
|
||
https://crash-stats.mozilla.com/query/query?product=Fennec&product=FennecAndroid&version=ALL%3AALL&range_value=4&range_unit=weeks&date=04%2F18%2F2012+17%3A14%3A26&query_search=signature&query_type=contains&query=HttpChannelParent%3A%3ARecvRedirect2Verify&reason=&build_id=&process_type=any&hang_type=any&do_query=1
doesn't return anything helpful. Either NS_RUNTIMEABORT to force a crash that gets reported doesn't work or the bug has gone. I assume the former is more likely.
If I don't get any useful data soon, I'll just add a null check and close this bug. It's inconsistent behavior, but better get a report of a broken page load we could one day investigate, then to let the application continuously crash.
Assignee | ||
Comment 75•13 years ago
|
||
- removes NS_RUNTIMEABORTS
- leaves some of the investigation logic and provides log
- adds a non-null check before call to mRedirectCallback (workaround)
Assignee | ||
Comment 76•13 years ago
|
||
Comment on attachment 620774 [details] [diff] [review]
fix v1
I remember I was asking for r here when attaching the patch...
Jason will you have time to look at this? Quit simple...
Attachment #620774 -
Flags: review?(jduell.mcbugs)
Updated•13 years ago
|
Attachment #620774 -
Flags: review?(jduell.mcbugs) → review+
Assignee | ||
Comment 77•13 years ago
|
||
Comment on attachment 620774 [details] [diff] [review]
fix v1
https://hg.mozilla.org/integration/mozilla-inbound/rev/d16947d62333
Attachment #620774 -
Flags: checkin+
Assignee | ||
Updated•13 years ago
|
Whiteboard: [mobile-crash] [leave open] → [mobile-crash]
Updated•13 years ago
|
Target Milestone: mozilla13 → mozilla15
Comment 78•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•