[libcxx-commits] [libcxx] [libc++][hardening] Classify assertions related to leaks and syscalls. (PR #77164)

Konstantin Varlamov via libcxx-commits libcxx-commits at lists.llvm.org
Fri Jan 5 17:07:08 PST 2024


================
@@ -668,7 +668,9 @@ path __read_symlink(const path& p, error_code* ec) {
   detail::SSizeT ret;
   if ((ret = detail::readlink(p.c_str(), buff.get(), size)) == -1)
     return err.report(capture_errno());
-  _LIBCPP_ASSERT_UNCATEGORIZED(ret > 0, "TODO");
+  // `ret` indicates the number of bytes written to the buffer, `0` means that the attempt to read the symlink produced
+  // an empty string.
+  _LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(ret > 0, "TODO");
----------------
var-const wrote:

Something I wanted to point out about the new `valid-api-call` category is that this is (so far, at least) the only category that also covers spurious failures. All other categories are (or are supposed to be) completely deterministic -- they are direct consequences of the user giving us invalid arguments. System calls, however, can (at least in theory) fail sporadically, with no fault on the user's part.

I'd like to make sure we think it's reasonable. Alternatively, we could:
- only leave checks that we only expect to fail due to bad user input that is hard to check. A failure to lock a mutex might be a good example -- it's most likely to happen if the user didn't lock the mutex in the first place (but we don't have an easy way to check for that, so we have to settle for checking the consequences). However, even this category still might cover spurious errors;
- separate the user-triggered failures and the spurious failures. For the latter, either create a new category or mark them `internal`. The downsides are a) arguably too fine a granularity; b) the separation between what is triggered by a user and what is spurious isn't completely deterministic.



https://github.com/llvm/llvm-project/pull/77164


More information about the libcxx-commits mailing list