Skip to content

[libc++][test] Use LIBCPP_ASSERT in some system_category-related tests #78834

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

Merged
merged 3 commits into from
Jan 23, 2024

Conversation

cpplearner
Copy link
Contributor

@cpplearner cpplearner commented Jan 20, 2024

  • eq_error_code_error_code.pass.cpp: The comparison between error_code and error_category internally uses default_error_condition to map the error_code to its corresponding error_condition. When the error_code's category is system_category, what constitutes correspondence is unspecified.
  • message.pass.cpp assumes that the result of system_category().message(5) is equal to generic_category().message(5).
  • system_category.pass.cpp assumes that system_category().default_error_condition(5).value() == 5.

Although libc++'s system_category always behave as if the incoming value is a POSIX error code (and thus is very similar to generic_category), other implementations do something different on Windows.

@cpplearner cpplearner requested a review from a team as a code owner January 20, 2024 08:15
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 20, 2024

@llvm/pr-subscribers-libcxx

Author: S. B. Tam (cpplearner)

Changes
  • eq_error_code_error_code.pass.cpp: The comparison between error_code and error_category internally uses default_error_condition to map the error_code to its corresponding error_condition. When the error_code's category is system_category, [what constitutes correspondence is unspecified](http://eel.is/c++draft/syserr.errcat.objects#4.sentence-7).
  • message.pass.cpp assumes that the result of system_category().message(5) is equal to generic_category().message(5).
  • system_category.pass.cpp assumes that system_category().default_error_condition(5).value() == 5.

Although libc++'s system_category always behave as if the incoming value is a POSIX error code, other implementations do something different on Windows.


Full diff: https://github.com/llvm/llvm-project/pull/78834.diff

3 Files Affected:

  • (modified) libcxx/test/std/diagnostics/syserr/syserr.compare/eq_error_code_error_code.pass.cpp (+73-74)
  • (modified) libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.derived/message.pass.cpp (+11-12)
  • (modified) libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp (+32-33)
diff --git a/libcxx/test/std/diagnostics/syserr/syserr.compare/eq_error_code_error_code.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.compare/eq_error_code_error_code.pass.cpp
index c63bfcd955a69c..f1f49733280b1d 100644
--- a/libcxx/test/std/diagnostics/syserr/syserr.compare/eq_error_code_error_code.pass.cpp
+++ b/libcxx/test/std/diagnostics/syserr/syserr.compare/eq_error_code_error_code.pass.cpp
@@ -22,88 +22,87 @@
 
 #include "test_macros.h"
 
