[Guile-commits] 06/07: peek-u8 correctness and speed refactor

[Andy Wingo]

wingo pushed a commit to branch wip-port-refactor
in repository guile.

commit bb6edc5a35c570b3355dd69f89cbc8d0f85fc21c
Author: Andy Wingo <address@hidden>
Date:   Tue Apr 19 22:58:33 2016 +0200

    peek-u8 correctness and speed refactor
    
    * libguile/ports-internal.h (scm_port_buffer_size): Verify that the
      bytevector field is a bytevector, in anticipation of Schemification.
      (scm_port_buffer_can_take, scm_port_buffer_can_put)
      (scm_port_buffer_can_putback): Enforce invariants on cur and end
      here.
      (scm_port_buffer_did_take, scm_port_buffer_did_put): Relax to not call
      other functions.
    * libguile/ports.h (scm_get_byte_or_eof_unlocked)
      (scm_peek_byte_or_eof_unlocked): Refactor to call no functions on the
      fast path.
---
 libguile/ports-internal.h |   42 ++++++++++++++++++++++++++++++++++++------
 libguile/ports.h          |   18 +++++++++++-------
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/libguile/ports-internal.h b/libguile/ports-internal.h
index 862d858..a8c4ea9 100644
--- a/libguile/ports-internal.h
+++ b/libguile/ports-internal.h
@@ -27,10 +27,26 @@
 #include "libguile/_scm.h"
 #include "libguile/ports.h"
 
+/* The port buffers are exposed to Scheme, which can mutate their
+   fields.  We have to do dynamic checks to ensure that
+   potentially-malicious Scheme doesn't invalidate our invariants.
+   However these dynamic checks are slow, so we need to avoid them where
+   they are unnecessary.  An unnecessary check is a check which has
+   already been performed, or one which would already be performed by
+   the time that memory is accessed.  Given that the "can_take",
+   "can_put", or "can_putback" functions are eventually called before
+   any access to the buffer, we hoist the necessary type checks the
+   can_foo and size functions, and otherwise assume that the cur and end
+   values are inums within the right ranges.  */
+
 static inline size_t
 scm_port_buffer_size (scm_t_port_buffer *buf)
 {
-  return scm_c_bytevector_length (buf->bytevector);
+  if (SCM_LIKELY (SCM_BYTEVECTOR_P (buf->bytevector)))
+    return SCM_BYTEVECTOR_LENGTH (buf->bytevector);
+  scm_misc_error (NULL, "invalid port buffer ~a",
+                  scm_list_1 (buf->bytevector));
+  return -1;
 }
 
 static inline void
@@ -48,31 +64,45 @@ scm_port_buffer_reset_end (scm_t_port_buffer *buf)
 static inline size_t
 scm_port_buffer_can_take (scm_t_port_buffer *buf)
 {
-  return scm_to_size_t (buf->end) - scm_to_size_t (buf->cur);
+  size_t cur, end;
+  cur = scm_to_size_t (buf->cur);
+  end = scm_to_size_t (buf->end);
+  if (cur > end || end > scm_port_buffer_size (buf))
+    scm_misc_error (NULL, "invalid port buffer cursors ~a, ~a",
+                    scm_list_2 (buf->cur, buf->end));
+  return end - cur;
 }
 
 static inline size_t
 scm_port_buffer_can_put (scm_t_port_buffer *buf)
 {
-  return scm_port_buffer_size (buf) - scm_to_size_t (buf->end);
+  size_t end = scm_to_size_t (buf->end);
+  if (end > scm_port_buffer_size (buf))
+    scm_misc_error (NULL, "invalid port buffer cursor ~a",
+                    scm_list_1 (buf->end));
+  return scm_port_buffer_size (buf) - end;
 }
 
 static inline size_t
 scm_port_buffer_can_putback (scm_t_port_buffer *buf)
 {
-  return scm_to_size_t (buf->cur);
+  size_t cur = scm_to_size_t (buf->cur);
+  if (cur > scm_port_buffer_size (buf))
+    scm_misc_error (NULL, "invalid port buffer cursor ~a",
+                    scm_list_1 (buf->cur));
+  return cur;
 }
 
 static inline void
 scm_port_buffer_did_take (scm_t_port_buffer *buf, size_t count)
 {
-  buf->cur = scm_from_size_t (scm_to_size_t (buf->cur) + count);
+  buf->cur = SCM_I_MAKINUM (SCM_I_INUM (buf->cur) + count);
 }
 
 static inline void
 scm_port_buffer_did_put (scm_t_port_buffer *buf, size_t count)
 {
-  buf->end = scm_from_size_t (scm_to_size_t (buf->end) + count);
+  buf->end = SCM_I_MAKINUM (SCM_I_INUM (buf->end) + count);
 }
 
 static inline const scm_t_uint8 *
diff --git a/libguile/ports.h b/libguile/ports.h
index 1735623..997b755 100644
--- a/libguile/ports.h
+++ b/libguile/ports.h
@@ -429,13 +429,15 @@ SCM_INLINE_IMPLEMENTATION int
 scm_get_byte_or_eof_unlocked (SCM port)
 {
   scm_t_port_buffer *buf = SCM_PTAB_ENTRY (port)->read_buf;
-  size_t cur;
+  size_t cur = SCM_I_INUM (buf->cur);
 
-  cur = scm_to_size_t (buf->cur);
-  if (SCM_LIKELY (cur < scm_to_size_t (buf->end)))
+  if (SCM_LIKELY (SCM_I_INUMP (buf->cur))
+      && SCM_LIKELY (SCM_I_INUMP (buf->end))
+      && SCM_LIKELY (cur < SCM_I_INUM (buf->end))
+      && SCM_LIKELY (cur < SCM_BYTEVECTOR_LENGTH (buf->bytevector)))
     {
       scm_t_uint8 ret = SCM_BYTEVECTOR_CONTENTS (buf->bytevector)[cur];
-      buf->cur = scm_from_size_t (cur + 1);
+      buf->cur = SCM_I_MAKINUM (cur + 1);
       return ret;
     }
 
@@ -459,10 +461,12 @@ SCM_INLINE_IMPLEMENTATION int
 scm_peek_byte_or_eof_unlocked (SCM port)
 {
   scm_t_port_buffer *buf = SCM_PTAB_ENTRY (port)->read_buf;
-  size_t cur;
+  size_t cur = SCM_I_INUM (buf->cur);
 
-  cur = scm_to_size_t (buf->cur);
-  if (SCM_LIKELY (cur < scm_to_size_t (buf->end)))
+  if (SCM_LIKELY (SCM_I_INUMP (buf->cur))
+      && SCM_LIKELY (SCM_I_INUMP (buf->end))
+      && SCM_LIKELY (cur < SCM_I_INUM (buf->end))
+      && SCM_LIKELY (cur < SCM_BYTEVECTOR_LENGTH (buf->bytevector)))
     {
       scm_t_uint8 ret = SCM_BYTEVECTOR_CONTENTS (buf->bytevector)[cur];
       return ret;