Merge pull request #539 from svaarala/fix-utf8-clen-arith-order

Fix utf8 clen fast path pointer arithmetic issue

RELEASES.rst

@@ -1372,6 +1372,10 @@ Planned
   immediately which makes the cause of the unsafe behavior difficult to
   diagnose (GH-531)
 
+* Fix pointer arithmetic portability issues with platforms/compilers with
+  exotic pointer models by avoiding arithmetic and binary operations on
+  (u)intptr_t values (GH-530, GH-539)
+
 2.0.0 (XXXX-XX-XX)
 ------------------
 

doc/code-issues.rst

@@ -535,6 +535,7 @@ There doesn't seem to be a nice portable approach:
 * Casting through an integer (e.g. ``(void *) (duk_uintptr_t) const_ptr``)
   works but assumes that pointers can be safely cast through an integer.
   This is not necessarily portable to platforms with segmented pointers.
+  Also, ``(u)intptr_t`` is an optional type in C99.
 
 If a const-losing cast is required internally, the following macro is used
 to cast an arbitrary const pointer into a ``void *``::
@@ -600,6 +601,36 @@ One workaround would be to implement all comparisons by looking at the IEEE
 byte representation directly (using a union with double and byte array).
 This is a rather heavy workaround though.
 
+Avoid (u)intptr_t arithmetic
+----------------------------
+
+The ``(u)intptr_t`` types are optional in C99 so it's best to avoid using
+them whenever possible.  Duktape provides ``duk_(u)intptr_t`` even when
+they're missing.
+
+Platforms/compilers with exotic pointer models may have unexpected behavior
+when a pointer is cast to ``(u)intptr_t`` and then used in arithmetic or
+binary operations.  For more details, see:
+
+* https://github.com/svaarala/duktape/issues/530#issuecomment-171654860
+
+* https://github.com/svaarala/duktape/issues/530#issuecomment-171697759
+
+Arithmetic on integer cast pointer values may be needed e.g. for alignment::
+
+    /* Align to 4. */
+    while (((duk_size_t) (p)) & 0x03) {
+        p++;
+    }
+
+**Don't** use ``duk_(u)intptr_t`` in such cases to avoid portability issues
+with exotic pointer models::
+
+    /* AVOID THIS */
+    while (((duk_uintptr_t) (p)) & 0x03) {
+        p++;
+    }
+
 Symbol visibility
 =================
 

src/duk_api_codec.c

@@ -473,7 +473,7 @@ DUK_EXTERNAL const char *duk_hex_encode(duk_context *ctx, duk_idx_t index) {
 	DUK_ASSERT(buf != NULL);
 
 #if defined(DUK_USE_HEX_FASTPATH)
-	DUK_ASSERT((((duk_uintptr_t) buf) & 0x01U) == 0);   /* pointer is aligned, guaranteed for fixed buffer */
+	DUK_ASSERT((((duk_size_t) buf) & 0x01U) == 0);   /* pointer is aligned, guaranteed for fixed buffer */
 	p16 = (duk_uint16_t *) (void *) buf;
 	len_safe = len & ~0x03U;
 	for (i = 0; i < len_safe; i += 4) {

src/duk_bi_json.c

@@ -1460,7 +1460,7 @@ DUK_LOCAL duk_uint8_t *duk__enc_buffer_data_hex(const duk_uint8_t *src, duk_size
 #if defined(DUK_USE_UNALIGNED_ACCESSES_POSSIBLE)
 	q16 = (duk_uint16_t *) (void *) dst;
 #else
-	shift_dst = (duk_bool_t) (((duk_uintptr_t) dst) & 0x01U);
+	shift_dst = (duk_bool_t) (((duk_size_t) dst) & 0x01U);
 	if (shift_dst) {
 		DUK_DD(DUK_DDPRINT("unaligned accesses not possible, dst not aligned -> step to dst + 1"));
 		q16 = (duk_uint16_t *) (void *) (dst + 1);
@@ -1468,7 +1468,7 @@ DUK_LOCAL duk_uint8_t *duk__enc_buffer_data_hex(const duk_uint8_t *src, duk_size
 		DUK_DD(DUK_DDPRINT("unaligned accesses not possible, dst is aligned"));
 		q16 = (duk_uint16_t *) (void *) dst;
 	}
-	DUK_ASSERT((((duk_uintptr_t) q16) & 0x01U) == 0);
+	DUK_ASSERT((((duk_size_t) q16) & 0x01U) == 0);
 #endif
 
 	len_safe = src_len & ~0x03U;

src/duk_unicode_support.c

@@ -311,7 +311,7 @@ DUK_INTERNAL duk_size_t duk_unicode_unvalidated_utf8_length(const duk_uint8_t *d
 	/* Align 'p' to 4; the input data may have arbitrary alignment.
 	 * End of string check not needed because blen >= 16.
 	 */
-	while (((duk_uintptr_t) (const void *) p) & 0x03) {
+	while (((duk_size_t) (const void *) p) & 0x03U) {
 		duk_uint8_t x;
 		x = *p++;
 		if (DUK_UNLIKELY(x >= 0x80 && x <= 0xbf)) {