Skip to content

Commit 4423dc0

Browse files
stephenkyle-ARMjuj
authored andcommitted
platform: fix unaligned 64-bit accesses on AArch32 (google#699)
Ensures that Aarch32 Arm builds with an Armv8 compiler do not set BROTLI_64_BITS. This scenario is possible with ChromeOS builds, as they may use a toolchain with the target armv7-cros-gnueabi, but with -march=armv8. This will set __ARM_ARCH to 8 (defining BROTLI_TARGET_ARMV8), but will also set __ARM_32BIT_STATE and not __ARM_64BIT_STATE. Without this, illegal 64-bit non-word-aligned reads (LDRD) may be emitted. Also fix unaligned 64-bit reads on AArch32 - STRD was still possible to emit.
1 parent 3f60c8f commit 4423dc0

File tree

1 file changed

+35
-10
lines changed

1 file changed

+35
-10
lines changed

c/common/platform.h

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,14 @@ To apply compiler hint, enclose the branching condition into macros, like this:
187187

188188
#if (defined(__ARM_ARCH) && (__ARM_ARCH == 8)) || \
189189
defined(__aarch64__) || defined(__ARM64_ARCH_8__)
190-
#define BROTLI_TARGET_ARMV8
190+
#define BROTLI_TARGET_ARMV8_ANY
191+
192+
#if defined(__ARM_32BIT_STATE)
193+
#define BROTLI_TARGET_ARMV8_32
194+
#elif defined(__ARM_64BIT_STATE)
195+
#define BROTLI_TARGET_ARMV8_64
196+
#endif
197+
191198
#endif /* ARMv8 */
192199

193200
#if defined(__i386) || defined(_M_IX86)
@@ -210,7 +217,7 @@ To apply compiler hint, enclose the branching condition into macros, like this:
210217
#define BROTLI_64_BITS 1
211218
#elif defined(BROTLI_BUILD_32_BIT)
212219
#define BROTLI_64_BITS 0
213-
#elif defined(BROTLI_TARGET_X64) || defined(BROTLI_TARGET_ARMV8) || \
220+
#elif defined(BROTLI_TARGET_X64) || defined(BROTLI_TARGET_ARMV8_64) || \
214221
defined(BROTLI_TARGET_POWERPC64) || defined(BROTLI_TARGET_RISCV64)
215222
#define BROTLI_64_BITS 1
216223
#else
@@ -261,7 +268,7 @@ To apply compiler hint, enclose the branching condition into macros, like this:
261268
#if defined(BROTLI_BUILD_PORTABLE)
262269
#define BROTLI_ALIGNED_READ (!!1)
263270
#elif defined(BROTLI_TARGET_X86) || defined(BROTLI_TARGET_X64) || \
264-
defined(BROTLI_TARGET_ARMV7) || defined(BROTLI_TARGET_ARMV8) || \
271+
defined(BROTLI_TARGET_ARMV7) || defined(BROTLI_TARGET_ARMV8_ANY) || \
265272
defined(BROTLI_TARGET_RISCV64)
266273
/* Allow unaligned read only for white-listed CPUs. */
267274
#define BROTLI_ALIGNED_READ (!!0)
@@ -306,15 +313,33 @@ static BROTLI_INLINE void BrotliUnalignedWrite64(void* p, uint64_t v) {
306313
}
307314
#else /* BROTLI_64_BITS */
308315
/* Avoid emitting LDRD / STRD, which require properly aligned address. */
316+
317+
#if BROTLI_GNUC_HAS_ATTRIBUTE(aligned, 2, 7, 0)
318+
typedef __attribute__((aligned(1))) uint64_t unaligned_uint64_t;
319+
320+
static BROTLI_INLINE uint64_t BrotliUnalignedRead64(const void* p) {
321+
return (uint64_t) ((unaligned_uint64_t*) p)[0];
322+
}
323+
static BROTLI_INLINE void BrotliUnalignedWrite64(void* p, uint64_t v) {
324+
unaligned_uint64_t* dwords = (unaligned_uint64_t*) p;
325+
dwords[0] = (unaligned_uint64_t) v;
326+
}
327+
#else /* BROTLI_GNUC_HAS_ATTRIBUTE(aligned, 2, 7, 0) */
328+
329+
/* Alternative way to avoid LDRD/STRD is use volatile pointers. See:
330+
* http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka16346.html
331+
*/
309332
static BROTLI_INLINE uint64_t BrotliUnalignedRead64(const void* p) {
310-
const uint32_t* dwords = (const uint32_t*)p;
311-
return dwords[0] | ((uint64_t)dwords[1] << 32);
333+
return (uint64_t) *((volatile uint32_t*) p)
334+
| ((uint64_t) *((volatile uint32_t*) p+1) << 32);
312335
}
336+
313337
static BROTLI_INLINE void BrotliUnalignedWrite64(void* p, uint64_t v) {
314-
uint32_t* dwords = (uint32_t *)p;
315-
dwords[0] = (uint32_t)v;
316-
dwords[1] = (uint32_t)(v >> 32);
338+
*((volatile uint32_t*) p) = (uint32_t) v;
339+
*((volatile uint32_t*) p+1) = (uint32_t) (v >> 32);
317340
}
341+
342+
#endif /* BROTLI_GNUC_HAS_ATTRIBUTE(aligned, 2, 7, 0) */
318343
#endif /* BROTLI_64_BITS */
319344
#endif /* BROTLI_ALIGNED_READ */
320345

@@ -400,7 +425,7 @@ static BROTLI_INLINE void BROTLI_UNALIGNED_STORE64LE(void* p, uint64_t v) {
400425
#define BROTLI_IS_CONSTANT(x) (!!0)
401426
#endif
402427

403-
#if defined(BROTLI_TARGET_ARMV7) || defined(BROTLI_TARGET_ARMV8)
428+
#if defined(BROTLI_TARGET_ARMV7) || defined(BROTLI_TARGET_ARMV8_ANY)
404429
#define BROTLI_HAS_UBFX (!!1)
405430
#else
406431
#define BROTLI_HAS_UBFX (!!0)
@@ -427,7 +452,7 @@ static BROTLI_INLINE void BrotliDump(const char* f, int l, const char* fn) {
427452
/* TODO: add appropriate icc/sunpro/arm/ibm/ti checks. */
428453
#if (BROTLI_GNUC_VERSION_CHECK(3, 0, 0) || defined(__llvm__)) && \
429454
!defined(BROTLI_BUILD_NO_RBIT)
430-
#if defined(BROTLI_TARGET_ARMV7) || defined(BROTLI_TARGET_ARMV8)
455+
#if defined(BROTLI_TARGET_ARMV7) || defined(BROTLI_TARGET_ARMV8_ANY)
431456
/* TODO: detect ARMv6T2 and enable this code for it. */
432457
static BROTLI_INLINE brotli_reg_t BrotliRBit(brotli_reg_t input) {
433458
brotli_reg_t output;

0 commit comments

Comments
 (0)