Change search from CTRL-S to CTRL-F & fix off by one errors in data submission

mubes · mubes · commit 1c26db0fac40 · 2022-06-03T00:17:04.000+01:00
diff --git a/Inc/etmDecoder.h b/Inc/etmDecoder.h
@@ -73,7 +73,7 @@ enum ETMchanges
 // Messages out of the decoder
 // ============================================================================
 enum ETMDecoderMsgType { ETMDEC_MSG_NONE, ETM_BRANCH, ETMDEC_MSG_NUM_MSGS };
-enum Mode { ETM_ADDRMODE_ARM, ETM_ADDRMODE_THUMB, ETM_ADDRMODE_JAZELLE };
+enum Mode { ETM_ADDRMODE_THUMB, ETM_ADDRMODE_ARM, ETM_ADDRMODE_JAZELLE };
 enum Reason { ETM_REASON_PERIODIC, ETM_REASON_TRACEON, ETM_REASON_TRACEOVF, ETM_REASON_EXITDBG };
 
 /* ETM Decoder statistics */
@@ -156,11 +156,12 @@ void ETMDecoderForceSync( struct ETMDecoder *i, bool isSynced );
 void ETMDecoderZeroStats( struct ETMDecoder *i );
 bool ETMDecoderIsSynced( struct ETMDecoder *i );
 
-inline struct ETMCPUState *ETMCPUState( struct ETMDecoder *i )
+ALWAYS_INLINE struct ETMCPUState *ETMCPUState( struct ETMDecoder *i )
 {
     return &i->cpu;
 }
