1082817 - Conditionalize exidx sorting in ARM EH ABI unwinder

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

The ARM exception handling ABI requires that the exception index be sorted by the referenced address, so that an unwinder can use in-place binary search, but the linker we're using on ICS B2G is buggy and doesn't always ensure that.

tools/profiler/EHABIStackWalk.cpp has a workaround, by contructing a meta-index of pointers to index entries and sorting that.  This wastes about 1/2 MiB of memory per process where the profiler has been started — even if native stack walking isn't being used, and even on newer B2Gs where the linker is fixed.

To put this in context, actually using the stack unwinder will fault in the exception tables, and there's about 1.3 MiB of that in the typical B2G process — but it would be shared among all processes using it, and it's subject to demand paging and eviction.

So, at the very least this could be made conditional on ANDROID_VERSION, so that JB/KK devices don't have to pay the small but nonzero cost.  It will also make the unwinder somewhat faster, but it's not particularly slow as it is.

(And to preemptively answer the question of "what about LUL": its memory usage is much higher, the last I heard; and fixing this isn't much actual work.)

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug1082817-exidx-unsort-hg0.diff

Tested locally on keon and flame-kk.  (I haven't tried to measure the change in heap-unclassified yet, but it does reduce the compiled code size from about 5.0 KiB to 3.8 KiB.)

Comment 2

[Benoit Girard (:BenWa)]

Comment on attachment 8507325 [details] [diff] [review]
bug1082817-exidx-unsort-hg0.diff

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

::: tools/profiler/EHABIStackWalk.cpp
@@ +42,5 @@
>  #define PT_ARM_EXIDX 0x70000001
>  #endif
>  
> +#if defined(ANDROID_VERSION) && ANDROID_VERSION < 16
> +#define HAVE_UNSORTED_EXIDX

Can you put a comment similar to Comment 0 in the code here so that people don't need to hg blame to understand what this?

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug1082817-exidx-unsort-hg1.diff

Now with added comment; thanks for pointing that out.  Carrying over r+.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

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

Comment 5

[Carsten Book [:Tomcat]]

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