Re: [patch 1/2] i386: add CFI macros for stack manipulation

Re: [patch 1/2] i386: add CFI macros for stack manipulation

[Chuck Ebbert]

In-Reply-To: <200607282036.45608.ak@suse.de>

On Fri, 28 Jul 2006 20:36:45 +0200, Andi Kleen wrote:
> 
> On Friday 28 July 2006 19:50, Chuck Ebbert wrote:
> > Add macros to dwarf2.h to simplify pushing and popping stack
> > variables.
> 
> I feared someone would do that patch. I've thought about it myself.
> 
> However it's not a good idea. I've already had complaints that some code in 
> x86-64 is too hard to read/debug because it uses too many macros. I think 
> it's better  if the core core still uses "real" instructions and keep the 
> CFI_* stuff as annotation that most people can just ignore.

I did this because it should prevent:

| Date: Thu, 27 Jul 2006 07:45:35 +0200
| From: Markus Armbruster <armbru@redhat.com>
| Subject: [PATCH] Fix trivial unwind info bug
| To: linux-kernel@vger.kernel.org
| Message-ID: <874px31tz4.fsf@pike.pond.sub.org>
| Content-Type: text/plain; charset=us-ascii
| 
| CFA needs to be adjusted upwards for push, and downwards for pop.
| arch/i386/kernel/entry.S gets it wrong in one place.
| 
| Signed-off-by: Markus Armbruster <armbru@redhat.com>
| 
| 
| diff --git a/arch/i386/kernel/entry.S b/arch/i386/kernel/entry.S
| index d9a260f..37a7d2e 100644
| --- a/arch/i386/kernel/entry.S
| +++ b/arch/i386/kernel/entry.S
| @@ -204,7 +204,7 @@ #define RING0_PTREGS_FRAME \
|  ENTRY(ret_from_fork)
|         CFI_STARTPROC
|         pushl %eax
| -       CFI_ADJUST_CFA_OFFSET -4
| +       CFI_ADJUST_CFA_OFFSET 4
|         call schedule_tail
|         GET_THREAD_INFO(%ebp)
|         popl %eax

Otherwise I was just going to indent the annotations an additional
tabstop to move them out of the way.  (They really annoy me.)

-- 
Chuck

Re: [patch 1/2] i386: add CFI macros for stack manipulation

[Andi Kleen]

On Friday 28 July 2006 21:40, Chuck Ebbert wrote:

> Otherwise I was just going to indent the annotations an additional
> tabstop to move them out of the way.  (They really annoy me.)

That would annoy me then.

-Andi

Re: [patch 1/2] i386: add CFI macros for stack manipulation

[Andi Kleen]

On Friday 28 July 2006 19:50, Chuck Ebbert wrote:
> Add macros to dwarf2.h to simplify pushing and popping stack
> variables.

I feared someone would do that patch. I've thought about it myself.

However it's not a good idea. I've already had complaints that some code in 
x86-64 is too hard to read/debug because it uses too many macros. I think 
it's better  if the core core still uses "real" instructions and keep the 
CFI_* stuff as annotation that most people can just ignore.

With your change that wouldn't be the case and everybody hacking
the code would need to know all of CFI too, which is still quite arcane
stuff.

So while it would make the source shorter and require less typing 
I don't think it's good for readability.

What would be a good thing if someone could write it up would
be a short tutorial for Documentation/* on CFI

-Andi

[patch 1/2] i386: add CFI macros for stack manipulation

[Chuck Ebbert]

Add macros to dwarf2.h to simplify pushing and popping stack
variables.

Signed-off-by: Chuck Ebbert <76306.1226@compuserve.com>

--- 2.6.18-rc2-32.orig/include/asm-i386/dwarf2.h
+++ 2.6.18-rc2-32/include/asm-i386/dwarf2.h
@@ -51,4 +51,26 @@
 
 #endif
 
-#endif
+.macro	CFI_pushl val
+	pushl \val
+	CFI_ADJUST_CFA_OFFSET 4
+.endm
+
+.macro	CFI_pushl_reg reg
+	pushl %\reg
+	CFI_ADJUST_CFA_OFFSET 4
+	CFI_REL_OFFSET \reg, 0
+.endm
+
+.macro	CFI_popl val
+	popl \val
+	CFI_ADJUST_CFA_OFFSET -4
+.endm
+
+.macro	CFI_popl_reg reg
+	popl %\reg
+	CFI_ADJUST_CFA_OFFSET -4
+	CFI_RESTORE \reg
+.endm
+
+#endif /* _DWARF2_H */
-- 
Chuck