mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "H. Peter Anvin" <hpa@linux.intel.com>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>,
	kbuild test robot <fengguang.wu@intel.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: sound/pci/asihpi/hpioctl.c:125:6: warning: cast to pointer from integer of different size
Date: Fri, 8 Feb 2013 20:50:57 +0200	[thread overview]
Message-ID: <20130208185057.GK9135@intel.com> (raw)
In-Reply-To: <511530AC.1070203@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]

On Fri, Feb 08, 2013 at 09:06:52AM -0800, H. Peter Anvin wrote:
> On 02/08/2013 07:28 AM, Russell King - ARM Linux wrote:
> > 
> > Whether that's safe for x86 or not, I don't know, but my suspicions are
> > that it's unsafe on x86 as it's possible to refer to the various bytes/
> > half-words of eax separately.
> > 
> > So, I came to the conclusion that if x86 remains a problem, there's
> > little point supporting it on ARM.
> > 
> 
> It is possible to access bytes separately, but gcc generally doesn't.
> However, whether or not that can be relied upon safely is a tricky question.
> 
> It *is* also worth nothing that the x86 ABI does allow two words to be
> returned in registers from a normal C function, simply by returning a
> structure.  That doesn't solve the problem at hand, though, which is how
> to make a type-neutral macro which can handle doublewords...

I just tried to compare a couple of options:

1)
	unsigned long __val_gu;
	unsigned long long __val_gu8;

2)
	register typeof(x) __val_gu asm("eax");

	I was still using a test app based on my orignal patch which
	used eax and edx for the value.

3)
	typeof(x) __val_gu;

While options 2 and 3 get rid of the warnings, they unfortunately
produce different results as well.

I've attached my test app in case you want to see it.

One other note is that if I include the memtest() call in the test,
option 1 and 2 produce the same results, but w/o the memtest() the
results differ. I didn't look into it any further.

-- 
Ville Syrjälä
Intel OTC