-int main(int, char**)
-{
-    std::error_code e_code1(5, std::generic_category());
-    std::error_code e_code2(5, std::system_category());
-    std::error_code e_code3(6, std::generic_category());
-    std::error_code e_code4(6, std::system_category());
-    std::error_condition e_condition1(5, std::generic_category());
-    std::error_condition e_condition2(5, std::system_category());
-    std::error_condition e_condition3(6, std::generic_category());
-    std::error_condition e_condition4(6, std::system_category());
+int main(int, char**) {
+  std::error_code e_code1(5, std::generic_category());
+  std::error_code e_code2(5, std::system_category());
+  std::error_code e_code3(6, std::generic_category());
+  std::error_code e_code4(6, std::system_category());
+  std::error_condition e_condition1(5, std::generic_category());
+  std::error_condition e_condition2(5, std::system_category());
+  std::error_condition e_condition3(6, std::generic_category());
+  std::error_condition e_condition4(6, std::system_category());
 
-    assert(e_code1 == e_code1);
-    assert(e_code1 != e_code2);
-    assert(e_code1 != e_code3);
-    assert(e_code1 != e_code4);
-    assert(e_code1 == e_condition1);
-    assert(e_code1 != e_condition2);
-    assert(e_code1 != e_condition3);
-    assert(e_code1 != e_condition4);
+  assert(e_code1 == e_code1);
+  assert(e_code1 != e_code2);
+  assert(e_code1 != e_code3);
+  assert(e_code1 != e_code4);
+  assert(e_code1 == e_condition1);
+  assert(e_code1 != e_condition2);
+  assert(e_code1 != e_condition3);
+  assert(e_code1 != e_condition4);
 
-    assert(e_code2 != e_code1);
-    assert(e_code2 == e_code2);
-    assert(e_code2 != e_code3);
-    assert(e_code2 != e_code4);
-    assert(e_code2 == e_condition1);  // ?
-    assert(e_code2 == e_condition2);
-    assert(e_code2 != e_condition3);
-    assert(e_code2 != e_condition4);
+  assert(e_code2 != e_code1);
+  assert(e_code2 == e_code2);
+  assert(e_code2 != e_code3);
+  assert(e_code2 != e_code4);
+  LIBCPP_ASSERT(e_code2 == e_condition1);
+  assert(e_code2 == e_condition2);
+  LIBCPP_ASSERT(e_code2 != e_condition3);
+  assert(e_code2 != e_condition4);
 
-    assert(e_code3 != e_code1);
-    assert(e_code3 != e_code2);
-    assert(e_code3 == e_code3);
-    assert(e_code3 != e_code4);
-    assert(e_code3 != e_condition1);
-    assert(e_code3 != e_condition2);
-    assert(e_code3 == e_condition3);
-    assert(e_code3 != e_condition4);
+  assert(e_code3 != e_code1);
+  assert(e_code3 != e_code2);
+  assert(e_code3 == e_code3);
+  assert(e_code3 != e_code4);
+  assert(e_code3 != e_condition1);
+  assert(e_code3 != e_condition2);
+  assert(e_code3 == e_condition3);
+  assert(e_code3 != e_condition4);
 
-    assert(e_code4 != e_code1);
-    assert(e_code4 != e_code2);
-    assert(e_code4 != e_code3);
-    assert(e_code4 == e_code4);
-    assert(e_code4 != e_condition1);
-    assert(e_code4 != e_condition2);
-    assert(e_code4 == e_condition3);  // ?
-    assert(e_code4 == e_condition4);
+  assert(e_code4 != e_code1);
+  assert(e_code4 != e_code2);
+  assert(e_code4 != e_code3);
+  assert(e_code4 == e_code4);
+  LIBCPP_ASSERT(e_code4 != e_condition1);
+  assert(e_code4 != e_condition2);
+  LIBCPP_ASSERT(e_code4 == e_condition3);
+  assert(e_code4 == e_condition4);
 
-    assert(e_condition1 == e_code1);
-    assert(e_condition1 == e_code2);  // ?
-    assert(e_condition1 != e_code3);
-    assert(e_condition1 != e_code4);
-    assert(e_condition1 == e_condition1);
-    assert(e_condition1 != e_condition2);
-    assert(e_condition1 != e_condition3);
-    assert(e_condition1 != e_condition4);
+  assert(e_condition1 == e_code1);
+  LIBCPP_ASSERT(e_condition1 == e_code2);
+  assert(e_condition1 != e_code3);
+  LIBCPP_ASSERT(e_condition1 != e_code4);
+  assert(e_condition1 == e_condition1);
+  assert(e_condition1 != e_condition2);
+  assert(e_condition1 != e_condition3);
+  assert(e_condition1 != e_condition4);
 
-    assert(e_condition2 != e_code1);
-    assert(e_condition2 == e_code2);
-    assert(e_condition2 != e_code3);
-    assert(e_condition2 != e_code4);
-    assert(e_condition2 != e_condition1);
-    assert(e_condition2 == e_condition2);
-    assert(e_condition2 != e_condition3);
-    assert(e_condition2 != e_condition4);
+  assert(e_condition2 != e_code1);
+  assert(e_condition2 == e_code2);
+  assert(e_condition2 != e_code3);
+  assert(e_condition2 != e_code4);
+  assert(e_condition2 != e_condition1);
+  assert(e_condition2 == e_condition2);
+  assert(e_condition2 != e_condition3);
+  assert(e_condition2 != e_condition4);
 
-    assert(e_condition3 != e_code1);
-    assert(e_condition3 != e_code2);
-    assert(e_condition3 == e_code3);
-    assert(e_condition3 == e_code4);  // ?
-    assert(e_condition3 != e_condition1);
-    assert(e_condition3 != e_condition2);
-    assert(e_condition3 == e_condition3);
-    assert(e_condition3 != e_condition4);
+  assert(e_condition3 != e_code1);
+  LIBCPP_ASSERT(e_condition3 != e_code2);
+  assert(e_condition3 == e_code3);
+  LIBCPP_ASSERT(e_condition3 == e_code4);
+  assert(e_condition3 != e_condition1);
+  assert(e_condition3 != e_condition2);
+  assert(e_condition3 == e_condition3);
+  assert(e_condition3 != e_condition4);
 
-    assert(e_condition4 != e_code1);
-    assert(e_condition4 != e_code2);
-    assert(e_condition4 != e_code3);
-    assert(e_condition4 == e_code4);
-    assert(e_condition4 != e_condition1);
-    assert(e_condition4 != e_condition2);
-    assert(e_condition4 != e_condition3);
-    assert(e_condition4 == e_condition4);
+  assert(e_condition4 != e_code1);
+  assert(e_condition4 != e_code2);
+  assert(e_condition4 != e_code3);
+  assert(e_condition4 == e_code4);
+  assert(e_condition4 != e_condition1);
+  assert(e_condition4 != e_condition2);
+  assert(e_condition4 != e_condition3);
+  assert(e_condition4 == e_condition4);
 
   return 0;
 }
diff --git a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.derived/message.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.derived/message.pass.cpp
index a899638ce169ae..9f7eb42bc78d97 100644
--- a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.derived/message.pass.cpp
+++ b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.derived/message.pass.cpp
@@ -20,18 +20,17 @@
 
 #include "test_macros.h"
 
-int main(int, char**)
-{
-    const std::error_category& e_cat1 = std::generic_category();
-    const std::error_category& e_cat2 = std::system_category();
-    std::string m1 = e_cat1.message(5);
-    std::string m2 = e_cat2.message(5);
-    std::string m3 = e_cat2.message(6);
-    assert(!m1.empty());
-    assert(!m2.empty());
-    assert(!m3.empty());
-    assert(m1 == m2);
-    assert(m1 != m3);
+int main(int, char**) {
+  const std::error_category& e_cat1 = std::generic_category();
+  const std::error_category& e_cat2 = std::system_category();
+  std::string m1                    = e_cat1.message(5);
+  std::string m2                    = e_cat2.message(5);
+  std::string m3                    = e_cat2.message(6);
+  assert(!m1.empty());
+  assert(!m2.empty());
+  assert(!m3.empty());
+  LIBCPP_ASSERT(m1 == m2);
+  assert(m1 != m3);
 
   return 0;
 }
diff --git a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
index 7c98a42b52010d..35107061bc366e 100644
--- a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
+++ b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
@@ -23,47 +23,46 @@
 
 // See https://llvm.org/D65667
 struct StaticInit {
-    const std::error_category* ec;
-    ~StaticInit() {
-        std::string str = ec->name();
-        assert(str == "system") ;
-    }
+  const std::error_category* ec;
+  ~StaticInit() {
+    std::string str = ec->name();
+    assert(str == "system");
+  }
 };
 static StaticInit foo;
 
-int main(int, char**)
-{
-    {
-        const std::error_category& e_cat1 = std::system_category();
-        std::error_condition e_cond = e_cat1.default_error_condition(5);
-        assert(e_cond.value() == 5);
-        assert(e_cond.category() == std::generic_category());
-        e_cond = e_cat1.default_error_condition(5000);
-        assert(e_cond.value() == 5000);
-        assert(e_cond.category() == std::system_category());
-    }
+int main(int, char**) {
+  {
+    const std::error_category& e_cat1 = std::system_category();
+    std::error_condition e_cond       = e_cat1.default_error_condition(5);
+    LIBCPP_ASSERT(e_cond.value() == 5);
+    LIBCPP_ASSERT(e_cond.category() == std::generic_category());
+    e_cond = e_cat1.default_error_condition(5000);
+    LIBCPP_ASSERT(e_cond.value() == 5000);
+    LIBCPP_ASSERT(e_cond.category() == std::system_category());
+  }
 
-    // Test the result of message(int cond) when given a bad error condition
-    {
-            errno = E2BIG; // something that message will never generate
-            const std::error_category& e_cat1 = std::system_category();
-            const std::string msg = e_cat1.message(-1);
-            // Exact message format varies by platform.
+  // Test the result of message(int cond) when given a bad error condition
+  {
+    errno                             = E2BIG; // something that message will never generate
+    const std::error_category& e_cat1 = std::system_category();
+    const std::string msg             = e_cat1.message(-1);
+    // Exact message format varies by platform.
 #if defined(_AIX)
-            LIBCPP_ASSERT(msg.rfind("Error -1 occurred", 0) == 0);
+    LIBCPP_ASSERT(msg.rfind("Error -1 occurred", 0) == 0);
 #elif defined(_NEWLIB_VERSION)
-            LIBCPP_ASSERT(msg.empty());
+    LIBCPP_ASSERT(msg.empty());
 #else
-            LIBCPP_ASSERT(msg.rfind("Unknown error", 0) == 0);
+    LIBCPP_ASSERT(msg.rfind("Unknown error", 0) == 0);
 #endif
-            assert(errno == E2BIG);
-    }
+    assert(errno == E2BIG);
+  }
 
-    {
-        foo.ec = &std::system_category();
-        std::string m = foo.ec->name();
-        assert(m == "system");
-    }
+  {
+    foo.ec        = &std::system_category();
+    std::string m = foo.ec->name();
+    assert(m == "system");
+  }
 
-    return 0;
+  return 0;
 }

@cpplearner cpplearner changed the title [libc++][test] Use LIBCPP_ASSERT in some tests involving system_category [libc++][test] Use LIBCPP_ASSERT in some system_category-related tests Jan 20, 2024
{
const std::error_category& e_cat1 = std::system_category();
std::error_condition e_cond = e_cat1.default_error_condition(5);
LIBCPP_ASSERT(e_cond.value() == 5);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this fascinating. Thank you for doing these checks. Does it seem like we should rework this test so that we check whether the result of

e_cat1.default_error_condition(e)

,where e corresponds to a POSIX errno value), compares equal to

error_condition(e, generic_category())

as required by the standard?

@ldionne ldionne added hardening Issues related to the hardening effort and removed hardening Issues related to the hardening effort labels Jan 22, 2024
@ldionne ldionne assigned ldionne and unassigned var-const Jan 22, 2024
@cpplearner cpplearner merged commit f1d3ebc into llvm:main Jan 23, 2024
@cpplearner cpplearner deleted the syserr branch January 29, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants