Last Comment Bug 621446 - Crash [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify ]
: Crash [@ mozilla::net::HttpChannelParent::RecvRedirect2Verify ]
Status: RESOLVED FIXED
[mobile-crash]
: crash
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: ARM Android
: -- critical with 1 vote (vote)
: mozilla15
Assigned To: Honza Bambas (:mayhemer)
:
:
Mentors:
Depends on: 721502
Blocks: 741104 745296
  Show dependency treegraph
 
Reported: 2010-12-26 09:04 PST by Josh Matthews [:jdm]
Modified: 2012-05-16 03:54 PDT (History)
17 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
investigation patch (4.99 KB, patch)
2011-12-13 13:44 PST, Honza Bambas (:mayhemer)
jduell.mcbugs: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta-
honzab.moz: checkin+
Details | Diff | Splinter Review
Investigation patch update (1.19 KB, patch)
2012-02-16 13:00 PST, Honza Bambas (:mayhemer)
jduell.mcbugs: review+
akeybl: approval‑mozilla‑aurora+
honzab.moz: checkin+
Details | Diff | Splinter Review
followup to fix build warning from investigation patch (1.10 KB, patch)
2012-03-13 14:05 PDT, Daniel Holbert [:dholbert]
jduell.mcbugs: review+
Details | Diff | Splinter Review
Patch (1.45 KB, patch)
2012-03-16 08:08 PDT, Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
jduell.mcbugs: review+
Details | Diff | Splinter Review
Patch v0.2 (1.49 KB, patch)
2012-04-02 19:48 PDT, Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
no flags Details | Diff | Splinter Review
gcc 4.4 internal compiler error - Fix v3 (1.21 KB, patch)
2012-04-05 03:37 PDT, Ben Bucksch (:BenB)
honzab.moz: review+
Details | Diff | Splinter Review
gcc 4.4 internal compiler error, all pragmas on ANDROID only - Fix v4 (1.30 KB, patch)
2012-04-05 03:53 PDT, Ben Bucksch (:BenB)
mh+mozilla: review-
Details | Diff | Splinter Review
fix v1 (3.13 KB, patch)
2012-05-03 10:55 PDT, Honza Bambas (:mayhemer)
jduell.mcbugs: review+
honzab.moz: checkin+
Details | Diff | Splinter Review

