[DynInst_API:] [PATCH 1/2] Fully separate symtab and symlite


Date: Fri, 14 Mar 2014 17:44:46 -0700
From: Josh Stone <jistone@xxxxxxxxxx>
Subject: [DynInst_API:] [PATCH 1/2] Fully separate symtab and symlite
The cmake LIGHTWEIGHT_SYMTAB option is documented to toggle between
symtab and symlite for ParseAPI, ProcControl, and Stackwalker.  However,
these libraries didn't fully commit one way or the other, so we were
left in limbo with both loaded most of the time.  This patch fixes them
to be fully separated into just symtab or just symlite.

- All CMakeLists now respect LIGHTWEIGHT_SYMTAB when setting target
  libraries, so they'll fail to link if something is wrong.

- ParseAPI now builds with symlite, but note that DyninstAPI won't work
  yet in this mode, because it assumes full SymtabCodeSource.

- ProcControl's getElfReader() now works with either symtab or symlite.

- Stackwalker's DebugStepperImpl::GetReg is refactored a bit to work
  without symtab, and the latent cap_stackwalker_use_symtab is now
  converted to WITH_SYMTAB_API and enabled.
---
 parseAPI/CMakeLists.txt         | 14 ++++-----
 patchAPI/CMakeLists.txt         |  1 -
 proccontrol/CMakeLists.txt      |  5 ++--
 proccontrol/src/bluegenep.C     | 12 ++++++++
 proccontrol/src/bluegeneq.C     | 12 ++++++++
 proccontrol/src/freebsd.C       | 13 +++++++++
 proccontrol/src/linux.C         | 13 +++++++++
 stackwalk/src/bluegenep-swk.C   |  8 ------
 stackwalk/src/dbginfo-stepper.C | 36 +++++++++++------------
 stackwalk/src/frame.C           |  2 +-
 stackwalk/src/linuxbsd-swk.C    | 13 +++++++++
 stackwalk/src/symtab-swk.C      | 64 +----------------------------------------
 stackwalk/src/symtab-swk.h      |  2 +-
 13 files changed, 93 insertions(+), 102 deletions(-)

diff --git a/parseAPI/CMakeLists.txt b/parseAPI/CMakeLists.txt
index 21da129be0ab..b2fda92af364 100644
--- a/parseAPI/CMakeLists.txt
+++ b/parseAPI/CMakeLists.txt
@@ -44,10 +44,14 @@ set (SRC_LIST
         ../dataflowAPI/src/Visitors.C 
     )
 
-if (NOT PLATFORM MATCHES nt)
+if (LIGHTWEIGHT_SYMTAB)
 set (SRC_LIST ${SRC_LIST}
     src/SymLiteCodeSource.C
 )
