2008-08-29 14:54:37

by Geert Uytterhoeven

[permalink] [raw]
Subject: endianness and sparse warnings

With `make C=1 CF="-D__CHECK_ENDIAN__"', you can let sparse check for bad
handling of endian-annotated data.

Unfortunately several of the accessors for endian-annotated data do not cause
sparse warnings.

Summarized:
- [bl]e{16,32,64}_to_cpu() is OK
- [bl]e{16,32,64}_to_cpup() (aka get_aligned_[bl]e{16,32,64}() ;-) is OK
- get_unaligned_[bl]e{16,32,64} is not OK
- __get_unaligned_[bl]e() is partially OK, as long as you don't use it on
non-annotated data, but
o it's meant for internal use only
o it incorrectly causes sparse warnings when assigning the resulting
value to a non-annotated variable

To check it for yourself, plug in the code below into any kernel source file.

#include <asm/unaligned.h>

__u8 d8;
__u16 d16;
__u32 d32;
__u64 d64;

__be16 be16;
__be32 be32;
__be64 be64;

__le16 le16;
__le32 le32;
__le64 le64;

char dr[16];

#define get_aligned_be16 be16_to_cpup
#define get_aligned_be32 be32_to_cpup
#define get_aligned_be64 be64_to_cpup
#define get_aligned_le16 le16_to_cpup
#define get_aligned_le32 le32_to_cpup
#define get_aligned_le64 le64_to_cpup

int test(void)
{
u64 x = 0;

x ^= d8;

// ----------------------------------------------------------------------------

x ^= d16; /* OK */
x ^= d32; /* OK */
x ^= d64; /* OK */
x ^= be16; /* sparse warning */
x ^= be32; /* sparse warning */
x ^= be64; /* sparse warning */
x ^= le16; /* sparse warning */
x ^= le32; /* sparse warning */
x ^= le64; /* sparse warning */

// ----------------------------------------------------------------------------

x ^= be16_to_cpu(d16); /* sparse warning */
x ^= be16_to_cpu(d32); /* sparse warning */
x ^= be16_to_cpu(d64); /* sparse warning */
x ^= be16_to_cpu(be16); /* OK */
x ^= be16_to_cpu(be32); /* sparse warning */
x ^= be16_to_cpu(be64); /* sparse warning */
x ^= be16_to_cpu(le16); /* sparse warning */
x ^= be16_to_cpu(le32); /* sparse warning */
x ^= be16_to_cpu(le64); /* sparse warning */

x ^= be32_to_cpu(d16); /* sparse warning */
x ^= be32_to_cpu(d32); /* sparse warning */
x ^= be32_to_cpu(d64); /* sparse warning */
x ^= be32_to_cpu(be16); /* sparse warning */
x ^= be32_to_cpu(be32); /* OK */
x ^= be32_to_cpu(be64); /* sparse warning */
x ^= be32_to_cpu(le16); /* sparse warning */
x ^= be32_to_cpu(le32); /* sparse warning */
x ^= be32_to_cpu(le64); /* sparse warning */

x ^= be64_to_cpu(d16); /* sparse warning */
x ^= be64_to_cpu(d32); /* sparse warning */
x ^= be64_to_cpu(d64); /* sparse warning */
x ^= be64_to_cpu(be16); /* sparse warning */
x ^= be64_to_cpu(be32); /* sparse warning */
x ^= be64_to_cpu(be64); /* OK */
x ^= be64_to_cpu(le16); /* sparse warning */
x ^= be64_to_cpu(le32); /* sparse warning */
x ^= be64_to_cpu(le64); /* sparse warning */

x ^= le16_to_cpu(d16); /* sparse warning */
x ^= le16_to_cpu(d32); /* sparse warning */
x ^= le16_to_cpu(d64); /* sparse warning */
x ^= le16_to_cpu(be16); /* sparse warning */
x ^= le16_to_cpu(be32); /* sparse warning */
x ^= le16_to_cpu(be64); /* sparse warning */
x ^= le16_to_cpu(le16); /* OK */
x ^= le16_to_cpu(le32); /* sparse warning */
x ^= le16_to_cpu(le64); /* sparse warning */

x ^= le32_to_cpu(d16); /* sparse warning */
x ^= le32_to_cpu(d32); /* sparse warning */
x ^= le32_to_cpu(d64); /* sparse warning */
x ^= le32_to_cpu(be16); /* sparse warning */
x ^= le32_to_cpu(be32); /* sparse warning */
x ^= le32_to_cpu(be64); /* sparse warning */
x ^= le32_to_cpu(le16); /* sparse warning */
x ^= le32_to_cpu(le32); /* OK */
x ^= le32_to_cpu(le64); /* sparse warning */

x ^= le64_to_cpu(d16); /* sparse warning */
x ^= le64_to_cpu(d32); /* sparse warning */
x ^= le64_to_cpu(d64); /* sparse warning */
x ^= le64_to_cpu(be16); /* sparse warning */
x ^= le64_to_cpu(be32); /* sparse warning */
x ^= le64_to_cpu(be64); /* sparse warning */
x ^= le64_to_cpu(le16); /* sparse warning */
x ^= le64_to_cpu(le32); /* sparse warning */
x ^= le64_to_cpu(le64); /* OK */

// ----------------------------------------------------------------------------

x ^= get_unaligned_be16(&d16); /* NO WARNING!! */
x ^= get_unaligned_be16(&d32); /* NO WARNING!! */
x ^= get_unaligned_be16(&d64); /* NO WARNING!! */
x ^= get_unaligned_be16(&be16); /* OK */
x ^= get_unaligned_be16(&be32); /* NO WARNING!! */
x ^= get_unaligned_be16(&be64); /* NO WARNING!! */
x ^= get_unaligned_be16(&le16); /* NO WARNING!! */
x ^= get_unaligned_be16(&le32); /* NO WARNING!! */
x ^= get_unaligned_be16(&le64); /* NO WARNING!! */
x ^= get_unaligned_be16(&dr[1]); /* OK */

x ^= get_unaligned_be32(&d16); /* NO WARNING!! */
x ^= get_unaligned_be32(&d32); /* NO WARNING!! */
x ^= get_unaligned_be32(&d64); /* NO WARNING!! */
x ^= get_unaligned_be32(&be16); /* NO WARNING!! */
x ^= get_unaligned_be32(&be32); /* OK */
x ^= get_unaligned_be32(&be64); /* NO WARNING!! */
x ^= get_unaligned_be32(&le16); /* NO WARNING!! */
x ^= get_unaligned_be32(&le32); /* NO WARNING!! */
x ^= get_unaligned_be32(&le64); /* NO WARNING!! */
x ^= get_unaligned_be32(&dr[1]); /* OK */

x ^= get_unaligned_be64(&d16); /* NO WARNING!! */
x ^= get_unaligned_be64(&d32); /* NO WARNING!! */
x ^= get_unaligned_be64(&d64); /* NO WARNING!! */
x ^= get_unaligned_be64(&be16); /* NO WARNING!! */
x ^= get_unaligned_be64(&be32); /* NO WARNING!! */
x ^= get_unaligned_be64(&be64); /* OK */
x ^= get_unaligned_be64(&le16); /* NO WARNING!! */
x ^= get_unaligned_be64(&le32); /* NO WARNING!! */
x ^= get_unaligned_be64(&le64); /* NO WARNING!! */
x ^= get_unaligned_be64(&dr[1]); /* OK */

x ^= get_unaligned_le16(&d16); /* NO WARNING!! */
x ^= get_unaligned_le16(&d32); /* NO WARNING!! */
x ^= get_unaligned_le16(&d64); /* NO WARNING!! */
x ^= get_unaligned_le16(&be16); /* NO WARNING!! */
x ^= get_unaligned_le16(&be32); /* NO WARNING!! */
x ^= get_unaligned_le16(&be64); /* NO WARNING!! */
x ^= get_unaligned_le16(&le16); /* OK */
x ^= get_unaligned_le16(&le32); /* NO WARNING!! */
x ^= get_unaligned_le16(&le64); /* NO WARNING!! */
x ^= get_unaligned_le16(&dr[1]); /* OK */

x ^= get_unaligned_le32(&d16); /* NO WARNING!! */
x ^= get_unaligned_le32(&d32); /* NO WARNING!! */
x ^= get_unaligned_le32(&d64); /* NO WARNING!! */
x ^= get_unaligned_le32(&be16); /* NO WARNING!! */
x ^= get_unaligned_le32(&be32); /* NO WARNING!! */
x ^= get_unaligned_le32(&be64); /* NO WARNING!! */
x ^= get_unaligned_le32(&le16); /* NO WARNING!! */
x ^= get_unaligned_le32(&le32); /* OK */
x ^= get_unaligned_le32(&le64); /* NO WARNING!! */
x ^= get_unaligned_le16(&dr[1]); /* OK */

x ^= get_unaligned_le64(&d16); /* NO WARNING!! */
x ^= get_unaligned_le64(&d32); /* NO WARNING!! */
x ^= get_unaligned_le64(&d64); /* NO WARNING!! */
x ^= get_unaligned_le64(&be16); /* NO WARNING!! */
x ^= get_unaligned_le64(&be32); /* NO WARNING!! */
x ^= get_unaligned_le64(&be64); /* NO WARNING!! */
x ^= get_unaligned_le64(&le16); /* NO WARNING!! */
x ^= get_unaligned_le64(&le32); /* NO WARNING!! */
x ^= get_unaligned_le64(&le64); /* OK */
x ^= get_unaligned_le16(&dr[1]); /* OK */

// ----------------------------------------------------------------------------

x ^= get_unaligned(&d16); /* OK */
x ^= get_unaligned(&d32); /* OK */
x ^= get_unaligned(&d64); /* OK */
x ^= get_unaligned(&be16); /* sparse warning */
x ^= get_unaligned(&be32); /* sparse warning */
x ^= get_unaligned(&be64); /* sparse warning */
x ^= get_unaligned(&le16); /* sparse warning */
x ^= get_unaligned(&le32); /* sparse warning */
x ^= get_unaligned(&le64); /* sparse warning */
x ^= get_unaligned(&dr[1]); /* OK */

x ^= __get_unaligned_be(&d16); /* NO WARNING!! */
x ^= __get_unaligned_be(&d32); /* NO WARNING!! */
x ^= __get_unaligned_be(&d64); /* NO WARNING!! */
x ^= __get_unaligned_be(&be16); /* sparse warning */
x ^= __get_unaligned_be(&be32); /* sparse warning */
x ^= __get_unaligned_be(&be64); /* sparse warning */
x ^= __get_unaligned_be(&le16); /* sparse warning */
x ^= __get_unaligned_be(&le32); /* sparse warning */
x ^= __get_unaligned_be(&le64); /* sparse warning */
x ^= __get_unaligned_be(&dr[1]); /* NO WARNING!! */

x ^= __get_unaligned_le(&d16); /* NO WARNING!! */
x ^= __get_unaligned_le(&d32); /* NO WARNING!! */
x ^= __get_unaligned_le(&d64); /* NO WARNING!! */
x ^= __get_unaligned_le(&be16); /* sparse warning */
x ^= __get_unaligned_le(&be32); /* sparse warning */
x ^= __get_unaligned_le(&be64); /* sparse warning */
x ^= __get_unaligned_le(&le16); /* sparse warning */
x ^= __get_unaligned_le(&le32); /* sparse warning */
x ^= __get_unaligned_le(&le64); /* sparse warning */
x ^= __get_unaligned_le(&dr[1]); /* NO WARNING!! */

// ----------------------------------------------------------------------------

x ^= get_aligned_be16(&d16); /* sparse warning */
x ^= get_aligned_be16(&d32); /* gcc/sparse warning */
x ^= get_aligned_be16(&d64); /* gcc/sparse warning */
x ^= get_aligned_be16(&be16); /* OK */
x ^= get_aligned_be16(&be32); /* gcc/sparse warning */
x ^= get_aligned_be16(&be64); /* gcc/sparse warning */
x ^= get_aligned_be16(&le16); /* sparse warning */
x ^= get_aligned_be16(&le32); /* gcc/sparse warning */
x ^= get_aligned_be16(&le64); /* gcc/sparse warning */
x ^= get_aligned_be16(&dr[1]); /* gcc/sparse warning */

x ^= get_aligned_be32(&d16); /* gcc/sparse warning */
x ^= get_aligned_be32(&d32); /* sparse warning */
x ^= get_aligned_be32(&d64); /* gcc/sparse warning */
x ^= get_aligned_be32(&be16); /* gcc/sparse warning */
x ^= get_aligned_be32(&be32); /* OK */
x ^= get_aligned_be32(&be64); /* gcc/sparse warning */
x ^= get_aligned_be32(&le16); /* gcc/sparse warning */
x ^= get_aligned_be32(&le32); /* sparse warning */
x ^= get_aligned_be32(&le64); /* gcc/sparse warning */
x ^= get_aligned_be32(&dr[1]); /* gcc/sparse warning */

x ^= get_aligned_be64(&d16); /* gcc/sparse warning */
x ^= get_aligned_be64(&d32); /* gcc/sparse warning */
x ^= get_aligned_be64(&d64); /* sparse warning */
x ^= get_aligned_be64(&be16); /* gcc/sparse warning */
x ^= get_aligned_be64(&be32); /* gcc/sparse warning */
x ^= get_aligned_be64(&be64); /* OK */
x ^= get_aligned_be64(&le16); /* gcc/sparse warning */
x ^= get_aligned_be64(&le32); /* gcc/sparse warning */
x ^= get_aligned_be64(&le64); /* sparse warning */
x ^= get_aligned_be64(&dr[1]); /* gcc/sparse warning */

x ^= get_aligned_le16(&d16); /* sparse warning */
x ^= get_aligned_le16(&d32); /* gcc/sparse warning */
x ^= get_aligned_le16(&d64); /* gcc/sparse warning */
x ^= get_aligned_le16(&be16); /* sparse warning */
x ^= get_aligned_le16(&be32); /* gcc/sparse warning */
x ^= get_aligned_le16(&be64); /* gcc/sparse warning */
x ^= get_aligned_le16(&le16); /* OK */
x ^= get_aligned_le16(&le32); /* gcc/sparse warning */
x ^= get_aligned_le16(&le64); /* gcc/sparse warning */
x ^= get_aligned_le16(&dr[1]); /* gcc/sparse warning */

x ^= get_aligned_le32(&d16); /* gcc/sparse warning */
x ^= get_aligned_le32(&d32); /* sparse warning */
x ^= get_aligned_le32(&d64); /* gcc/sparse warning */
x ^= get_aligned_le32(&be16); /* gcc/sparse warning */
x ^= get_aligned_le32(&be32); /* sparse warning */
x ^= get_aligned_le32(&be64); /* gcc/sparse warning */
x ^= get_aligned_le32(&le16); /* gcc/sparse warning */
x ^= get_aligned_le32(&le32); /* OK */
x ^= get_aligned_le32(&le64); /* gcc/sparse warning */
x ^= get_aligned_le16(&dr[1]); /* gcc/sparse warning */

x ^= get_aligned_le64(&d16); /* gcc/sparse warning */
x ^= get_aligned_le64(&d32); /* gcc/sparse warning */
x ^= get_aligned_le64(&d64); /* sparse warning */
x ^= get_aligned_le64(&be16); /* gcc/sparse warning */
x ^= get_aligned_le64(&be32); /* gcc/sparse warning */
x ^= get_aligned_le64(&be64); /* sparse warning */
x ^= get_aligned_le64(&le16); /* gcc/sparse warning */
x ^= get_aligned_le64(&le32); /* gcc/sparse warning */
x ^= get_aligned_le64(&le64); /* OK */
x ^= get_aligned_le16(&dr[1]); /* gcc/sparse warning */

// ----------------------------------------------------------------------------

return x;
}

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010


2008-08-29 16:24:57

by Harvey Harrison

[permalink] [raw]
Subject: Re: endianness and sparse warnings

On Fri, 2008-08-29 at 16:54 +0200, Geert Uytterhoeven wrote:
> With `make C=1 CF="-D__CHECK_ENDIAN__"', you can let sparse check for bad
> handling of endian-annotated data.
>
> Unfortunately several of the accessors for endian-annotated data do not cause
> sparse warnings.

