[vm/ffi] Non-aligned reads and writes

[dcharkes]

The getters and setters on Pointer<Double> etc, require memory to be aligned.

sdk/sdk/lib/ffi/ffi.dart

Lines 475 to 480 in 4216dc8

	/// The double at [address].
	///
	/// The [address] must be 8-byte aligned.
	external double get value;
	external void set value(double value);

Attempting to read/write with a non-aligned pointer causes a segmentation fault on arm32-Android. (On many other hardware/OS combinations it seems to be fine or just slower.)

We currently do not provide a way to do non-aligned reads and writes.

(The workaround is to manually turn the double into 8 uint8's, and write those.)

We should add unaligned reads/writes.

   /// The double at [address]. 
   /// 
   /// The [address] must be 8-byte aligned. Use [unalignedValue] for unaligned access.
   external double get value; 
  
   external void set value(double value); 

   /// The double at [address]. 
   /// 
   /// The [address] may be unaligned. Use [value] for aligned access.
   external double get unalignedValue; 
  
   external void set unalignedValue(double unalignedValue); 

@lrhn unalignedValue getter and setter as naming? wdyt?

Should we add unaligned access for index operators as well? That might not be that useful.

sdk/sdk/lib/ffi/ffi.dart

Lines 482 to 490 in 4216dc8

	/// The double at `address + 8 * index`.
	///
	/// The [address] must be 8-byte aligned.
	external double operator [](int index);
	/// The double at `address + 8 * index`.
	///
	/// The [address] must be 8-byte aligned.
	external void operator []=(int index, double value);

We need this feature for interacting with packed structs #38158.

[lrhn]

What are the failure modes?

If you read value and the pointer was not aligned, it throws/dumps core/whatever low-level failure the hardware provides.
If you read unaligned value, it's guaranteed to read at any address point, but might be slower. (What ByteData.getFloat64 does, vs. the aligned Float64List).
You don't want to pre-check the direct read. The slow/safe-mode can probably check whether the pointer is aligned and do a quick read, and still be faster on average than not checking.

What are the alternatives?

• Always allowing slower unaligned reads (requires a check first to see if it's aligned, so not good).
• A separate type (UnalignedDouble) with a different read/write mode. You could then also check that the pointer is aligned when constructing the Pointer<Double>, and throw if it isn't, but that's still an overhead on aligned pointers.
• This. (The name is probably fine, but it's slightly weird that it's a negative name, and the positive one is unused.)
• The opposite. (value reads unaligned, alignedValue reads only aligned values. Naming is better. Usability ... depends on what people usually do, which is probably mostly aligned values.)

It's an issue that intel CPUs won't complain about unaligned pointers being read. That means that many users won't ever need to use unalignedValue, and won't even notice that it's slower. Then their code fails to work on ARM.

I'd consider the UnalignedDouble type approach. Then you have to choose up-front whether you want your double to be readable unaligned or not, and it will have the same API as all other pointers.

I think that's potentially a more consistent API than adding extra methods to all more-than-one-byte types.

It matches the typed-array story of having aligned Float64List and general unaligned reads using ByteData, so if a Pointer<Double> can be converted to a Float64List, then a Pointer<UnalignedDouble> should only be convertible to ByteData.

[mkustermann]

@dcharkes Could you be more clear about where unaligned reads/writes are not supported?

We have landed cl/152322, which removed the last support for ARMv6. ARMv7 requires support for unaligned reads and writes afaik.

/cc @mraleph Correct me if I'm wrong.

[dcharkes]

Only ARMv7 with the testing I've done locally (that does not include all OS/hardware combinations yet).

Always allowing slower unaligned reads (requires a check first to see if it's aligned, so not good).

I'll make a CL for this to see what is the impact on arm.

[mraleph]

A3.2.1 Unaligned data access in ARM Architecture Reference Manual ARMv7-A and ARMv7-R edition specifies that VLDR and VSTR require at least word aligned addresses otherwise they trigger Alignment fault independent of SCTLR.A state. That's a bummer.

To be honest I think we should just do what C/C++ would do a segfault.

[mkustermann]

@mraleph Some instructions still require alignment, that's true (also for ldm, ...). Though in general the CPU supports it. So maybe in cases where we don't know whether data is aligned we can use a set of instructions that do support unaligned access.

[dcharkes]

For packed structs we need support for unaligned reads/writes or our Struct API will segfault.

#pragma pack(push, 1)
struct Struct9BytesPackedMixed {
  uint8_t a0;
  double a1;
};
#pragma pack(pop)

Dart API

@Packed(1)
class Struct9BytesPackedMixed extends Struct {
  @Uint8()
  external int a0;

  @Double()
  external double a1; // This getter/setter needs to do an unaligned read/write.
}

[mraleph]

Yeah I agree that we should support reading unaligned data from packed structs - basically follow how C does it: normally it assumes aligned access, except for certain cases like packed structs.

For convenience we can also have either unaligned getters or other APIs, but I would suggest against making Pointers themselves defensive against misalignment.

[mkustermann]

Out of curiosity, here's the code our C compiler generates for packed (and therefore possibly unaligned) float/double members.

typedef struct __attribute__((packed)) {
    char _;
    double d;
    float f;
    char __;
} Foo ;

int getSize() {
  return sizeof(Foo);
}
double getDouble(Foo* f) {
  return f->d;
}
float getFloat(Foo* f) {
  return f->f;
}

In normal arm abi:

% buildtools/linux-x64/clang/bin/clang --target=armv7-linux-gnueabi --sysroot=build/linux/debian_jessie_arm-sysroot -c -O3 -o test test.c
% arm-linux-gnueabihf-objdump -d test
00000000 <getSize>:
   0:   e3a0000e        mov     r0, #14
   4:   e12fff1e        bx      lr

00000008 <getDouble>:
   8:   e2800001        add     r0, r0, #1
   c:   f460070f        vld1.8  {d16}, [r0]
  10:   ec510b30        vmov    r0, r1, d16
  14:   e12fff1e        bx      lr

00000018 <getFloat>:
  18:   e5900009        ldr     r0, [r0, #9]
  1c:   e12fff1e        bx      lr

And hard-fp abi:

% buildtools/linux-x64/clang/bin/clang --target=armv7-linux-gnueabihf --sysroot=build/linux/debian_jessie_arm-sysroot  -O3 -c -o test test.c

% arm-linux-gnueabihf-objdump -d test
00000000 <getSize>:
   0:   e3a0000e        mov     r0, #14
   4:   e12fff1e        bx      lr

00000008 <getDouble>:
   8:   e2800001        add     r0, r0, #1
   c:   f420070f        vld1.8  {d0}, [r0]
  10:   e12fff1e        bx      lr

00000014 <getFloat>:
  14:   e5900009        ldr     r0, [r0, #9]
  18:   ee000a10        vmov    s0, r0
  1c:   e12fff1e        bx      lr

The ARM ISA mentions:

A3.2.1 Unaligned data access

An ARMv7 implementation must support unaligned data accesses by some load and store instructions, as
Table A3-1 shows. Software can set the SCTLR.A bit to control whether a misaligned access by one of these
instructions causes an Alignment fault Data Abort exception.

| Instructions Alignment  | check        | SCTLR.A is 0     | SCTLR.A is 1    |
-------------------------------------------------------------------------------
| VLD1 ...                | Element size | Unaligned access | Alignment fault |

[dcharkes]

We will not expose unaligned reads/writes in a public API, only implement it for packed structs.