LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [development-gcc] Re: do_exit stuck
       [not found]     ` <200608301740.41729.ak@suse.de>
@ 2006-09-11 15:37       ` Jan Beulich
  2006-09-11 20:17         ` Andi Kleen
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2006-09-11 15:37 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Michael Matz, Richard Guenther, linux-kernel

>>> Andi Kleen <ak@suse.de> 30.08.06 17:40 >>>
>On Wednesday 30 August 2006 17:28, Jan Beulich wrote:
>> >Hmm, yes.  Sigh, so it's either gcc changes or binutils changes, and none 
>> >of that can be relied upon, except someone has a better idea to identify 
>> >signal frames with high enough assurance to be sensible for debugging.
>> 
>> The only alternative idea I have is to take into consideration the location
>> of the return address: if it's at the default location (top of call frame),
>> then assume this is a normal frame, in all other cases assume it's an
>> interrupt/exception one. This will probably get a few cases wrong
>> (where the return address is being played with), but it might be better
>> than the current situation. Andi?
>
>Fine by me.
>
>At least it will likely be less controversal than changing all the noreturns :)

Here's a patch, including all that I think is necessary once we selectively
enable (auto-detect) CONFIG_AS_CFI_SIGNAL_FRAME for newer binutils.
It doesn't include adjustment of the printed address (i.e. for the original
example, it'd still be kernel_math_error+0 that gets displayed, but the
unwinder wouldn't get confused anymore.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- linux-2.6.18-rc6/arch/i386/kernel/entry.S	2006-09-11 17:18:19.000000000 +0200
+++ 2.6.18-rc6-unwind-adjust-caller-pc/arch/i386/kernel/entry.S	2006-09-11 15:26:14.000000000 +0200
@@ -176,18 +176,21 @@ VM_MASK		= 0x00020000
 
 #define RING0_INT_FRAME \
 	CFI_STARTPROC simple;\
+	CFI_SIGNAL_FRAME;\
 	CFI_DEF_CFA esp, 3*4;\
 	/*CFI_OFFSET cs, -2*4;*/\
 	CFI_OFFSET eip, -3*4
 
 #define RING0_EC_FRAME \
 	CFI_STARTPROC simple;\
+	CFI_SIGNAL_FRAME;\
 	CFI_DEF_CFA esp, 4*4;\
 	/*CFI_OFFSET cs, -2*4;*/\
 	CFI_OFFSET eip, -3*4
 
 #define RING0_PTREGS_FRAME \
 	CFI_STARTPROC simple;\
+	CFI_SIGNAL_FRAME;\
 	CFI_DEF_CFA esp, OLDESP-EBX;\
 	/*CFI_OFFSET cs, CS-OLDESP;*/\
 	CFI_OFFSET eip, EIP-OLDESP;\
@@ -263,6 +266,7 @@ need_resched:
 	# sysenter call handler stub
 ENTRY(sysenter_entry)
 	CFI_STARTPROC simple
+	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA esp, 0
 	CFI_REGISTER esp, ebp
 	movl TSS_sysenter_esp0(%esp),%esp
--- linux-2.6.18-rc6/arch/x86_64/ia32/ia32entry.S	2006-09-11 17:18:24.000000000 +0200
+++ 2.6.18-rc6-unwind-adjust-caller-pc/arch/x86_64/ia32/ia32entry.S	2006-09-11 15:28:10.000000000 +0200
@@ -71,6 +71,7 @@
  */ 	
 ENTRY(ia32_sysenter_target)
 	CFI_STARTPROC32	simple
+	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA	rsp,0
 	CFI_REGISTER	rsp,rbp
 	swapgs
@@ -186,6 +187,7 @@ ENDPROC(ia32_sysenter_target)
  */ 	
 ENTRY(ia32_cstar_target)
 	CFI_STARTPROC32	simple
+	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA	rsp,PDA_STACKOFFSET
 	CFI_REGISTER	rip,rcx
 	/*CFI_REGISTER	rflags,r11*/
@@ -293,6 +295,7 @@ ia32_badarg:
 
 ENTRY(ia32_syscall)
 	CFI_STARTPROC	simple
+	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA	rsp,SS+8-RIP
 	/*CFI_REL_OFFSET	ss,SS-RIP*/
 	CFI_REL_OFFSET	rsp,RSP-RIP
@@ -370,6 +373,7 @@ ENTRY(ia32_ptregs_common)
 	popq %r11
 	CFI_ENDPROC
 	CFI_STARTPROC32	simple
+	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA	rsp,SS+8-ARGOFFSET
 	CFI_REL_OFFSET	rax,RAX-ARGOFFSET
 	CFI_REL_OFFSET	rcx,RCX-ARGOFFSET
--- linux-2.6.18-rc6/arch/x86_64/kernel/entry.S	2006-09-11 17:18:24.000000000 +0200
+++ 2.6.18-rc6-unwind-adjust-caller-pc/arch/x86_64/kernel/entry.S	2006-09-11 15:26:47.000000000 +0200
@@ -115,6 +115,7 @@
 	.macro	CFI_DEFAULT_STACK start=1
 	.if \start
 	CFI_STARTPROC	simple
+	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA	rsp,SS+8
 	.else
 	CFI_DEF_CFA_OFFSET SS+8
@@ -199,6 +200,7 @@ END(ret_from_fork)
 
 ENTRY(system_call)
 	CFI_STARTPROC	simple
+	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA	rsp,PDA_STACKOFFSET
 	CFI_REGISTER	rip,rcx
 	/*CFI_REGISTER	rflags,r11*/
@@ -316,6 +318,7 @@ END(system_call)
  */ 	
 ENTRY(int_ret_from_sys_call)
 	CFI_STARTPROC	simple
+	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA	rsp,SS+8-ARGOFFSET
 	/*CFI_REL_OFFSET	ss,SS-ARGOFFSET*/
 	CFI_REL_OFFSET	rsp,RSP-ARGOFFSET
@@ -476,6 +479,7 @@ END(stub_rt_sigreturn)
  */
 	.macro _frame ref
 	CFI_STARTPROC simple
+	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA rsp,SS+8-\ref
 	/*CFI_REL_OFFSET ss,SS-\ref*/
 	CFI_REL_OFFSET rsp,RSP-\ref
--- linux-2.6.18-rc6/include/asm-i386/dwarf2.h	2006-09-11 17:18:42.000000000 +0200
+++ 2.6.18-rc6-unwind-adjust-caller-pc/include/asm-i386/dwarf2.h	2006-09-11 15:16:45.000000000 +0200
@@ -28,6 +28,11 @@
 #define CFI_RESTORE .cfi_restore
 #define CFI_REMEMBER_STATE .cfi_remember_state
 #define CFI_RESTORE_STATE .cfi_restore_state
+#ifdef CONFIG_AS_CFI_SIGNAL_FRAME
+#define CFI_SIGNAL_FRAME .cfi_signal_frame
+#else
+#define CFI_SIGNAL_FRAME
+#endif
 
 #else
 
@@ -48,6 +53,7 @@
 #define CFI_RESTORE	ignore
 #define CFI_REMEMBER_STATE ignore
 #define CFI_RESTORE_STATE ignore
+#define CFI_SIGNAL_FRAME ignore
 
 #endif
 
--- linux-2.6.18-rc6/include/asm-i386/unwind.h	2006-09-11 17:18:42.000000000 +0200
+++ 2.6.18-rc6-unwind-adjust-caller-pc/include/asm-i386/unwind.h	2006-09-11 16:10:20.000000000 +0200
@@ -18,6 +18,7 @@ struct unwind_frame_info
 {
 	struct pt_regs regs;
 	struct task_struct *task;
+	unsigned call_frame:1;
 };
 
 #define UNW_PC(frame)        (frame)->regs.eip
@@ -42,6 +43,10 @@ struct unwind_frame_info
 	PTREGS_INFO(edi), \
 	PTREGS_INFO(eip)
 
+#define UNW_DEFAULT_RA(raItem, dataAlign) \
+	((raItem).where == Memory && \
+	 !((raItem).value * (dataAlign) + 4))
+
 static inline void arch_unw_init_frame_info(struct unwind_frame_info *info,
                                             /*const*/ struct pt_regs *regs)
 {
--- linux-2.6.18-rc6/include/asm-x86_64/dwarf2.h	2006-09-11 17:18:44.000000000 +0200
+++ 2.6.18-rc6-unwind-adjust-caller-pc/include/asm-x86_64/dwarf2.h	2006-09-11 15:17:13.000000000 +0200
@@ -28,6 +28,11 @@
 #define CFI_REMEMBER_STATE .cfi_remember_state
 #define CFI_RESTORE_STATE .cfi_restore_state
 #define CFI_UNDEFINED .cfi_undefined
+#ifdef CONFIG_AS_CFI_SIGNAL_FRAME
+#define CFI_SIGNAL_FRAME .cfi_signal_frame
+#else
+#define CFI_SIGNAL_FRAME
+#endif
 
 #else
 
@@ -45,6 +50,7 @@
 #define CFI_REMEMBER_STATE	#
 #define CFI_RESTORE_STATE	#
 #define CFI_UNDEFINED	#
+#define CFI_SIGNAL_FRAME	#
 
 #endif
 
--- linux-2.6.18-rc6/include/asm-x86_64/unwind.h	2006-09-11 17:18:44.000000000 +0200
+++ 2.6.18-rc6-unwind-adjust-caller-pc/include/asm-x86_64/unwind.h	2006-09-11 16:10:29.000000000 +0200
@@ -18,6 +18,7 @@ struct unwind_frame_info
 {
 	struct pt_regs regs;
 	struct task_struct *task;
+	unsigned call_frame:1;
 };
 
 #define UNW_PC(frame)        (frame)->regs.rip
@@ -57,6 +58,10 @@ struct unwind_frame_info
 	PTREGS_INFO(r15), \
 	PTREGS_INFO(rip)
 
+#define UNW_DEFAULT_RA(raItem, dataAlign) \
+	((raItem).where == Memory && \
+	 !((raItem).value * (dataAlign) + 8))
+
 static inline void arch_unw_init_frame_info(struct unwind_frame_info *info,
                                             /*const*/ struct pt_regs *regs)
 {
--- linux-2.6.18-rc6/kernel/unwind.c	2006-09-11 17:18:46.000000000 +0200
+++ 2.6.18-rc6-unwind-adjust-caller-pc/kernel/unwind.c	2006-09-11 16:44:02.000000000 +0200
@@ -603,6 +603,7 @@ int unwind(struct unwind_frame_info *fra
 #define FRAME_REG(r, t) (((t *)frame)[reg_info[r].offs])
 	const u32 *fde = NULL, *cie = NULL;
 	const u8 *ptr = NULL, *end = NULL;
+	unsigned long pc = UNW_PC(frame) - frame->call_frame;
 	unsigned long startLoc = 0, endLoc = 0, cfa;
 	unsigned i;
 	signed ptrType = -1;
@@ -612,7 +613,7 @@ int unwind(struct unwind_frame_info *fra
 
 	if (UNW_PC(frame) == 0)
 		return -EINVAL;
-	if ((table = find_table(UNW_PC(frame))) != NULL
+	if ((table = find_table(pc)) != NULL
 	    && !(table->size & (sizeof(*fde) - 1))) {
 		unsigned long tableSize = table->size;
 
@@ -647,7 +648,7 @@ int unwind(struct unwind_frame_info *fra
 			                        ptrType & DW_EH_PE_indirect
 			                        ? ptrType
 			                        : ptrType & (DW_EH_PE_FORM|DW_EH_PE_signed));
-			if (UNW_PC(frame) >= startLoc && UNW_PC(frame) < endLoc)
+			if (pc >= startLoc && pc < endLoc)
 				break;
 			cie = NULL;
 		}
@@ -657,16 +658,28 @@ int unwind(struct unwind_frame_info *fra
 		state.cieEnd = ptr; /* keep here temporarily */
 		ptr = (const u8 *)(cie + 2);
 		end = (const u8 *)(cie + 1) + *cie;
+		frame->call_frame = 1;
 		if ((state.version = *ptr) != 1)
 			cie = NULL; /* unsupported version */
 		else if (*++ptr) {
 			/* check if augmentation size is first (and thus present) */
 			if (*ptr == 'z') {
-				/* check for ignorable (or already handled)
-				 * nul-terminated augmentation string */
-				while (++ptr < end && *ptr)
-					if (strchr("LPR", *ptr) == NULL)
+				while (++ptr < end && *ptr) {
+					switch(*ptr) {
+					/* check for ignorable (or already handled)
+					 * nul-terminated augmentation string */
+					case 'L':
+					case 'P':
+					case 'R':
+						continue;
+					case 'S':
+						frame->call_frame = 0;
+						continue;
+					default:
 						break;
+					}
+					break;
+				}
 			}
 			if (ptr >= end || *ptr)
 				cie = NULL;
@@ -755,7 +768,7 @@ int unwind(struct unwind_frame_info *fra
 	state.org = startLoc;
 	memcpy(&state.cfa, &badCFA, sizeof(state.cfa));
 	/* process instructions */
-	if (!processCFI(ptr, end, UNW_PC(frame), ptrType, &state)
+	if (!processCFI(ptr, end, pc, ptrType, &state)
 	   || state.loc > endLoc
 	   || state.regs[retAddrReg].where == Nowhere
 	   || state.cfa.reg >= ARRAY_SIZE(reg_info)
@@ -763,6 +776,11 @@ int unwind(struct unwind_frame_info *fra
 	   || state.cfa.offs % sizeof(unsigned long))
 		return -EIO;
 	/* update frame */
+#ifndef CONFIG_AS_CFI_SIGNAL_FRAME
+	if(frame->call_frame
+	   && !UNW_DEFAULT_RA(state.regs[retAddrReg], state.dataAlign))
+		frame->call_frame = 0;
+#endif
 	cfa = FRAME_REG(state.cfa.reg, unsigned long) + state.cfa.offs;
 	startLoc = min((unsigned long)UNW_SP(frame), cfa);
 	endLoc = max((unsigned long)UNW_SP(frame), cfa);
@@ -866,6 +884,7 @@ int unwind_init_frame_info(struct unwind
                            /*const*/ struct pt_regs *regs)
 {
 	info->task = tsk;
+	info->call_frame = 0;
 	arch_unw_init_frame_info(info, regs);
 
 	return 0;
@@ -879,6 +898,7 @@ int unwind_init_blocked(struct unwind_fr
                         struct task_struct *tsk)
 {
 	info->task = tsk;
+	info->call_frame = 0;
 	arch_unw_init_blocked(info);
 
 	return 0;
@@ -894,6 +914,7 @@ int unwind_init_running(struct unwind_fr
                         void *arg)
 {
 	info->task = current;
+	info->call_frame = 0;
 
 	return arch_unwind_init_running(info, callback, arg);
 }



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [development-gcc] Re: do_exit stuck
  2006-09-11 15:37       ` [development-gcc] Re: do_exit stuck Jan Beulich
@ 2006-09-11 20:17         ` Andi Kleen
  2006-09-12  6:57           ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2006-09-11 20:17 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Michael Matz, Richard Guenther, linux-kernel

On Monday 11 September 2006 17:37, Jan Beulich wrote:
> >>> Andi Kleen <ak@suse.de> 30.08.06 17:40 >>>
> >
> >On Wednesday 30 August 2006 17:28, Jan Beulich wrote:
> >> >Hmm, yes.  Sigh, so it's either gcc changes or binutils changes, and
> >> > none of that can be relied upon, except someone has a better idea to
> >> > identify signal frames with high enough assurance to be sensible for
> >> > debugging.
> >>
> >> The only alternative idea I have is to take into consideration the
> >> location of the return address: if it's at the default location (top of
> >> call frame), then assume this is a normal frame, in all other cases
> >> assume it's an interrupt/exception one. This will probably get a few
> >> cases wrong (where the return address is being played with), but it
> >> might be better than the current situation. Andi?
> >
> >Fine by me.
> >
> >At least it will likely be less controversal than changing all the
> > noreturns :)
>
> Here's a patch, including all that I think is necessary once we selectively
> enable (auto-detect) CONFIG_AS_CFI_SIGNAL_FRAME for newer binutils.
> It doesn't include adjustment of the printed address (i.e. for the original
> example, it'd still be kernel_math_error+0 that gets displayed, but the
> unwinder wouldn't get confused anymore.

Thanks.

Isn't a Kconfig patch missing? I don't see any place that defines 
CONFIG_AS_CFI_SIGNAL_FRAME. Actually Kconfig wouldn't 
be very good for this, so auto testing would be preferable
(like the cfi test is doing) 

BTW the tree you generated it against doesn't seem to match the latest
tree. I had to fix some rejects.

Also it would be nice if you could give a full description that could
be used as a commit message.

Other than that it looks good.

Ok maybe a one liner comment on why UNW_DEFAULT_RA does this magic.

-Andi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [development-gcc] Re: do_exit stuck
  2006-09-12  6:57           ` Jan Beulich
