1060115 - B2G Desktop from VS2013 doesn't work on XP SP2

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[(Away)]

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.

Comment 1

[(Away)]

Does this matter? I hear that Mulet is going to replace B2G Desktop anyway.

Comment 2

[(not currently active) Ted Mielczarek]

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).

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(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.

Comment 4

[Alexandre Poirot [:ochameau]]

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.

Comment 5

[(Away)]

> 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.

Comment 6

[(no longer active)]

But fixing it is *so simple*.  I think we should just fix it.

Comment 7

[(no longer active)]

Attachment: Avoid using malloc/free in b2g.exe on b2g desktop so that we can make it work on XPSP2 with MSVC2013; r=dmajor

Comment 8

[(Away)]

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

Comment 9

[(no longer active)]

lol, OK I'll do that.

Comment 10

[(no longer active)]

Attachment: Avoid using malloc/free in b2g.exe on b2g desktop so that we can make it work on XPSP2 with MSVC2013; r=dmajor

Comment 11

[(no longer active)]

Attachment: Avoid using malloc/free in b2g.exe on b2g desktop so that we can make it work on XPSP2 with MSVC2013; r=dmajor

Comment 12

[(Away)]

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?

Comment 13

[(no longer active)]

(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!

Comment 14

[(no longer active)]

https://hg.mozilla.org/integration/b2g-inbound/rev/0dac04937a65

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0dac04937a65

Comment 16

[(Away)]

Attachment: This didn't work

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)

Comment 17

[(no longer active)]

OK let's have this bug open until someone else complains. :)

Comment 18

[Yuhong Bao]

I predict nobody will, considering the target audience.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

I think we can safely call this wontfix now :)

Comment 20

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.