Description Josh Matthews [:jdm] 2010-12-26 09:04:54 PST
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);
Comment 1 Honza Bambas (:mayhemer) 2010-12-28 13:23:20 PST
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.
Comment 2 Honza Bambas (:mayhemer) 2011-01-03 11:34:52 PST
Josh, ping for answers on comment 1, thanks.
Comment 3 Josh Matthews [:jdm] 2011-01-03 11:43:02 PST
Sorry, this isn't a crash that I received, it just came up in the daily crash reports I was looking at.
Comment 5 Honza Bambas (:mayhemer) 2011-01-03 12:00:19 PST
Thanks.
Comment 6 Honza Bambas (:mayhemer) 2011-01-19 10:31:13 PST
This is not that rare crash in Fennec.  I think we should investigate.
Comment 7 Honza Bambas (:mayhemer) 2011-01-23 14:38:18 PST
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 :(

*** This bug has been marked as a duplicate of bug 591707 ***
Comment 8 Josh Matthews [:jdm] 2011-02-05 09:43:22 PST
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.
Comment 9 Honza Bambas (:mayhemer) 2011-02-07 06:24:16 PST
I didn't find this report.  Thanks Josh, I'll try to figure it out.
Comment 10 Stuart Parmenter 2011-02-09 11:43:58 PST
Very low occurrences on crash-stats, but would like to get a fix in
Comment 12 Robert Kaiser 2011-10-31 08:04:28 PDT
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.
Comment 13 Honza Bambas (:mayhemer) 2011-12-13 13:44:06 PST
Created attachment 581409 [details] [diff] [review]
investigation patch

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.
Comment 14 Honza Bambas (:mayhemer) 2011-12-13 13:46:22 PST
Comment on attachment 581409 [details] [diff] [review]
investigation patch

https://tbpl.mozilla.org/?tree=Try&rev=a7e5ed5f73aa
Comment 15 Jason Duell [:jduell] (needinfo me) 2012-01-05 19:50:23 PST
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"
Comment 16 Honza Bambas (:mayhemer) 2012-01-13 07:37:51 PST
Comment on attachment 581409 [details] [diff] [review]
investigation patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/63c3d103610c
Comment 18 Honza Bambas (:mayhemer) 2012-01-14 06:34:42 PST
This was not a fix.  Just a debugging patch.
Comment 19 Honza Bambas (:mayhemer) 2012-01-20 14:00:14 PST
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.
Comment 20 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-01-23 07:24:24 PST
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 Alex Keybl [:akeybl] 2012-01-23 08:42:43 PST
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.
Comment 22 Honza Bambas (:mayhemer) 2012-01-24 15:39:26 PST
(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
Comment 23 Honza Bambas (:mayhemer) 2012-01-24 16:22:15 PST
Comment on attachment 581409 [details] [diff] [review]
investigation patch

https://hg.mozilla.org/releases/mozilla-aurora/rev/8aaa33c0d915
Comment 24 Honza Bambas (:mayhemer) 2012-02-16 13:00:04 PST
Created attachment 597971 [details] [diff] [review]
Investigation patch update

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.
Comment 25 Honza Bambas (:mayhemer) 2012-02-17 07:25:03 PST
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.
Comment 26 Ed Morley [:emorley] 2012-02-17 17:59:06 PST
https://hg.mozilla.org/mozilla-central/rev/5002199145a9
Comment 27 Honza Bambas (:mayhemer) 2012-02-19 09:26:08 PST
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.
Comment 28 Glenn Randers-Pehrson 2012-02-20 07:22:49 PST
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 Alex Keybl [:akeybl] 2012-02-21 11:04:23 PST
Comment on attachment 597971 [details] [diff] [review]
Investigation patch update

[Triage Comment]
Approved for Aurora 12.
Comment 30 Honza Bambas (:mayhemer) 2012-02-23 10:00:06 PST
Comment on attachment 597971 [details] [diff] [review]
Investigation patch update

https://hg.mozilla.org/releases/mozilla-aurora/rev/010814586207
Comment 31 Daniel Holbert [:dholbert] 2012-03-13 14:05:45 PDT
Created attachment 605532 [details] [diff] [review]
followup to fix build warning from investigation patch

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.
Comment 32 Daniel Holbert [:dholbert] 2012-03-13 14:58:47 PDT
Pushed followup to fix build warning:
http://hg.mozilla.org/integration/mozilla-inbound/rev/90f6b58ce0fa
Comment 33 Ed Morley [:emorley] 2012-03-14 14:35:30 PDT
(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
Comment 34 Honza Bambas (:mayhemer) 2012-03-15 15:30:49 PDT
Thanks, Daniel.
Comment 35 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-03-16 08:08:30 PDT
Created attachment 606580 [details] [diff] [review]
Patch

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).
Comment 36 Guillermo López :willyaranda (probably SLOW response) 2012-03-16 13:25:16 PDT
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 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-03-16 13:35:16 PDT
(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 Guillermo López :willyaranda (probably SLOW response) 2012-03-16 13:40:55 PDT
(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 Jason Duell [:jduell] (needinfo me) 2012-03-16 15:30:57 PDT
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.
Comment 40 Honza Bambas (:mayhemer) 2012-03-19 10:11:21 PDT
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 Ben Bucksch (:BenB) 2012-04-01 12:10:27 PDT
(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 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-04-02 19:48:21 PDT
Created attachment 611700 [details] [diff] [review]
Patch v0.2

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.
Comment 43 Jason Duell [:jduell] (needinfo me) 2012-04-03 14:32:00 PDT
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.
Comment 44 Honza Bambas (:mayhemer) 2012-04-03 15:58:58 PDT
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 Steve Workman [:sworkman] (INACTIVE) 2012-04-03 16:40:08 PDT
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.
Comment 46 Honza Bambas (:mayhemer) 2012-04-03 16:59:25 PDT
Vivien, can you please use #ifdef ANDROID in your Patch v0.2?
Comment 47 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-04-03 22:32:18 PDT
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 Mike Hommey [:glandium] 2012-04-03 23:08:32 PDT
(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.
Comment 49 Honza Bambas (:mayhemer) 2012-04-04 16:46:26 PDT
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.
Comment 50 Ben Bucksch (:BenB) 2012-04-04 16:50:56 PDT
> 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 Ben Bucksch (:BenB) 2012-04-04 16:52:08 PDT
Comment on attachment 611700 [details] [diff] [review]
Patch v0.2

see above
Comment 52 Honza Bambas (:mayhemer) 2012-04-04 17:00:51 PDT
(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 ?
Comment 53 Honza Bambas (:mayhemer) 2012-04-04 17:02:46 PDT
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 Ben Bucksch (:BenB) 2012-04-05 03:31:15 PDT
> 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 Mike Hommey [:glandium] 2012-04-05 03:33:06 PDT
(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 Ben Bucksch (:BenB) 2012-04-05 03:37:43 PDT
Created attachment 612491 [details] [diff] [review]
gcc 4.4 internal compiler error - Fix v3
Comment 57 Ben Bucksch (:BenB) 2012-04-05 03:46:01 PDT
> 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 Ben Bucksch (:BenB) 2012-04-05 03:53:11 PDT
Created attachment 612496 [details] [diff] [review]
gcc 4.4 internal compiler error, all pragmas on ANDROID only - Fix v4
Comment 59 Mike Hommey [:glandium] 2012-04-05 04:32:44 PDT
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.
Comment 60 Ben Bucksch (:BenB) 2012-04-05 04:48:00 PDT
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 Mike Hommey [:glandium] 2012-04-05 04:56:27 PDT
(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 Ben Bucksch (:BenB) 2012-04-05 04:59:27 PDT
Well, you said:
> This is the direct opposite of what we want.

My interest is described in its entirety in comment 41.
Comment 63 Mike Hommey [:glandium] 2012-04-05 05:01:11 PDT
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 Mike Hommey [:glandium] 2012-04-05 05:03:42 PDT
On the other hand, if the investigation is only for Android, why is there a specific pragma for msvc ?
Comment 65 Ben Bucksch (:BenB) 2012-04-05 05:17:00 PDT
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 Ben Bucksch (:BenB) 2012-04-05 05:18:37 PDT
> 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 Mike Hommey [:glandium] 2012-04-05 05:29:45 PDT
(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.
Comment 68 Honza Bambas (:mayhemer) 2012-04-05 11:37:09 PDT
(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?
Comment 69 Honza Bambas (:mayhemer) 2012-04-05 13:21:54 PDT
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).
Comment 70 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-04-06 02:43:04 PDT
(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 Peter Teoh 2012-04-08 19:09:53 PDT
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 Ben Bucksch (:BenB) 2012-04-09 02:13:18 PDT
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 74 Honza Bambas (:mayhemer) 2012-04-18 10:24:47 PDT
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.
Comment 75 Honza Bambas (:mayhemer) 2012-05-03 10:55:07 PDT
Created attachment 620774 [details] [diff] [review]
fix v1

- removes NS_RUNTIMEABORTS
- leaves some of the investigation logic and provides log
- adds a non-null check before call to mRedirectCallback (workaround)
Comment 76 Honza Bambas (:mayhemer) 2012-05-14 11:16:59 PDT
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...
Comment 78 Ed Morley [:emorley] 2012-05-16 03:54:55 PDT
https://hg.mozilla.org/mozilla-central/rev/d16947d62333

Note You need to log in before you can comment on or make changes to this bug.