tile: Check for pointer add overflow in memchr
Commit Message
On 1/13/2017 5:42 AM, Adhemerval Zanella wrote:
> On 12/01/2017 19:37, Chris Metcalf wrote:
>> + /* Handle possible addition overflow. */
>> + if (__glibc_unlikely ((unsigned long) last_byte_ptr < (unsigned long) s))
>> + last_byte_ptr = (const char *) UINTPTR_MAX;
>> +
> Wouldn't a branchfree saturating addition be better for tile?
Actually, it turns out that branching here is pretty reasonable.
Due to the way tilegx bundles instructions, we only add a single
bundle as a result of doing the comparison and branch. (If we
actually take the branch, that adds a couple of extra cycles, but I
think this is a pretty unlikely case, so we don't really care.)
We could use a cmov instruction to avoid the branch, but that
actually takes an additional cycle since there are more dependencies,
so the branch-taken case gets faster at the expense of the
branch-not-taken case, which seems like the wrong thing.
You're right that with the 32-bit case (either tilegx32 or tilepro)
we can use a saturating add instruction directly, which complexifies
the tilegx code a bit with #ifdefs, but we might as well do, I think.
Unfortunately, we don't have a 64-bit saturating add instruction.
@@ -20,6 +20,17 @@
#include <stdint.h>
#include "string-endian.h"
+static uintptr_t
+saturating_add (uintptr_t p, size_t n)
+{
+#ifdef _LP64
+ uintptr_t result = p + n;
+ return __glibc_likely (result >= p) ? result : UINTPTR_MAX;
+#else
+ return __insn_addxsc (p, n);
+#endif
+}
+
void *
__memchr (const void *s, int c, size_t n)
{
@@ -49,7 +60,7 @@ __memchr (const void *s, int c, size_t n)
v = (*p | before_mask) ^ (goal & before_mask);
/* Compute the address of the last byte. */
- last_byte_ptr = (const char *) s + n - 1;
+ last_byte_ptr = (const char *) saturating_add ((uintptr_t) s, n - 1);
/* Compute the address of the word containing the last byte. */
last_word_ptr = (const uint64_t *) ((uintptr_t) last_byte_ptr & -8);
@@ -48,8 +48,8 @@ __memchr (const void *s, int c, size_t n)
before_mask = (1 << (s_int << 3)) - 1;
v = (*p | before_mask) ^ (goal & before_mask);
- /* Compute the address of the last byte. */
- last_byte_ptr = (const char *) s + n - 1;
+ /* Compute the address of the last byte using a saturating add. */
+ last_byte_ptr = (const char *) __insn_adds ((uintptr_t) s, n - 1);
/* Compute the address of the word containing the last byte. */
last_word_ptr = (const uint32_t *) ((uintptr_t) last_byte_ptr & -4);