Make Detect_Rebol_Pointer 100% robust, simpler

This simplifies the UTF8 detection and makes it basically foolproof.

The leftmost bit is always 1 on Rebol series/values ("NODE_FLAG_NODE"),
then the second bit is NODE_FLAG_FREE, which should be 0 on all valid
series/values. That means all non-"free" series and non-trash values
will start with 10, an invalid leading bit pattern in UTF-8.

Then the special bytes 11000000 and 11000001, 192 and 193, are also
illegal unicode (while in general 11xxxxxx are valid). So that means
bit #8 is used for NODE_FLAG_CELL.

Free series nodes use 11000000 as their first byte, trash cells use
11000001 for theirs.  With this strategy, it is possible to tell the
difference between basically everything with no false positives: free
states of Rebol values and series are still distinguishable from UTF8.
The simplified (and complete) list of detectable pointers are:

    DETECTED_AS_UTF8
    DETECTED_AS_SERIES
    DETECTED_AS_FREED_SERIES
    DETECTED_AS_VALUE
    DETECTED_AS_END
    DETECTED_AS_TRASH_CELL

The cost of the scheme is one bit: the high one that's always 1, which
could be used for other things otherwise.  However, this required a lot
of flags to get shuffled around.

As a side-effect of the new layout, an END cell does not need to have
NODE_FLAG_MANAGED set.  This helps with logic that needs to operate
on a node that doesn't know if it's dealing with a series or a value
cell, but wants to quickly test the managed flag for meaning.

src/core/b-init.c

@@ -147,6 +147,18 @@ static void Assert_Basics(void)
     // you'd like to catch IS_END() tests on trash.)
     //
     assert(REB_MAX < 256);
+
+    // Make sure tricks for "internal END markers" are lined up as expected.
+    //
+    assert(SERIES_INFO_0_IS_TRUE == NODE_FLAG_NODE);
+    assert(SERIES_INFO_1_IS_FALSE == NODE_FLAG_FREE);
+    assert(SERIES_INFO_4_IS_TRUE == NODE_FLAG_END);
+    assert(SERIES_INFO_7_IS_FALSE == NODE_FLAG_CELL);
+
+    assert(DO_FLAG_0_IS_TRUE == NODE_FLAG_NODE);
+    assert(DO_FLAG_1_IS_FALSE == NODE_FLAG_FREE);
+    assert(DO_FLAG_4_IS_TRUE == NODE_FLAG_END);
+    assert(DO_FLAG_7_IS_FALSE == NODE_FLAG_CELL);
 }
 
 

src/core/c-context.c

@@ -1344,7 +1344,7 @@ void Assert_Context_Core(REBCTX *c)
     if (!CTX_KEYLIST(c))
         panic (c);
 
-    if (GET_SER_FLAG(keylist, CONTEXT_FLAG_STACK))
+    if (GET_SER_INFO(keylist, CONTEXT_INFO_STACK))
         panic (keylist);
 
     REBVAL *rootvar = CTX_VALUE(c);
@@ -1357,7 +1357,7 @@ void Assert_Context_Core(REBCTX *c)
     if (keys_len < 1)
         panic (keylist);
 
