Hello Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6326
to look at the new patch set (#3).
Change subject: IMPALA-4929: Safe concurrent access to IR function call graph
......................................................................
IMPALA-4929: Safe concurrent access to IR function call graph
Previously, LlvmCodeGen creates a map (fn_refs_map_) which
maps an IR function to its set of callees. That map was initialized
once at Impalad's start time and it is supposed to be read-only
afterwards.
However, the map was unintentionally modified by its user
LlvmCodeGen::MaterializeFunctionHelper() due to the use of
operator []. In particular, that operator may create a new
entry in the map if it doesn't exist already. This is possible
because the map initially only contains entries for functions
with at least one callee function. Using the operator [] with
functions with no callee function may modify the map. Since the
map is shared by all fragment instances, such unsafe concurrent
modification can cause missing or wrong callee functions to be
materialized, leading to failure to resolve symbols in LLVM.
This change fixes the problem above by:
1. Create an entry for all functions even if it has no callee.
In fact, LlvmCodeGen::LinkModule() assumes all functions
defined in main module will have entries in the map.
2. Introduce a new class CodegenCallGraph which encapsulates
the map described above. The map was established once at
initialization time and there is no interface to modify the
map afterwards, thus preventing any accidental modification
to the map at run time.
Change-Id: I1acd6bad80341121c8189d817e0fe62f2862f28a
---
M be/src/codegen/CMakeLists.txt
A be/src/codegen/codegen-callgraph.cc
A be/src/codegen/codegen-callgraph.h
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
5 files changed, 188 insertions(+), 81 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/6326/3
--
To view, visit http://gerrit.cloudera.org:8080/6326
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1acd6bad80341121c8189d817e0fe62f2862f28a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com>
