Skip to contentSkip to conversation

Fix in process.c

PATCH In review Siddha, Suresh B 2 participants 6 messages Jan 18, 2003 – Jan 21, 2003

lore
Share & tools
lore

Siddha, Suresh B

author
#0 lore raw

PATCH 1/1 Fix in process.c

Attached/appended fix for another problem(getting the address of a function), which depends on the compiler behaviour.

thanks,
suresh

arch/ia64/kernel/process.c renamed from arch/ia64/kernel/process.c~ +9 -3
diff -Nru linux-2.5.52/arch/ia64/kernel/process.c~ linux-2.5.52/arch/ia64/kernel/process.c
@@ -281,7 +281,10 @@
 {
 	unsigned long rbs, child_rbs, rbs_size, stack_offset, stack_top, stack_used;
 	struct switch_stack *child_stack, *stack;
-	extern char ia64_ret_from_clone, ia32_ret_from_clone;
+	extern void  ia64_ret_from_clone(void);
+#ifdef CONFIG_IA32_SUPPORT
+	extern void ia32_ret_from_clone(void);
+#endif
 	struct pt_regs *child_ptregs;
 	int retval = 0;
 

@@ -332,10 +335,13 @@
 		child_ptregs->r12 = (unsigned long) (child_ptregs + 1); /* kernel sp */
 		child_ptregs->r13 = (unsigned long) p;		/* set `current' pointer */
 	}
+#ifdef CONFIG_IA32_SUPPORT
 	if (IS_IA32_PROCESS(regs))
-		child_stack->b0 = (unsigned long) &ia32_ret_from_clone;
+		child_stack->b0 = ((unsigned long *) &ia32_ret_from_clone)[0];
 	else
-		child_stack->b0 = (unsigned long) &ia64_ret_from_clone;
+#else
+		child_stack->b0 = ((unsigned long *) &ia64_ret_from_clone)[0];
+#endif
 	child_stack->ar_bspstore = child_rbs + rbs_size;
 
 	/* copy parts of thread_struct: */
arch/ia64/kernel/process.c renamed from arch/ia64/kernel/process.c~ +9 -3
diff -Nru linux-2.5.52/arch/ia64/kernel/process.c~ linux-2.5.52/arch/ia64/kernel/process.c
@@ -281,7 +281,10 @@
 {
 	unsigned long rbs, child_rbs, rbs_size, stack_offset, stack_top, stack_used;
 	struct switch_stack *child_stack, *stack;
-	extern char ia64_ret_from_clone, ia32_ret_from_clone;
+	extern void  ia64_ret_from_clone(void);
+#ifdef CONFIG_IA32_SUPPORT
+	extern void ia32_ret_from_clone(void);
+#endif
 	struct pt_regs *child_ptregs;
 	int retval = 0;
 

@@ -332,10 +335,13 @@
 		child_ptregs->r12 = (unsigned long) (child_ptregs + 1); /* kernel sp */
 		child_ptregs->r13 = (unsigned long) p;		/* set `current' pointer */
 	}
+#ifdef CONFIG_IA32_SUPPORT
 	if (IS_IA32_PROCESS(regs))
-		child_stack->b0 = (unsigned long) &ia32_ret_from_clone;
+		child_stack->b0 = ((unsigned long *) &ia32_ret_from_clone)[0];
 	else
-		child_stack->b0 = (unsigned long) &ia64_ret_from_clone;
+#else
+		child_stack->b0 = ((unsigned long *) &ia64_ret_from_clone)[0];
+#endif
 	child_stack->ar_bspstore = child_rbs + rbs_size;
 
 	/* copy parts of thread_struct: */
#1 lore raw

This one I REALLY don't like:

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Suresh B
  Suresh> - extern char ia64_ret_from_clone;
  Suresh> + extern void  ia64_ret_from_clone(void);

  Suresh> - child_stack->b0 = (unsigned long) &ia64_ret_from_clone;
  Suresh> + child_stack->b0 = ((unsigned long *) &ia64_ret_from_clone)[0];

Why materialize a function pointer for ABSOLUTELY no reason? Furthermore, &FUNC can be used as a static initializer, whereas ((long*)&FUNC)[0] cannot.

	--david
author
#2 lore raw
In reply to Siddha, Suresh B

hi David,

Type of "ia64_ret_from_clone" is declared differently here(char as against function) and we end up linking object files with different types for the symbol(which the linker will not like).

Current C-code is assuming gcc specific behaviour(gcc does not fix the type for extern variables, leaves it to the linker).

thanks,
suresh

Quoted 26 lines

-----Original Message-----
From: David Mosberger [mailto:[email protected]]
Sent: Friday, January 17, 2003 5:53 PM
To: Siddha, Suresh B
Cc: [email protected]
Subject: Re: [Linux-ia64] [Patch] Fix in process.c

This one I REALLY don't like:

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

On Fri, 17 Jan 2003 17:43:44 -0800, "Siddha, Suresh B"

<[email protected]> said:

  Suresh> - extern char ia64_ret_from_clone;
  Suresh> + extern void  ia64_ret_from_clone(void);

Suresh> - child_stack->b0 = (unsigned long) &ia64_ret_from_clone; Suresh> + child_stack->b0 = ((unsigned long *)
&ia64_ret_from_clone)[0];

Why materialize a function pointer for ABSOLUTELY no reason? Furthermore, &FUNC can be used as a static initializer, whereas ((long*)&FUNC)[0] cannot.

	--david
#3 lore raw
In reply to Siddha, Suresh B
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Suresh B

Suresh> hi David, Type of "ia64_ret_from_clone" is declared Suresh> differently here(char as against function) and we end up Suresh> linking object files with different types for the Suresh> symbol(which the linker will not like).

