820540 - Some new DMD stacks stop at malloc() on Gonk

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

We get a bunch of stacks which start and stop at malloc() on Gonk.

I originally thought this was due to the JIT somehow.

But actually, it looks more prosaic: I think I forgot to turn on -funwind-tables for JS code.

I'm compiling now to see if this fixes the problem.

Comment 1

[Justin Lebar (not reading bugmail)]

JS was definitely part of the problem, and fixed by adding -funwind-tables there.  Cairo appears to be another perpetrator.

Comment 2

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

This fixes all the places we can't trace into, as far as I can tell.

Comment 3

[Justin Lebar (not reading bugmail)]

I found the offenders by running B2G+DMD in GDB and setting a breakpoint tripped when I saw a stack trace of length 1.

Comment 4

[Nicholas Nethercote [inactive]]

Comment on attachment 691031 [details] [diff] [review]
Patch, v1

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

Nice.

In js/src/configure.in, you need to modify the MOZ_DEMANGLE_SYMBOLS definition to match configure.in.  Likewise for MOZ_COMPONENTS_VERSION_SCRIPT_LDFLAGS.

Bonus points if you add the missing "dnl ===..." line above "Enable trace malloc in js/src/configure.in.

Comment 5

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v2

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 691106 [details] [diff] [review]
Patch, v2

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

::: gfx/cairo/cairo/src/Makefile.in
@@ +13,5 @@
>  LIBRARY_NAME    = mozcairo
>  LIBXUL_LIBRARY = 1
>  
>  ifdef GNU_CC
> +OS_CFLAGS := $(filter-out -pedantic,$(OS_CFLAGS))

Ouch

::: js/src/configure.in
@@ +3500,5 @@
> +if test "$NS_TRACE_MALLOC"; then        # trace-malloc disables DMD
> +    MOZ_DMD=
> +fi
> +if test "$MOZ_DMD"; then
> +    USE_ELF_DYNSTR_GC=

You can skip USE_ELF_DYNSTR_GC

@@ +3501,5 @@
> +    MOZ_DMD=
> +fi
> +if test "$MOZ_DMD"; then
> +    USE_ELF_DYNSTR_GC=
> +    AC_DEFINE(MOZ_DMD)

You can skip this.

@@ +3510,5 @@
> +    fi
> +
> +    MOZ_MEMORY=1                        # DMD enables jemalloc
> +    MOZ_REPLACE_MALLOC=1                # DMD enables replace-malloc
> +    MOZ_DMDV=                           # DMD disables DMDV

You can skip these.

@@ +3512,5 @@
> +    MOZ_MEMORY=1                        # DMD enables jemalloc
> +    MOZ_REPLACE_MALLOC=1                # DMD enables replace-malloc
> +    MOZ_DMDV=                           # DMD disables DMDV
> +fi
> +AC_SUBST(MOZ_DMD)

You can skip this.

Comment 7

[Justin Lebar (not reading bugmail)]

>  LIBRARY_NAME    = mozcairo
>  LIBXUL_LIBRARY = 1
>  
>  ifdef GNU_CC
> +OS_CFLAGS := $(filter-out -pedantic,$(OS_CFLAGS))
>
> Ouch

srsly.

Should I worry about landing this on branches, given that we're going to change the compile flags for Cairo?

Comment 8

[Justin Lebar (not reading bugmail)]

> Should I worry about landing this on branches, given that we're going to change the 
> compile flags for Cairo?

I guess it's not as big a deal now, since we've branched from beta.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Justin Lebar [:jlebar] from comment #7)
> Should I worry about landing this on branches, given that we're going to
> change the compile flags for Cairo?

The flags in question are only a bunch of -W (but that includes one -Werror=return-type), -fno-strict-aliasing, -ffunction-sections, -fdata-sections, -pthread, -pipe. Besides -fno-strict-aliasing they are all harmless. I'm tempted to say that if we don't have any problem on m-c, there shouldn't be any problem on branches, provided cairo hasn't changed since 18.

Comment 10

[Justin Lebar (not reading bugmail)]

It looks like there's just one change affecting tier-1 platforms in m-c that isn't on Aurora: Bug 813124.  Scanning the code, it doesn't appear to do any hanky-panky.  So let's say we're probably OK!

Comment 11

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f17233a2e6ab

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f17233a2e6ab

Comment 13

[Justin Lebar (not reading bugmail)]

Comment on attachment 691106 [details] [diff] [review]
Patch, v2

[Triage Comment]
DMD is npotb.

Comment 14

[Justin Lebar (not reading bugmail)]

(In reply to Justin Lebar [:jlebar] from comment #13)
> Comment on attachment 691106 [details] [diff] [review]
> Patch, v2
> 
> [Triage Comment]
> DMD is npotb.

Hm, that's not entirely true in this situation; we're modifying Cairo's build flags.  I still think this is likely safe, however; see comment 10.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1c4fecb5edbb
https://hg.mozilla.org/releases/mozilla-b2g18/rev/3efa3f9cbe15