From 1e30a1200d2aef0b46b048088d99a7dd99ff5675 Mon Sep 17 00:00:00 2001 From: Xiaozhu Meng Date: Sun, 5 Aug 2018 13:15:33 -0500 Subject: [PATCH] Fix undeterministic tail call identification results exposed in parallel code parsing The tail call identification algorithm contains two related heuristics: (1) A jump to a block within the same function is not a tail call (2) A jump to a known entry point is a tail call For (1), whether or not the jump target is within the current function depends on the parsing order of other functions, especially the callee functions. To have a consistent tail call identification results, I add a tail call cleaning phase in the parsing finalizing phase. Because we already know the complete function boundary at function finalizing time, we can rectify the bogus tail calls and removed the functions caused by those bogus tail calls. For (2), if a known entry point is created by a bogus tail call, (2) will lead to more bogus tail calls. This is exposed by a special case. Suppose function A contains multiple jumps to block B. Block B has no incoming edges from any other function, nor does it has a symbol associated. So, B should be part of A. One of the jump in A (denoted as J1) will be marked as tail call because it tears down the stack frame before the jump, while other jumps (denoted as non-J1) will not be marked as tail calls. If J1 is parsed before non-J1, block B will be marked as a tail call entry, and all non-J1 will be marked as tail calls because they jump to ``a known entry''. Function A will thus not contain B, and the rectify method mentioned in the previous parapgraph will not work. So, I change to only mark a jump as a tail call when the target is a hint. --- parseAPI/src/IA_aarch64.C | 5 +++- parseAPI/src/IA_power.C | 6 +++-- parseAPI/src/IA_x86.C | 2 ++ parseAPI/src/Parser.C | 50 +++++++++++++++++++++++++++++++++++++++ parseAPI/src/Parser.h | 1 + 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/parseAPI/src/IA_aarch64.C b/parseAPI/src/IA_aarch64.C index 5fe3a8b843..67dd94780a 100644 --- a/parseAPI/src/IA_aarch64.C +++ b/parseAPI/src/IA_aarch64.C @@ -144,7 +144,10 @@ bool IA_aarch64::isTailCall(const Function* context, EdgeTypeEnum type, unsigned valid && callee && callee != context && - !context->contains(target) + // We can only trust entry points from hints + callee->src() == HINT && + /* the target can either be not parsed or not within the current context */ + ((target == NULL) || (target && !context->contains(target))) ) { parsing_printf("\tjump to 0x%lx, TAIL CALL\n", addr); diff --git a/parseAPI/src/IA_power.C b/parseAPI/src/IA_power.C index f74adc74bf..81e492a1fc 100644 --- a/parseAPI/src/IA_power.C +++ b/parseAPI/src/IA_power.C @@ -139,8 +139,10 @@ bool IA_power::isTailCall(const Function* context, EdgeTypeEnum type, unsigned i valid && callee && callee != context && - target && - !context->contains(target) + // We can only trust entry points from hints + callee->src() == HINT && + /* the target can either be not parsed or not within the current context */ + ((target == NULL) || (target && !context->contains(target))) ) { parsing_printf("\tjump to 0x%lx, TAIL CALL\n", addr); diff --git a/parseAPI/src/IA_x86.C b/parseAPI/src/IA_x86.C index 7bc954a8c7..503b160dc2 100644 --- a/parseAPI/src/IA_x86.C +++ b/parseAPI/src/IA_x86.C @@ -265,6 +265,8 @@ bool IA_x86::isTailCall(const Function *context, EdgeTypeEnum type, unsigned int valid && callee && callee != context && + // We can only trust entry points from hints + callee->src() == HINT && /* the target can either be not parsed or not within the current context */ ((target == NULL) || (target && !context->contains(target))) ) diff --git a/parseAPI/src/Parser.C b/parseAPI/src/Parser.C index d272e7cbe2..5f1fe38214 100644 --- a/parseAPI/src/Parser.C +++ b/parseAPI/src/Parser.C @@ -848,6 +848,27 @@ Parser::finalize(Function *f) // finish delayed parsing and sorting Function::blocklist blocks = f->blocks_int(); + // Check whether there are tail calls to blocks within the same function + dyn_hash_map visited; + for (auto bit = blocks.begin(); bit != blocks.end(); ++bit) { + Block * b = *bit; + visited[b] = true; + } + for (auto bit = blocks.begin(); bit != blocks.end(); ++bit) { + Block * b = *bit; + for (auto eit = b->targets().begin(); eit != b->targets().end(); ++eit) { + Edge *e = *eit; + if (e->interproc() && (e->type() == DIRECT || e->type() == COND_TAKEN)) { + if (visited.find(e->trg()) != visited.end()) { + // Find a tail call targeting a block within the same function + // So, this edge is not a tail call + e->_type._interproc = false; + } + } + } + } + + // is this the first time we've parsed this function? if (unlikely( !f->_extents.empty() )) { _parse_data->remove_extents(f->_extents); @@ -922,6 +943,7 @@ Parser::finalize() if(_parse_state < FINALIZED) { finalize_funcs(hint_funcs); finalize_funcs(discover_funcs); + clean_bogus_funcs(discover_funcs); finalize_ranges(hint_funcs); finalize_ranges(discover_funcs); _parse_state = FINALIZED; @@ -967,6 +989,34 @@ Parser::finalize_ranges(vector &funcs) } } +void +Parser::clean_bogus_funcs(vector &funcs) +{ + for (auto fit = funcs.begin(); fit != funcs.end(); ) { + Function *f = *fit; + if (f->src() == HINT) { + fit++; + continue; + } + bool interprocEdge = false; + for (auto eit = f->entry()->sources().begin(); !interprocEdge && eit != f->entry()->sources().end(); ++eit) + if ((*eit)->interproc()) interprocEdge = true; + if (!interprocEdge) { + // This is a discovered function that has no inter-procedural entry edge. + // This function should be created because tail call heuristic makes a mistake + // We have already fixed such bogos tail calls in the previous step of finalizing, + // so now we should remove such bogus function + if (sorted_funcs.end() != sorted_funcs.find(f)) { + sorted_funcs.erase(f); + } + fit = funcs.erase(fit); + _parse_data->remove_func(f); + } else { + fit++; + } + } +} + void Parser::record_func(Function *f) { diff --git a/parseAPI/src/Parser.h b/parseAPI/src/Parser.h index c311a826ab..139784eb59 100644 --- a/parseAPI/src/Parser.h +++ b/parseAPI/src/Parser.h @@ -252,6 +252,7 @@ namespace Dyninst { void finalize(); void finalize_funcs(vector &funcs); + void clean_bogus_funcs(vector &funcs); void finalize_ranges(vector &funcs);