Thread (9 messages) 9 messages, 4 authors, 2019-03-31

Re: [PATCH v3] powerpc/64: Fix memcmp reading past the end of src/dest

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2019-03-25 12:36:06

Segher Boessenkool [off-list ref] writes:
On Fri, Mar 22, 2019 at 11:37:24PM +1100, Michael Ellerman wrote:
quoted
 .Lcmp_rest_lt8bytes:
-	/* Here we have only less than 8 bytes to compare with. at least s1
-	 * Address is aligned with 8 bytes.
-	 * The next double words are load and shift right with appropriate
-	 * bits.
+	/*
+	 * Here we have less than 8 bytes to compare. At least s1 is aligned to
+	 * 8 bytes, but s2 may not be. We must make sure s2 + 8 doesn't cross a
"s2 + 7"?  The code is fine though (bgt, not bge).
Duh, thanks for catching it.
quoted
+	 * page boundary, otherwise we might read past the end of the buffer and
+	 * trigger a page fault. We use 4K as the conservative minimum page
+	 * size. If we detect that case we go to the byte-by-byte loop.
+	 *
+	 * Otherwise the next double word is loaded from s1 and s2, and shifted
+	 * right to compare the appropriate bits.
 	 */
+	clrldi	r6,r4,(64-12)	// r6 = r4 & 0xfff
You can just write
  rlwinm r6,r4,0,0x0fff
if that is clearer?  Or do you still want a comment with that :-)
I don't think it's clearer doing a rotate of zero bits :)

And yeah I'd probably still leave the comment, so I'm inclined to stick
with the clrldi?

Would be nice if the assembler could support:

	andi	r6, r4, 0x0fff

And turn it into the rlwinm, or rldicl :)
quoted
+	cmpdi	r6,0xff8
+	bgt	.Lshort
Reviewed-by: Segher Boessenkool <redacted>
I'll fixup the comment. Thanks.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help