Changeset a5db488 for libcfa/src

Timestamp:

May 5, 2021, 2:41:50 PM (4 years ago)

Author:

Michael Brooks <mlbrooks@…>

Branches:

ADT, arm-eh, ast-experimental, enum, forall-pointer-decay, jacob/cs343-translation, master, new-ast-unique-expr, pthread-emulation, qualifiedEnum

Children:

7f54356

Parents:

d653faf

Message:

Fixing two bugs in new linked list, which last night's build failure revealed.

1. Adding more gcc-optimization reorder barriers and removing an unnecesary factoring around an incumbent one.

2. Adjusting pointer-tagging to work on 32-bit.

File:

• libcfa/src/containers/list2.hfa (modified) (7 diffs)

libcfa/src/containers/list2.hfa

@@ -39 +39 @@
 
 
-
-
-#define ORIGIN_TAG_BITNO 63
+// The origin is the position encountered at the start of iteration,
+// signifying, "need to advance to the first element," and at the end
+// of iteration, signifying, "no more elements."  Normal comsumption of
+// an iterator runs ?`moveNext as the first step, and uses the return
+// of ?`moveNext as a guard, before dereferencing the iterator.  So
+// normal consumption of an iterator does not dereference an iterator
+// in origin position.  The value of a pointer (underlying a refence)
+// that is exposed publicly as an iteraor, and also a pointer stored
+// internally in a link field, is tagged, to indicate "is the origin"
+// (internally, is the list-head sentinel node), or untagged, to indicate
+// "is a regular node."  Intent is to help a user who dereferences an
+// iterator in origin position (which would be an API-use error on their
+// part), by failing fast.
+
+#if defined( __x86_64 )
+    // Preferred case: tag in the most-significant bit.  Dereference
+    // has been shown to segfault consistently.  Maintenance should
+    // list more architectures as "ok" here, to let them use the
+    // preferred case, when valid.
+    #define ORIGIN_TAG_BITNO ( 8 * sizeof( size_t ) - 1 )
+#else
+    // Fallback case: tag in the least-significant bit.  Dereference
+    // will often give an alignment error, but may not, e.g. if
+    // accessing a char-typed member.  32-bit x86 uses the most-
+    // significant bit for real room on the heap.
+    #define ORIGIN_TAG_BITNO 0
+#endif
 #define ORIGIN_TAG_MASK (((size_t)1) << ORIGIN_TAG_BITNO)
 
@@ -47 +71 @@
 #define ORIGIN_TAG_CLEAR(p) ((p) & ~ORIGIN_TAG_MASK)
 #define ORIGIN_TAG_QUERY(p) ((p) &  ORIGIN_TAG_MASK)
-
-// #define ORIGIN_TAG_SET(p)   (p)
-// #define ORIGIN_TAG_CLEAR(p) (p)
-// #define ORIGIN_TAG_QUERY(p) (0)
 
 
@@ -110 +130 @@
         tE & list_pos_raw = list_pos;
         tE & list_pos_elem = * (tE *) ORIGIN_TAG_CLEAR( (size_t) & list_pos_raw );
+        asm( "" : : : "memory" );
         tE & after_raw = * (list_pos_elem`inner`inner).next;
         tE & after_elem = * (tE *) ORIGIN_TAG_CLEAR( (size_t) & after_raw );
@@ -116 +137 @@
         (after_elem`inner`inner).prev = &to_insert;
                 (list_pos_elem`inner`inner).next = &to_insert;
+        asm( "" : : : "memory" );
         }
 
@@ -126 +148 @@
         tE & list_pos_raw = list_pos;
         tE & list_pos_elem = * (tE *) ORIGIN_TAG_CLEAR( (size_t) & list_pos_raw );
+        asm( "" : : : "memory" );
         tE & before_raw = * (list_pos_elem`inner`inner).prev;
         tE & before_elem = * (tE *) ORIGIN_TAG_CLEAR( (size_t) & before_raw );
@@ -132 +155 @@
         (before_elem`inner`inner).next = &to_insert;
                 (list_pos_elem`inner`inner).prev = &to_insert;
-        }
-
-    static inline void doRemove(
-        dlink(tE) & this_links, dlink(tE) & before_links, dlink(tE) & after_links,
-        tE & before_target, tE & after_target ) {
-
-        before_links.next = &after_target;
-        after_links.prev = &before_target;
-
-        asm( "" : : : "memory" );
-
-                this_links.prev = 0p;
-                this_links.next = 0p;
-    }
-
-        static inline tE & remove(diref(tE, tLinks) list_pos) {
-                verify (&list_pos != 0p);
-        tE & list_pos_elem = list_pos;
-        verify( ! ORIGIN_TAG_QUERY((size_t) & list_pos_elem) );
-        dlink(tE) & list_pos_links = list_pos_elem`inner`inner;
-        tE & before_raw = * list_pos_links.prev;
-        tE & before_elem = * (tE *) ORIGIN_TAG_CLEAR( (size_t) & before_raw );
-        dlink(tE) & before_links = before_elem`inner`inner;
-        tE & after_raw = * list_pos_links.next;
-        tE & after_elem = * (tE *) ORIGIN_TAG_CLEAR( (size_t) & after_raw );
-        dlink(tE) & after_links = after_elem`inner`inner;
-
-        doRemove(list_pos_links, before_links, after_links, before_raw, after_raw);
-
-        return list_pos_elem;
-        }
-/*    
+        asm( "" : : : "memory" );
+        }
+
         static inline tE & remove(diref(tE, tLinks) list_pos) {
                 verify (&list_pos != 0p);
@@ -177 +171 @@
         before_links.next = &after_raw;
         after_links.prev = &before_raw;
-asm( "" : : : "memory" );
+        asm( "" : : : "memory" );
                 list_pos_links.prev = 0p;
                 list_pos_links.next = 0p;
+        asm( "" : : : "memory" );
         return list_pos_elem;
         }
-*/
+
     static inline diref(tE, tLinks) ?`first( dlist(tE, tLinks) &lst ) {
         tE * firstPtr = lst.next;