-inline bool ETMStateChanged( struct ETMDecoder *i, enum ETMchanges c )
+
+ALWAYS_INLINE bool ETMStateChanged( struct ETMDecoder *i, enum ETMchanges c )
 {
     bool r = ( i->cpu.changeRecord & ( 1 << c ) ) != 0;
     i->cpu.changeRecord &= ~( 1 << c );
diff --git a/Inc/generics.h b/Inc/generics.h
@@ -59,6 +59,7 @@ extern "C" {
 #define C_WHITE   "\033[1;37m"
 #define C_MONO    ""
 
+#define ALWAYS_INLINE inline __attribute__((always_inline))
 // ====================================================================================================
 enum verbLevel {V_ERROR, V_WARN, V_INFO, V_DEBUG, V_MAX_VERBLEVEL};
 
diff --git a/README.md b/README.md
@@ -279,6 +279,8 @@ For `orbuculum`, the specific command line options of note are;
 
  `-m`: Monitor interval (in mS) for reporting on state of the link. If baudrate is specified (using `-a`) and is greater than 100bps then the percentage link occupancy is also reported.
 
+  `-o [filename]`: Record trace data locally. This is unfettered data directly from the source device, can be useful for replay purposes or other tool testing.
+  
   `-p [serialPort]`: to use. If not specified then the program defaults to Blackmagic probe.
 
   `-s [address]:[port]`: Set address for explicit TCP Source connection, (default none:2332).
@@ -513,7 +515,7 @@ configured to stream parallel trace info (clue; the `startETM` option).
 
 The command line options of note are;
 
- `-a`: Use alternate address encoding. Select this if decodes don't seem to arrive correctly. You can discover if you need this option by using the `describeETM` command inside the debugger.
+ `-a`: Don't use alternate address encoding. Select this if decodes don't seem to arrive correctly. You can discover if you need this option by using the `describeETM` command inside the debugger.
  
  `-b [Length]`: Set length of post-mortem buffer, in KBytes (Default 32 KBytes)
  
diff --git a/Src/etmDecoder.c b/Src/etmDecoder.c
@@ -76,19 +76,16 @@ static void _ETMDecoderPumpAction( struct ETMDecoder *i, uint8_t c, etmDecodeCB
                 break;
 
             // -----------------------------------------------------
+
             case ETM_IDLE:
 
                 // *************************************************
                 // ************** BRANCH PACKET ********************
                 // *************************************************
                 if ( c & 0b1 )
                 {
-                    if ( report )
-                    {
-                        report( V_DEBUG, "BRANCH " );
-                    }
-
                     /* The lowest order 6 bits of address info... */
+
                     switch ( cpu->addrMode )
                     {
                         case ETM_ADDRMODE_ARM:
@@ -105,7 +102,7 @@ static void _ETMDecoderPumpAction( struct ETMDecoder *i, uint8_t c, etmDecodeCB
                     }
 
                     i->byteCount = 1;
-                    C = c & 0x80;
+                    C = ( c & 0x80 ) != 0;
                     X = false;
                     _stateChange( i, EV_CH_ADDRESS );
 
@@ -312,7 +309,7 @@ static void _ETMDecoderPumpAction( struct ETMDecoder *i, uint8_t c, etmDecodeCB
 
                             if ( report )
                             {
-                                report( V_DEBUG, "PHdr FMT1 (E=%d, N=%d)" EOL, cpu->eatoms, cpu->natoms );
+                                report( V_DEBUG, "PHdr FMT1 (%02x E=%d, N=%d)" EOL, c, cpu->eatoms, cpu->natoms );
                             }
 
                             break;
@@ -466,10 +463,10 @@ static void _ETMDecoderPumpAction( struct ETMDecoder *i, uint8_t c, etmDecodeCB
                 C = c & 0x80;
                 /* This is a proper mess. Mask and collect bits according to address mode in use and */
                 /* if it's the last byte of the sequence */
-
                 mask = C ? 0x7f : 0x3f;
                 ofs = ( cpu->addrMode == ETM_ADDRMODE_ARM ) ? 1 : ( cpu->addrMode == ETM_ADDRMODE_THUMB ) ? 0 : -1;
 
+
                 i->addrConstruct = ( i->addrConstruct &   ~( mask << ( 7 * i->byteCount + ofs ) ) )
                                    | ( ( c & mask ) << ( 7 * i->byteCount + ofs ) );
                 /* There is exception information only if no continuation and bit 6 set */
@@ -488,6 +485,10 @@ static void _ETMDecoderPumpAction( struct ETMDecoder *i, uint8_t c, etmDecodeCB
                 X = ( i->byteCount == 5 ) && C;
                 goto terminateAddrByte;
 
+                // -----------------------------------------------------
+
+                /* For all cases, see if the address is complete, and process if so */
+                /* this is a continuation of ETM_COLLECT_BA_???_FORMAT.             */
             terminateAddrByte:
 
                 /* Check to see if this packet is complete, and encode to return if so */
@@ -849,7 +850,8 @@ static void _ETMDecoderPumpAction( struct ETMDecoder *i, uint8_t c, etmDecodeCB
                         if ( i->addrConstruct & ( 1 << 0 ) )
                         {
                             cpu->addrMode = ETM_ADDRMODE_THUMB;
-                            cpu->addr = i->addrConstruct & 0xFFFFFFFE;
+                            i->addrConstruct &= ~( 1 << 0 );
+                            cpu->addr = i->addrConstruct;
                         }
                         else
                         {
@@ -986,7 +988,7 @@ void ETMDecoderPump( struct ETMDecoder *i, uint8_t *buf, int len, etmDecodeCB cb
 {
     while ( len-- )
     {
-        _ETMDecoderPumpAction( i, *buf++, cb, report, d );
+        _ETMDecoderPumpAction( i, *( buf++ ), cb, report, d );
     }
 }
 // ====================================================================================================
diff --git a/Src/nwclient.c b/Src/nwclient.c
@@ -15,6 +15,8 @@
 #include <arpa/inet.h>
 #include <assert.h>
 #include <strings.h>
+#include <stdio.h>
+#include <linux/tcp.h>
 #include "generics.h"
 #include "nwclient.h"
 
@@ -315,8 +317,6 @@ void nwclientShutdown( struct nwclientsHandle *h )
         return;
     }
 
-    struct nwClient *c = h->firstClient;
-
     /* Flag that we're ending */
     h->finish = true;
 
@@ -325,6 +325,8 @@ void nwclientShutdown( struct nwclientsHandle *h )
         genericsExit( -1, "Failed to acquire mutex" EOL );
     }
 
+    struct nwClient *c = h->firstClient;
+
     /* Tell all the clients to terminate */
     while ( c )
     {
diff --git a/Src/orbmortem.c b/Src/orbmortem.c
@@ -54,7 +54,7 @@ struct Options
     int channel;                        /* When TPIU is in use, which channel to decode? */
     int port;                           /* Source information */
     char *server;
-    bool altAddr;                       /* Should alternate addressing be used? */
+    bool noAltAddr;                     /* Flag to *not* use alternate addressing */
     char *openFileCL;                   /* Command line for opening refernced file */
 
 } _options =
@@ -92,8 +92,11 @@ struct RunTime
     struct SymbolSet *s;                /* Symbols read from elf */
     bool     ending;                    /* Flag indicating app is terminating */
     bool     singleShot;                /* Flag indicating take a single buffer then stop */
-    uint64_t intervalBytes;             /* Number of bytes transferred in current interval */
-    uint64_t oldintervalBytes;          /* Number of bytes transferred previously */
+    uint64_t newTotalBytes;             /* Number of bytes of real data transferred in total */
+    uint64_t oldTotalBytes;             /* Old number of bytes of real data transferred in total */
+    uint64_t oldTotalIntervalBytes;     /* Number of bytes transferred in previous interval */
+    uint64_t oldTotalHangBytes;         /* Number of bytes transferred in previous hang interval */
+
     uint8_t *pmBuffer;                  /* The post-mortem buffer */
     int wp;                             /* Index pointers for ring buffer */
     int rp;
@@ -149,7 +152,7 @@ static void _printHelp( struct RunTime *r )
 
 {
     genericsPrintf( "Usage: %s [options]" EOL, r->progName );
-    genericsPrintf( "       -a: Use alternate address encoding" EOL );
+    genericsPrintf( "       -a: Do not use alternate address encoding" EOL );
     genericsPrintf( "       -b: <Length> Length of post-mortem buffer, in KBytes (Default %d KBytes)" EOL, DEFAULT_PM_BUFLEN_K );
     genericsPrintf( "       -c: <command> Command line for external editor (%f = filename, %l = line)" EOL );
     genericsPrintf( "       -D: Switch off C++ symbol demangling" EOL );
@@ -177,7 +180,7 @@ static int _processOptions( int argc, char *argv[], struct RunTime *r )
         {
             // ------------------------------------
             case 'a':
-                r->options->altAddr = true;
+                r->options->noAltAddr = true;
                 break;
 
             // ------------------------------------
@@ -309,9 +312,6 @@ static void _processBlock( struct RunTime *r )
 
     genericsReport( V_DEBUG, "RXED Packet of %d bytes" EOL, y );
 
-    /* Account for this reception */
-    r->intervalBytes += y;
-
     if ( y )
     {
 #ifdef DUMP_BLOCK
@@ -351,6 +351,7 @@ static void _processBlock( struct RunTime *r )
                             if ( r->options->channel == p.packet[g].s )
                             {
                                 r->pmBuffer[r->wp] = p.packet[g].d;
+                                r->newTotalBytes++;
                                 uint32_t nwp = ( r->wp + 1 ) % r->options->buflen;
 
                                 if ( nwp == r->rp )
@@ -375,6 +376,8 @@ static void _processBlock( struct RunTime *r )
         }
         else
         {
+            r->newTotalBytes += y;
+
             while ( y-- )
             {
                 r->pmBuffer[r->wp] = *c++;
@@ -498,6 +501,12 @@ static void _etmCB( void *d )
     /* Deal with changes introduced by this event ========================= */
     if ( ETMStateChanged( &r->i, EV_CH_ADDRESS ) )
     {
+        /* Make debug report if calculated and reported addresses differ. This is most useful for testing when exhaustive  */
+        /* address reporting is switched on. It will give 'false positives' for uncalculable instructions (e.g. bx lr) but */
+        /* it's a decent safety net to be sure the jump decoder is working correctly.                                      */
+        _appendToOPBuffer( r, r->op.currentLine, LT_DEBUG, "%sCommanded CPU Address change (Was:0x%08x Commanded:0x%08x)" EOL,
+                           ( r->op.workingAddr == cpu->addr ) ? "" : "***INCONSISTENT*** ", r->op.workingAddr, cpu->addr );
+
         r->op.workingAddr = cpu->addr;
     }
 
@@ -666,6 +675,7 @@ static void _etmCB( void *d )
         else
         {
             /* We didn't have a symbol for this address, so let's just assume a short instruction */
+            _appendRefToOPBuffer( r, r->op.currentLine, LT_DEBUG, "*** No Symbol found ***" EOL );
             r->op.workingAddr += 2;
         }
 
@@ -705,8 +715,8 @@ static void _dumpBuffer( struct RunTime *r )
     if ( ( bytesAvailable + r->rp ) > r->options->buflen )
     {
         /* Buffer is wrapped - submit both parts */
-        ETMDecoderPump( &r->i, &r->pmBuffer[r->rp], r->options->buflen - r->rp - 1, _etmCB, _etmReport, r );
-        ETMDecoderPump( &r->i, &r->pmBuffer[0], r->rp, _etmCB, _etmReport, r );
+        ETMDecoderPump( &r->i, &r->pmBuffer[r->rp], r->options->buflen - r->rp, _etmCB, _etmReport, r );
+        ETMDecoderPump( &r->i, &r->pmBuffer[0], r->wp, _etmCB, _etmReport, r );
     }
     else
     {
@@ -1034,7 +1044,7 @@ int main( int argc, char *argv[] )
     struct hostent *server;
     int flag = 1;
 
-    int32_t lastTime, lastTTime, lastTSTime;
+    int32_t lastTTime, lastTSTime, lastHTime;
     int r;
     struct timeval tv;
     fd_set readfds;
@@ -1055,7 +1065,7 @@ int main( int argc, char *argv[] )
     _r.sio = SIOsetup( _r.progName, _r.options->elffile, ( _r.options->file != NULL ) );
 
     /* Fill in a time to start from */
-    lastTime = lastTTime = lastTSTime = genericsTimestampmS();
+    lastHTime = lastTTime = lastTSTime = genericsTimestampmS();
 
     /* This ensures the atexit gets called */
     if ( SIG_ERR == signal( SIGINT, _intHandler ) )
@@ -1072,7 +1082,7 @@ int main( int argc, char *argv[] )
     /* Create the buffer memory */
     _r.pmBuffer = ( uint8_t * )calloc( 1, _r.options->buflen );
 
-    ETMDecoderInit( &_r.i, &_r.options->altAddr );
+    ETMDecoderInit( &_r.i, !( _r.options->noAltAddr ) );
 
     if ( _r.options->useTPIU )
     {
@@ -1108,7 +1118,7 @@ int main( int argc, char *argv[] )
 
             if ( !server )
             {
-                perror( "Cannot find host" );
+                perror( "Cannot find host" EOL );
                 return -EIO;
             }
 
@@ -1120,7 +1130,7 @@ int main( int argc, char *argv[] )
 
             if ( connect( sourcefd, ( struct sockaddr * ) &serv_addr, sizeof( serv_addr ) ) < 0 )
             {
-                perror( "Could not connect" );
+                /* This can happen when the feeder has gone missing... */
                 close( sourcefd );
                 usleep( 1000000 );
                 continue;
@@ -1135,7 +1145,7 @@ int main( int argc, char *argv[] )
             }
         }
 
-        FD_ZERO( &readfds );
+	FD_ZERO( &readfds );
 
         /* ----------------------------------------------------------------------------- */
         /* This is the main active loop...only break out of this when ending or on error */
@@ -1177,12 +1187,11 @@ int main( int argc, char *argv[] )
                 {
                     /* Pump all of the data through the protocol handler */
                     _processBlock( &_r );
-                    lastTime = genericsTimestampmS();
                 }
             }
 
             /* Update the outputs and deal with any keys that made it up this high */
-            switch ( SIOHandler( _r.sio, ( genericsTimestampmS() - lastTTime ) > TICK_TIME_MS, _r.oldintervalBytes ) )
+            switch ( SIOHandler( _r.sio, ( genericsTimestampmS() - lastTTime ) > TICK_TIME_MS, _r.oldTotalIntervalBytes ) )
             {
                 case SIO_EV_HOLD:
                     if ( !_r.options->file )
@@ -1239,15 +1248,15 @@ int main( int argc, char *argv[] )
                     break;
             }
 
-            /* Update the various timers that are running */
-            if ( ( genericsTimestampmS() - lastTTime ) > TICK_TIME_MS )
-            {
-                lastTTime = genericsTimestampmS();
-            }
+            /* Deal with possible timeout on sampling, or if this is a read-from-file that is finished */
+            if ( ( !_r.numLines )  &&
+                    (
+                                ( _r.options->file && !sourcefd ) ||
 
-            /* Deal with possible timeout on sampling, or if this is a read-from-file */
-            if ( ( !_r.numLines ) &&
-                    ( _r.options->file || ( ( ( genericsTimestampmS() - lastTime ) > HANG_TIME_MS ) && ( _r.wp != _r.rp ) ) )
+                                ( ( ( genericsTimestampmS() - lastHTime ) > HANG_TIME_MS ) &&
+				  ( _r.newTotalBytes - _r.oldTotalHangBytes == 0 ) &&
+				  ( _r.wp != _r.rp ) )
+                    )
                )
             {
                 _dumpBuffer( &_r );
@@ -1256,10 +1265,21 @@ int main( int argc, char *argv[] )
             }
 
             /* Update the intervals */
+            if ( ( genericsTimestampmS() - lastHTime ) > HANG_TIME_MS )
+            {
+                _r.oldTotalHangBytes = _r.newTotalBytes;
+                lastHTime = genericsTimestampmS();
+            }
+
+            if ( ( genericsTimestampmS() - lastTTime ) > TICK_TIME_MS )
+            {
+                lastTTime = genericsTimestampmS();
+            }
+
             if ( ( genericsTimestampmS() - lastTSTime ) > INTERVAL_TIME_MS )
             {
-                _r.oldintervalBytes = _r.intervalBytes;
-                _r.intervalBytes = 0;
+                _r.oldTotalIntervalBytes = _r.newTotalBytes - _r.oldTotalBytes;
+                _r.oldTotalBytes = _r.newTotalBytes;
                 lastTSTime = genericsTimestampmS();
             }
         }
diff --git a/Src/orbuculum.c b/Src/orbuculum.c
diff --git a/Src/sio.c b/Src/sio.c
diff --git a/Src/tpiuDecoder.c b/Src/tpiuDecoder.c
