mod-core rework

squashed some init commands together
fixed a problem with non-atomic shm command reading on the DF side that caused segfaults

library/DFHackAPI.cpp

@@ -297,9 +297,10 @@ bool API::ReadBlock40d(uint32_t x, uint32_t y, uint32_t z, mapblock40d * buffer)
         SHMMAPSHDR->x = x;
         SHMMAPSHDR->y = y;
         SHMMAPSHDR->z = z;
-        const uint32_t cmd = Maps::MAP_READ_BLOCK_BY_COORDS + (d->maps_module << 16);
+        volatile uint32_t cmd = Maps::MAP_READ_BLOCK_BY_COORDS + (d->maps_module << 16);
         full_barrier
         SHMCMD = cmd;
+        full_barrier
         if(!g_pProcess->waitWhile(cmd))
         {
             return false;

library/DFProcess-linux-SHM.cpp

@@ -67,9 +67,8 @@ class Process::Private
     bool useYield;
     
     bool validate(char* exe_file, uint32_t pid, std::vector< memory_info* >& known_versions);
-    bool DF_TestBridgeVersion(bool & ret);
-    bool DF_GetPID(pid_t & ret);
-    void DF_SyncAffinity(void);
+    
+    bool Aux_Core_Attach(bool & versionOK, pid_t & PID);
     bool waitWhile (uint32_t state);
 };
 
@@ -80,6 +79,9 @@ class Process::Private
 #define SHMHDR ((shm_core_hdr *)my_shm)
 #define D_SHMHDR ((shm_core_hdr *)d->my_shm)
 
+#define SHMDATA(type) ((type *)(my_shm + SHM_HEADER))
+#define D_SHMDATA(type) ((type *)(d->my_shm + SHM_HEADER))
+
 /*
 Yeah. with no way to synchronize things (locks are slow, the OS doesn't give us enough control over scheduling)
 we end up with this silly thing
@@ -111,7 +113,7 @@ bool Process::Private::waitWhile (uint32_t state)
         }
         cnt++;
     }
-    if(SHMCMD == CORE_SV_ERROR)
+    if(SHMCMD == CORE_ERROR)
     {
         SHMCMD = CORE_RUNNING;
         attached = suspended = false;
@@ -131,53 +133,31 @@ bool Process::waitWhile (uint32_t state)
     return d->waitWhile(state);
 }
 
-bool Process::Private::DF_TestBridgeVersion(bool & ret)
-{
-    SHMCMD = CORE_GET_VERSION;
-    gcc_barrier
-    if(!waitWhile(CORE_GET_VERSION))
-        return false;
-    gcc_barrier
-    SHMCMD = CORE_SUSPENDED;
-    ret =( SHMHDR->value == CORE_VERSION );
-    return true;
-}
-
-bool Process::Private::DF_GetPID(pid_t & ret)
-{
-    SHMCMD = CORE_GET_PID;
-    gcc_barrier
-    if(!waitWhile(CORE_GET_PID))
-        return false;
-    gcc_barrier
-    SHMCMD = CORE_SUSPENDED;
-    ret = SHMHDR->value;
-    return true;
-}
-
 uint32_t OS_getAffinity()
 {
     cpu_set_t mask;
     sched_getaffinity(0,sizeof(cpu_set_t),&mask);
     // FIXME: truncation
-        uint32_t affinity = *(uint32_t *) &mask;
-        return affinity;
+    uint32_t affinity = *(uint32_t *) &mask;
+    return affinity;
 }
 
-void Process::Private::DF_SyncAffinity( void )
+
+bool Process::Private::Aux_Core_Attach(bool & versionOK, pid_t & PID)
 {
-    SHMHDR->value = OS_getAffinity();
+    SHMDATA(coreattach)->cl_affinity = OS_getAffinity();
     gcc_barrier
-    SHMCMD = CORE_SYNC_YIELD;
-    gcc_barrier
-    if(!waitWhile(CORE_SYNC_YIELD))
-        return;
+    SHMCMD = CORE_ATTACH;
+    if(!waitWhile(CORE_ATTACH))
+        return false;
     gcc_barrier
-    SHMCMD = CORE_SUSPENDED;
-    useYield = SHMHDR->value;
+    versionOK =( SHMDATA(coreattach)->sv_version == CORE_VERSION );
+    PID = SHMDATA(coreattach)->sv_PID;
+    useYield = SHMDATA(coreattach)->sv_useYield;
     #ifdef DEBUG
-    if(useYield) cerr << "Using Yield!" << endl;
+        if(useYield) cerr << "Using Yield!" << endl;
     #endif
+    return true;
 }
 
 Process::Process(uint32_t PID, vector< memory_info* >& known_versions)
@@ -190,7 +170,7 @@ Process::Process(uint32_t PID, vector< memory_info* >& known_versions)
     /*
      * Locate the segment.
      */
