[PATCH-RFC] arch/i386/kernel/: kill some sparse warnings

[PATCH-RFC] arch/i386/kernel/: kill some sparse warnings

[Sam Ravnborg]

When running
make C=2 arch/i386/kernel/
sparse complains about access past end of variable ''

The following patch silence these sparse warnings.
RELOC_HIDE uses an asm("") trick to hide the size of the variable for
sparse. I've cheched the generated code and with -O2 the code does not
change with or without RELOC_HIDE.

loadsegment take the pointer to second argument and cast it to unsigned
int *. Using a properly sized variable as argument to loadsegment kills
this warning.
For this fix I wonder what happened to the upper bits in the old
implmentation - they were undefined per definition.

This is the relevant code smippet from system.h:
		".align 4\n\t"			\
		".long 1b,3b\n"			\
		".previous"			\
		: :"m" (*(unsigned int *)&(value)))

'value' is the variable passed as second argument to loadsegment.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---

===== arch/i386/kernel/ioport.c 1.14 vs edited =====
--- 1.14/arch/i386/kernel/ioport.c	2004-09-17 08:58:37 +02:00
+++ edited/arch/i386/kernel/ioport.c	2005-01-16 20:24:09 +01:00
@@ -129,7 +129,7 @@
 
 asmlinkage long sys_iopl(unsigned long unused)
 {
-	volatile struct pt_regs * regs = (struct pt_regs *) &unused;
+	volatile struct pt_regs * regs = (struct pt_regs *) RELOC_HIDE(&unused, 0);
 	unsigned int level = regs->ebx;
 	unsigned int old = (regs->eflags >> 12) & 3;
 
===== arch/i386/kernel/signal.c 1.51 vs edited =====
--- 1.51/arch/i386/kernel/signal.c	2005-01-12 01:42:47 +01:00
+++ edited/arch/i386/kernel/signal.c	2005-01-16 21:16:43 +01:00
@@ -120,7 +120,7 @@
 sys_sigaltstack(unsigned long ebx)
 {
 	/* This is needed to make gcc realize it doesn't own the "struct pt_regs" */
-	struct pt_regs *regs = (struct pt_regs *)&ebx;
+	struct pt_regs *regs = (struct pt_regs *) RELOC_HIDE(&ebx, 0);
 	const stack_t __user *uss = (const stack_t __user *)ebx;
 	stack_t __user *uoss = (stack_t __user *)regs->ecx;
 
@@ -154,8 +154,10 @@
 
 #define GET_SEG(seg)							\
 	{ unsigned short tmp;						\
+	  unsigned int tmp2;						\
 	  err |= __get_user(tmp, &sc->seg);				\
-	  loadsegment(seg,tmp); }
+	  tmp2 = tmp;							\
+	  loadsegment(seg,tmp2); }
 
 #define	FIX_EFLAGS	(X86_EFLAGS_AC | X86_EFLAGS_OF | X86_EFLAGS_DF | \
 			 X86_EFLAGS_TF | X86_EFLAGS_SF | X86_EFLAGS_ZF | \
@@ -208,7 +210,7 @@
 
 asmlinkage int sys_sigreturn(unsigned long __unused)
 {
-	struct pt_regs *regs = (struct pt_regs *) &__unused;
+	struct pt_regs *regs = (struct pt_regs *) RELOC_HIDE(&__unused, 0);
 	struct sigframe __user *frame = (struct sigframe __user *)(regs->esp - 8);
 	sigset_t set;
 	int eax;
@@ -238,7 +240,7 @@
 
 asmlinkage int sys_rt_sigreturn(unsigned long __unused)
 {
-	struct pt_regs *regs = (struct pt_regs *) &__unused;
+	struct pt_regs *regs = (struct pt_regs *) RELOC_HIDE(&__unused, 0);
 	struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(regs->esp - 4);
 	sigset_t set;
 	int eax;

Re: [PATCH-RFC] arch/i386/kernel/: kill some sparse warnings

[Linus Torvalds]

On Sun, 16 Jan 2005, Sam Ravnborg wrote:
> 
> loadsegment take the pointer to second argument and cast it to unsigned
> int *. Using a properly sized variable as argument to loadsegment kills
> this warning.

I think the bug here is in "loadsegment".

I don't really see why it uses "m" in the first place, since you can 
certainly move to a segment register from a reg too.

Afaik, that

	"m" (*(unsigned int *)&(value)))

is likely from some old bogus code for totally historical reasons, and it 
should likely just be

	"rm" (value)

instead.

Of course, there may be some strange mis-use of the thing somewhere which 
explains why the code does something that strange, and thus it might be 
best to check that all users are ok.

> For this fix I wonder what happened to the upper bits in the old
> implmentation - they were undefined per definition.

They are ignored by the definition of the instruction, and the 32-bit 
version (without a data size override) is selected just because it is 
faster. Which may be why it does that strange cast too: to make sure that 
the size of the operand matches (not that it should _matter_ for a memory 
op).

Oh, and the "%0" in the asm descriptor should probably have the operand 
character override for a full word, to make sure that the operand size 
(when we use a register - where it _does_ matter) matches the "movl". 

I think that means it should be "%k0" instead of "%0", ie something like

	"movl %k0,%%" #seg "\n"

for the move itself, along with the fix for the strange value thing.

Willing to see if that works ok?

		Linus