+else()
+set (SRC_LIST ${SRC_LIST}
+    src/SymtabCodeSource.C
+)
 endif()
 
 if (PLATFORM MATCHES amd64 OR PLATFORM MATCHES i386 OR PLATFORM
@@ -69,12 +73,6 @@ set (SRC_LIST ${SRC_LIST}
     )
 endif()
 
-if (NOT ${LIGHTWEIGHT_SYMTAB})
-set (SRC_LIST ${SRC_LIST}
-    src/SymtabCodeSource.C
-    )
-endif()
-
 SET_SOURCE_FILES_PROPERTIES(${SRC_LIST} PROPERTIES LANGUAGE CXX)
 
 ADD_DEFINITIONS(-DPARSER_LIB)
@@ -89,7 +87,7 @@ if (UNIX)
 target_link_libraries(parseAPI ${LIBELF_LIBRARIES})
 endif()
 
-if (${LIGHTWEIGHT_SYMTAB})
+if (LIGHTWEIGHT_SYMTAB)
 target_link_libraries(parseAPI symLite)
 else()
 target_link_libraries(parseAPI symtabAPI)
diff --git a/patchAPI/CMakeLists.txt b/patchAPI/CMakeLists.txt
index b246f06436d6..1753674527dd 100644
--- a/patchAPI/CMakeLists.txt
+++ b/patchAPI/CMakeLists.txt
@@ -29,7 +29,6 @@ ADD_DEFINITIONS(-DPATCHAPI_LIB)
 add_library (patchAPI ${SRC_LIST})
 add_library (patchAPI_static STATIC ${SRC_LIST})
 target_link_libraries(patchAPI common)
-target_link_libraries(patchAPI symtabAPI)
 target_link_libraries(patchAPI instructionAPI)
 target_link_libraries(patchAPI parseAPI)
 
diff --git a/proccontrol/CMakeLists.txt b/proccontrol/CMakeLists.txt
index 4498d2066250..79515444b2a9 100644
--- a/proccontrol/CMakeLists.txt
+++ b/proccontrol/CMakeLists.txt
@@ -87,10 +87,11 @@ target_link_libraries(pcontrol ${CMAKE_DL_LIBS})
 
 if (UNIX)
 target_link_libraries(pcontrol dynElf)
-target_link_libraries(pcontrol symLite)
 endif()
 
-if(WIN32)
+if (LIGHTWEIGHT_SYMTAB)
+target_link_libraries(pcontrol symLite)
+else()
 target_link_libraries(pcontrol symtabAPI)
 endif()
 
diff --git a/proccontrol/src/bluegenep.C b/proccontrol/src/bluegenep.C
index 4ac1f5841aaa..4aadc19156f7 100644
--- a/proccontrol/src/bluegenep.C
+++ b/proccontrol/src/bluegenep.C
@@ -43,7 +43,13 @@
 #include "proccontrol/src/irpc.h"
 
 #include "common/h/SymReader.h"
+#if defined(WITH_SYMLITE)
 #include "symlite/h/SymLite-elf.h"
+#elif defined(WITH_SYMTAB_API)
+#include "symtabAPI/h/SymtabReader.h"
+#else
+#error "No defined symbol reader"
+#endif
 
 #include <sys/types.h>
 #include <sys/socket.h>
@@ -1356,12 +1362,18 @@ bool bgp_process::plat_supportDOTF()
 
 SymbolReaderFactory *getElfReader()
 {
+#if defined(WITH_SYMLITE)
   static SymbolReaderFactory *symreader_factory = NULL;
   if (symreader_factory)
     return symreader_factory;
 
   symreader_factory = (SymbolReaderFactory *) new SymElfFactory();
   return symreader_factory;
+#elif defined(WITH_SYMTAB_API)
+  return SymtabAPI::getSymtabReaderFactory();
+#else
+#error "No defined symbol reader"
+#endif
 }
 
 SymbolReaderFactory *bgp_process::plat_defaultSymReader()
diff --git a/proccontrol/src/bluegeneq.C b/proccontrol/src/bluegeneq.C
index 46b017e9d18f..eaa43c031ee8 100644
--- a/proccontrol/src/bluegeneq.C
+++ b/proccontrol/src/bluegeneq.C
@@ -63,7 +63,13 @@ using namespace Dyninst;
 using namespace ProcControlAPI;
 using namespace std;
 
+#if defined(WITH_SYMLITE)
 #include "symlite/h/SymLite-elf.h"
+#elif defined(WITH_SYMTAB_API)
+#include "symtabAPI/h/SymtabReader.h"
+#else
+#error "No defined symbol reader"
+#endif
 
 using namespace bgq;
 using namespace std;
@@ -630,12 +636,18 @@ bool bgq_process::plat_supportLWPPostDestroy()
 
 static SymbolReaderFactory *getElfReader()
 {
+#if defined(WITH_SYMLITE)
    static SymbolReaderFactory *symreader_factory = NULL;
    if (symreader_factory)
       return symreader_factory;
 
    symreader_factory = (SymbolReaderFactory *) new SymElfFactory();
    return symreader_factory;
+#elif defined(WITH_SYMTAB_API)
+   return SymtabAPI::getSymtabReaderFactory();
+#else
+#error "No defined symbol reader"
+#endif
 }
 
 SymbolReaderFactory *bgq_process::plat_defaultSymReader()
diff --git a/proccontrol/src/freebsd.C b/proccontrol/src/freebsd.C
index 7f57c4cdabd6..b455528a8811 100644
--- a/proccontrol/src/freebsd.C
+++ b/proccontrol/src/freebsd.C
@@ -42,7 +42,14 @@
 #include "proccontrol/src/int_handler.h"
 #include "proccontrol/src/int_event.h"
 #include "common/src/freebsdKludges.h"
+
+#if defined(WITH_SYMLITE)
 #include "symlite/h/SymLite-elf.h"
+#elif defined(WITH_SYMTAB_API)
+#include "symtabAPI/h/SymtabReader.h"
+#else
+#error "No defined symbol reader"
+#endif
 
 #include <iostream>
 
@@ -1745,12 +1752,18 @@ bool freebsd_process::plat_individualRegAccess()
 
 SymbolReaderFactory *freebsd_process::plat_defaultSymReader()
 {
+#if defined(WITH_SYMLITE)
   static SymbolReaderFactory *symreader_factory = NULL;
   if (symreader_factory)
     return symreader_factory;
 
   symreader_factory = (SymbolReaderFactory *) new SymElfFactory();
   return symreader_factory;
+#elif defined(WITH_SYMTAB_API)
+  return SymtabAPI::getSymtabReaderFactory();
+#else
+#error "No defined symbol reader"
+#endif
 }
 
 bool freebsd_process::plat_threadOpsNeedProcStop()
diff --git a/proccontrol/src/linux.C b/proccontrol/src/linux.C
index c0de7ba77bc6..2a0ece0561af 100644
--- a/proccontrol/src/linux.C
+++ b/proccontrol/src/linux.C
@@ -73,7 +73,14 @@
 
 using namespace Dyninst;
 using namespace ProcControlAPI;
+
+#if defined(WITH_SYMLITE)
 #include "symlite/h/SymLite-elf.h"
+#elif defined(WITH_SYMTAB_API)
+#include "symtabAPI/h/SymtabReader.h"
+#else
+#error "No defined symbol reader"
+#endif
 
 #if !defined(PTRACE_GETREGS) && defined(PPC_PTRACE_GETREGS)
 #define PTRACE_GETREGS PPC_PTRACE_GETREGS
@@ -1289,12 +1296,18 @@ bool linux_thread::plat_cont()
 
 SymbolReaderFactory *getElfReader()
 {
+#if defined(WITH_SYMLITE)
   static SymbolReaderFactory *symreader_factory = NULL;
   if (symreader_factory)
     return symreader_factory;
 
   symreader_factory = (SymbolReaderFactory *) new SymElfFactory();
   return symreader_factory;
+#elif defined(WITH_SYMTAB_API)
+  return SymtabAPI::getSymtabReaderFactory();
+#else
+#error "No defined symbol reader"
+#endif
 }
 
 SymbolReaderFactory *linux_process::plat_defaultSymReader()
diff --git a/stackwalk/src/bluegenep-swk.C b/stackwalk/src/bluegenep-swk.C
index 4dfa943b1396..0aeee3df7b62 100644
--- a/stackwalk/src/bluegenep-swk.C
+++ b/stackwalk/src/bluegenep-swk.C
@@ -92,14 +92,6 @@ namespace Dyninst {
         }
       }
 
