From 0a5abca4e9be3a3ab729f32e2459594b437185d9 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Mon, 18 Oct 2021 12:37:36 -0400 Subject: [PATCH] Fix garbled stack traces. Newer versions of VS and Windows use ASLR for security purposes, meaning that the module may be relocated from what is expected by the linker map file. While we could kludge around that by disabling ASLR, it would be better to just map the actual stack addresses to the expected addresses. This also simplifies all of the weird segment math to simply use the rvabase value in the map file. --- .../FeatureLib/pfStackTrace/pfMapFile.cpp | 41 +++++++++---------- .../pfStackTrace/pfMapFileEntry.cpp | 9 +++- .../FeatureLib/pfStackTrace/pfMapFileEntry.h | 6 ++- .../FeatureLib/pfStackTrace/pfStackTrace.cpp | 11 ++++- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFile.cpp b/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFile.cpp index 71772933..fe2adade 100644 --- a/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFile.cpp +++ b/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFile.cpp @@ -6,8 +6,11 @@ #include #include #include + #ifdef WIN32 #include + +extern "C" IMAGE_DOS_HEADER __ImageBase; #endif //----------------------------------------------------------------------------- @@ -151,7 +154,7 @@ private: parse( 'H' ); char buf[256]; m_file.readString( buf, sizeof(buf) ); - segments.add( MapFileEntry(seg,offs,len,buf) ); + segments.add( MapFileEntry(seg,offs,len,buf,0) ); // break at empty line if ( nextLineEmpty() ) @@ -212,8 +215,9 @@ private: entryname = newName; } + long rvabase = m_file.readHex(); - entries.add( MapFileEntry(seg,offs,0,entryname) ); + entries.add( MapFileEntry(seg,offs,0,entryname,rvabase) ); // break at empty line if ( nextLineEmpty() ) @@ -271,26 +275,21 @@ int MapFile::line() const int MapFile::findEntry( long addr ) const { - for ( int j = 0 ; j < segments() ; ++j ) - { - const MapFileEntry& segment = getSegment( j ); - long section = segment.section(); - long segmentBegin = loadAddress() + (segment.section() << 12) + segment.offset(); - long segmentEnd = segmentBegin + segment.length(); +#ifdef WIN32 + // Cope with Windows ASLR. Note that these operations are not commutative. + addr -= (long)&__ImageBase; + addr += loadAddress(); +#endif - if ( addr >= segmentBegin && addr < segmentEnd ) - { - for ( int i = entries()-1 ; i >= 0 ; --i ) - { - const MapFileEntry entry = getEntry( i ); - if ( entry.section() == section ) - { - long entryAddr = loadAddress() + (entry.section() << 12) + entry.offset(); - if ( entryAddr <= addr ) - return i; - } - } - } + // The code before ASLR-proofing tried to compute things by segment. + // It didn't work for whatever reason, so here's something simpler + // by address. Just be sure to toss anything larger than the + // highest address we know about. + if (addr == 0 || addr > getEntry(entries() - 1).rvabase()) + return -1; + for (int i = entries() - 1; i >= 0; --i) { + if (getEntry(i).rvabase() <= addr) + return i; } return -1; } diff --git a/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFileEntry.cpp b/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFileEntry.cpp index 6b4915c0..6834de99 100644 --- a/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFileEntry.cpp +++ b/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFileEntry.cpp @@ -12,14 +12,16 @@ MapFileEntry::MapFileEntry() m_sec = 0; m_addr = 0; m_len = 0; + m_rvabase = 0; m_name[0] = 0; } -MapFileEntry::MapFileEntry( long section, long offset, long length, const char* name ) +MapFileEntry::MapFileEntry( long section, long offset, long length, const char* name, long rvabase ) { m_sec = section; m_addr = offset; m_len = length; + m_rvabase = rvabase; strncpy( m_name, name, MAX_NAME ); m_name[MAX_NAME] = 0; @@ -45,6 +47,11 @@ const char* MapFileEntry::name() const return m_name; } +long MapFileEntry::rvabase() const +{ + return m_rvabase; +} + bool MapFileEntry::operator<( const MapFileEntry& other ) const { if ( m_sec < other.m_sec ) diff --git a/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFileEntry.h b/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFileEntry.h index 74b8e93e..b7414651 100644 --- a/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFileEntry.h +++ b/Sources/Plasma/FeatureLib/pfStackTrace/pfMapFileEntry.h @@ -21,7 +21,7 @@ public: MapFileEntry(); /** Creates an entry with specified section, offset, length and name. */ - MapFileEntry( long section, long offset, long length, const char* name ); + MapFileEntry( long section, long offset, long length, const char* name, long rvabase ); /** Returns section of the entry. */ long section() const; @@ -35,6 +35,9 @@ public: /** Returns name of the entry. */ const char* name() const; + /** Returns the location of the entry. */ + long rvabase() const; + /** Returns true if the offset of this entry is before the other one. */ bool operator<( const MapFileEntry& other ) const; @@ -43,6 +46,7 @@ private: long m_addr; long m_len; char m_name[MAX_NAME+1]; + long m_rvabase; }; diff --git a/Sources/Plasma/FeatureLib/pfStackTrace/pfStackTrace.cpp b/Sources/Plasma/FeatureLib/pfStackTrace/pfStackTrace.cpp index 4fbadaf1..fd8d0524 100644 --- a/Sources/Plasma/FeatureLib/pfStackTrace/pfStackTrace.cpp +++ b/Sources/Plasma/FeatureLib/pfStackTrace/pfStackTrace.cpp @@ -6,6 +6,8 @@ #pragma optimize( "y", off ) +extern "C" struct IMAGE_DOS_HEADER __ImageBase; + //----------------------------------------------------------------------------- #define MAX_DEPTH 32 @@ -155,9 +157,14 @@ int StackTrace::printStackTrace( MapFile** map, int maps, else { const MapFileEntry &en = entryMap->getEntry( entry ); - long entryAddr = entryMap->loadAddress() + (en.section() << 12) + en.offset(); + long entryAddr = en.rvabase(); + long stackAddr = addr; +#ifdef WIN32 + stackAddr -= (long)&__ImageBase; + stackAddr += entryMap->loadAddress(); +#endif - sprintf( buf+strlen(buf), "%s (0x%08X + 0x%08X)\r\n", en.name(), entryAddr, addr - entryAddr ); + sprintf( buf+strlen(buf), "%s (0x%08X + 0x%08X)\r\n", en.name(), entryAddr, stackAddr - entryAddr ); } // append temporary buf to output buffer if space left