[armel] non-ISO-C misaligned pointer punning causes slowness and SIGILLs

Bug #494667 reported by Dave Martin
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
squashfs (Ubuntu)
Fix Released
Critical
Unassigned
Lucid
Fix Released
Critical
Unassigned
squashfs-tools (Ubuntu)
Fix Released
Critical
Emmet Hikory
Lucid
Fix Released
Critical
Emmet Hikory

Bug Description

Binary package hint: squashfs-tools

mksquashfs.c currently accesses the following structures through misaligned pointers: at least the following:

squashfs_base_inode_header *
squashfs_dir_entry *
squashfs_dir_header *
squashfs_dir_inode_header *
squashfs_ldir_inode_header *
squashfs_reg_inode_header *
squashfs_symlink_inode_header *
unsigned short *

I haven't checked the rest of this package, but there are likely to be issues elsewhere as well.

Not all memory access instructions can access at unaligned addresses on ARM; GCC assumes that pointers are properly aligned for their type because attempts by a C program to access an lvalue via a pointer which is not properly aligned for the lvalue's type are not permitted under strict interpretation of the C language specification.

Particularly when doing block transfers of whole structures (*obj_p_dest = *obj_p_src) and equivalent operations, LDM or STM instructions are emitted by the compiler: these _always_ fault into the kernel if the base address is unaligned, and can cause a significant performance hit.

In addition, prior to Linux 2.6.31, the kernel cannot successfully emulate all misaligned LDM or STM instructions for Thumb ... leading to the SIGILLs which have been observed on the lucid buildds (2.6.28 kernel) when moving to v7.

Possible workarounds:
  * Build squashfs-tools with -marm
  * fix squashfs-tools to use strict C only (laborious... either build squashfs filesystem structures at true alignment and then copy them into place in the output, or
  * declare all the affected types with __attribute__ (( __packed__ )) so GCC knows the structures may be accessed at misaligned locations (care must be taken to make sure the structures' sizes and internal arrangement do not change as a result of this, and the resulting code may run significantly slower --- I've not checked for the implications of this. Some other check is needed for unsigned short, or it must be wrapped in a structure or union type, because GCC cannot tag basic types with the __packed__ attribute)
  * apply the appropriate LDM/STM emulation patches to the kernel used on the buildds (if feasible --- I'll try and feed back on this)

Tags: armel armv7
description: updated
Revision history for this message
Dave Martin (dave-martin-arm) wrote :

I tried experimentally buiding this package for -marm, and this seems to solve the SIGILLs on 2.6.28 kernels.

Running mksquash and unsquash on /etc now works without SIGILL, for example.

Revision history for this message
Alexander Sack (asac) wrote :

this seems to happen with 2.6.28 kernel, workaround until we have our builders on newer kernel is to build with -marm.

Assigning to David to prepare the debdiff for sponsoring.

Changed in squashfs (Ubuntu):
assignee: nobody → David Sugar (dyfet)
importance: Undecided → Critical
milestone: none → lucid-alpha-2
status: New → Triaged
Revision history for this message
David Sugar (dyfet-deactivatedaccount) wrote :

This adds a conditional in the Makefile to use -marm when DEB_BUILD_ARCH_CPU is arm, which does for arm as suggested by David Martin. The lucid version of squashfs was still debian upstream. I am not sure if we wish to push this back to debian at all, or even if we later want to address this in a better way, so I assume we are for now keeping as a delta, at least for present production needs.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package squashfs - 1:4.0-3ubuntu1

---------------
squashfs (1:4.0-3ubuntu1) lucid; urgency=high

  * Temporary conditional -marm fix for arm7 gcc build issue (LP: #494667),
    required for current lucid production.
 -- David Sugar <email address hidden> Fri, 11 Dec 2009 14:44:00 -0500

Changed in squashfs (Ubuntu Lucid):
status: Triaged → Fix Released
Revision history for this message
Michael Casadevall (mcasadevall) wrote :

The debdiff is invalid, build architecture for ARM is armel, not arm, which will fail to pass -marm on the resulting binaries.

Changed in squashfs (Ubuntu Lucid):
status: Fix Released → In Progress
Revision history for this message
Loïc Minier (lool) wrote :

Michael, the debdiff uses _ARCH_CPU which is arm on armel; the actual issue is using _BUILD instead of _HOST.

Revision history for this message
Loïc Minier (lool) wrote :

-marm is passed when building natively and I see it in the build log

Revision history for this message
Daniel Holbach (dholbach) wrote :

Unsubscribing ubuntu-core-dev.

Revision history for this message
Dave Martin (dave-martin-arm) wrote :

> This adds a conditional in the Makefile to use -marm when
> DEB_BUILD_ARCH_CPU is arm, which does for arm as suggested by
> David Martin. The lucid version of squashfs was still debian
> upstream. I am not sure if we wish to push this back to
> debian at all, or even if we later want to address this in a
> better way, so I assume we are for now keeping as a delta, at
> least for present production needs.

I think it makes sense to keep it as a delta--- the need for it should go away in the future once the kernels for all platforms are sufficiently new.

Revision history for this message
Loïc Minier (lool) wrote :

Nothing to sponsor right now, unsub-ing main-sponsors

Revision history for this message
Emmet Hikory (persia) wrote :

The attached patch applies a substantially similar fix to the squashfs-tools package (using -marm for arm)

Changed in squashfs-tools (Ubuntu Lucid):
status: New → Triaged
importance: Undecided → Critical
assignee: nobody → Emmet Hikory (persia)
milestone: none → lucid-alpha-2
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package squashfs-tools - 1:4.0-6ubuntu1

---------------
squashfs-tools (1:4.0-6ubuntu1) lucid; urgency=low

  * Build with -marm to avoid using Thumb2 instructions
      (Older kernels don't support these instructions: LP: #494667)
 -- Emmet Hikory <email address hidden> Thu, 17 Dec 2009 23:04:46 +0900

Changed in squashfs-tools (Ubuntu Lucid):
status: Triaged → Fix Released
Revision history for this message
Alexander Sack (asac) wrote :

was fix released for squashfs source ... the -marm flag was confirmed to work afaict.

Changed in squashfs (Ubuntu Lucid):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.