-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[libcxx] Initialize vcruntime __std_exception_data in the exception copy ctor #144329
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
Conversation
@llvm/pr-subscribers-libcxx Author: Martin Storsjö (mstorsjo) ChangesThis fixes failures in a number of tests, in the Previously, with optimization enabled, the compiler concluded that these fields would be uninitialized at the points of asserts in the tests. This fixes the following tests in this configuration:
Full diff: https://github.com/llvm/llvm-project/pull/144329.diff 1 Files Affected:
diff --git a/libcxx/include/__exception/exception.h b/libcxx/include/__exception/exception.h
index f7dab6e83ad14..161cc49979e4a 100644
--- a/libcxx/include/__exception/exception.h
+++ b/libcxx/include/__exception/exception.h
@@ -48,7 +48,7 @@ class exception { // base of all library exceptions
__data_._DoFree = true;
}
- exception(exception const&) _NOEXCEPT {}
+ exception(exception const&) _NOEXCEPT : __data_() {}
exception& operator=(exception const&) _NOEXCEPT { return *this; }
|
10913df
to
38f3ff4
Compare
Ping; this one should be super trivial. We provide a dummy no-op implementation of |
Ping |
@@ -48,7 +48,7 @@ class exception { // base of all library exceptions | |||
__data_._DoFree = true; | |||
} | |||
|
|||
exception(exception const&) _NOEXCEPT {} | |||
exception(exception const&) _NOEXCEPT : __data_() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we don't do a proper copy? What happens if two TUs are linked where we provide our own implementation while the other is provided by vcruntime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we don't do a proper copy?
To properly do a copy, the real vcruntime here calls the __std_exception_copy
helper function (to do a proper deep copy of the contained data structure). This dummy implementation here, used when compiling in the no-vcruntime
setup, is intentionally a no-op - which is usually fine. However here the too-much-no-op case makes the compiler do incorrect conclusions when compiling with optimizations enabled.
What happens if two TUs are linked where we provide our own implementation while the other is provided by vcruntime?
Not sure it this dummy implementation is being mangled differently to avoid collisions, or if that's a latent bug for anybody doing mixed bugs. Clearly a potential issue - but mostly orthogonal to this patch.
…opy ctor This fixes failures in a number of tests, in the clang-cl-no-vcruntime configuration (where libcxx provides dummy, no-op replacements of some vcruntime base exception classes), if building with optimization enabled. Previously, with optimization enabled, the compiler concluded that these fields would be uninitialized at the points of asserts in the tests. This fixes the following tests in this configuration: llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.dynamic/alloc.errors/bad.alloc/bad_alloc.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.dynamic/alloc.errors/new.badlength/bad_array_new_length.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.exception/bad.exception/bad_exception.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.exception/exception/exception.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.rtti/bad.cast/bad_cast.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.rtti/bad.typeid/bad_typeid.pass.cpp
38f3ff4
to
2f83bba
Compare
…opy ctor (llvm#144329) This fixes failures in a number of tests, in the clang-cl-no-vcruntime configuration (where libcxx provides dummy, no-op replacements of some vcruntime base exception classes), if building with optimization enabled. Previously, with optimization enabled, the compiler concluded that these fields would be uninitialized at the points of asserts in the tests. This fixes the following tests in this configuration: llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.dynamic/alloc.errors/bad.alloc/bad_alloc.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.dynamic/alloc.errors/new.badlength/bad_array_new_length.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.exception/bad.exception/bad_exception.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.exception/exception/exception.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.rtti/bad.cast/bad_cast.pass.cpp llvm-libc++-shared-no-vcruntime-clangcl.cfg.in :: std/language.support/support.rtti/bad.typeid/bad_typeid.pass.cpp
This fixes failures in a number of tests, in the
clang-cl-no-vcruntime configuration (where libcxx provides dummy, no-op replacements of some vcruntime base exception classes), if building with optimization enabled.
Previously, with optimization enabled, the compiler concluded that these fields would be uninitialized at the points of asserts in the tests.
This fixes the following tests in this configuration: