From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932985AbcFLRom (ORCPT ); Sun, 12 Jun 2016 13:44:42 -0400 Received: from mail-bl2on0077.outbound.protection.outlook.com ([65.55.169.77]:40538 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752555AbcFLRoj (ORCPT ); Sun, 12 Jun 2016 13:44:39 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Yuri.Norov@caviumnetworks.com; Date: Sun, 12 Jun 2016 20:44:30 +0300 From: Yury Norov To: "Zhangjian (Bamvor)" CC: , , , , , , , , , , , , , , , , , , , , , , Andrew Pinski , Andrew Pinski , Hanjun Guo Subject: Re: [PATCH 21/23] arm64: ilp32: introduce ilp32-specific handlers for sigframe and ucontext Message-ID: <20160612174430.GA12067@yury-N73SV> References: <1464048292-30136-1-git-send-email-ynorov@caviumnetworks.com> <1464048292-30136-22-git-send-email-ynorov@caviumnetworks.com> <5752BCC8.7080205@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <5752BCC8.7080205@huawei.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [96.95.216.225] X-ClientProxiedBy: CY1PR12CA0073.namprd12.prod.outlook.com (10.163.230.41) To DM3PR07MB2249.namprd07.prod.outlook.com (10.164.33.147) X-MS-Office365-Filtering-Correlation-Id: d3411aca-9dd4-46cd-4b57-08d392e937b7 X-Microsoft-Exchange-Diagnostics: 1;DM3PR07MB2249;2:1iqmuovhqNfwcPMOj93lOdj/Ns039AMtuXI9ET7S0ianu97USvRnixj9whJ60sbvoloGmEyRweMA8fOZfYoW7XBYSXa92W4amHXU9btDHVEVMxvu7icbmC1TOwmMfYbA8C8GSkXI/Xiol2XXssfAA/SSxREFLUHcQMCHfOR+xIoBv2ER5j2OLUihdQcLEulc;3:9ZXu2PZZ4EbYkAbEx85BbPBQzRF5G+r7atwnovQSvLicq/yC3HhtOZqGD5X/bliGu4kI+nevOxFW0MKk08kPLjAzAo4CNmttj2+AA/zDntALtG6UE6SClIJ/4xRAZkyg;25:sOms/LjCG6m+D7uq2QPqqN6tM2txLuyA4prAv+v7ay0AP1OaFrzWJLACZSp2vT6K3j9UUcj3QuilVK1B45kw7lJoy/aE6zTgaBd8tKCnTHix371C6jy9Qb8HconGdfo9kx6sSqOalLX73xqUr1UmROLe+LeOcc2J3CvWw14i55xNhjrKjdMb9cNyyDZzmOszNP468SR36bkBX0ZMs0rfl88JUdDdSpigl5kjkqvImbliL6FbL2NXb9DAXSOEbFA2ErGGX1cZ2rkRP+zFn+ZdCbTdnh+0/TnX3URKn0ZchvV/vk9b4LmUCMR+vkguiIxpv4SGMIecHd7Ni5EI9sOI6oGOfAL4qmsMqNePikaX/hIlFE3IZMRgs+VidH2Ep5QmOClt/99ymOH6KKWIzp21uyrCIcsm7n/LCNc5UKEvtto= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM3PR07MB2249; X-Microsoft-Exchange-Diagnostics: 1;DM3PR07MB2249;20:CLjmpiOp5OqO//R2XM55VyTbF28SVKsrMMfiDMiFib3ZDIU/v7OQscI9dAUEoNAuFIR7vd3OlE8IumA2ya/vsC2bq6mDw+tY/zoOwhGllTdFasuTv9CfuB6WaumEkwxvsXAG7mSNIuh2rP8YFS+DkJ0aUALXyNqgAu+901SvlFJp3OMqNHNEdnCDjEtjZA3rYGrFJ1h8vk7QrD/wNVwIGBLbxafohpkizr+QoziAVPGD3DPg1TMAHO4BfBidwpAud5VwD0dwg4L5nHz0pw1ViRi4Ljw1aF2bO3wZnfa/dUYt+riZkklHfjgztD5prayyBMGU0iQ//XyAhPErfaOEgD6xf076QmOKWj3QbeHIME4ABj82lIjKU9DLVXxjME0eoz75H967ez5f5oj2jFvGsEjn3fS+DztCfRdOdLmI/hGUIHYZmLmvoNzqLbHZYKX6hgVAyxkUuzXEkcHpajRhWGNMTz39oF4hT4ds7xnZs+UQrD/Bu0DEoo9ZBWVPA476x+TwLBkboMnn/sDlKIdsIIl7KZbD4gImENYh3mh4mr12ZvIGfZpOOF5VBr2C+shnnNTnhWmHARzkgbnlQMp7Uk4WF70AHmr7RapTGxPoOAg= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(50582790962513)(17755550239193); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046);SRVR:DM3PR07MB2249;BCL:0;PCL:0;RULEID:;SRVR:DM3PR07MB2249; X-Microsoft-Exchange-Diagnostics: 1;DM3PR07MB2249;4:nHuQtItwrn93/7p3Q8ead+ehBp06/ThCJd+8V1UkMbHS/AYBVcOQ3GA+ZUepHinHHpnA2e28NEYfDxdApD51/Thk2hD8FFTpXsZ48uty8+cdNgfQtIsebObf5zE6QB1mo/bOIOUgehrjdrrx756J8kiVGKe4Cf+ihebtlrI8AlG3jpJP2k2ai82mGJqQaWNhSBHhJtOUfvk3iF5clAoYJpkxqfiu5oBmqLjRxDoPLCUqKxXKKjrb9qLsatuYdZSfvOpjzkRBbF3aHfvMvk0T32AaG/QDJj+/itCyhsj2VxQsyegn7Nm/NF2goQk1sC0SjyBSXHi6Lncug4j5SOTQZDhqXcrGDade1VpKVs7h9cCKbfHcPmJA75yTS9r2myIDjAtQQcZtq/GbMm91q0tRNtmZw9sd6Pyr9AYTKCrtxjvNCNzv450tJSpDqVyDpNZefB/C/DNLjzImTsfYPZyZ5Q== X-Forefront-PRVS: 0971922F40 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(6069001)(7916002)(189002)(199003)(24454002)(19580395003)(9686002)(19580405001)(76506005)(1076002)(23726003)(3846002)(6116002)(5004730100002)(4001350100001)(92566002)(97736004)(586003)(101416001)(97756001)(105586002)(106356001)(33656002)(83506001)(189998001)(42186005)(5008740100001)(77096005)(68736007)(4326007)(2906002)(50466002)(66066001)(46406003)(47776003)(110136002)(2950100001)(54356999)(76176999)(50986999)(8676002)(33716001)(81166006)(81156014);DIR:OUT;SFP:1101;SCL:1;SRVR:DM3PR07MB2249;H:localhost;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;DM3PR07MB2249;23:rmN3Detv2VEvplWRpnkmxiNs+r8BUldq+JucfreSA?= =?us-ascii?Q?KGHnZsDf0q17aOG9wz9qYQIlgVZGBdK/8sJYOGeG7wLpPFUyK4c5QAA8GhnC?= =?us-ascii?Q?xqY+2kCgTQGzVwHyB7PEJNIAz837MEAKklA0CwxHJfipSF5PyP/M9JBhrB0t?= =?us-ascii?Q?t5lBjgDDU3pTMoR0IiuglWPY6xGBWIlkPUCbMlfOT8S5mKNxmwapzWSVpnN4?= =?us-ascii?Q?LQYM1B3QccV+gazpT6nEIqnQ4Fr0IGr+y6G4ercLuULua9SNjj9KTFHBVo6K?= =?us-ascii?Q?1P0w4/T55ohQ1OqKm+zBKBoGMr0wUt31qCxyACw01PEmyG3aROWXnAC2ztBu?= =?us-ascii?Q?KCW5to6gs8pCAS9+BtbZSwzqrP1GM9sTWs0eohYM+f9Yl/lRcFupDrqRFCwU?= =?us-ascii?Q?h6g58PWSN1pd8yF6HpPtax9kS9gqgk30vmoudm5jq8rvZPBff07eLeOL+blW?= =?us-ascii?Q?qgbV+5Gz7DZAbRpqx0RLbOqLxJNwlUQ/V41BNFhgHKyoHnh+6gdK/+F8gy8B?= =?us-ascii?Q?J2BHhMal2XjV5W3qjNwT4C9FeSwByE+x1oKYYVv7p3A9CRdO4M5T5T1N5KIx?= =?us-ascii?Q?/OsXNDe73GiKYhQAI8WcMyK3/S1tKIL4bSF476kAGeBc/u/RenYfWT2OvjCG?= =?us-ascii?Q?GMUHQ5HMXTr/mwNBsaFLuhSzZ5r+YtijTy4irl2bPzIigOCpuBVx1B2YKNc0?= =?us-ascii?Q?aDOyqZjwHS2xJW+3nDGs+rX7LoTTsg47mcYuAryYpgv4UeiRJED6E6ZnCXhV?= =?us-ascii?Q?9vwjX86XSmZoL/WRQoaHwDMkHIH1+JgDAUXMGL6flu9LjVs8XpIofmEGTSWV?= =?us-ascii?Q?kuBnTzvQXabUIthoZf8VFIVErOX5q9a/QY2KZzoPPwrRJxAUIYcRqpBmHi7U?= =?us-ascii?Q?909IJMlqKPrJh6pWSBr9trzRPS8xeAWNM6sPYmKUZYIVNjQASlROgyPOSGa3?= =?us-ascii?Q?FH5+HTHOzSBxGJ+B3817O3IsEYxQv1aUPTJ8zCNVttbXwYYNIcM8pG9JlRYN?= =?us-ascii?Q?o/j86lJR0hab0JS7Em8FXwatS3I2D9fFNo2uQDlkfgpoeNP6M42Mr1Ndv9Ht?= =?us-ascii?Q?YI6m6/saTlem78j23pH4o93jXRa6S7wrseTwqKqGPcL3m75owdjEVm7tsRVq?= =?us-ascii?Q?Ki61925Qa8Adot4yVHiZRfpMLWVcZlIDmrPQQoRSEF2OI7i3ZhQ88TBaDvey?= =?us-ascii?Q?n7L9NVFH98ULF0=3D?= X-Microsoft-Exchange-Diagnostics: 1;DM3PR07MB2249;5:hoCjYrBxZtlOsYX4nZCnlssDgziRgf18bVb9XLjZpk92ZGWXpey7i91kTTvJTi/F9MpaxWjFT196AYBoVMnXKylRJ0O2+NVAaaWIHN3TmJB3O0ZHdKw3CghnD+WOvlDRrWyiDdGzvgaJnwVofx3Y+g==;24:o78bsAI501VWF8se5JK9ULsC+gxQpF+9QElXB6gZpehNNC9b/cC0Nrknx3TmgGHrRqi7qdccS8VZqFEMudL859Pab+69c9IiJa+EkNXNTQQ=;7:dwgKUtnpSHDtubdGANNSPMaOLy4V93Km+Wj+Pzn9KMLMAfPIt1p96HsF2optsGv9EABiM63lgSSbljkqzPrbBugYOkJfu729UdV5+8CbxjiZBT95vHIwxt5YtwIYJr15oFKpwmrUBTjlwZOYDjBqHWRPi8JZlX7bZBI6nzMS+4zGtFGKGyGRTgcbB4JtUmFXtP+sG9eDDdq3GtSvVrR/9xV3OoQMERQgSezA44t+K/M= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jun 2016 17:44:34.5602 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR07MB2249 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Bamvor, Sorry, I missed this patch. On Sat, Jun 04, 2016 at 07:34:32PM +0800, Zhangjian (Bamvor) wrote: > Hi, > > I found an issue of unwind with the following code. The correct backtrace > should be: > (gdb) where > #0 0x004004d0 in my_sig (sig=11) at test_force3.c:16 > #1 > #2 func2 (num=0) at test_force3.c:22 > #3 0x00400540 in func1 (num=1) at test_force3.c:28 > #4 0x00400574 in main (argc=1, argv=0xffd7bc04) at test_force3.c:33 > > Without my patch, the backtrace is: > (gdb) where > #0 0x00400490 in my_sig (sig=11) at test_force3.c:16 > #1 > #2 0x004004e4 in main (argc=1, argv=0xffe6f8f4) at test_force3.c:33 > > With my patch which fix the wrong frame pointer(setup_return calculate the offset > of fp through ilp32_sigframe instead of sigfreame), the backtrace is: > (gdb) where > #0 0x00400490 in my_sig (sig=11) at test_force3.c:16 > #1 > #2 func1 () at test_force3.c:28 > #3 0x004004e4 in main (argc=1, argv=0xffe6f8f4) at test_force3.c:33 > > I am not sure there is still some issue in kernel. But it seem that the gdb of ilp32 > does not work correctly when unwind without framepointer. > > The test code is: > > From 7e364a765097f57aed2d73f94c1688c2e7343e79 Mon Sep 17 00:00:00 2001 > From: Bamvor Jian Zhang > Date: Sat, 4 Jun 2016 14:30:05 +0800 > Subject: [PATCH] arm64: ilp32: fix for wrong fp offset when calculate the > new fp > > ILP32 define its own sigframe(ilp32_sigframe) because of the > difference uc_context. setup_return do not use ilp32 specific > sigframe to calculate the new offset of fp which lead to wrong > fp in signal handler. At this circumstance, gdb backtrace will miss > one item: > (gdb) where > > It should be: > (gdb) where > > The test code is as follows: > > void my_sig(int sig) > { > printf("sig=%d\n", sig); > *(int *)0 = 0x0; > } > > void func2(int num) > { > printf("%s: %d\n", __FUNCTION__, num); > *(int *)0 = 0x0; > func2(num-1); > } > > void func1(int num) > { > printf("%s\n", __FUNCTION__); > func2(num - 1); > } > > int main(int argc, char **argv) > { > signal(11, my_sig); > func1(argc); > return 0; > } > > This patch fix this by passing the correct offset of fp to > setup_return. > Test pass on both ILP32 and LP64 in aarch64 EE. > > Signed-off-by: Bamvor Jian Zhang > --- > arch/arm64/include/asm/signal_common.h | 3 ++- > arch/arm64/kernel/signal.c | 9 +++++---- > arch/arm64/kernel/signal_ilp32.c | 4 ++-- > 3 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/arch/arm64/include/asm/signal_common.h b/arch/arm64/include/asm/signal_common.h > index de93c71..a5d7b63 100644 > --- a/arch/arm64/include/asm/signal_common.h > +++ b/arch/arm64/include/asm/signal_common.h > @@ -29,6 +29,7 @@ int setup_sigcontex(struct sigcontext __user *uc_mcontext, > struct pt_regs *regs); > int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sf); > void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > - void __user *frame, off_t sigframe_off, int usig); > + void __user *frame, off_t sigframe_off, off_t fp_off, > + int usig); > > #endif /* __ASM_SIGNAL_COMMON_H */ > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > index 038bebe..e66a6e9 100644 > --- a/arch/arm64/kernel/signal.c > +++ b/arch/arm64/kernel/signal.c > @@ -256,14 +256,14 @@ static struct rt_sigframe __user *get_sigframe(struct ksignal *ksig, > } > > void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > - void __user *frame, off_t sigframe_off, int usig) > + void __user *frame, off_t sigframe_off, off_t fp_off, > + int usig) > { > __sigrestore_t sigtramp; > > regs->regs[0] = usig; > regs->sp = (unsigned long)frame; > - regs->regs[29] = regs->sp + sigframe_off + > - offsetof(struct sigframe, fp); > + regs->regs[29] = regs->sp + sigframe_off + fp_off; I think you are right here. The only nitpick is what for we send 2 offsets just to add one to another inside setup_return()? We can do like this: void setup_return(struct pt_regs *regs, struct k_sigaction *ka, void __user *frame, off_t fp_off, int usig) { __sigrestore_t sigtramp; regs->regs[0] = usig; regs->sp = (unsigned long)frame; regs->regs[29] = regs->sp + fp_off; [...] } Where fp_off calculation is done by caller. setup_return(regs, &ksig->ka, frame, offsetof(struct rt_sigframe, sig) + offsetof(struct sigframe, fp), usig); For me it's more clear to understand what happens with this approach. I don't think struct rt_sigframe will grow, but we can even introduce some helper for it: #define RT_SIGFRAME_FP_POS (offsetof(struct rt_sigframe, sig) + offsetof(struct sigframe, fp)) If no objections, I'll apply your patch with my fix in next series. > regs->pc = (unsigned long)ka->sa.sa_handler; > > if (ka->sa.sa_flags & SA_RESTORER) > @@ -294,7 +294,8 @@ static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set, > err |= setup_sigframe(&frame->sig, regs, set); > if (err == 0) { > setup_return(regs, &ksig->ka, frame, > - offsetof(struct rt_sigframe, sig), usig); > + offsetof(struct rt_sigframe, sig), > + offsetof(struct sigframe, fp), usig); > if (ksig->ka.sa.sa_flags & SA_SIGINFO) { > err |= copy_siginfo_to_user(&frame->info, &ksig->info); > regs->regs[1] = (unsigned long)&frame->info; > diff --git a/arch/arm64/kernel/signal_ilp32.c b/arch/arm64/kernel/signal_ilp32.c > index a8ea73e..9030f14 100644 > --- a/arch/arm64/kernel/signal_ilp32.c > +++ b/arch/arm64/kernel/signal_ilp32.c > @@ -147,7 +147,6 @@ static struct ilp32_rt_sigframe __user *ilp32_get_sigframe(struct ksignal *ksig, > struct ilp32_rt_sigframe __user *frame; > > sp = sp_top = sigsp(regs->sp, ksig); > - > sp = (sp - sizeof(struct ilp32_rt_sigframe)) & ~15; > frame = (struct ilp32_rt_sigframe __user *)sp; > > @@ -183,7 +182,8 @@ int ilp32_setup_rt_frame(int usig, struct ksignal *ksig, > err |= setup_ilp32_sigframe(&frame->sig, regs, set); > if (err == 0) { > setup_return(regs, &ksig->ka, frame, > - offsetof(struct ilp32_rt_sigframe, sig), usig); > + offsetof(struct ilp32_rt_sigframe, sig), > + offsetof(struct ilp32_sigframe, fp), usig); > regs->regs[1] = (unsigned long)&frame->info; > regs->regs[2] = (unsigned long)&frame->sig.uc; > } > -- > 1.8.4.5 > > Regards > > Bamvor > >