From 56bc77324934bb0ee5817529679c58e6b2000737 Mon Sep 17 00:00:00 2001 From: Vitaliy Lyudvichenko Date: Mon, 12 Feb 2018 17:57:25 +0300 Subject: [PATCH] Fixed exception catching thrown from compiled .so files. [#CLICKHOUSE-3573] --- dbms/src/Interpreters/CMakeLists.txt | 7 ++-- dbms/src/Interpreters/Compiler.cpp | 16 ++++----- dbms/src/Interpreters/config_compile.h.in | 6 +++- dbms/src/Interpreters/tests/compiler_test.cpp | 35 ++++++++++++++++--- .../00568_compile_catch_throw.reference | 1 + .../0_stateless/00568_compile_catch_throw.sh | 6 ++++ 6 files changed, 53 insertions(+), 18 deletions(-) create mode 100644 dbms/tests/queries/0_stateless/00568_compile_catch_throw.reference create mode 100755 dbms/tests/queries/0_stateless/00568_compile_catch_throw.sh diff --git a/dbms/src/Interpreters/CMakeLists.txt b/dbms/src/Interpreters/CMakeLists.txt index 3dea20ff290..d558883a75e 100644 --- a/dbms/src/Interpreters/CMakeLists.txt +++ b/dbms/src/Interpreters/CMakeLists.txt @@ -10,8 +10,9 @@ set (INTERNAL_LINKER_EXECUTABLE "clickhouse-lld" CACHE STRING "") # Disabling asan reporting for these tools if (CMAKE_BUILD_TYPE_UC STREQUAL "ASAN") - set(INTERNAL_COMPILER_EXECUTABLE "env ASAN_OPTIONS=detect_leaks=0 ${INTERNAL_COMPILER_EXECUTABLE}") - set(INTERNAL_LINKER_EXECUTABLE "env ASAN_OPTIONS=detect_leaks=0 ${INTERNAL_LINKER_EXECUTABLE}") + set(INTERNAL_COMPILER_ENV "env ASAN_OPTIONS=detect_leaks=0" CACHE STRING "") +else () + set(INTERNAL_COMPILER_ENV "" CACHE STRING "") endif () set (INTERNAL_COMPILER_NO_WARNING OFF CACHE INTERNAL "") @@ -39,7 +40,7 @@ string (REPLACE ${ClickHouse_SOURCE_DIR} ${INTERNAL_COMPILER_HEADERS} INTERNAL_B string (REPLACE ${ClickHouse_SOURCE_DIR} ${INTERNAL_COMPILER_HEADERS} INTERNAL_Poco_Foundation_INCLUDE_DIR ${Poco_Foundation_INCLUDE_DIR}) string (REPLACE ${ClickHouse_SOURCE_DIR} ${INTERNAL_COMPILER_HEADERS} INTERNAL_Poco_Util_INCLUDE_DIR ${Poco_Util_INCLUDE_DIR}) -message (STATUS "Using internal compiler: headers=${INTERNAL_COMPILER_HEADERS} : ${INTERNAL_COMPILER_EXECUTABLE} ${INTERNAL_COMPILER_FLAGS}; ${INTERNAL_LINKER_EXECUTABLE}") +message (STATUS "Using internal compiler: headers=${INTERNAL_COMPILER_HEADERS} : ${INTERNAL_COMPILER_ENV} ${INTERNAL_COMPILER_EXECUTABLE} ${INTERNAL_COMPILER_FLAGS}; ${INTERNAL_LINKER_EXECUTABLE}") set (CONFIG_COMPILE ${ClickHouse_BINARY_DIR}/dbms/src/Interpreters/config_compile.h) configure_file (${ClickHouse_SOURCE_DIR}/dbms/src/Interpreters/config_compile.h.in ${CONFIG_COMPILE}) diff --git a/dbms/src/Interpreters/Compiler.cpp b/dbms/src/Interpreters/Compiler.cpp index 25b749f02b0..0012dba3add 100644 --- a/dbms/src/Interpreters/Compiler.cpp +++ b/dbms/src/Interpreters/Compiler.cpp @@ -203,7 +203,6 @@ void Compiler::compile( std::string prefix = path + "/" + file_name; std::string cpp_file_path = prefix + ".cpp"; - std::string o_file_path = prefix + ".o"; std::string so_file_path = prefix + ".so"; std::string so_tmp_file_path = prefix + ".so.tmp"; @@ -219,8 +218,12 @@ void Compiler::compile( /// Slightly unconvenient. command << "(" - INTERNAL_COMPILER_EXECUTABLE + INTERNAL_COMPILER_ENV + " " INTERNAL_COMPILER_EXECUTABLE " " INTERNAL_COMPILER_FLAGS + /// It is hard to correctly call a ld program manually, because it is easy to skip critical flags, which might lead to + /// unhandled exceptions. Therefore pass path to llvm's lld directly to clang. + " -fuse-ld=/usr/bin/" INTERNAL_LINKER_EXECUTABLE #if INTERNAL_COMPILER_CUSTOM_ROOT @@ -247,13 +250,7 @@ void Compiler::compile( " -I " INTERNAL_Boost_INCLUDE_DIRS " -I " INTERNAL_COMPILER_HEADERS "/libs/libcommon/include/" " " << additional_compiler_flags << - " -c -o " << o_file_path << " " << cpp_file_path - << " 2>&1" - ") && (" - INTERNAL_LINKER_EXECUTABLE - " -shared" - " -o " << so_tmp_file_path - << " " << o_file_path + " -shared -o " << so_tmp_file_path << " " << cpp_file_path << " 2>&1" ") || echo Return code: $?"; @@ -270,7 +267,6 @@ void Compiler::compile( /// If there was an error before, the file with the code remains for viewing. Poco::File(cpp_file_path).remove(); - Poco::File(o_file_path).remove(); Poco::File(so_tmp_file_path).renameTo(so_file_path); SharedLibraryPtr lib(new SharedLibrary(so_file_path)); diff --git a/dbms/src/Interpreters/config_compile.h.in b/dbms/src/Interpreters/config_compile.h.in index 19f91fd3449..8d3b0512ab8 100644 --- a/dbms/src/Interpreters/config_compile.h.in +++ b/dbms/src/Interpreters/config_compile.h.in @@ -6,8 +6,12 @@ #endif #cmakedefine PATH_SHARE "@PATH_SHARE@" #cmakedefine INTERNAL_COMPILER_FLAGS "@INTERNAL_COMPILER_FLAGS@" -#cmakedefine INTERNAL_COMPILER_EXECUTABLE "@INTERNAL_COMPILER_EXECUTABLE@" #cmakedefine INTERNAL_LINKER_EXECUTABLE "@INTERNAL_LINKER_EXECUTABLE@" +#cmakedefine INTERNAL_COMPILER_EXECUTABLE "@INTERNAL_COMPILER_EXECUTABLE@" +#cmakedefine INTERNAL_COMPILER_ENV "@INTERNAL_COMPILER_ENV@" +#ifndef INTERNAL_COMPILER_ENV +#define INTERNAL_COMPILER_ENV "" +#endif #cmakedefine INTERNAL_COMPILER_HEADERS "@INTERNAL_COMPILER_HEADERS@" #cmakedefine INTERNAL_COMPILER_HEADERS_ROOT "@INTERNAL_COMPILER_HEADERS_ROOT@" #cmakedefine01 INTERNAL_COMPILER_CUSTOM_ROOT diff --git a/dbms/src/Interpreters/tests/compiler_test.cpp b/dbms/src/Interpreters/tests/compiler_test.cpp index 9ead9b6aa19..f0fe667a04f 100644 --- a/dbms/src/Interpreters/tests/compiler_test.cpp +++ b/dbms/src/Interpreters/tests/compiler_test.cpp @@ -12,18 +12,45 @@ int main(int, char **) Logger::root().setChannel(channel); Logger::root().setLevel("trace"); + /// Check exception handling and catching try { Compiler compiler(".", 1); - auto lib = compiler.getOrCount("xxx", 1, "", []() -> std::string + auto lib = compiler.getOrCount("catch_me_if_you_can", 0, "", []() -> std::string { - return "void f() __attribute__((__visibility__(\"default\"))); void f() {}"; + return + "#include \n" + "void f() __attribute__((__visibility__(\"default\")));\n" + "void f()" + "{" + "try { throw std::runtime_error(\"Catch me if you can\"); }" + "catch (const std::runtime_error & e) { std::cout << \"Caught in .so: \" << e.what() << std::endl; throw; }\n" + "}" + ; }, [](SharedLibraryPtr&){}); + + auto f = lib->template get("_Z1fv"); + + try + { + f(); + } + catch (const std::exception & e) + { + std::cout << "Caught in main(): " << e.what() << "\n"; + return 0; + } + catch (...) + { + std::cout << "Unknown exception\n"; + return -1; + } } - catch (const DB::Exception & e) + catch (...) { - std::cerr << e.displayText() << std::endl; + std::cerr << getCurrentExceptionMessage(true) << "\n"; + return -1; } return 0; diff --git a/dbms/tests/queries/0_stateless/00568_compile_catch_throw.reference b/dbms/tests/queries/0_stateless/00568_compile_catch_throw.reference new file mode 100644 index 00000000000..d00491fd7e5 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00568_compile_catch_throw.reference @@ -0,0 +1 @@ +1 diff --git a/dbms/tests/queries/0_stateless/00568_compile_catch_throw.sh b/dbms/tests/queries/0_stateless/00568_compile_catch_throw.sh new file mode 100755 index 00000000000..120b11478f4 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00568_compile_catch_throw.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash + +if $CLICKHOUSE_CLIENT -q "select length(groupArray(number)) FROM (SELECT * FROM system.numbers LIMIT 1000000)" --compile=1 --min_count_to_compile=0 --max_threads=1 --max_memory_usage=8000000 2>/dev/null ; then + echo 'There is no expected exception "Memory limit (for query) exceeded: would use..."' +fi +$CLICKHOUSE_CLIENT -q "SELECT 1"