-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[YCQL] Uncaught exception when handling wrong CQL timestamp format #13064
Comments
This is an LTO specific issue. Also it seems to be specific to Boost exceptions. If I throw an Getting this stack trace with lldb:
Built yugabyte-db-thirdparty with this patch, off of the commit yugabyte/yugabyte-db-thirdparty@52b25a9:
Then built yugabyte-db off of commit f27f2a3 like so:
Created a script #!/usr/bin/env bash
. "${BASH_SOURCE%/*}/common-build-env.sh"
activate_virtualenv
set_pythonpath
if [[ $( uname -m ) == "x86_64" ]]; then
build_root_basename=release-clang13-linuxbrew-full-lto-ninja
else
build_root_basename=release-clang12-full-lto-ninja
fi
set -x
"$YB_SRC_ROOT/python/yb/dependency_graph.py" \
--build-root "$YB_SRC_ROOT/build/${build_root_basename}" \
--file-regex "^.*/tostring-test$" \
link-whole-program \
"$@" Modified a test:
The test case that throws an exception in the same test works fine:
(there is some additional logging added to cxa_persionality.cc, part of libc++abi). However, the test that attempts to catch an exception thrown from Boost, crashes:
|
Potentially relevant issues: |
A test that calls
Crashes with this stack trace:
The debug logging lines were produced by the following code added to
|
Also a useful document on exception handling: https://itanium-cxx-abi.github.io/cxx-abi/exceptions.pdf |
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
Here are some details on the workaround:
if (native_exception && actions == _UA_CLEANUP_PHASE) {
// This might happen for LTO binaries. Assume there is no handler and no cleanup.
results.reason = _URC_CONTINUE_UNWIND;
return;
} It remains to be verified that all the destructors are being called correctly. |
It does appear that the destructor gets called, added to Test (with the exception being thrown inside libc++ -- this bug does not affect exceptions being thrown from the same compilation unit for some reason):
Output: https://gist.githubusercontent.com/mbautin/a177b3ac12817f9ac72b541ffc2d93af/raw (notice the "MyClass destructor called" line). |
Reopening until the updated third-party dependencies are pulled into master. |
Third-party dependencies pulled into master with f2702bb |
…crash when enabling SSL Summary: This diff fixes a crash when enable SSL for the webserver, due to an old version of Squeasel: 1. searching for and loading OpenSSL from the system rather than using the one bundled with thirdparty. 2. depending on the dynamic library exposing symbols that have been removed with OpenSSL 1.1.0 (SSLv23_... methods, which have been replaced with a macro). This is fixed by rebasing to a newer upstream commit, which does not require those symbols, and which further presents the ssl_global_init flag to stop Squeasel from loading OpenSSL on its own. This diff also adds the `webserver_private_key_file` and `webserver_private_key_password` gflags, corresponding to the `ssl_private_key` and `ssl_private_key_password` options present in newer versions of Squeasel. This diff also fixes a crash when pthread_setspecific fails in libcds, by catching the exception and forcing the worker thread to exit instead of letting it bubble up and kill the whole process. Also pulling yugabyte/yugabyte-db-thirdparty@fc8dae1 to fix C++ exception handling in Clang-based LTO builds (yugabyte#13064). Test Plan: `ybd --cxx_test server_webserver-test --gtest_filter WebserverSecureTest.TestIndexPage` Reviewers: mbautin Reviewed By: mbautin Subscribers: cwang, sanketh, ybase, bogdan Differential Revision: https://phabricator.dev.yugabyte.com/D18419
…ugabyte#127) The bug fix in libc++abi is needed to fix yugabyte/yugabyte-db#13064. Picking up these fixes in our LLVM branches: - yugabyte/llvm-project@4047555 (branch: 14.0.6-yb, tag: https://github.com/yugabyte/llvm-project/releases/tag/llvmorg-14.0.6-yb-1) - yugabyte/llvm-project@0131d8e (branch: 13.0.1-yb, tag: https://github.com/yugabyte/llvm-project/releases/tag/llvmorg-13.0.1-yb-2) - yugabyte/llvm-project@2276446 (branch: 12.0.1-yb, tag: https://github.com/yugabyte/llvm-project/releases/tag/llvmorg-12.0.1-yb-2) Other changes: - Clean up Snyk integration to be more in line with the style of the rest of this codebase. - Remove Clang 12 LTO builds, because we now only use Clang 13 or later with LTO. - Replace the --single-compiler-type flag with --compiler-family (there is always a single compiler family, gcc or clang, in use for any given build.) - Support building multiple build types in the same source directory. We now create subdirectories under build/ and installed/, named e.g. clang13-full-lto-x86_64, gcc11-x86_64., etc. This is disabled by default and can be enabled using the --use-per-build-subdirs flags. We will need changes to yugabyte-db's build system to understand this new directory structure.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
…inary. See yugabyte/yugabyte-db#13064 for details.
New issue: #22191 |
We were previously building Boost with C++14, and potentially also building other dependencies with C++ standard versions other than C++20, which is currently used across the main YugabyteDB codebase. This is a suspected root cause of failures to catch exceptions thrown from Boost. Relevant issues: yugabyte/yugabyte-db#22191 and yugabyte/yugabyte-db#13064. For the older issue yugabyte/yugabyte-db#13064 a custom workaround ( e.g. see the commit yugabyte/llvm-project@8efd900 ) was implemented in libc++abi, which will be unnecessary with this fix. Also removing most of the Clang 16 builds (only keeping the one on Ubuntu 20.04).
…when parsing an invalid timestamp Summary: Fixing an issue with failing to catch an exception thrown from inside of the Boost time zone parsing code. This only appears in an LTO build. - Updating third-party dependencies to make sure all C++ dependencies are all built with C++20. Previously Boost was built with C++14. This alone was insufficient for fixing the bug, but is a good thing to do anyway. - Avoid parsing a VERIFY_RESULT expression to a function that might throw an exception. Instead, compute VERIFY_RESULT and store its return value in a local variable, and then pass that varaible to the make_shared call that might thrown an exception from its template argument class's constructor. This diff also includes CMakeLists.txt build infrastructure changes to allow easily building a C++ unit test as a separate LTO executable. Another related issue (#13064) -- an older occurrence of a similar error that was fixed at the time. Jira: DB-11115 Test Plan: Jenkins Details of unit tests: - New test TestSelect.testInvalidTimestampBadLexicalCast provided by @OlegLoginov (D34768) - In date_time-test.cc there are two new tests, DateTimeTest.InvalidTimestampFromString and DateTimeTest.InvalidBoostTimeZoneFromString. DateTimeTest.InvalidTimestampFromString most closely matches the production issue. To reproduce the failure, date_time-test has to be built with LTO by uncommenting the `yb_add_lto_target(date_time-test date_time-test-lto "")` line in CMakeLists.txt. Manual test (with the LTO build) -- provided by @OlegLoginov as well: ``` ycqlsh> create keyspace n; ycqlsh> create table n.b(c1 int, c2 timestamp, primary key(c1)); ycqlsh> select * from n.b where c2 >= '2021-10-10 10:00:00 UTC_b'; -- Incorrect result: NoHostAvailable -- Correct result: InvalidRequest: Error from server: code=2200 [Invalid query] message="Invalid Arguments. Invalid timestamp: bad lexical cast: source type value could not be interpreted as target select * from n.b where c2 >= '2021-10-10 10:00:00 UTC_b'; ^^^^^^ (ql error -304)" ``` Reviewers: loginov, mihnea Reviewed By: loginov Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D34970
…when parsing an invalid timestamp Summary: Fixing an issue with failing to catch an exception thrown from inside of the Boost time zone parsing code. This only appears in an LTO build. - Updating third-party dependencies to make sure all C++ dependencies are all built with C++20. Previously Boost was built with C++14. This alone was insufficient for fixing the bug, but is a good thing to do anyway. - Avoid parsing a VERIFY_RESULT expression to a function that might throw an exception. Instead, compute VERIFY_RESULT and store its return value in a local variable, and then pass that varaible to the make_shared call that might thrown an exception from its template argument class's constructor. This diff also includes CMakeLists.txt build infrastructure changes to allow easily building a C++ unit test as a separate LTO executable. Another related issue (#13064) -- an older occurrence of a similar error that was fixed at the time. Jira: DB-11115 Test Plan: Jenkins Details of unit tests: - New test TestSelect.testInvalidTimestampBadLexicalCast provided by @OlegLoginov (D34768) - In date_time-test.cc there are two new tests, DateTimeTest.InvalidTimestampFromString and DateTimeTest.InvalidBoostTimeZoneFromString. DateTimeTest.InvalidTimestampFromString most closely matches the production issue. To reproduce the failure, date_time-test has to be built with LTO by uncommenting the `yb_add_lto_target(date_time-test date_time-test-lto "")` line in CMakeLists.txt. Manual test (with the LTO build) -- provided by @OlegLoginov as well: ``` ycqlsh> create keyspace n; ycqlsh> create table n.b(c1 int, c2 timestamp, primary key(c1)); ycqlsh> select * from n.b where c2 >= '2021-10-10 10:00:00 UTC_b'; -- Incorrect result: NoHostAvailable -- Correct result: InvalidRequest: Error from server: code=2200 [Invalid query] message="Invalid Arguments. Invalid timestamp: bad lexical cast: source type value could not be interpreted as target select * from n.b where c2 >= '2021-10-10 10:00:00 UTC_b'; ^^^^^^ (ql error -304)" ``` Reviewers: loginov, mihnea Reviewed By: loginov Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D34970
Jira Link: DB-2787
Description
The text was updated successfully, but these errors were encountered: