The default bug view has changed. See this FAQ.

Linker needs to align to 16k boundaries on armv6

RESOLVED FIXED in mozilla13

Status

()

Core
mozglue
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ted, Assigned: glandium)

Tracking

unspecified
mozilla13
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

5 years ago
After some detective work by glandium, sewardj, and jbramley, we found out that the reason the linker is failing on my armv6 builds is because there's a 16k alignment requirement.
(Assignee)

Updated

5 years ago
Component: General → mozglue
Product: Fennec Native → Core
QA Contact: general → mozglue
(Assignee)

Comment 1

5 years ago
Created attachment 595355 [details] [diff] [review]
When reserving memory for the loaded library, ensure correct alignment for future MAP_SHARED mappings
Attachment #595355 - Flags: review?(taras.mozilla)
(Assignee)

Comment 2

5 years ago
As mentioned on IRC, another part of the problem is that the static linker doesn't help either, and we want it to align segments at 16k boundaries (that is, we need (p_vaddr - p_offset) % 16384 == 0).
This can be done with -Wl,-z,max-page-size=0x4000 in LDFLAGS. I'm not completely sure which of configure.in or mozconfig this should live in.
(Reporter)

Comment 3

5 years ago
I think we should automatically add that to LDFLAGS in configure if we're targeting Android-armv6.
(Assignee)

Comment 4

5 years ago
armv6 or less, since armv5 code can run on armv6.
(Reporter)

Updated

5 years ago
Depends on: 725284

Comment 5

5 years ago
Comment on attachment 595355 [details] [diff] [review]
When reserving memory for the loaded library, ensure correct alignment for future MAP_SHARED mappings

seems harmless
Attachment #595355 - Flags: review?(taras.mozilla) → review+
(Assignee)

Comment 6

5 years ago
Mmmmm interestingly, this breaks the linker on desktop :-/
(Assignee)

Comment 7

5 years ago
Created attachment 597384 [details] [diff] [review]
When reserving memory for the loaded library, ensure correct alignment for future MAP_SHARED mappings

I'm not entirely sure why it was breaking desktop and not android, but there seems to be pretty bad side effects keeping the mapping MAP_SHARED. So once we get a base address that can hold the library and that will work with MAP_SHARED later, just remap it MAP_PRIVATE instead.
Attachment #597384 - Flags: review?(taras.mozilla)
(Assignee)

Updated

5 years ago
Attachment #595355 - Attachment is obsolete: true
(Assignee)

Comment 8

5 years ago
Created attachment 597402 [details] [diff] [review]
part 2 - Ensure 16k alignment of ELF segments when building for ARM < v7
Attachment #597402 - Flags: review?(ted.mielczarek)

Updated

5 years ago
Attachment #597384 - Flags: review?(taras.mozilla) → review+
(Assignee)

Comment 9

5 years ago
Created attachment 597711 [details] [diff] [review]
part 2 - Ensure 16k alignment of ELF segments when building for ARM < v7

There were single quotes instead of double quotes in the previous patch
Attachment #597711 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #597402 - Attachment is obsolete: true
Attachment #597402 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 10

5 years ago
Created attachment 597872 [details] [diff] [review]
part 2 - Ensure 16k alignment of ELF segments when building for ARM < v7

This applies the flag to nspr, too.
Attachment #597872 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #597711 - Attachment is obsolete: true
Attachment #597711 - Flags: review?(ted.mielczarek)
(Reporter)

Comment 11

5 years ago
Comment on attachment 597872 [details] [diff] [review]
part 2 - Ensure 16k alignment of ELF segments when building for ARM < v7

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

::: configure.in
@@ +4091,5 @@
> +dnl Using the custom linker on ARMv6 requires 16k alignment of ELF segments.
> +if test -n "$MOZ_LINKER"; then
> +  if test "$CPU_ARCH" = arm; then
> +    dnl Determine the target ARM architecture (5 for ARMv5, v5T, v5E, etc.; 6 for ARMv6, v6K, etc.)
> +    ARM_ARCH=`${CC-cc} ${CFLAGS} -dM -E - < /dev/null | sed -n 's/.*__ARM_ARCH_\([[0-9]]*\).*/\1/p'`

Kind of awkward, but I don't have a better suggestion.

@@ +4095,5 @@
> +    ARM_ARCH=`${CC-cc} ${CFLAGS} -dM -E - < /dev/null | sed -n 's/.*__ARM_ARCH_\([[0-9]]*\).*/\1/p'`
> +    dnl When building for < ARMv7, we need to ensure 16k alignment of ELF segments
> +    if test -n "$ARM_ARCH" && test "$ARM_ARCH" -lt 7; then
> +      LDFLAGS="$LDFLAGS -Wl,-z,max-page-size=0x4000"
> +      _SUBDIR_LDFLAGS="$LDFLAGS"

Do we really want to stick all of our LDFLAGS in SUBDIR_LDFLAGS, or just this one thing?
Attachment #597872 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a9d1df1c762
https://hg.mozilla.org/integration/mozilla-inbound/rev/a69b347c4d35
https://hg.mozilla.org/mozilla-central/rev/8a9d1df1c762
https://hg.mozilla.org/mozilla-central/rev/a69b347c4d35
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.