Page MenuHomePhabricator

[libc++] These throwing allocation functions shouldn't return null.
Needs ReviewPublic

Authored by DianQK on Sun, Mar 19, 5:48 AM.

Details

Reviewers
xbolva00
Mordante
rmaprath
Group Reviewers
Restricted Project
Summary

According to https://isocpp.org/wiki/faq/freestore-mgmt#new-never-returns-null and https://en.cppreference.com/w/cpp/memory/new/operator_new, a new operator that throws an exception should not return null.

This D20677 breaks that convention with -fno-exceptions.

I noticed the C++ standard doesn't mention such a convention under using -fno-exceptions. I'm new to the C++ standard library, so please let me know if I'm missing something.

But this patch breaks another convention.

  1. Called by the non-throwing non-array new-expressions. The standard library implementation calls the version (1) and returns a null pointer on failure instead of propagating the exception.

I am submitting another candidate. (Mark the void* operator new ( std::size_t count ) as noexcept?)

If there are no relevant standards, are these the possible ways?

  1. The current patch (Don't call the throwing function). This broken the standard.
  2. Mark the void* operator new ( std::size_t count ) as noexcept.
  3. Return the -1 in the throwing function? (Too tricky).
  4. Remove the throwing function declaration (Breaking many api).
  5. Revert D20677. Because the standard requires that exceptions be thrown.

Diff Detail

Unit TestsFailed

TimeTest
260 mslibcxx CI C++03 > llvm-libc++-shared-cfg-in.std/language_support/support_dynamic/new_delete/new_delete_array::new_array_nothrow_replace.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_array_nothrow_replace.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++03 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/test/std/language.support/support.dynamic/new.delete/new.delete.array/Output/new_array_nothrow_replace.pass.cpp.dir/t.tmp.exe
260 mslibcxx CI C++03 > llvm-libc++-shared-cfg-in.std/language_support/support_dynamic/new_delete/new_delete_single::new_nothrow_replace.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_nothrow_replace.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++03 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/59b15ba2fdf6-1/llvm-project/libcxx-ci/build/generic-cxx03/test/std/language.support/support.dynamic/new.delete/new.delete.single/Output/new_nothrow_replace.pass.cpp.dir/t.tmp.exe
260 mslibcxx CI C++11 > llvm-libc++-shared-cfg-in.std/language_support/support_dynamic/new_delete/new_delete_array::new_array_nothrow_replace.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_array_nothrow_replace.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++11 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/test/std/language.support/support.dynamic/new.delete/new.delete.array/Output/new_array_nothrow_replace.pass.cpp.dir/t.tmp.exe
280 mslibcxx CI C++11 > llvm-libc++-shared-cfg-in.std/language_support/support_dynamic/new_delete/new_delete_single::new_nothrow_replace.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_nothrow_replace.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++11 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/19cb8500e747-1/llvm-project/libcxx-ci/build/generic-cxx11/test/std/language.support/support.dynamic/new.delete/new.delete.single/Output/new_nothrow_replace.pass.cpp.dir/t.tmp.exe
250 mslibcxx CI C++2b > llvm-libc++-shared-cfg-in.std/language_support/support_dynamic/new_delete/new_delete_array::new_align_val_t_nothrow_replace.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/16eac77cfeaf-1/llvm-project/libcxx-ci/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t_nothrow_replace.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/16eac77cfeaf-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/16eac77cfeaf-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/16eac77cfeaf-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/16eac77cfeaf-1/llvm-project/libcxx-ci/build/generic-cxx2b/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/16eac77cfeaf-1/llvm-project/libcxx-ci/build/generic-cxx2b/lib -lc++ -pthread -o /home/libcxx-builder/.buildkite-agent/builds/16eac77cfeaf-1/llvm-project/libcxx-ci/build/generic-cxx2b/test/std/language.support/support.dynamic/new.delete/new.delete.array/Output/new_align_val_t_nothrow_replace.pass.cpp.dir/t.tmp.exe
View Full Test Results (16 Failed)

Event Timeline

DianQK created this revision.Sun, Mar 19, 5:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptSun, Mar 19, 5:48 AM
DianQK requested review of this revision.Sun, Mar 19, 5:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptSun, Mar 19, 5:48 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
This comment was removed by DianQK.
DianQK edited the summary of this revision. (Show Details)Sun, Mar 19, 5:55 AM
DianQK edited the summary of this revision. (Show Details)Sun, Mar 19, 7:17 AM
DianQK edited the summary of this revision. (Show Details)
DianQK edited the summary of this revision. (Show Details)Sun, Mar 19, 8:01 AM
DianQK edited the summary of this revision. (Show Details)

Thanks for the quick fix. I just saw on Discord @ldionne wants to look into this further.

libcxxabi/src/stdlib_new_delete.cpp
43

I really dislike std::__libcpp_unreachable since it just results in undefined behaviour. Either we should do nothing or assert(false);.