-    if ((d->my_shmid = shmget(SHM_KEY, SHM_SIZE, 0666)) < 0)
+    if ((d->my_shmid = shmget(SHM_KEY + PID, SHM_SIZE, 0666)) < 0)
     {
         return;
     }
@@ -216,41 +196,40 @@ Process::Process(uint32_t PID, vector< memory_info* >& known_versions)
     }
     
     /*
-     * Test bridge version, will also detect when we connect to something that doesn't respond
+     * Test bridge version, get PID, sync Yield
      */
     bool bridgeOK;
-    if(!d->DF_TestBridgeVersion(bridgeOK))
+    if(!d->Aux_Core_Attach(bridgeOK,d->my_pid))
     {
         fprintf(stderr,"DF terminated during reading\n");
+        shmdt(d->my_shm);
         return;
     }
     if(!bridgeOK)
     {
         fprintf(stderr,"SHM bridge version mismatch\n");
+        shmdt(d->my_shm);
         return;
     }
-    /*
-     * get the PID from DF
-     */
-    if(d->DF_GetPID(d->my_pid) && d->my_pid == PID)
+    
+    // find the binary
+    sprintf(exe_link_name,"/proc/%d/exe", d->my_pid);
+    target_result = readlink(exe_link_name, target_name, sizeof(target_name)-1);
+    if (target_result == -1)
     {
-        // find its binary
-        sprintf(exe_link_name,"/proc/%d/exe", d->my_pid);
-        target_result = readlink(exe_link_name, target_name, sizeof(target_name)-1);
-        if (target_result == -1)
-        {
-            perror("readlink");
-            return;
-        }
-        // make sure we have a null terminated string...
-        // see http://www.opengroup.org/onlinepubs/000095399/functions/readlink.html
-        target_name[target_result] = 0;
-        
-        // try to identify the DF version
-        d->validate(target_name, d->my_pid, known_versions);
-        d->DF_SyncAffinity();
-        d->my_window = new DFWindow(this);
+        perror("readlink");
+        shmdt(d->my_shm);
+        return;
     }
+    
+    // make sure we have a null terminated string...
+    // see http://www.opengroup.org/onlinepubs/000095399/functions/readlink.html
+    target_name[target_result] = 0;
+    
+    // try to identify the DF version
+    d->validate(target_name, d->my_pid, known_versions);
+    d->my_window = new DFWindow(this);
+
     gcc_barrier
     // at this point, DF is stopped and waiting for commands. make it run again
     D_SHMCMD = CORE_RUNNING;
@@ -708,16 +687,30 @@ string Process::readClassName (uint32_t vptr)
     return raw.substr(start,end-start - 2); // trim the 'st' from the end
 }
 
+// FIXME: having this around could lead to bad things in the hands of unsuspecting fools
+// *!!DON'T BE AN UNSUSPECTING FOOL!!*
+// the whole SHM thing works only because copying DWORDS is an atomic operation on i386 and x86_64 archs
 // get module index by name and version. bool 1 = error
 bool Process::getModuleIndex (const char * name, const uint32_t version, uint32_t & OUTPUT)
 {
     modulelookup * payload = (modulelookup *) (d->my_shm + SHM_HEADER);
     payload->version = version;
-    strcpy(payload->name,name);
+    
+    strncpy(payload->name,name,255);
+    payload->name[255] = 0;
+    
     full_barrier
+    
     D_SHMCMD = CORE_ACQUIRE_MODULE;
-    waitWhile(CORE_ACQUIRE_MODULE);
-    if(D_SHMHDR->error) return false;
+    if(!waitWhile(CORE_ACQUIRE_MODULE))
+    {
+        return false; // FIXME: throw a fatal exception instead
+    }
+    if(D_SHMHDR->error)
+    {
+        return false;
+    }
+    //fprintf(stderr,"%s v%d : %d\n", name, version, D_SHMHDR->value);
     OUTPUT = D_SHMHDR->value;
     return true;
 }

