mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Matt Redfearn <matt.redfearn@imgtec.com>
To: Ralf Baechle <ralf@linux-mips.org>
Cc: <linux-mips@linux-mips.org>,
	Matt Redfearn <matt.redfearn@imgtec.com>,
	Marcin Nowakowski <marcin.nowakowski@imgtec.com>,
	<linux-kernel@vger.kernel.org>,
	James Hogan <james.hogan@imgtec.com>,
	Ingo Molnar <mingo@kernel.org>,
	Paul Burton <paul.burton@imgtec.com>
Subject: [PATCH v3 1/6] MIPS: Handle non word sized instructions when examining frame
Date: Tue, 8 Aug 2017 13:22:30 +0100	[thread overview]
Message-ID: <1502194955-18018-2-git-send-email-matt.redfearn@imgtec.com> (raw)
In-Reply-To: <1502194955-18018-1-git-send-email-matt.redfearn@imgtec.com>

Commit 34c2f668d0f6b ("MIPS: microMIPS: Add unaligned access support.")
added fairly broken support for handling 16bit microMIPS instructions in
get_frame_info(). It adjusts the instruction pointer by 16bits in the
case of a 16bit sp move instruction, but not any other 16bit
instruction.

Commit b6c7a324df37 ("MIPS: Fix get_frame_info() handling of microMIPS
function size") goes some way to fixing get_frame_info() to iterate over
microMIPS instuctions, but the instruction pointer is still manipulated
using a postincrement, and is of union mips_instruction type. Since the
union is sized to the largest member (a word), but microMIPS
instructions are a mix of halfword and word sizes, the function does not
always iterate correctly, ending up misaligned with the instruction
stream and interpreting it incorrectly.

Since the instruction modifying the stack pointer is usually the first
in the function, that one is usually handled correctly. But the
instruction which saves the return address to the sp is some variable
number of instructions into the frame and is frequently missed due to
not being on a word boundary, leading to incomplete walking of the
stack.

Fix this by incrementing the instruction pointer based on the size of
the previously decoded instruction (& remove the hack introduced by
commit 34c2f668d0f6b ("MIPS: microMIPS: Add unaligned access support.")
which adjusts the instruction pointer in the case of a 16bit sp move
instruction, but not any other).

Fixes: 34c2f668d0f6b ("MIPS: microMIPS: Add unaligned access support.")
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>

---

Changes in v3:
- Remove hack which attempted to cope with 16bit instructions

Changes in v2:
- Keep locals in reverse christmas tree order

 arch/mips/kernel/process.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 5351e1f3950d..5950ecf469e9 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -348,6 +348,7 @@ static int get_frame_info(struct mips_frame_info *info)
 	bool is_mmips = IS_ENABLED(CONFIG_CPU_MICROMIPS);
 	union mips_instruction insn, *ip, *ip_end;
 	const unsigned int max_insns = 128;
+	unsigned int last_insn_size = 0;
 	unsigned int i;
 
 	info->pc_offset = -1;
@@ -359,15 +360,19 @@ static int get_frame_info(struct mips_frame_info *info)
 
 	ip_end = (void *)ip + info->func_size;
 
-	for (i = 0; i < max_insns && ip < ip_end; i++, ip++) {
+	for (i = 0; i < max_insns && ip < ip_end; i++) {
+		ip = (void *)ip + last_insn_size;
 		if (is_mmips && mm_insn_16bit(ip->halfword[0])) {
 			insn.halfword[0] = 0;
 			insn.halfword[1] = ip->halfword[0];
+			last_insn_size = 2;
 		} else if (is_mmips) {
 			insn.halfword[0] = ip->halfword[1];
 			insn.halfword[1] = ip->halfword[0];
+			last_insn_size = 4;
 		} else {
 			insn.word = ip->word;
+			last_insn_size = 4;
 		}
 
 		if (is_jump_ins(&insn))
@@ -389,8 +394,6 @@ static int get_frame_info(struct mips_frame_info *info)
 						tmp = (ip->halfword[0] >> 1);
 						info->frame_size = -(signed short)(tmp & 0xf);
 					}
-					ip = (void *) &ip->halfword[1];
-					ip--;
 				} else
 #endif
 				info->frame_size = - ip->i_format.simmediate;
-- 
2.7.4

  reply	other threads:[~2017-08-08 12:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-08 12:22 [PATCH v3 0/6] MIPS: Further microMIPS stack unwinding fixes Matt Redfearn
2017-08-08 12:22 ` Matt Redfearn [this message]
2017-08-08 12:22 ` [PATCH v3 2/6] MIPS: microMIPS: Fix detection of addiusp instruction Matt Redfearn
2017-08-08 12:22 ` [PATCH v3 3/6] MIPS: microMIPS: Fix decoding " Matt Redfearn
2017-08-08 12:22 ` [PATCH v3 4/6] MIPS: microMIPS: Fix decoding of swsp16 instruction Matt Redfearn
2017-08-08 12:22 ` [PATCH v3 5/6] MIPS: Stacktrace: Fix microMIPS stack unwinding on big endian systems Matt Redfearn
2017-08-08 12:22 ` [PATCH v3 6/6] MIPS: Refactor handling of stack pointer in get_frame_info Matt Redfearn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1502194955-18018-2-git-send-email-matt.redfearn@imgtec.com \
    --to=matt.redfearn@imgtec.com \
    --cc=james.hogan@imgtec.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=marcin.nowakowski@imgtec.com \
    --cc=mingo@kernel.org \
    --cc=paul.burton@imgtec.com \
    --cc=ralf@linux-mips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®