From 40550ec3a2572df9586b3ae060f0f8cf0cf357ab Mon Sep 17 00:00:00 2001 From: Daniel McNulty Date: Wed, 28 Apr 2010 12:52:53 -0500 Subject: [PATCH] BPatch_binaryEdit::staticExecutableLoaded was poorly named. Changing its name to isStaticExecutable (per feedback from others in the group). This also involves changes to the testsuite. Fixed some bugs in the rewriter of static binaries: - was using replaceFunctionCall to replace ctor and dtor handlers -- this appears to be having some problems on x86. Instead just instrumented the original ctor/dtor handler with a funcJump to the new handler. In retrospect, this is the better solution. - Variables in relocatable files that are in COMMON were not being created correctly by Symtab. If a variable is in COMMON, there is no way to have an alias for this variable so new variables are created for every COMMON symbol that corresponds to a variable. --- dyninstAPI/h/BPatch_binaryEdit.h | 4 +- dyninstAPI/src/BPatch_binaryEdit.C | 2 +- dyninstAPI/src/linux-x86.C | 161 ++++++++++++++----------------------- symtabAPI/src/Symtab.C | 17 +++- symtabAPI/src/emitElf-64.C | 2 +- testsuite/src/dyninst/test1_14.C | 2 +- testsuite/src/dyninst/test1_21.C | 2 +- testsuite/src/dyninst/test1_22.C | 2 +- 8 files changed, 81 insertions(+), 111 deletions(-) diff --git a/dyninstAPI/h/BPatch_binaryEdit.h b/dyninstAPI/h/BPatch_binaryEdit.h index d7d6a02..f1c5d94 100644 --- a/dyninstAPI/h/BPatch_binaryEdit.h +++ b/dyninstAPI/h/BPatch_binaryEdit.h @@ -148,11 +148,11 @@ class BPATCH_DLL_EXPORT BPatch_binaryEdit : public BPatch_addressSpace { API_EXPORT_VIRT(Int, (libname, reload), bool, loadLibrary,(const char *libname, bool reload = false)); - // BPatch_binaryEdit::staticExecutableLoaded + // BPatch_binaryEdit::isStaticExecutable // // Returns true if the original binary opened was a static executable API_EXPORT_VIRT(Int, (), - bool, staticExecutableLoaded, ()); + bool, isStaticExecutable, ()); }; diff --git a/dyninstAPI/src/BPatch_binaryEdit.C b/dyninstAPI/src/BPatch_binaryEdit.C index 5565aee..71a63d0 100644 --- a/dyninstAPI/src/BPatch_binaryEdit.C +++ b/dyninstAPI/src/BPatch_binaryEdit.C @@ -399,7 +399,7 @@ bool BPatch_binaryEdit::loadLibraryInt(const char *libname, bool deps) return true; } -bool BPatch_binaryEdit::staticExecutableLoadedInt() { +bool BPatch_binaryEdit::isStaticExecutableInt() { return origBinEdit->getMappedObject()->isStaticExec(); } diff --git a/dyninstAPI/src/linux-x86.C b/dyninstAPI/src/linux-x86.C index 0847a0e..ad919f8 100644 --- a/dyninstAPI/src/linux-x86.C +++ b/dyninstAPI/src/linux-x86.C @@ -1842,6 +1842,47 @@ static const std::string DYNINST_DTOR_LIST("DYNINSTdtors_addr"); static const std::string SYMTAB_CTOR_LIST_REL("__SYMTABAPI_CTOR_LIST__"); static const std::string SYMTAB_DTOR_LIST_REL("__SYMTABAPI_DTOR_LIST__"); +static bool replaceHandler(int_function *origHandler, int_function *newHandler, + int_symbol *newList, const std::string &listRelName) +{ + // Add instrumentation to replace the function + const pdvector &entries = origHandler->funcEntries(); + AstNodePtr funcJump = AstNode::funcReplacementNode(const_cast(newHandler)); + for(unsigned j = 0; j < entries.size(); ++j) { + miniTramp *mini = entries[j]->instrument(funcJump, + callPreInsn, orderFirstAtPoint, true, false); + if( !mini ) { + return false; + } + } + + /* create the special relocation for the new list -- search the RT library for + * the symbol + */ + Symbol *newListSym = const_cast(newList->sym()); + + std::vector allRegions; + if( !newListSym->getSymtab()->getAllRegions(allRegions) ) { + return false; + } + + bool success = false; + std::vector::iterator reg_it; + for(reg_it = allRegions.begin(); reg_it != allRegions.end(); ++reg_it) { + std::vector ®ion_rels = (*reg_it)->getRelocations(); + vector::iterator rel_it; + for( rel_it = region_rels.begin(); rel_it != region_rels.end(); ++rel_it) { + if( rel_it->getDynSym() == newListSym ) { + relocationEntry *rel = &(*rel_it); + rel->setName(listRelName); + success = true; + } + } + } + + return success; +} + bool BinaryEdit::doStaticBinarySpecialCases() { Symtab *origBinary = mobj->parse_img()->getObject(); @@ -1863,6 +1904,7 @@ bool BinaryEdit::doStaticBinarySpecialCases() { return false; } + // First, find all the necessary symbol info. int_function *globalCtorHandler = findOnlyOneFunction(LIBC_CTOR_HANDLER); if( !globalCtorHandler ) { logLine("failed to find libc constructor handler\n"); @@ -1887,58 +1929,6 @@ bool BinaryEdit::doStaticBinarySpecialCases() { return false; } - /* - * Replace all calls to the global ctor and dtor handlers with the special - * handler - */ - pdvector allFuncs; - getAllFunctions(allFuncs); - - bool success = false; - for(unsigned i = 0; i < allFuncs.size(); ++i) { - int_function *func = allFuncs[i]; - if( !func ) continue; - - const pdvector &calls = func->funcCalls(); - - for(unsigned j = 0; j < calls.size(); ++j) { - instPoint *point = calls[j]; - if ( !point ) continue; - - Address target = point->callTarget(); - - if( !target ) continue; - - if( target == globalCtorHandler->getAddress() ) { - if( !replaceFunctionCall(point, dyninstCtorHandler) ) { - success = false; - }else{ - inst_printf("%s[%d]: replaced call to %s in %s with %s\n", - FILE__, __LINE__, LIBC_CTOR_HANDLER.c_str(), - func->prettyName().c_str(), DYNINST_CTOR_HANDLER.c_str()); - success = true; - } - }else if( target == globalDtorHandler->getAddress() ) { - if( !replaceFunctionCall(point, dyninstDtorHandler) ) { - success = false; - }else{ - inst_printf("%s[%d]: replaced call to %s in %s with %s\n", - FILE__, __LINE__, LIBC_DTOR_HANDLER.c_str(), - func->prettyName().c_str(), DYNINST_DTOR_HANDLER.c_str()); - success = true; - } - } - } - } - - if( !success ) { - logLine("failed to replace libc ctor or dtor handler with special handler\n"); - return false; - } - - /* create the special relocation for the ctors and dtors list -- search the - * RT library for the symbol - */ int_symbol ctorsListInt; int_symbol dtorsListInt; bool ctorFound = false, dtorFound = false; @@ -1965,60 +1955,31 @@ bool BinaryEdit::doStaticBinarySpecialCases() { return false; } - Symbol *ctorsList = const_cast(ctorsListInt.sym()); - - std::vector allRegions; - if( !ctorsList->getSymtab()->getAllRegions(allRegions) ) { - logLine("failed to find ctors list relocation\n"); - return false; - } - - success = false; - std::vector::iterator reg_it; - for(reg_it = allRegions.begin(); reg_it != allRegions.end(); ++reg_it) { - std::vector ®ion_rels = (*reg_it)->getRelocations(); - vector::iterator rel_it; - for( rel_it = region_rels.begin(); rel_it != region_rels.end(); ++rel_it) { - if( rel_it->getDynSym() == ctorsList ) { - relocationEntry *rel = &(*rel_it); - rel->setName(SYMTAB_CTOR_LIST_REL); - success = true; - } - } - } - - if( !success ) { - logLine("failed to change relocation for ctors list\n"); - return false; - } - - Symbol *dtorsList = const_cast(dtorsListInt.sym()); - - if( !dtorsList->getSymtab()->getAllRegions(allRegions) ) { - logLine("failed to find dtors list relocation\n"); + /* + * Replace the libc ctor and dtor handlers with our special handlers + */ + if( !replaceHandler(globalCtorHandler, dyninstCtorHandler, + &ctorsListInt, SYMTAB_CTOR_LIST_REL) ) { + logLine("Failed to replace libc ctor handler with special handler"); return false; + }else{ + inst_printf("%s[%d]: replaced ctor function %s with %s\n", + FILE__, __LINE__, LIBC_CTOR_HANDLER.c_str(), + DYNINST_CTOR_HANDLER.c_str()); } - success = false; - for(reg_it = allRegions.begin(); reg_it != allRegions.end(); ++reg_it) { - std::vector ®ion_rels = (*reg_it)->getRelocations(); - vector::iterator rel_it; - for( rel_it = region_rels.begin(); rel_it != region_rels.end(); ++rel_it) { - if( rel_it->getDynSym() == dtorsList ) { - relocationEntry *rel = &(*rel_it); - rel->setName(SYMTAB_DTOR_LIST_REL); - success = true; - } - } - } - - if( !success ) { - logLine("failed to change relocation for dtors list\n"); + if( !replaceHandler(globalDtorHandler, dyninstDtorHandler, + &dtorsListInt, SYMTAB_DTOR_LIST_REL) ) { + logLine("Failed to replace libc dtor handler with special handler"); return false; + }else{ + inst_printf("%s[%d]: replaced dtor function %s with %s\n", + FILE__, __LINE__, LIBC_DTOR_HANDLER.c_str(), + DYNINST_DTOR_HANDLER.c_str()); } /* - * Special Case 3: Issue a warning if attempting to link pthreads into a binary + * Special Case 2: Issue a warning if attempting to link pthreads into a binary * that originally did not support it or into a binary that is stripped. This * scenario is not supported with the initial release of the binary rewriter for * static binaries. @@ -2055,7 +2016,7 @@ bool BinaryEdit::doStaticBinarySpecialCases() { } /* - * Special Case 4: + * Special Case 3: * The RT library has some dependencies -- Symtab always needs to know * about these dependencies. So if the dependencies haven't already been * loaded, load them. diff --git a/symtabAPI/src/Symtab.C b/symtabAPI/src/Symtab.C index 57b8dda..fad0bb8 100644 --- a/symtabAPI/src/Symtab.C +++ b/symtabAPI/src/Symtab.C @@ -824,14 +824,23 @@ bool Symtab::addSymbolToAggregates(Symbol *&sym) } else { /* XXX - * See comment above about uniqueness of symbols in relocatable - * files + * For relocatable files, the offset is not a unique identifier for + * a Symbol. With functions, the Region and offset could be used to + * identify the symbol. With variables, the Region and offset may + * not uniquely identify the symbol. The only case were this occurs + * is with COMMON symbols -- their offset is their memory alignment + * and their Region is undefined. In this case, always create a + * new variable. */ - if( var->getRegion() != sym->getRegion() ) { + if( obj_RelocatableFile == getObjectType() && + ( var->getRegion() != sym->getRegion() || + NULL == sym->getRegion() ) ) + { var = new Variable(sym); everyVariable.push_back(var); + }else{ + var->addSymbol(sym); } - var->addSymbol(sym); } sym->setVariable(var); break; diff --git a/symtabAPI/src/emitElf-64.C b/symtabAPI/src/emitElf-64.C index eed8121..13f805f 100644 --- a/symtabAPI/src/emitElf-64.C +++ b/symtabAPI/src/emitElf-64.C @@ -1876,7 +1876,7 @@ void emitElf64::createRelocationSections(Symtab *obj, std::vectord_un.d_val; - dynamic_reloc_size = old_reloc_size+ l*sizeof(Elf32_Rel)+ m*sizeof(Elf32_Rela); + dynamic_reloc_size = old_reloc_size+ l*sizeof(Elf64_Rel)+ m*sizeof(Elf64_Rela); string name; if (secTagRegionMapping.find(dtype) != secTagRegionMapping.end()) name = secTagRegionMapping[dtype]->getRegionName(); diff --git a/testsuite/src/dyninst/test1_14.C b/testsuite/src/dyninst/test1_14.C index 303be2f..46f57e1 100644 --- a/testsuite/src/dyninst/test1_14.C +++ b/testsuite/src/dyninst/test1_14.C @@ -84,7 +84,7 @@ test_results_t test1_14_Mutator::executeTest() { bool isStatic = false; if( NULL != appBinEdit ) { - isStatic = appBinEdit->staticExecutableLoaded(); + isStatic = appBinEdit->isStaticExecutable(); } strncpy(libNameA, libNameAroot, 127); addLibArchExt(libNameA,127, pointer_size, isStatic); diff --git a/testsuite/src/dyninst/test1_21.C b/testsuite/src/dyninst/test1_21.C index 4672fa3..8eccecf 100644 --- a/testsuite/src/dyninst/test1_21.C +++ b/testsuite/src/dyninst/test1_21.C @@ -204,7 +204,7 @@ test_results_t test1_21_Mutator::executeTest() bool isStatic = false; if( NULL != appBinEdit ) { - isStatic = appBinEdit->staticExecutableLoaded(); + isStatic = appBinEdit->isStaticExecutable(); } strncpy(libNameA, libNameAroot, 127); diff --git a/testsuite/src/dyninst/test1_22.C b/testsuite/src/dyninst/test1_22.C index ea981f4..3bcee28 100644 --- a/testsuite/src/dyninst/test1_22.C +++ b/testsuite/src/dyninst/test1_22.C @@ -272,7 +272,7 @@ test_results_t test1_22_Mutator::executeTest() #endif bool isStatic = false; if( NULL != appBinEdit ) { - isStatic = appBinEdit->staticExecutableLoaded(); + isStatic = appBinEdit->isStaticExecutable(); } strncpy(libNameA, libNameAroot, 127); -- 1.8.3.1