library/DFProcess-windows-SHM.cpp

@@ -58,9 +58,7 @@ class Process::Private
     
     bool waitWhile (uint32_t state);
     bool isValidSV();
-    bool DF_TestBridgeVersion(bool & ret);
-    bool DF_GetPID(uint32_t & ret);
-    void DF_SyncAffinity(void);
+    bool Aux_Core_Attach(bool & versionOK, uint32_t & PID);
 };
 
 // some helpful macros to keep the code bloat in check
@@ -70,6 +68,9 @@ class Process::Private
 #define SHMHDR ((shm_core_hdr *)my_shm)
 #define D_SHMHDR ((shm_core_hdr *)d->my_shm)
 
+#define SHMDATA(type) ((type *)(my_shm + SHM_HEADER))
+#define D_SHMDATA(type) ((type *)(d->my_shm + SHM_HEADER))
+
 // is the other side still there?
 bool Process::Private::isValidSV()
 {
@@ -129,7 +130,7 @@ bool Process::Private::waitWhile (uint32_t state)
         }
         cnt++;
     }
-    if(SHMCMD == CORE_SV_ERROR)
+    if(SHMCMD == CORE_ERROR)
     {
         SHMCMD = CORE_RUNNING;
         attached = suspended = false;
@@ -139,30 +140,6 @@ bool Process::Private::waitWhile (uint32_t state)
     return true;
 }
 
-bool Process::Private::DF_TestBridgeVersion(bool & ret)
-{
-    SHMCMD = CORE_GET_VERSION;
-    full_barrier
-    if(!waitWhile(CORE_GET_VERSION))
-        return false;
-    full_barrier
-    SHMCMD = CORE_SUSPENDED;
-    ret =( SHMHDR->value == CORE_VERSION );
-    return true;
-}
-
-bool Process::Private::DF_GetPID(uint32_t & ret)
-{
-    SHMCMD = CORE_GET_PID;
-    full_barrier
-    if(!waitWhile(CORE_GET_PID))
-        return false;
-    full_barrier
-    SHMCMD = CORE_SUSPENDED;
-    ret = SHMHDR->value;
-    return true;
-}
-
 uint32_t OS_getAffinity()
 {
     HANDLE hProcess = GetCurrentProcess();
@@ -171,22 +148,24 @@ uint32_t OS_getAffinity()
     return dwProcessAffinityMask;
 }
 
-void Process::Private::DF_SyncAffinity( void )
+bool Process::Private::Aux_Core_Attach(bool & versionOK, uint32_t & PID)
 {
-    SHMHDR->value = OS_getAffinity();
-    full_barrier
-    SHMCMD = CORE_SYNC_YIELD;
+    SHMDATA(coreattach)->cl_affinity = OS_getAffinity();
     full_barrier
-    if(!waitWhile(CORE_SYNC_YIELD))
-        return;
+    SHMCMD = CORE_ATTACH;
+    if(!waitWhile(CORE_ATTACH))
+        return false;
     full_barrier
-    SHMCMD = CORE_SUSPENDED;
-    useYield = SHMHDR->value;
-    if(useYield) cerr << "Using Yield!" << endl;
+    versionOK =( SHMDATA(coreattach)->sv_version == CORE_VERSION );
+    PID = SHMDATA(coreattach)->sv_PID;
+    useYield = SHMDATA(coreattach)->sv_useYield;
+    #ifdef DEBUG
+        if(useYield) cerr << "Using Yield!" << endl;
+    #endif
+    return true;
 }
 
-
-Process::Process(vector <memory_info *> & known_versions)
+Process::Process(uint32_t PID, vector <memory_info *> & known_versions)
 : d(new Private())
 {
     // get server and client mutex
@@ -200,29 +179,28 @@ Process::Process(vector <memory_info *> & known_versions)
     {
         return;
     }
+    
+    // attach the SHM
     if(!attach())
     {
         return;
     }
     
-    // All seems to be OK so far. Attached and connected to something that looks like DF
-    
-    // Test bridge version, will also detect when we connect to something that doesn't respond
+    // Test bridge version, get PID, sync Yield
     bool bridgeOK;
-    if(!d->DF_TestBridgeVersion(bridgeOK))
+    bool error = 0;
+    if(!d->Aux_Core_Attach(bridgeOK,d->my_pid))
     {
         fprintf(stderr,"DF terminated during reading\n");
-        UnmapViewOfFile(d->my_shm);
-        ReleaseMutex(d->DFCLMutex);
-        CloseHandle(d->DFSVMutex);
-        d->DFSVMutex = 0;
-        CloseHandle(d->DFCLMutex);
-        d->DFCLMutex = 0;
-        return;
+        error = 1;
     }
-    if(!bridgeOK)
+    else if(!bridgeOK)
     {
         fprintf(stderr,"SHM bridge version mismatch\n");
+        error = 1;
+    }
+    if(error)
+    {
         D_SHMCMD = CORE_RUNNING;
         UnmapViewOfFile(d->my_shm);
         ReleaseMutex(d->DFCLMutex);
@@ -232,84 +210,78 @@ Process::Process(vector <memory_info *> & known_versions)
         d->DFCLMutex = 0;
         return;
     }
-    /*
-     * get the PID from DF
-     */
-    if(d->DF_GetPID(d->my_pid))
-    {
-        // try to identify the DF version
-        do // glorified goto
-        {  
-            IMAGE_NT_HEADERS32 pe_header;
-            IMAGE_SECTION_HEADER sections[16];
-            HMODULE hmod = NULL;
-            DWORD junk;
-            HANDLE hProcess;
-            bool found = false;
-            d->identified = false;
-            // open process, we only need the process open 
-            hProcess = OpenProcess( PROCESS_ALL_ACCESS, FALSE, d->my_pid );
-            if (NULL == hProcess)
-                break;
-            
-            // try getting the first module of the process
-            if(EnumProcessModules(hProcess, &hmod, 1 * sizeof(HMODULE), &junk) == 0)
-            {
-                CloseHandle(hProcess);
-                cout << "EnumProcessModules fail'd" << endl;
-                break;
-            }
-            // got base ;)
-            uint32_t base = (uint32_t)hmod;
-            
-            // read from this process
-            uint32_t pe_offset = readDWord(base+0x3C);
-            read(base + pe_offset                   , sizeof(pe_header), (uint8_t *)&pe_header);
-            read(base + pe_offset+ sizeof(pe_header), sizeof(sections) , (uint8_t *)&sections );
-            
-            // iterate over the list of memory locations
-            vector<memory_info *>::iterator it;
-            for ( it=known_versions.begin() ; it < known_versions.end(); it++ )
-            {
-                uint32_t pe_timestamp;
-                try
-                {
-                    pe_timestamp = (*it)->getHexValue("pe_timestamp");
-                }
-                catch(Error::MissingMemoryDefinition& e)
-                {
-                    continue;
-                }
-                if (pe_timestamp == pe_header.FileHeader.TimeDateStamp)
-                {
-                    memory_info *m = new memory_info(**it);
-                    m->RebaseAll(base);
-                    d->my_descriptor = m;
-                    d->identified = true;
-                    cerr << "identified " << m->getVersion() << endl;
-                    break;
-                }
-            }
-            CloseHandle(hProcess);
-        } while (0); // glorified goto end
+
+    // try to identify the DF version
+    do // glorified goto
+    {  
+        IMAGE_NT_HEADERS32 pe_header;
+        IMAGE_SECTION_HEADER sections[16];
+        HMODULE hmod = NULL;
+        DWORD junk;
+        HANDLE hProcess;
+        bool found = false;
+        d->identified = false;
+        // open process, we only need the process open 
+        hProcess = OpenProcess( PROCESS_ALL_ACCESS, FALSE, d->my_pid );
+        if (NULL == hProcess)
+            break;
         
-        if(d->identified)
+        // try getting the first module of the process
+        if(EnumProcessModules(hProcess, &hmod, 1 * sizeof(HMODULE), &junk) == 0)
         {
-            d->my_window = new DFWindow(this);
-            d->DF_SyncAffinity();
+            CloseHandle(hProcess);
+            cout << "EnumProcessModules fail'd" << endl;
+            break;
         }
-        else
+        // got base ;)
+        uint32_t base = (uint32_t)hmod;
+        
+        // read from this process
+        uint32_t pe_offset = readDWord(base+0x3C);
+        read(base + pe_offset                   , sizeof(pe_header), (uint8_t *)&pe_header);
+        read(base + pe_offset+ sizeof(pe_header), sizeof(sections) , (uint8_t *)&sections );
+        
+        // iterate over the list of memory locations
+        vector<memory_info *>::iterator it;
+        for ( it=known_versions.begin() ; it < known_versions.end(); it++ )
         {
-            D_SHMCMD = CORE_RUNNING;
-            UnmapViewOfFile(d->my_shm);
-            d->my_shm = 0;
-            ReleaseMutex(d->DFCLMutex);
-            CloseHandle(d->DFSVMutex);
-            d->DFSVMutex = 0;
-            CloseHandle(d->DFCLMutex);
-            d->DFCLMutex = 0;
-            return;
+            uint32_t pe_timestamp;
+            try
+            {
+                pe_timestamp = (*it)->getHexValue("pe_timestamp");
+            }
+            catch(Error::MissingMemoryDefinition& e)
+            {
+                continue;
+            }
+            if (pe_timestamp == pe_header.FileHeader.TimeDateStamp)
+            {
+                memory_info *m = new memory_info(**it);
+                m->RebaseAll(base);
+                d->my_descriptor = m;
+                d->identified = true;
+                cerr << "identified " << m->getVersion() << endl;
+                break;
+            }
         }
+        CloseHandle(hProcess);
+    } while (0); // glorified goto end
+    
+    if(d->identified)
+    {
+        d->my_window = new DFWindow(this);
+    }
+    else
+    {
+        D_SHMCMD = CORE_RUNNING;
+        UnmapViewOfFile(d->my_shm);
+        d->my_shm = 0;
+        ReleaseMutex(d->DFCLMutex);
+        CloseHandle(d->DFSVMutex);
+        d->DFSVMutex = 0;
+        CloseHandle(d->DFCLMutex);
+        d->DFCLMutex = 0;
+        return;
     }
     full_barrier
     // at this point, DF is attached and suspended, make it run

library/DFProcessEnumerator-windows.cpp

@@ -41,12 +41,23 @@ bool ProcessEnumerator::findProcessess()
 {
     // Get the list of process identifiers.
     DWORD ProcArray[2048], memoryNeeded, numProccesses;
+    //EnableDebugPriv();
+    if ( !EnumProcesses( ProcArray, sizeof(ProcArray), &memoryNeeded ) )
     {
-        Process * p = new Process(d->meminfo->meminfo);
+        cout << "EnumProcesses fail'd" << endl;
+        return false;
+    }
+
+    // Calculate how many process identifiers were returned.
+    numProccesses = memoryNeeded / sizeof(DWORD);
+
+    // iterate through processes
+    for ( int i = 0; i < (int)numProccesses; i++ )
+    {
+        Process *p = new Process(ProcArray[i],d->meminfo->meminfo);
         if(p->isIdentified())
         {
             d->processes.push_back(p);
-            return true;
         }
         else
         {
@@ -55,30 +66,20 @@ bool ProcessEnumerator::findProcessess()
         }
     }
     /*
-    EnableDebugPriv();
-    if ( !EnumProcesses( ProcArray, sizeof(ProcArray), &memoryNeeded ) )
     {
-        cout << "EnumProcesses fail'd" << endl;
-        return false;
-    }
-
-    // Calculate how many process identifiers were returned.
-    numProccesses = memoryNeeded / sizeof(DWORD);
-
-    // iterate through processes
-    for ( int i = 0; i < (int)numProccesses; i++ )
-    {
-        Process *q = new NormalProcess(ProcArray[i],d->meminfo->meminfo);
-        if(q->isIdentified())
+        Process * p = new Process(d->meminfo->meminfo);
+        if(p->isIdentified())
         {
-            d->processes.push_back(q);
+            d->processes.push_back(p);
+            return true;
         }
         else
         {
-            delete q;
-            q = 0;
+            delete p;
+            p = 0;
         }
-    }*/
+    }
+    */
     if(d->processes.size())
         return true;
     return false;

shmserver/mod-core.cpp

@@ -49,16 +49,7 @@ bool useYield = 0;
 
 #define SHMHDR ((shm_core_hdr *)shm)
 #define SHMCMD ((shm_cmd *)shm)->pingpong
-
-void GetCoreVersion (void * data)
-{
-    SHMHDR->value = module_registry[0].version;
-}
-
-void GetPID (void * data)
-{
-    SHMHDR->value = OS_getPID();
-}
+#define SHMDATA(type) ((type *)(shm + SHM_HEADER))
 
 void ReadRaw (void * data)
 {
@@ -126,19 +117,17 @@ int bitcount(uint32_t n)
 }
 
 // get local and remote affinity, set up yield if required (single core available)
-void SyncYield (void * data)
+void CoreAttach (void * data)
 {
+    // sync affinity
     uint32_t local = OS_getAffinity();
-    uint32_t remote = SHMHDR->value;
+    uint32_t remote = SHMDATA(coreattach)->cl_affinity;
     uint32_t pool = local | remote;
-    if(bitcount(pool) == 1)
-    {
-        SHMHDR->value = useYield = 1;
-    }
-    else
-    {
-        SHMHDR->value = useYield = 0;
-    }
+    SHMDATA(coreattach)->sv_useYield = useYield = (bitcount(pool) == 1);
+    // return our PID
+    SHMDATA(coreattach)->sv_PID = OS_getPID();
+    // return core version
+    SHMDATA(coreattach)->sv_version = module_registry[0].version;
 }
 
 void FindModule (void * data)
@@ -157,14 +146,33 @@ void FindModule (void * data)
             break;
         }
     }