-/*   
-    sw_printf("[%s:%u] - Successfully polled for threads in debug_post_attach on %d\n", FILE__, __LINE__, pid);      
-#   if defined(cap_stackwalker_use_symtab)
-      library_tracker = new SymtabLibState(this, executable_path);
-#   else
-      library_tracker = new DefaultLibState(this);
-#   endif
-*/
       if (!library_tracker)
          setDefaultLibraryTracker();
       // This should really be checking for exceptions, since the above are constructors.
diff --git a/stackwalk/src/dbginfo-stepper.C b/stackwalk/src/dbginfo-stepper.C
index e9742078c74f..9e76ffe47538 100644
--- a/stackwalk/src/dbginfo-stepper.C
+++ b/stackwalk/src/dbginfo-stepper.C
@@ -43,7 +43,9 @@
 #include "dwarf/h/dwarfFrameParser.h"
 #include "dwarf/h/dwarfHandle.h"
 
+#if defined(WITH_SYMTAB_API)
 #include "symtabAPI/h/Symtab.h"
+#endif
 
 using namespace Dyninst;
 using namespace Stackwalker;
@@ -149,10 +151,6 @@ location_t DebugStepperImpl::getLastComputedLocation(unsigned long value)
 
 bool DebugStepperImpl::GetReg(MachRegister reg, MachRegisterVal &val)
 {
-   using namespace SymtabAPI;
-   
-   const Frame *prevDepthFrame = depth_frame;
-  
    if (reg.isFramePointer()) {
       val = static_cast<MachRegisterVal>(depth_frame->getFP());
       return true;
@@ -162,32 +160,34 @@ bool DebugStepperImpl::GetReg(MachRegister reg, MachRegisterVal &val)
       val = static_cast<MachRegisterVal>(depth_frame->getSP());
       return true;
    }
-   
+
    if (reg.isPC()) {
       val = static_cast<MachRegisterVal>(depth_frame->getRA());
       return true;
    }
 
+   bool result = false;
+   const Frame *prevDepthFrame = depth_frame;
    depth_frame = depth_frame->getPrevFrame();
    if (!depth_frame)
    {
-      bool bres =  getProcessState()->getRegValue(reg, cur_frame->getThread(), val);
-      depth_frame = prevDepthFrame;
-      return bres;
+      result = getProcessState()->getRegValue(reg, cur_frame->getThread(), val);
    }
-
-   Offset offset;
-   void *symtab_v = NULL;
-   std::string lib;
-   depth_frame->getLibOffset(lib, offset, symtab_v);
-   Symtab *symtab = (Symtab*) symtab_v;
-   if (!symtab)
+#if defined(WITH_SYMTAB_API)
+   else
    {
-     depth_frame = prevDepthFrame;
-     return false;
+      Offset offset;
+      void *symtab_v = NULL;
+      std::string lib;
+      depth_frame->getLibOffset(lib, offset, symtab_v);
+      SymtabAPI::Symtab *symtab = (SymtabAPI::Symtab*) symtab_v;
+      if (symtab)
+      {
+         result = symtab->getRegValueAtFrame(offset, reg, val, this);
+      }
    }
+#endif
 
-   bool result = symtab->getRegValueAtFrame(offset, reg, val, this);
    depth_frame = prevDepthFrame;
    return result;
 }