@ 2006-09-12  6:32             ` Andi Kleen
  2006-09-12  8:35               ` Jakub Jelinek
  2006-09-12  8:45               ` Jan Beulich
  0 siblings, 2 replies; 6+ messages in thread
From: Andi Kleen @ 2006-09-12  6:32 UTC (permalink / raw)
  To: Jan Beulich, sam; +Cc: Michael Matz, Richard Guenther, linux-kernel

On Tuesday 12 September 2006 08:57, Jan Beulich wrote:
> >Isn't a Kconfig patch missing? I don't see any place that defines
> >CONFIG_AS_CFI_SIGNAL_FRAME. Actually Kconfig wouldn't
> >be very good for this, so auto testing would be preferable
> >(like the cfi test is doing)
>
> Using that framework was the intention (you used a CONFIG_
> prefix there, and so did I), but as I wasn't sure about its status,
> and as I also was doing this against plain 2.6.18-rc6, I didn't add
> the actual detection logic. Actually I also think that should be
> done a little differently to allow for better future extension, i.e.
> instead of adding to CFLAGS store the auto-detected results in
> a header and forcibly -include it.

Ok. I guess I'll do it in the same way as the CFI detection
and maybe one of the kbuild folks can figure out a better way longer term.

BTW which binutils release started supporting this properly?

-Andi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [development-gcc] Re: do_exit stuck
  2006-09-11 20:17         ` Andi Kleen