-    if(found)
-    {
-        SHMHDR->error = false;
-    }
-    else
+    SHMHDR->error = !found;
+}
+
+void FindCommand (void * data)
+{
+    bool found = false;
+    commandlookup * payload = SHMDATA(commandlookup);
+    std::string modname = payload->module;
+    std::string cmdname = payload->name;
+    uint32_t version = payload->version;
+    for(unsigned int i = 0; i < module_registry.size();i++)
     {
-        SHMHDR->error = true;
+        if(module_registry[i].name == modname && module_registry[i].version == version)
+        {
+            for(unsigned int j = 0 ; j < module_registry[i].commands.size();j++)
+            {
+                if(module_registry[i].commands[j].name == cmdname)
+                {
+                    // gotcha
+                    SHMHDR->value = j + (i << 16);
+                    SHMHDR->error = false;
+                    return;
+                }
+            }
+        }
     }
+    SHMHDR->error = true;
 }
 
 DFPP_module InitCore(void)
@@ -176,45 +184,29 @@ DFPP_module InitCore(void)
     
     core.reserve(NUM_CORE_CMDS);
     core.set_command(CORE_RUNNING, CANCELLATION, "Running");
+    core.set_command(CORE_SUSPEND, CLIENT_WAIT, "Suspend", 0 , CORE_SUSPENDED);
+    core.set_command(CORE_SUSPENDED, CLIENT_WAIT, "Suspended");
     
-    core.set_command(CORE_GET_VERSION, FUNCTION,"Get core version",GetCoreVersion, CORE_RET_VERSION);
-    core.set_command(CORE_RET_VERSION, CLIENT_WAIT,"Core version return");
-    
-    core.set_command(CORE_GET_PID, FUNCTION, "Get PID", GetPID, CORE_RET_PID);
-    core.set_command(CORE_RET_PID, CLIENT_WAIT, "PID return");
-    
-    core.set_command(CORE_DFPP_READ, FUNCTION,"Raw read",ReadRaw, CORE_RET_DATA);
-    core.set_command(CORE_RET_DATA, CLIENT_WAIT,"Raw read return");
-
-    core.set_command(CORE_READ_DWORD, FUNCTION,"Read DWORD",ReadDWord, CORE_RET_DWORD);
-    core.set_command(CORE_RET_DWORD, CLIENT_WAIT,"Read DWORD return");
-
-    core.set_command(CORE_READ_WORD, FUNCTION,"Read WORD",ReadWord, CORE_RET_WORD);
-    core.set_command(CORE_RET_WORD, CLIENT_WAIT,"Read WORD return");
+    core.set_command(CORE_ATTACH, FUNCTION,"Core attach",CoreAttach, CORE_SUSPENDED);
     
-    core.set_command(CORE_READ_BYTE, FUNCTION,"Read BYTE",ReadByte, CORE_RET_BYTE);
-    core.set_command(CORE_RET_BYTE, CLIENT_WAIT,"Read BYTE return");
+    core.set_command(CORE_DFPP_READ, FUNCTION,"Raw read",ReadRaw, CORE_SUSPENDED);
+    core.set_command(CORE_READ_DWORD, FUNCTION,"Read DWORD",ReadDWord, CORE_SUSPENDED);
+    core.set_command(CORE_READ_WORD, FUNCTION,"Read WORD",ReadWord, CORE_SUSPENDED);
+    core.set_command(CORE_READ_BYTE, FUNCTION,"Read BYTE",ReadByte, CORE_SUSPENDED);
     
-    core.set_command(CORE_SV_ERROR, CANCELLATION, "Server error");
-    core.set_command(CORE_CL_ERROR, CANCELLATION, "Client error");
+    core.set_command(CORE_ERROR, CANCELLATION, "Error");
     
     core.set_command(CORE_WRITE, FUNCTION, "Raw write", WriteRaw, CORE_SUSPENDED);
     core.set_command(CORE_WRITE_DWORD, FUNCTION, "Write DWORD", WriteDWord, CORE_SUSPENDED);
     core.set_command(CORE_WRITE_WORD, FUNCTION, "Write WORD", WriteWord, CORE_SUSPENDED);
     core.set_command(CORE_WRITE_BYTE, FUNCTION, "Write BYTE", WriteByte, CORE_SUSPENDED);
     
-    core.set_command(CORE_SUSPEND, CLIENT_WAIT, "Suspend", 0 , CORE_SUSPENDED);
-    core.set_command(CORE_SUSPENDED, CLIENT_WAIT, "Suspended");
-    
-    core.set_command(CORE_READ_STL_STRING, FUNCTION, "Read STL string", ReadSTLString, CORE_RET_STRING);
+    core.set_command(CORE_READ_STL_STRING, FUNCTION, "Read STL string", ReadSTLString, CORE_SUSPENDED);
     core.set_command(CORE_READ_C_STRING, CLIENT_WAIT, "RESERVED");
-    core.set_command(CORE_RET_STRING, CLIENT_WAIT, "Return string");
     core.set_command(CORE_WRITE_STL_STRING, FUNCTION, "Write STL string", WriteSTLString, CORE_SUSPENDED);
-    core.set_command(CORE_SYNC_YIELD, FUNCTION, "Synchronize affinity/yield", SyncYield, CORE_SYNC_YIELD_RET);
-    core.set_command(CORE_SYNC_YIELD_RET, CLIENT_WAIT, "Synchronize affinity/yield return");
     
-    core.set_command(CORE_ACQUIRE_MODULE, FUNCTION, "Module lookup", FindModule, CORE_RET_MODULE);
-    core.set_command(CORE_RET_MODULE, CLIENT_WAIT, "Return module index or error");
+    core.set_command(CORE_ACQUIRE_MODULE, FUNCTION, "Module lookup", FindModule, CORE_SUSPENDED);
+    core.set_command(CORE_ACQUIRE_COMMAND, FUNCTION, "Command lookup", FindCommand, CORE_SUSPENDED);
     return core;
 }
 
@@ -253,24 +245,37 @@ void SHM_Act (void)
         }
         else
         {
-            // full_barrier
+            full_barrier
             SHMCMD = CORE_RUNNING;
             fprintf(stderr,"dfhack: Broke out of loop, other process disappeared.\n");
         }
     }
-    DFPP_module & mod = module_registry[((shm_cmd *)shm)->parts.module];
-    DFPP_command & cmd = mod.commands[((shm_cmd *)shm)->parts.command];
-    //fprintf(stderr, "Client invoked %d:%d = ",((shm_cmd *)shm)->parts.module,((shm_cmd *)shm)->parts.command);
-    //fprintf(stderr, "%s\n",cmd.name.c_str());
     