-    if (GET_SER_FLAG(CTX_VARLIST(c), CONTEXT_FLAG_STACK)) {
+    if (GET_SER_INFO(CTX_VARLIST(c), CONTEXT_INFO_STACK)) {
         if (vars_len != 1)
             panic (varlist);
     }

src/core/c-error.c

@@ -246,35 +246,33 @@ ATTRIBUTE_NO_RETURN void Fail_Core(const void *p)
     REBCTX *error;
 
     switch (Detect_Rebol_Pointer(p)) {
-    case DETECTED_AS_NON_EMPTY_UTF8: {
+    case DETECTED_AS_UTF8: {
         DECLARE_LOCAL (string);
         Init_String(string, Make_UTF8_May_Fail(cast(const char*, p)));
         error = Error(RE_USER, string, END);
         break; }
 
-    case DETECTED_AS_EMPTY_UTF8:
-        panic (p);
-
     case DETECTED_AS_SERIES: {
         REBSER *s = m_cast(REBSER*, cast(const REBSER*, p)); // don't mutate
         if (NOT_SER_FLAG(s, ARRAY_FLAG_VARLIST))
             panic (s);
         error = CTX(s);
         break; }
 
+    case DETECTED_AS_FREED_SERIES:
+        panic (p);
+
     case DETECTED_AS_VALUE: {
         const REBVAL *v = cast(const REBVAL*, p);
         error = Error(RE_INVALID_ARG, v, END);
         break; }
 
-    case DETECTED_AS_CELL_END:
-        panic (p);
-
-    case DETECTED_AS_INTERNAL_END:
+    case DETECTED_AS_END:
+    case DETECTED_AS_TRASH_CELL:
         panic (p);
 
     default:
-        panic (p);
+        panic (p); // suppress compiler error from non-smart compilers
     }
 
     ASSERT_CONTEXT(error);

src/core/c-function.c

@@ -1121,9 +1121,8 @@ done_caching:;
 //
 REBCTX *Make_Expired_Frame_Ctx_Managed(REBFUN *func)
 {
-    REBARR *varlist = Alloc_Singular_Array_Core(
-        ARRAY_FLAG_VARLIST | CONTEXT_FLAG_STACK
-    );
+    REBARR *varlist = Alloc_Singular_Array_Core(ARRAY_FLAG_VARLIST);
+    SET_SER_INFO(varlist, CONTEXT_INFO_STACK);
     SET_BLANK(ARR_HEAD(varlist));
     MANAGE_ARRAY(varlist);
 

src/core/c-value.c

@@ -122,8 +122,8 @@ void Assert_Cell_Writable(const RELVAL *v, const char *file, int line)
 //
 void SET_END_Debug(RELVAL *v, const char *file, int line) {
     ASSERT_CELL_WRITABLE(v, file, line);
-    v->header.bits &= CELL_MASK_RESET;
-    v->header.bits |= NODE_FLAG_VALID | FLAGBYTE_FIRST(255);
+    v->header.bits &= CELL_MASK_RESET; // leaves NODE_FLAG_CELL, etc.
+    v->header.bits |= NODE_FLAG_NODE | NODE_FLAG_END;
     Set_Track_Payload_Debug(v, file, line);
 }
 
@@ -132,14 +132,14 @@ void SET_END_Debug(RELVAL *v, const char *file, int line) {
 //  IS_END_Debug: C
 //
 REBOOL IS_END_Debug(const RELVAL *v, const char *file, int line) {
-    if (NOT(v->header.bits & NODE_FLAG_VALID)) {
+    if (v->header.bits & NODE_FLAG_FREE) {
         printf("IS_END() called on garbage\n");
         panic_at(v, file, line);
     }
 
     if (IS_END_MACRO(v)) {
         if (v->header.bits & NODE_FLAG_CELL)
-            assert(LEFT_N_BITS(v->header.bits, 8) == 255);
+            assert(VAL_TYPE_RAW(v) == REB_0);
         return TRUE;
     }
     return FALSE;

src/core/d-crash.c

@@ -98,40 +98,14 @@ ATTRIBUTE_NO_RETURN void Panic_Core(
     strncat(buf, "\n", PANIC_BUF_SIZE - strlen(buf));
 
     switch (Detect_Rebol_Pointer(p)) {
-    case DETECTED_AS_NON_EMPTY_UTF8:
+    case DETECTED_AS_UTF8: // string might be empty...handle specially?
         strncat(
             buf,
             cast(const char*, p),
             PANIC_BUF_SIZE - strlen(buf)
         );
         break;
 
-    case DETECTED_AS_EMPTY_UTF8: {
-        //
-        // More often than not, what this actually is would be a freed series.
-        // Which ordinary code shouldn't run into, so you have to assume in
-        // general it's not an empty string.  But how often is an empty string
-        // passed to panic?
-        //
-        // If it is an empty string, you run the risk of causing an access
-        // violation by reading beyond that first 0 byte to see the full
-        // header.
-        //
-    #if !defined(NDEBUG)
-        const struct Reb_Header *h = cast(const struct Reb_Header*, p);
-        if (h->bits == 0) {
-            REBSER *s = m_cast(REBSER*, cast(const REBSER*, p)); // read only
-            Panic_Series_Debug(s);
-        }
-    #endif
-        const char *no_message = "[empty UTF-8 string (or freed series)]";
-        strncat(
-            buf,
-            no_message,
-            PANIC_BUF_SIZE - strlen(no_message)
-        );
-        break; }
-
     case DETECTED_AS_SERIES: {
         REBSER *s = m_cast(REBSER*, cast(const REBSER*, p)); // don't mutate
     #if !defined(NDEBUG)
@@ -159,39 +133,27 @@ ATTRIBUTE_NO_RETURN void Panic_Core(
     #endif
         break; }
 
-    case DETECTED_AS_VALUE:
+    case DETECTED_AS_FREED_SERIES:
     #if !defined(NDEBUG)
-        Panic_Value_Debug(cast(const REBVAL*, p));
-    #else
-        strncat(buf, "value", PANIC_BUF_SIZE - strlen(buf));
+        Panic_Series_Debug(m_cast(REBSER*, cast(const REBSER*, p)));
     #endif
+        strncat(buf, "freed series", PANIC_BUF_SIZE - strlen(buf));
         break;
 
-    case DETECTED_AS_CELL_END:
+    case DETECTED_AS_VALUE:
+    case DETECTED_AS_END:
     #if !defined(NDEBUG)
         Panic_Value_Debug(cast(const REBVAL*, p));
     #else
-        strncat(buf, "full cell-sized end", PANIC_BUF_SIZE - strlen(buf));
+        strncat(buf, "value", PANIC_BUF_SIZE - strlen(buf));
     #endif
         break;
 
-    case DETECTED_AS_INTERNAL_END:
+    case DETECTED_AS_TRASH_CELL:
     #if !defined(NDEBUG)
-        printf("Internal END marker, if array then panic-ing container:\n");
-        Panic_Series_Debug(cast(REBSER*,
-            m_cast(REBYTE*, cast(const REBYTE*, p))
-            - offsetof(struct Reb_Series, info)
-            + offsetof(struct Reb_Series, header)
-        ));
-    #else
-        strncat(buf, "internal end marker", PANIC_BUF_SIZE - strlen(buf));
+        Panic_Value_Debug(cast(const RELVAL*, p));
     #endif
-
-    default:
-        strncat(buf,
-            "Detect_Rebol_Pointer() failure",
-            PANIC_BUF_SIZE - strlen(buf)
-        );
+        strncat(buf, "trash cell", PANIC_BUF_SIZE - strlen(buf));
         break;
     }
 
@@ -209,10 +171,5 @@ ATTRIBUTE_NO_RETURN void Panic_Core(
 
     OS_CRASH(cb_cast(Str_Panic_Title), cb_cast(buf));
 
-    // Note that since we crash, we never return so that the caller can run
-    // a va_end on the passed-in args.  This is illegal in the general case:
-    //
-    //    http://stackoverflow.com/a/587139/211160
-
     DEAD_END;
 }