Suresh> Current C-code is assuming gcc specific behaviour(gcc does Suresh> not fix the type for extern variables, leaves it to the Suresh> linker).

I understand that. What I'm saying is that GCC does the sane thing. There's got to be a way to get the address of a label without having to materialize a function pointer.

	--david
author
#4 lore raw
In reply to Siddha, Suresh B
Quoted 13 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

On Fri, 17 Jan 2003 19:04:55 -0800, "Siddha, Suresh B"

<[email protected]> said:

Suresh> hi David, Type of "ia64_ret_from_clone" is declared Suresh> differently here(char as against function) and we end up Suresh> linking object files with different types for the Suresh> symbol(which the linker will not like).

Suresh> Current C-code is assuming gcc specific behaviour(gcc does Suresh> not fix the type for extern variables, leaves it to the Suresh> linker).

Quoted 6 lines

I understand that. What I'm saying is that GCC does the sane thing. There's got to be a way to get the address of a label without having to materialize a function pointer.

	--david

We can do that through the linker script vmlinux.lds.S.

thanks,
suresh

author
#5 lore raw
In reply to Siddha, Suresh B
Quoted 10 lines
Quoted 7 lines

I understand that. What I'm saying is that GCC does the sane thing. There's got to be a way to get the address of a label without having to materialize a function pointer.

	--david

We can do that through the linker script vmlinux.lds.S.

hi David,

Here is the patch which does this. Please apply.

thanks,
suresh

arch/ia64/kernel/process.c +8 -3
diff -Nru linux-2.5.52/arch/ia64/kernel/process.c linux/arch/ia64/kernel/process.c
@@ -281,7 +281,10 @@
 {
 	unsigned long rbs, child_rbs, rbs_size, stack_offset, stack_top, stack_used;
 	struct switch_stack *child_stack, *stack;
-	extern char ia64_ret_from_clone, ia32_ret_from_clone;
+	extern int  __ia64_ret_from_clone;
+#ifdef CONFIG_IA32_SUPPORT
+	extern int __ia32_ret_from_clone;
+#endif
 	struct pt_regs *child_ptregs;
 	int retval = 0;
 

@@ -332,10 +335,12 @@
 		child_ptregs->r12 = (unsigned long) (child_ptregs + 1); /* kernel sp */
 		child_ptregs->r13 = (unsigned long) p;		/* set `current' pointer */
 	}
+#ifdef CONFIG_IA32_SUPPORT
 	if (IS_IA32_PROCESS(regs))
-		child_stack->b0 = (unsigned long) &ia32_ret_from_clone;
+		child_stack->b0 = (unsigned long) &__ia32_ret_from_clone;
 	else
-		child_stack->b0 = (unsigned long) &ia64_ret_from_clone;
+#endif
+		child_stack->b0 = (unsigned long) &__ia64_ret_from_clone;
 	child_stack->ar_bspstore = child_rbs + rbs_size;
 
 	/* copy parts of thread_struct: */
arch/ia64/vmlinux.lds.S +8 -0
diff -Nru linux-2.5.52/arch/ia64/vmlinux.lds.S linux/arch/ia64/vmlinux.lds.S
@@ -39,6 +39,14 @@
 #endif
   _etext = .;
 
+   /* We want to get the address of a label without having
+      to materialize a function pointer. "+0" is for changing
+      the symbol type */
+   __ia64_ret_from_clone = ia64_ret_from_clone + 0;
+#ifdef CONFIG_IA32_SUPPORT
+   __ia32_ret_from_clone = ia32_ret_from_clone + 0;
+#endif
+
   /* Read-only data */
 
   /* Global data */
arch/ia64/kernel/process.c +8 -3
diff -Nru linux-2.5.52/arch/ia64/kernel/process.c linux/arch/ia64/kernel/process.c
@@ -281,7 +281,10 @@
 {
 	unsigned long rbs, child_rbs, rbs_size, stack_offset, stack_top, stack_used;
 	struct switch_stack *child_stack, *stack;
-	extern char ia64_ret_from_clone, ia32_ret_from_clone;
+	extern int  __ia64_ret_from_clone;
+#ifdef CONFIG_IA32_SUPPORT
+	extern int __ia32_ret_from_clone;
+#endif
 	struct pt_regs *child_ptregs;
 	int retval = 0;
 

@@ -332,10 +335,12 @@
 		child_ptregs->r12 = (unsigned long) (child_ptregs + 1); /* kernel sp */
 		child_ptregs->r13 = (unsigned long) p;		/* set `current' pointer */
 	}
+#ifdef CONFIG_IA32_SUPPORT
 	if (IS_IA32_PROCESS(regs))
-		child_stack->b0 = (unsigned long) &ia32_ret_from_clone;
+		child_stack->b0 = (unsigned long) &__ia32_ret_from_clone;
 	else
-		child_stack->b0 = (unsigned long) &ia64_ret_from_clone;
+#endif
+		child_stack->b0 = (unsigned long) &__ia64_ret_from_clone;
 	child_stack->ar_bspstore = child_rbs + rbs_size;
 
 	/* copy parts of thread_struct: */
arch/ia64/vmlinux.lds.S +8 -0
diff -Nru linux-2.5.52/arch/ia64/vmlinux.lds.S linux/arch/ia64/vmlinux.lds.S
@@ -39,6 +39,14 @@
 #endif
   _etext = .;
 
+   /* We want to get the address of a label without having
+      to materialize a function pointer. "+0" is for changing
+      the symbol type */
+   __ia64_ret_from_clone = ia64_ret_from_clone + 0;
+#ifdef CONFIG_IA32_SUPPORT
+   __ia32_ret_from_clone = ia32_ret_from_clone + 0;
+#endif
+
   /* Read-only data */
 
   /* Global data */