@ 2006-09-12  6:57           ` Jan Beulich
  2006-09-12  6:32             ` Andi Kleen
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2006-09-12  6:57 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Michael Matz, Richard Guenther, linux-kernel

>Isn't a Kconfig patch missing? I don't see any place that defines 
>CONFIG_AS_CFI_SIGNAL_FRAME. Actually Kconfig wouldn't 
>be very good for this, so auto testing would be preferable
>(like the cfi test is doing) 

Using that framework was the intention (you used a CONFIG_
prefix there, and so did I), but as I wasn't sure about its status,
and as I also was doing this against plain 2.6.18-rc6, I didn't add
the actual detection logic. Actually I also think that should be
done a little differently to allow for better future extension, i.e.
instead of adding to CFLAGS store the auto-detected results in
a header and forcibly -include it.

>BTW the tree you generated it against doesn't seem to match the latest
>tree. I had to fix some rejects.

I didn't create it against your quilt tree, that's true.

>Also it would be nice if you could give a full description that could
>be used as a commit message.

Below.

>Other than that it looks good.
>
>Ok maybe a one liner comment on why UNW_DEFAULT_RA does this magic.

In order to deal with gcc's somewhat broken handling of noreturn
functions (the call instruction in which may be immediately followed
by a subsequent function, thus leading to the call's return address
pointing into that [wrong] function), add heuristics to the unwinder
to distinguish standard call frames from syscall, exception, or
interruption ones. Also provide for utilizing newer gas'
.cfi_signal_frame for non-heuristic based detection, pending
addition of the respective assembler feature detection logic.

Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [development-gcc] Re: do_exit stuck
  2006-09-12  6:32             ` Andi Kleen
@ 2006-09-12  8:35               ` Jakub Jelinek
  2006-09-12  8:45               ` Jan Beulich
  1 sibling, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2006-09-12  8:35 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Jan Beulich, sam, Michael Matz, Richard Guenther, linux-kernel

On Tue, Sep 12, 2006 at 08:32:06AM +0200, Andi Kleen wrote:
> On Tuesday 12 September 2006 08:57, Jan Beulich wrote:
> > >Isn't a Kconfig patch missing? I don't see any place that defines
> > >CONFIG_AS_CFI_SIGNAL_FRAME. Actually Kconfig wouldn't
> > >be very good for this, so auto testing would be preferable
> > >(like the cfi test is doing)
> >
> > Using that framework was the intention (you used a CONFIG_
> > prefix there, and so did I), but as I wasn't sure about its status,
> > and as I also was doing this against plain 2.6.18-rc6, I didn't add
> > the actual detection logic. Actually I also think that should be
> > done a little differently to allow for better future extension, i.e.
> > instead of adding to CFLAGS store the auto-detected results in
> > a header and forcibly -include it.
> 
> Ok. I guess I'll do it in the same way as the CFI detection
> and maybe one of the kbuild folks can figure out a better way longer term.
> 
> BTW which binutils release started supporting this properly?

2006-02-27 and later CVS, so in H.J's releases that's 2.16.91.0.7 and later
(and in upstream 2.17 and later).  But there are several distributions that
backported the changes even to older binutils, so doing a compile time check
if assembler groks .cfi_signal_frame is preferrable to a version check.

	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [development-gcc] Re: do_exit stuck
  2006-09-12  6:32             ` Andi Kleen
  2006-09-12  8:35               ` Jakub Jelinek
@ 2006-09-12  8:45               ` Jan Beulich
  1 sibling, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2006-09-12  8:45 UTC (permalink / raw)
  To: sam, Andi Kleen; +Cc: Michael Matz, Richard Guenther, linux-kernel

>BTW which binutils release started supporting this properly?

2.17

Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2006-09-12  8:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <200608291332.18499.ak@suse.de>
     [not found] ` <Pine.LNX.4.64.0608301626150.6582@wotan.suse.de>
     [not found]   ` <44F5CAD0.76E4.0078.0@novell.com>
     [not found]     ` <200608301740.41729.ak@suse.de>
2006-09-11 15:37       ` [development-gcc] Re: do_exit stuck Jan Beulich
2006-09-11 20:17         ` Andi Kleen
2006-09-12  6:57           ` Jan Beulich
2006-09-12  6:32             ` Andi Kleen
2006-09-12  8:35               ` Jakub Jelinek
2006-09-12  8:45               ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).