diff --git a/stackwalk/src/frame.C b/stackwalk/src/frame.C
index d1e13113d7af..c49f333963cf 100644
--- a/stackwalk/src/frame.C
+++ b/stackwalk/src/frame.C
@@ -337,7 +337,7 @@ bool Frame::getLibOffset(std::string &lib, Dyninst::Offset &offset, void*& symta
   lib = la.first;
   offset = getRA() - la.second;
 
-#if defined(cap_stackwalker_use_symtab)
+#if defined(WITH_SYMTAB_API)
   symtab = static_cast<void *>(SymtabWrapper::getSymtab(lib));
 #else
   symtab = NULL;
diff --git a/stackwalk/src/linuxbsd-swk.C b/stackwalk/src/linuxbsd-swk.C
index c88e8dba8fcd..e27e5186d010 100644
--- a/stackwalk/src/linuxbsd-swk.C
+++ b/stackwalk/src/linuxbsd-swk.C
@@ -43,6 +43,7 @@
 #include <cerrno>
 #include <cstring>
 
+#include <sys/mman.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <signal.h>
@@ -52,8 +53,14 @@
 using namespace Dyninst;
 using namespace Dyninst::Stackwalker;
 
+#if defined(WITH_SYMLITE)
 #include "symlite/h/SymLite-elf.h"
+#elif defined(WITH_SYMTAB_API)
 #include "symtabAPI/h/SymtabReader.h"
+#else
+#error "No defined symbol reader"
+#endif
+
 #include "linuxbsd-swk.h"
 
 
@@ -61,8 +68,14 @@ extern int P_gettid();
 
 SymbolReaderFactory *Dyninst::Stackwalker::getDefaultSymbolReader()
 {
+#if defined(WITH_SYMLITE)
    static SymElfFactory symelffact;
    return &symelffact;
+#elif defined(WITH_SYMTAB_API)
+   return SymtabAPI::getSymtabReaderFactory();
+#else
+#error "No defined symbol reader"
+#endif
 }
 
 static void registerLibSpotterSelf(ProcSelf *pself);
diff --git a/stackwalk/src/symtab-swk.C b/stackwalk/src/symtab-swk.C
index d2b74a582e7b..1ad2154de2b0 100644
--- a/stackwalk/src/symtab-swk.C
+++ b/stackwalk/src/symtab-swk.C
@@ -29,7 +29,7 @@
  */
 
 
-#if defined(cap_stackwalker_use_symtab)
+#if defined(WITH_SYMTAB_API)
 
 #include "stackwalk/h/symlookup.h"
 #include "stackwalk/h/swk_errors.h"
@@ -103,66 +103,4 @@ void SymtabWrapper::notifyOfSymtab(Symtab *symtab, std::string name)
   wrapper->map[name] = symtab;
 }
 