+    // this is very important! copying two words separately from the command variable leads to inconsistency.
+    // Always copy the thing in one go.
+    // Also, this whole SHM thing probably only works on intel processors
+    
+    volatile shm_cmd atomic = SHMHDR->cmd;
+    full_barrier
+    DFPP_module & mod = module_registry[atomic.parts.module];
+    DFPP_command & cmd = mod.commands[atomic.parts.command];
+    full_barrier
+    /*
+    fprintf(stderr, "Called %x\0", cmd._function);
+    fprintf(stderr, "Client invoked %d:%d = ",atomic.parts.module,atomic.parts.command);
+    fprintf(stderr, "%s\n",cmd.name.c_str());
+    */
+    full_barrier
     if(cmd._function)
     {
         cmd._function(mod.modulestate);
     }
+        full_barrier
     if(cmd.nextState != -1)
     {
         SHMCMD = cmd.nextState;
     }
+        full_barrier
     if(cmd.type != CANCELLATION)
     {
         if(useYield)

shmserver/mod-core.h

@@ -43,54 +43,56 @@ typedef struct
     char name[256];
 } modulelookup;
 
+typedef struct
+{
+    uint32_t version;
+    char module[256];
+    char name[256];
+} commandlookup;
+
+typedef struct
+{
+    uint32_t sv_version; // output
+    uint32_t cl_affinity; // input
+    uint32_t sv_PID; // output
+    uint32_t sv_useYield; // output
+} coreattach;
+
 enum CORE_COMMAND
 {
+    // suspend / resume
     CORE_RUNNING = 0, // no command, normal server execution
+    CORE_SUSPEND, // client notifies server to wait for commands (server is stalled in busy wait)
+    CORE_SUSPENDED, // response to WAIT, server is stalled in busy wait
     
-    CORE_GET_VERSION, // protocol version query
-    CORE_RET_VERSION, // return the protocol version
-    
-    CORE_GET_PID, // query for the process ID
-    CORE_RET_PID, // return process ID
+    // aux
+    CORE_ATTACH, // compare affinity, get core version and process ID
     
-    // version 1 stuff below
+    // reads
     CORE_DFPP_READ, // cl -> sv, read some data
-    CORE_RET_DATA, // sv -> cl, returned data
-    
     CORE_READ_DWORD, // cl -> sv, read a dword
-    CORE_RET_DWORD, // sv -> cl, returned dword
-
     CORE_READ_WORD, // cl -> sv, read a word
-    CORE_RET_WORD, // sv -> cl, returned word
-
     CORE_READ_BYTE, // cl -> sv, read a byte
-    CORE_RET_BYTE, // sv -> cl, returned byte
-    
-    CORE_SV_ERROR, // there was a server error
-    CORE_CL_ERROR, // there was a client error
-    
+
+    // writes
     CORE_WRITE,// client writes to server
     CORE_WRITE_DWORD,// client writes a DWORD to server
     CORE_WRITE_WORD,// client writes a WORD to server
     CORE_WRITE_BYTE,// client writes a BYTE to server
+
+    // error state
+    CORE_ERROR, // there was a server error
     
-    CORE_SUSPEND, // client notifies server to wait for commands (server is stalled in busy wait)
-    CORE_SUSPENDED, // response to WAIT, server is stalled in busy wait
-    
-    // all strings capped at 1MB
+    // string functions
     CORE_READ_STL_STRING,// client requests contents of STL string at address
     CORE_READ_C_STRING,// client requests contents of a C string at address, max length (0 means zero terminated)
-    CORE_RET_STRING, // sv -> cl length + string contents
     CORE_WRITE_STL_STRING,// client wants to set STL string at address to something
     
-    // compare affinity and determine if using yield is required
-    CORE_SYNC_YIELD,// cl sends affinity to sv, sv sets yield
-    CORE_SYNC_YIELD_RET,// sv returns yield bool
-    
-    // get index of a loaded module by name and version
-    CORE_ACQUIRE_MODULE, 
-    // returning module index
-    CORE_RET_MODULE,
+    // extension module enumeration 
+    CORE_ACQUIRE_MODULE, // get index of a loaded module by name and version
+    CORE_ACQUIRE_COMMAND, // get module::command callsign by module name, command name and module version
+
+    // total commands
     NUM_CORE_CMDS
 };
 #endif