Hard fault on format or write because unaligned memory access

[FabianInostroza]

Hi,

I'm using littlefs on a stm32l0, using only static buffers, the issue I found is that when using LTO the look ahead buffer (defined as uint8_t array) gets placed in a address that is not 32bit aligned and internally this buffer is used a pointer to uint32_t. When the program dereferences this unaligned pointer in lfs_alloc_lookahead() a hard fault exception occurs.

Wouldn't it be better to define lookahead_buffer as uint32_t * in struct lfs_config?, It would not prevent the unaligned access but this way a warning or error could be generated at compile time by using incompatible pointers. Also, if one doesn't use static buffers, does lfs_malloc() always returns 32bit aligned pointers? If this is not the case, then the library should be adapted to use uint8_t.

PD: To ensure that the buffer is 32bit aligned now I am declaring it as a uint32_t array. Using __attribute__((aligned(4)) on a uint8_t array also works.

[geky]

Hi, thanks for creating an issue. This is a good point, I'll see about changing the pointer type in v2.

Traditionally, malloc functions are word aligned. This is required in stdc malloc (ref), but you're right I should note this requirement for lfs_malloc

[geky]

One of the next area of improvements is the allocator (https://github.com/ARMmbed/littlefs/issues/75). Because of this, it's up in the air if lookahead_buffer needs to be uint64_t or uint32_t. For this reason I'm going to only provide an assert for now as the requirement can be weakened without worrying about backwards compatibility.

https://github.com/ARMmbed/littlefs/blob/9dafd89709c908b05420e5a9b9d13e182765e18a/lfs.c#L3230-L3233

At least this should help identify the issue if it happens for other issues. As an added plus this will catch unaligned lfs_malloc implementations.

[mon]

64bit alignment on a 32 bit system seems like overkill - should that be lfs->cfg->lookahead_buffer % sizeof(size_t) instead?

[geky]

Ah, you're right that the buffer's address only needs to be 32-bit aligned.

The size is needs to be 64-bit aligned, though this may be temporary. One of the possible solutions for #75 involves storing pairs of pointers (address+size pairs). The lookahead buffer would then need to be a multiple of 64-bits.

Well, it doesn't need to be, but any unaligned size would be wasted. It's cheaper and more transparent to the user to present this as a strict requirement to the user.

If we don't adopt this solution for #75, we can just remove the 64-bit requirement. But adopting it early makes it possible to introduce without breaking compatibility.

[pabigot]

I'm going through conniptions because of the 8-byte alignment requirement on a system that allocates on 4-byte boundaries. Yes, I could overallocate then adjust the pointer, but then I'd have to record that the adjustment was made so I can reconstruct the correct pointer to free. So I'm trying to figure out why 64-bit alignment is required. That led me to #147 and then here, and to this:

The size is needs to be 64-bit aligned, though this may be temporary. One of the possible solutions for #75 involves storing pairs of pointers (address+size pairs). The lookahead buffer would then need to be a multiple of 64-bits.

I don't see how this would require 64-bit alignment, unless the pointer is a 64-bit value. If it's 32-bits, then shouldn't 32-bit alignment should be sufficient?

[geky]

You're right. It's only the lookahead_size that needs to be 64-bits aligned. Not the address.

You're not the first person to be confused by this. Do you think the documentation could be improved somewhere?

[pabigot]

You're not the first person to be confused by this. Do you think the documentation could be improved somewhere?

Yes. See #239.