You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2016/05/14 08:30:11 UTC
[5/5] incubator-impala git commit: Kudu: Exclude non-Kudu symbols
during stub client generation
Kudu: Exclude non-Kudu symbols during stub client generation
Previously boost related symbols (and others) would get defined in the
Kudu client stub with a non-functional implementation. If these
implementations were used at runtime they would crash Impala.
Change-Id: I54292095692ce38c255a8df48cf8f3f655d797b0
Reviewed-on: http://gerrit.cloudera.org:8080/2864
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Internal Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/cd193f06
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/cd193f06
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/cd193f06
Branch: refs/heads/master
Commit: cd193f063f02bf7921c2d0f5b1231f91deed6694
Parents: 11ea79c
Author: Casey Ching <ca...@cloudera.com>
Authored: Mon Apr 25 13:54:57 2016 -0700
Committer: Tim Armstrong <ta...@cloudera.com>
Committed: Sat May 14 01:30:01 2016 -0700
----------------------------------------------------------------------
bin/bootstrap_toolchain.py | 174 +++++++++++++++++++++-------------------
1 file changed, 91 insertions(+), 83 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/cd193f06/bin/bootstrap_toolchain.py
----------------------------------------------------------------------
diff --git a/bin/bootstrap_toolchain.py b/bin/bootstrap_toolchain.py
index 3009332..2589d1f 100755
--- a/bin/bootstrap_toolchain.py
+++ b/bin/bootstrap_toolchain.py
@@ -124,6 +124,80 @@ def bootstrap(packages):
write_version_file(toolchain_root, pkg_name, pkg_version, compiler,
get_platform_release_label())
+def check_output(cmd_args):
+ """Run the command and return the output. Raise an exception if the command returns
+ a non-zero return code. Similar to subprocess.check_output() which is only provided
+ in python 2.7.
+ """
+ process = subprocess.Popen(cmd_args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+ stdout, _ = process.communicate()
+ if process.wait() != 0:
+ raise Exception("Command with args '%s' failed with exit code %s:\n%s"
+ % (cmd_args, process.returncode, stdout))
+ return stdout
+
+def package_directory(toolchain_root, pkg_name, pkg_version):
+ dir_name = "{0}-{1}".format(pkg_name, pkg_version)
+ return os.path.join(toolchain_root, dir_name)
+
+def version_file_path(toolchain_root, pkg_name, pkg_version):
+ return os.path.join(package_directory(toolchain_root, pkg_name, pkg_version),
+ "toolchain_package_version.txt")
+
+def check_custom_toolchain(toolchain_root, packages):
+ missing = []
+ for p in packages:
+ pkg_name, pkg_version = unpack_name_and_version(p)
+ pkg_dir = package_directory(toolchain_root, pkg_name, pkg_version)
+ if not os.path.isdir(pkg_dir):
+ missing.append((p, pkg_dir))
+
+ if missing:
+ print("The following packages are not in their expected locations.")
+ for p, pkg_dir in missing:
+ print(" %s (expected directory %s to exist)" % (p, pkg_dir))
+ print("Pre-built toolchain archives not available for your platform.")
+ print("Clone and build native toolchain from source using this repository:")
+ print(" https://github.com/cloudera/native-toolchain")
+ raise Exception("Toolchain bootstrap failed: required packages were missing")
+
+def check_for_existing_package(toolchain_root, pkg_name, pkg_version, compiler):
+ """Return true if toolchain_root already contains the package with the correct
+ version and compiler.
+ """
+ version_file = version_file_path(toolchain_root, pkg_name, pkg_version)
+ if not os.path.exists(version_file):
+ return False
+
+ label = get_platform_release_label()
+ pkg_version_string = "{0}-{1}-{2}-{3}".format(pkg_name, pkg_version, compiler, label)
+ with open(version_file) as f:
+ return f.read().strip() == pkg_version_string
+
+def write_version_file(toolchain_root, pkg_name, pkg_version, compiler, label):
+ with open(version_file_path(toolchain_root, pkg_name, pkg_version), 'w') as f:
+ f.write("{0}-{1}-{2}-{3}".format(pkg_name, pkg_version, compiler, label))
+
+def remove_existing_package(toolchain_root, pkg_name, pkg_version):
+ dir_path = package_directory(toolchain_root, pkg_name, pkg_version)
+ if os.path.exists(dir_path):
+ print "Removing existing package directory {0}".format(dir_path)
+ shutil.rmtree(dir_path)
+
+def unpack_name_and_version(package):
+ """A package definition is either a string where the version is fetched from the
+ environment or a tuple where the package name and the package version are fully
+ specified.
+ """
+ if isinstance(package, basestring):
+ env_var = "IMPALA_{0}_VERSION".format(package).replace("-", "_").upper()
+ try:
+ return package, os.environ[env_var]
+ except KeyError:
+ raise Exception("Could not find version for {0} in environment var {1}".format(
+ package, env_var))
+ return package[0], package[1]
+
def build_kudu_stub(toolchain_root, kudu_version, compiler):
# When Kudu isn't supported, the CentOS 7 package will be downloaded and the client
# lib will be replaced with a stubbed client.
@@ -159,7 +233,7 @@ def build_kudu_stub(toolchain_root, kudu_version, compiler):
# Extract the symbols and write the stubbed client source. There is a special method
# kudu::client::GetShortVersionString() that is overridden so that the stub can be
# identified by the caller.
- get_short_version_symbol = "_ZN4kudu6client21GetShortVersionStringEv"
+ get_short_version_sig = "kudu::client::GetShortVersionString()"
nm_out = check_output([nm_path, "--defined-only", "-D", client_lib_path])
stub_build_dir = tempfile.mkdtemp()
stub_client_src_file = open(os.path.join(stub_build_dir, "kudu_client.cc"), "w")
@@ -178,22 +252,30 @@ std::string GetShortVersionString() { return kFakeKuduVersion; }
}}
""")
found_start_version_symbol = False
+ cpp_filt_path = os.path.join(binutils_dir, "bin", "c++filt")
for line in nm_out.splitlines():
- addr, sym_type, name = line.split(" ")
- if name in ["_init", "_fini"]:
+ addr, sym_type, mangled_name = line.split(" ")
+ # Skip special functions an anything that isn't a strong symbol. Any symbols that
+ # get passed this check must be related to Kudu. If a symbol unrelated to Kudu
+ # (ex: a boost symbol) gets defined in the stub, there's a chance the symbol could
+ # get used and crash Impala.
+ if mangled_name in ["_init", "_fini"] or sym_type not in "Tt":
continue
- if name == get_short_version_symbol:
+ demangled_name = check_output([cpp_filt_path, mangled_name]).strip()
+ assert "kudu" in demangled_name, \
+ "Symbol doesn't appear to be related to Kudu: " + demangled_name
+ if demangled_name == get_short_version_sig:
found_start_version_symbol = True
continue
- if sym_type.upper() in "TW":
- stub_client_src_file.write("""
+ stub_client_src_file.write("""
extern "C" void %s() {
KuduNotSupported();
}
-""" % name)
+""" % mangled_name)
+
if not found_start_version_symbol:
- raise Exception("Expected to find symbol " + get_short_version_symbol +
- " corresponding to kudu::client::GetShortVersionString() but it was not found.")
+ raise Exception("Expected to find symbol a corresponding to"
+ " %s but it was not found." % get_short_version_sig)
stub_client_src_file.flush()
# The soname is needed to avoid problem in packaging builds. Without the soname,
@@ -221,80 +303,6 @@ extern "C" void %s() {
finally:
shutil.rmtree(stub_build_dir)
-def check_output(cmd_args):
- """Run the command and return the output. Raise an exception if the command returns
- a non-zero return code. Similar to subprocess.check_output() which is only provided
- in python 2.7.
- """
- process = subprocess.Popen(cmd_args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
- stdout, _ = process.communicate()
- if process.wait() != 0:
- raise Exception("Command with args '%s' failed with exit code %s:\n%s"
- % (cmd_args, process.returncode, stdout))
- return stdout
-
-def package_directory(toolchain_root, pkg_name, pkg_version):
- dir_name = "{0}-{1}".format(pkg_name, pkg_version)
- return os.path.join(toolchain_root, dir_name)
-
-def version_file_path(toolchain_root, pkg_name, pkg_version):
- return os.path.join(package_directory(toolchain_root, pkg_name, pkg_version),
- "toolchain_package_version.txt")
-
-def check_custom_toolchain(toolchain_root, packages):
- missing = []
- for p in packages:
- pkg_name, pkg_version = unpack_name_and_version(p)
- pkg_dir = package_directory(toolchain_root, pkg_name, pkg_version)
- if not os.path.isdir(pkg_dir):
- missing.append((p, pkg_dir))
-
- if missing:
- print("The following packages are not in their expected locations.")
- for p, pkg_dir in missing:
- print(" %s (expected directory %s to exist)" % (p, pkg_dir))
- print("Pre-built toolchain archives not available for your platform.")
- print("Clone and build native toolchain from source using this repository:")
- print(" https://github.com/cloudera/native-toolchain")
- raise Exception("Toolchain bootstrap failed: required packages were missing")
-
-def check_for_existing_package(toolchain_root, pkg_name, pkg_version, compiler):
- """Return true if toolchain_root already contains the package with the correct
- version and compiler.
- """
- version_file = version_file_path(toolchain_root, pkg_name, pkg_version)
- if not os.path.exists(version_file):
- return False
-
- label = get_platform_release_label()
- pkg_version_string = "{0}-{1}-{2}-{3}".format(pkg_name, pkg_version, compiler, label)
- with open(version_file) as f:
- return f.read().strip() == pkg_version_string
-
-def write_version_file(toolchain_root, pkg_name, pkg_version, compiler, label):
- with open(version_file_path(toolchain_root, pkg_name, pkg_version), 'w') as f:
- f.write("{0}-{1}-{2}-{3}".format(pkg_name, pkg_version, compiler, label))
-
-def remove_existing_package(toolchain_root, pkg_name, pkg_version):
- dir_path = package_directory(toolchain_root, pkg_name, pkg_version)
- if os.path.exists(dir_path):
- print "Removing existing package directory {0}".format(dir_path)
- shutil.rmtree(dir_path)
-
-def unpack_name_and_version(package):
- """A package definition is either a string where the version is fetched from the
- environment or a tuple where the package name and the package version are fully
- specified.
- """
- if isinstance(package, basestring):
- env_var = "IMPALA_{0}_VERSION".format(package).replace("-", "_").upper()
- try:
- return package, os.environ[env_var]
- except KeyError:
- raise Exception("Could not find version for {0} in environment var {1}".format(
- package, env_var))
- return package[0], package[1]
-
if __name__ == "__main__":
packages = ["avro", "binutils", "boost", "breakpad", "bzip2", "gcc", "gflags", "glog",
"gperftools", "gtest", "kudu", "llvm", ("llvm", "3.8.0-asserts-p1"), "lz4",