-bool SwkSymtab::lookupAtAddr(Address addr, std::string &out_name,
-				void* &out_value)
-{
-  Address load_addr;
-  std::string libname;
-  bool result;
-
-  LibraryState *libtracker = walker->getProcessState()->getLibraryTracker();
-  if (!libtracker) {
-     sw_printf("[%s:%u] - getLibraryTracker() failed\n", FILE__, __LINE__);
-     setLastError(err_nolibtracker, "No library tracker object registered");
-     return false;
-  }
-
-  LibAddrPair lib;
-  result = libtracker->getLibraryAtAddr(addr, lib);
-  if (!result) {
-     sw_printf("[%s:%u] - getLibraryAtAddr() failed: %s\n", FILE__, __LINE__, getLastErrorMsg());
-    return false;
-  }
-
-  Symtab *symtab = SymtabWrapper::getSymtab(lib.first);
-  assert(symtab);
-  load_addr = lib.second;
-
-  //TODO: Cache symbol vector and use binary search
-  std::vector<Symbol *> syms;
-  result = symtab->getAllSymbolsByType(syms, Symbol::ST_FUNCTION);
-  if (!result) {
-    sw_printf("[%s:%u] - Couldn't get symbols for %s\n", 
-              FILE__, __LINE__, libname.c_str());
-    return false;
-  }
-  Symbol *candidate = NULL;
-  unsigned long distance = 0;
-  for (unsigned i = 0; i < syms.size(); i++)
-  {
-    unsigned long cur_distance = (addr - load_addr) - syms[i]->getAddr();
-    if (!candidate || cur_distance < distance) {
-      distance = cur_distance;
-      candidate = syms[i];
-    }
-  }
-
-  out_name = candidate->getTypedName();
-  if (!out_name.length())
-    out_name = candidate->getPrettyName();
-  if (!out_name.length())
-    out_name = candidate->getName();
-  out_value = (void *) candidate;
-
-  sw_printf("[%s:%u] - Found name for %lx : %s\n", FILE__, __LINE__,
-	    addr, out_name.c_str());  
-  
-  return true;
-}
-
-SwkSymtab::SwkSymtab(Walker *w, std::string exec_name) :
-   SymbolLookup(w, exec_name)
-{
-}
-
 #endif
diff --git a/stackwalk/src/symtab-swk.h b/stackwalk/src/symtab-swk.h
index b5678bd22846..13f050f8a124 100644
--- a/stackwalk/src/symtab-swk.h
+++ b/stackwalk/src/symtab-swk.h
@@ -31,7 +31,7 @@
 #if !defined(SYMTAB_SWK_H_)
 #define SYMTAB_SWK_H_
 
-#if defined(cap_stackwalker_use_symtab)
+#if defined(WITH_SYMTAB_API)
 
 #include "stackwalk/h/framestepper.h"
 #include "symtabAPI/h/Symtab.h"
-- 
1.8.5.3

[← Prev in Thread] Current Thread [Next in Thread→]