I'll try and give some background on why the unaligned versions are implemented
this way.

The get_unaligned helpers were meant to replace two kinds of use (using le16 as
an example)

char *ptr;

1 - le16_to_cpu(get_unaligned((__le16 *)ptr))
2 - u16 val = ptr[0] | (ptr[1] << 8)

The places where 1 was replaced with the unaligned helpers would have been fine
with an annotated version as it already had the cast to a proper type.

The places where 2 was replaced would have required a new cast to __le16 *.

Lots of places that were using 2 are drivers that have some data area pointed
to by a char * and they are grabbing values from there at known offsets,
for these users, the need for extra casting was quite ugly and it was known
exactly how many bytes and in what endianness you are reading as it is
right in the function name so I thought it would be ok to omit the annotation
on the parameter.

u16 foo, bar;
char *my_data;

foo = get_unaligned_le16((__le16 *)my_data); /* if unaligned helpers were annotated */
bar = get_unaligned_le16(my_data); /* current version */

>
> Summarized:
> - [bl]e{16,32,64}_to_cpu() is OK
> - [bl]e{16,32,64}_to_cpup() (aka get_aligned_[bl]e{16,32,64}() ;-) is OK
> - get_unaligned_[bl]e{16,32,64} is not OK
> - __get_unaligned_[bl]e() is partially OK, as long as you don't use it on
> non-annotated data, but
> o it's meant for internal use only
> o it incorrectly causes sparse warnings when assigning the resulting
> value to a non-annotated variable

Almost... __get_unaligned_le16 etc are _never_ to be used...as some arches
choose to use memmove-based implementations, and on arches where unaligned
access is OK, they don't exist _at_all_.

Cheers,

Harvey