Closed Bug 1060115 Opened 8 years ago Closed 6 years ago

B2G Desktop from VS2013 doesn't work on XP SP2

Categories

(Firefox Build System :: General, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: away, Assigned: ehsan.akhgari)

References

Details

Attachments

(2 files, 2 obsolete files)

See bug 1023941 comment 32. The summary is that B2G Desktop from VS2013 won't work on XP SP2 unless it stops calling malloc from the main exe (from DLLs is fine).

For b2g.exe, the only use of malloc are the strdup/free on argv: http://hg.mozilla.org/mozilla-central/file/47c9418fbc28/b2g/app/nsBrowserApp.cpp#l280 

We can probably just work around those mallocs. Or maybe we don't care.
Does this matter? I hear that Mulet is going to replace B2G Desktop anyway.
Flags: needinfo?(poirot.alex)
I don't think this matters, we don't ship these to end users, they're just for app developers for testing (and AIUI they are going to be replaced by Mulet).
(In reply to David Major [:dmajor] from comment #0)
> See bug 1023941 comment 32. The summary is that B2G Desktop from VS2013
> won't work on XP SP2 unless it stops calling malloc from the main exe (from
> DLLs is fine).

No B2G developer should be running XP (any SP) anyway, because it is a non-secure and non-supported operating system.
b2g desktop isn't only for b2g developers but also for apps developers using the simulator (which is based on b2g desktop).
But I'm not sure we really care if it never worked as we never got any report from simulator users about not supporting XP.

Otherwise, yes, mulet is meant to replace b2g desktop. We don't have a precise timeframe about when it will happen yet.
Flags: needinfo?(poirot.alex)
> we never got any report from simulator users about not supporting XP.

Well, you haven't received complaints because we're not building with VS2013 yet. But in general I agree that this does not seem like a priority.
But fixing it is *so simple*.  I think we should just fix it.
Assignee: nobody → ehsan.akhgari
Attachment #8484445 - Flags: review?(dmajor)
The other day I learned another disturbing thing: |operator new| does not count as a use of malloc, for purposes of deciding whether the exe gets mozglue. That explains why [1] doesn't mess up firefox.exe. How would you feel about using |new| to avoid the ifdefs?

[1] http://hg.mozilla.org/mozilla-central/annotate/da04cf3f8a06/toolkit/xre/nsWindowsWMain.cpp#l92
Flags: needinfo?(ehsan.akhgari)
lol, OK I'll do that.
Flags: needinfo?(ehsan.akhgari)
Attachment #8484445 - Attachment is obsolete: true
Attachment #8484445 - Flags: review?(dmajor)
Attachment #8484575 - Attachment is obsolete: true
Attachment #8484576 - Flags: review?(dmajor)
Comment on attachment 8484576 [details] [diff] [review]
Avoid using malloc/free in b2g.exe on b2g desktop so that we can make it work on XPSP2 with MSVC2013; r=dmajor

Review of attachment 8484576 [details] [diff] [review]:
-----------------------------------------------------------------

To get b2g.exe fully fixed you'll also need to port the build stuff in Attachment #8475707 [details] [diff].

::: build/clang-plugin/clang-plugin.cpp
@@ +94,5 @@
> +    "angle",
> +    "harfbuzz",
> +    "hunspell",
> +    "scoped_ptr.h",
> +    "graphite2"

Wrong patch?
Attachment #8484576 - Flags: review?(dmajor) → review+
(In reply to David Major [:dmajor] from comment #12)
> Comment on attachment 8484576 [details] [diff] [review]
> Avoid using malloc/free in b2g.exe on b2g desktop so that we can make it
> work on XPSP2 with MSVC2013; r=dmajor
> 
> Review of attachment 8484576 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> To get b2g.exe fully fixed you'll also need to port the build stuff in
> Attachment #8475707 [details] [diff] [diff].

Do you mind taking care of that part, please?  I don't have an XP SP2 machine handy for testing...

> ::: build/clang-plugin/clang-plugin.cpp
> @@ +94,5 @@
> > +    "angle",
> > +    "harfbuzz",
> > +    "hunspell",
> > +    "scoped_ptr.h",
> > +    "graphite2"
> 
> Wrong patch?

Hah, yeah, I wrote this in a hurry.  Sorry!
Keywords: leave-open
Attached patch This didn't workSplinter Review
This patch wasn't enough. There were some more malloc calls that were only exposed once we removed the strdup. I don't really have time to take this further.

|link -verbose| blames nsStringAPI.obj:

      Found _free
        Referenced in xpcomglue_staticruntime.lib(nsStringAPI.obj)
        Loaded mozglue.lib(mozglue.dll)
      Found _malloc
        Referenced in xpcomglue_staticruntime.lib(nsStringAPI.obj)
        Loaded mozglue.lib(mozglue.dll)
      Found _realloc
        Referenced in xpcomglue_staticruntime.lib(nsStringAPI.obj)
        Loaded mozglue.lib(mozglue.dll)
OK let's have this bug open until someone else complains. :)
I predict nobody will, considering the target audience.
I think we can safely call this wontfix now :)
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.