[-- Attachment #2: get_user.S --]
[-- Type: text/x-asm, Size: 701 bytes --]

#ifdef AMD64
#define _ASM_AX rax
#define _ASM_CX rcx
#define _ASM_DX rdx
#else
#define _ASM_AX eax
#define _ASM_CX ecx
#define _ASM_DX edx
#endif

.text
.globl __get_user_1
.globl __get_user_2
.globl __get_user_4
#ifdef AMD64
.globl __get_user_8
#else
.globl __get_user_8_32
#endif

__get_user_1:
	movzb (%_ASM_CX),%eax
	xor %ecx,%ecx
	ret

__get_user_2:
	add $1,%_ASM_CX
	movzwl -1(%_ASM_CX),%eax
	xor %ecx,%ecx
	ret

__get_user_4:
	add $3,%_ASM_CX
	mov -3(%_ASM_CX),%eax
	xor %ecx,%ecx
	ret

#ifdef AMD64
__get_user_8:
	add $7,%_ASM_CX
	movq -7(%_ASM_CX),%_ASM_AX
	xor %ecx,%ecx
	ret
#else
__get_user_8_32:
	add $7,%_ASM_CX
	movl -7(%_ASM_CX),%eax
	movl -3(%_ASM_CX),%edx
	xor %ecx,%ecx
	ret
#endif

[-- Attachment #3: Makefile --]
[-- Type: text/plain, Size: 204 bytes --]

CFLAGS:=-std=gnu99 -O2

CPPFLAGS:=-DAMD64
#CFLAGS+=-m32
#ASFLAGS+=-m32

CPPFLAGS+=-Wint-to-pointer-cast

.PHONY: all clean

all: test

test: uaccess.o get_user.o
	$(CC) $(CFLAGS) -o $@ $^

clean:
	rm *.o

[-- Attachment #4: uaccess.c --]
[-- Type: text/x-c, Size: 3710 bytes --]

#include <stdio.h>
#include <stdint.h>
#include <inttypes.h>
#include <string.h>

#define OPT 1

#define __get_user_x(size, ret, x, ptr)				      \
	asm volatile("call __get_user_" #size			      \
		     : "=c" (ret), "=a" (x)			      \
		     : "0" (ptr))


#ifdef AMD64
#define __get_user_8(__ret_gu, __val_gu, ptr)                           \
	__get_user_x(8, __ret_gu, __val_gu, ptr)
#else
#define __get_user_8(ret, x, ptr)					\
	asm volatile("call __get_user_8_32"				\
		     : "=c" (ret), "=A" (x)				\
		     : "0" (ptr))
#endif

#if OPT == 1
#define get_user(x, ptr)						\
({									\
	int __ret_gu;							\
	unsigned long __val_gu;						\
	unsigned long long __val_gu8;					\
	switch (sizeof(*(ptr))) {					\
	case 1:                                                         \
		__get_user_x(1, __ret_gu, __val_gu, ptr);		\
		break;							\
	case 2:								\
		__get_user_x(2, __ret_gu, __val_gu, ptr);		\
		break;							\
	case 4:								\
		__get_user_x(4, __ret_gu, __val_gu, ptr);		\
		break;							\
	case 8:								\
		__get_user_8(__ret_gu, __val_gu8, ptr);			\
		break;							\
	default:							\
		__get_user_x(X, __ret_gu, __val_gu, ptr);		\
		break;							\
	}								\
	if (sizeof(*(ptr)) == 8)					\
		(x) = (typeof(*(ptr)))__val_gu8;			\
	else								\
		(x) = (typeof(*(ptr)))__val_gu;				\
	__ret_gu;							\
})
#endif

#if OPT == 2
#define get_user(x, ptr)						\
({									\
	int __ret_gu;							\
	register typeof(x) __val_gu asm("eax");				\
	switch (sizeof(*(ptr))) {					\
	case 1:                                                         \
		__get_user_x(1, __ret_gu, __val_gu, ptr);		\
		break;							\
	case 2:								\
		__get_user_x(2, __ret_gu, __val_gu, ptr);		\
		break;							\
	case 4:								\
		__get_user_x(4, __ret_gu, __val_gu, ptr);		\
		break;							\
	case 8:								\
		__get_user_8(__ret_gu, __val_gu, ptr);			\
		break;							\
	default:							\
		__get_user_x(X, __ret_gu, __val_gu, ptr);		\
		break;							\
	}								\
	(x) = (typeof(*(ptr)))__val_gu;					\
	__ret_gu;							\
})
#endif

#if OPT == 3
#define get_user(x, ptr)						\
({									\
	int __ret_gu;							\
	typeof(x) __val_gu;						\
	switch (sizeof(*(ptr))) {					\
	case 1:                                                         \
		__get_user_x(1, __ret_gu, __val_gu, ptr);		\
		break;							\
	case 2:								\
		__get_user_x(2, __ret_gu, __val_gu, ptr);		\
		break;							\
	case 4:								\
		__get_user_x(4, __ret_gu, __val_gu, ptr);		\
		break;							\
	case 8:								\
		__get_user_8(__ret_gu, __val_gu, ptr);			\
		break;							\
	default:							\
		__get_user_x(X, __ret_gu, __val_gu, ptr);		\
		break;							\
	}								\
	(x) = (typeof(*(ptr)))__val_gu;					\
	__ret_gu;							\
})
#endif

int main(void)
{
	uint64_t foo = 0x89abcdef76543210;

	printf(" foo: %"PRIx64"\n", foo);

	struct {
		uint8_t sika1;
		uint16_t sika2;
		uint32_t sika4;
		uint64_t sika8;
	} __attribute__((__packed__)) x;

	if (get_user(x.sika8, (const uint64_t*)&foo)) return 1;
	if (get_user(x.sika4, (const uint64_t*)&foo)) return 1;
	if (get_user(x.sika2, (const uint64_t*)&foo)) return 1;
	if (get_user(x.sika1, (const uint64_t*)&foo)) return 1;
	printf("sika8: %"PRIx64"\n", x.sika8);
	printf("sika4: %x\n", x.sika4);
	printf("sika2: %x\n", x.sika2);
	printf("sika1: %x\n", x.sika1);

	//memset(&x, 0, sizeof(x));

	if (get_user(x.sika1, (const uint64_t*)&foo)) return 1;
	if (get_user(x.sika2, (const uint64_t*)&foo)) return 1;
	if (get_user(x.sika4, (const uint64_t*)&foo)) return 1;
	if (get_user(x.sika8, (const uint64_t*)&foo)) return 1;
	printf("sika8: %"PRIx64"\n", x.sika8);
	printf("sika4: %x\n", x.sika4);
	printf("sika2: %x\n", x.sika2);
	printf("sika1: %x\n", x.sika1);

	return 0;
}

  reply	other threads:[~2013-02-08 18:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <51143ca4.bNX7TobZ2rDrz0zj%fengguang.wu@intel.com>
2013-02-08  2:19 ` H. Peter Anvin
2013-02-08 15:28   ` Russell King - ARM Linux
2013-02-08 17:06     ` H. Peter Anvin
2013-02-08 18:50       ` Ville Syrjälä [this message]
2013-02-08 22:57         ` Russell King - ARM Linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130208185057.GK9135@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=fengguang.wu@intel.com \
    --cc=hpa@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome