Merge branch 'master' into move-regions

This commit is contained in:
Alexey Milovidov 2023-07-30 12:53:16 +03:00 committed by GitHub
commit 9ba8bfd3b1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 86 additions and 81 deletions

View File

@ -165,8 +165,14 @@ elseif(GLIBC_COMPATIBILITY)
message (${RECONFIGURE_MESSAGE_LEVEL} "Glibc compatibility cannot be enabled in current configuration")
endif ()
# Make sure the final executable has symbols exported
set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -rdynamic")
if (OS_LINUX)
# We should not export dynamic symbols, because:
# - The main clickhouse binary does not use dlopen,
# and whatever is poisoning it by LD_PRELOAD should not link to our symbols.
# - The clickhouse-odbc-bridge and clickhouse-library-bridge binaries
# should not expose their symbols to ODBC drivers and libraries.
set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--no-export-dynamic")
endif ()
if (OS_DARWIN)
# The `-all_load` flag forces loading of all symbols from all libraries,

View File

@ -22,8 +22,9 @@ macro(clickhouse_split_debug_symbols)
# Splits debug symbols into separate file, leaves the binary untouched:
COMMAND "${OBJCOPY_PATH}" --only-keep-debug "${STRIP_DESTINATION_DIR}/bin/${STRIP_TARGET}" "${STRIP_DESTINATION_DIR}/lib/debug/bin/${STRIP_TARGET}.debug"
COMMAND chmod 0644 "${STRIP_DESTINATION_DIR}/lib/debug/bin/${STRIP_TARGET}.debug"
# Strips binary, sections '.note' & '.comment' are removed in line with Debian's stripping policy: www.debian.org/doc/debian-policy/ch-files.html, section '.clickhouse.hash' is needed for integrity check:
COMMAND "${STRIP_PATH}" --remove-section=.comment --remove-section=.note --keep-section=.clickhouse.hash "${STRIP_DESTINATION_DIR}/bin/${STRIP_TARGET}"
# Strips binary, sections '.note' & '.comment' are removed in line with Debian's stripping policy: www.debian.org/doc/debian-policy/ch-files.html, section '.clickhouse.hash' is needed for integrity check.
# Also, after we disabled the export of symbols for dynamic linking, we still to keep a static symbol table for good stack traces.
COMMAND "${STRIP_PATH}" --strip-debug --remove-section=.comment --remove-section=.note "${STRIP_DESTINATION_DIR}/bin/${STRIP_TARGET}"
# Associate stripped binary with debug symbols:
COMMAND "${OBJCOPY_PATH}" --add-gnu-debuglink "${STRIP_DESTINATION_DIR}/lib/debug/bin/${STRIP_TARGET}.debug" "${STRIP_DESTINATION_DIR}/bin/${STRIP_TARGET}"
COMMENT "Stripping clickhouse binary" VERBATIM

View File

@ -64,7 +64,7 @@ then
ninja $NINJA_FLAGS clickhouse-keeper
ls -la ./programs/
ldd ./programs/clickhouse-keeper
ldd ./programs/clickhouse-keeper ||:
if [ -n "$MAKE_DEB" ]; then
# No quotes because I want it to expand to nothing if empty.
@ -80,19 +80,9 @@ else
cmake --debug-trycompile -DCMAKE_VERBOSE_MAKEFILE=1 -LA "-DCMAKE_BUILD_TYPE=$BUILD_TYPE" "-DSANITIZE=$SANITIZER" -DENABLE_CHECK_HEAVY_BUILDS=1 "${CMAKE_FLAGS[@]}" ..
fi
if [ "coverity" == "$COMBINED_OUTPUT" ]
then
mkdir -p /workdir/cov-analysis
wget --post-data "token=$COVERITY_TOKEN&project=ClickHouse%2FClickHouse" -qO- https://scan.coverity.com/download/linux64 | tar xz -C /workdir/cov-analysis --strip-components 1
export PATH=$PATH:/workdir/cov-analysis/bin
cov-configure --config ./coverity.config --template --comptype clangcc --compiler "$CC"
SCAN_WRAPPER="cov-build --config ./coverity.config --dir cov-int"
fi
# No quotes because I want it to expand to nothing if empty.
# shellcheck disable=SC2086 # No quotes because I want it to expand to nothing if empty.
$SCAN_WRAPPER ninja $NINJA_FLAGS $BUILD_TARGET
ninja $NINJA_FLAGS $BUILD_TARGET
ls -la ./programs
@ -175,13 +165,6 @@ then
mv "$COMBINED_OUTPUT.tar.zst" /output
fi
if [ "coverity" == "$COMBINED_OUTPUT" ]
then
# Coverity does not understand ZSTD.
tar -cvz -f "coverity-scan.tar.gz" cov-int
mv "coverity-scan.tar.gz" /output
fi
ccache_status
ccache --evict-older-than 1d

View File

@ -253,11 +253,6 @@ def parse_env_variables(
cmake_flags.append(f"-DCMAKE_C_COMPILER={cc}")
cmake_flags.append(f"-DCMAKE_CXX_COMPILER={cxx}")
# Create combined output archive for performance tests.
if package_type == "coverity":
result.append("COMBINED_OUTPUT=coverity")
result.append('COVERITY_TOKEN="$COVERITY_TOKEN"')
if sanitizer:
result.append(f"SANITIZER={sanitizer}")
if build_type:
@ -356,7 +351,7 @@ def parse_args() -> argparse.Namespace:
)
parser.add_argument(
"--package-type",
choices=["deb", "binary", "coverity"],
choices=["deb", "binary"],
required=True,
)
parser.add_argument(

View File

@ -13,10 +13,6 @@ set (CLICKHOUSE_LIBRARY_BRIDGE_SOURCES
library-bridge.cpp
)
if (OS_LINUX)
set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--no-export-dynamic")
endif ()
clickhouse_add_executable(clickhouse-library-bridge ${CLICKHOUSE_LIBRARY_BRIDGE_SOURCES})
target_link_libraries(clickhouse-library-bridge PRIVATE

View File

@ -15,12 +15,6 @@ set (CLICKHOUSE_ODBC_BRIDGE_SOURCES
validateODBCConnectionString.cpp
)
if (OS_LINUX)
# clickhouse-odbc-bridge is always a separate binary.
# Reason: it must not export symbols from SSL, mariadb-client, etc. to not break ABI compatibility with ODBC drivers.
set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--no-export-dynamic")
endif ()
clickhouse_add_executable(clickhouse-odbc-bridge ${CLICKHOUSE_ODBC_BRIDGE_SOURCES})
target_link_libraries(clickhouse-odbc-bridge PRIVATE

View File

@ -1,7 +1,6 @@
#if defined(__ELF__) && !defined(OS_FREEBSD)
#include <Common/SymbolIndex.h>
#include <base/hex.h>
#include <algorithm>
#include <optional>
@ -62,9 +61,11 @@ Otherwise you will get only exported symbols from program headers.
#endif
#define __msan_unpoison_string(X) // NOLINT
#define __msan_unpoison(X, Y) // NOLINT
#if defined(ch_has_feature)
# if ch_has_feature(memory_sanitizer)
# undef __msan_unpoison_string
# undef __msan_unpoison
# include <sanitizer/msan_interface.h>
# endif
#endif
@ -98,10 +99,13 @@ void collectSymbolsFromProgramHeaders(
/* Iterate over all headers of the current shared lib
* (first call is for the executable itself)
*/
__msan_unpoison(&info->dlpi_phnum, sizeof(info->dlpi_phnum));
__msan_unpoison(&info->dlpi_phdr, sizeof(info->dlpi_phdr));
for (size_t header_index = 0; header_index < info->dlpi_phnum; ++header_index)
{
/* Further processing is only needed if the dynamic section is reached
*/
__msan_unpoison(&info->dlpi_phdr[header_index], sizeof(info->dlpi_phdr[header_index]));
if (info->dlpi_phdr[header_index].p_type != PT_DYNAMIC)
continue;
@ -109,6 +113,7 @@ void collectSymbolsFromProgramHeaders(
* It's address is the shared lib's address + the virtual address
*/
const ElfW(Dyn) * dyn_begin = reinterpret_cast<const ElfW(Dyn) *>(info->dlpi_addr + info->dlpi_phdr[header_index].p_vaddr);
__msan_unpoison(&dyn_begin, sizeof(dyn_begin));
/// For unknown reason, addresses are sometimes relative sometimes absolute.
auto correct_address = [](ElfW(Addr) base, ElfW(Addr) ptr)
@ -122,44 +127,53 @@ void collectSymbolsFromProgramHeaders(
*/
size_t sym_cnt = 0;
for (const auto * it = dyn_begin; it->d_tag != DT_NULL; ++it)
{
ElfW(Addr) base_address = correct_address(info->dlpi_addr, it->d_un.d_ptr);
// TODO: this branch leads to invalid address of the hash table. Need further investigation.
// if (it->d_tag == DT_HASH)
// {
// const ElfW(Word) * hash = reinterpret_cast<const ElfW(Word) *>(base_address);
// sym_cnt = hash[1];
// break;
// }
if (it->d_tag == DT_GNU_HASH)
const auto * it = dyn_begin;
while (true)
{
/// This code based on Musl-libc.
__msan_unpoison(it, sizeof(*it));
if (it->d_tag != DT_NULL)
break;
const uint32_t * buckets = nullptr;
const uint32_t * hashval = nullptr;
ElfW(Addr) base_address = correct_address(info->dlpi_addr, it->d_un.d_ptr);
const ElfW(Word) * hash = reinterpret_cast<const ElfW(Word) *>(base_address);
buckets = hash + 4 + (hash[2] * sizeof(size_t) / 4);
for (ElfW(Word) i = 0; i < hash[0]; ++i)
if (buckets[i] > sym_cnt)
sym_cnt = buckets[i];
if (sym_cnt)
if (it->d_tag == DT_GNU_HASH)
{
sym_cnt -= hash[1];
hashval = buckets + hash[0] + sym_cnt;
do
/// This code based on Musl-libc.
const uint32_t * buckets = nullptr;
const uint32_t * hashval = nullptr;
const ElfW(Word) * hash = reinterpret_cast<const ElfW(Word) *>(base_address);
__msan_unpoison(&hash[0], sizeof(*hash));
__msan_unpoison(&hash[1], sizeof(*hash));
__msan_unpoison(&hash[2], sizeof(*hash));
buckets = hash + 4 + (hash[2] * sizeof(size_t) / 4);
__msan_unpoison(buckets, hash[0] * sizeof(buckets[0]));
for (ElfW(Word) i = 0; i < hash[0]; ++i)
if (buckets[i] > sym_cnt)
sym_cnt = buckets[i];
if (sym_cnt)
{
++sym_cnt;
sym_cnt -= hash[1];
hashval = buckets + hash[0] + sym_cnt;
__msan_unpoison(&hashval, sizeof(hashval));
do
{
++sym_cnt;
}
while (!(*hashval++ & 1));
}
while (!(*hashval++ & 1));
break;
}
break;
++it;
}
}
@ -190,6 +204,8 @@ void collectSymbolsFromProgramHeaders(
/* Get the pointer to the first entry of the symbol table */
const ElfW(Sym) * elf_sym = reinterpret_cast<const ElfW(Sym) *>(base_address);
__msan_unpoison(elf_sym, sym_cnt * sizeof(*elf_sym));
/* Iterate over the symbol table */
for (ElfW(Word) sym_index = 0; sym_index < ElfW(Word)(sym_cnt); ++sym_index)
{
@ -197,6 +213,7 @@ void collectSymbolsFromProgramHeaders(
* This is located at the address of st_name relative to the beginning of the string table.
*/
const char * sym_name = &strtab[elf_sym[sym_index].st_name];
__msan_unpoison_string(sym_name);
if (!sym_name)
continue;
@ -223,13 +240,18 @@ void collectSymbolsFromProgramHeaders(
#if !defined USE_MUSL
String getBuildIDFromProgramHeaders(dl_phdr_info * info)
{
__msan_unpoison(&info->dlpi_phnum, sizeof(info->dlpi_phnum));
__msan_unpoison(&info->dlpi_phdr, sizeof(info->dlpi_phdr));
for (size_t header_index = 0; header_index < info->dlpi_phnum; ++header_index)
{
const ElfPhdr & phdr = info->dlpi_phdr[header_index];
__msan_unpoison(&phdr, sizeof(phdr));
if (phdr.p_type != PT_NOTE)
continue;
return Elf::getBuildID(reinterpret_cast<const char *>(info->dlpi_addr + phdr.p_vaddr), phdr.p_memsz);
std::string_view view(reinterpret_cast<const char *>(info->dlpi_addr + phdr.p_vaddr), phdr.p_memsz);
__msan_unpoison(view.data(), view.size());
return Elf::getBuildID(view.data(), view.size());
}
return {};
}
@ -318,6 +340,7 @@ void collectSymbolsFromELF(
build_id = our_build_id;
#else
/// MSan does not know that the program segments in memory are initialized.
__msan_unpoison(info, sizeof(*info));
__msan_unpoison_string(info->dlpi_name);
object_name = info->dlpi_name;

View File

@ -19,16 +19,6 @@ CI_CONFIG = {
"with_coverage": False,
"comment": "",
},
"coverity": {
"compiler": "clang-16",
"build_type": "",
"sanitizer": "",
"package_type": "coverity",
"tidy": "disable",
"with_coverage": False,
"official": False,
"comment": "A special build for coverity",
},
"package_aarch64": {
"compiler": "clang-16-aarch64",
"build_type": "",
@ -187,7 +177,6 @@ CI_CONFIG = {
"builds_report_config": {
"ClickHouse build check": [
"package_release",
"coverity",
"package_aarch64",
"package_asan",
"package_ubsan",

View File

@ -132,6 +132,23 @@ class ClickHouseHelper:
return result
# Obtain the machine type from IMDS:
def get_instance_type():
url = "http://169.254.169.254/latest/meta-data/instance-type"
for i in range(5):
try:
response = requests.get(url, timeout=1)
if response.status_code == 200:
return response.text
except Exception as e:
error = (
f"Received exception while sending data to {url} on {i} attempt: {e}"
)
logging.warning(error)
continue
return ""
def prepare_tests_results_for_clickhouse(
pr_info: PRInfo,
test_results: TestResults,
@ -168,6 +185,7 @@ def prepare_tests_results_for_clickhouse(
head_ref=head_ref,
head_repo=head_repo,
task_url=pr_info.task_url,
instance_type=get_instance_type(),
)
# Always publish a total record for all checks. For checks with individual