Skip to content
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

Different strerror_r function usage #231

Closed
Esonhugh opened this issue Dec 14, 2023 · 7 comments
Closed

Different strerror_r function usage #231

Esonhugh opened this issue Dec 14, 2023 · 7 comments

Comments

@Esonhugh
Copy link
Contributor

Esonhugh commented Dec 14, 2023

When i was building Havoc Framework which imports toml11 on my m1 mac, I got error like

error stack

In file included from /tmp/Havoc/client/src/Havoc/PythonApi/UI/PyLoggerClass.cc:6:
In file included from /tmp/Havoc/client/include/Havoc/PythonApi/PythonApi.h:4:
In file included from /tmp/Havoc/client/include/global.hpp:36:
In file included from /tmp/Havoc/client/include/External.h:6:
In file included from /tmp/Havoc/client/external/toml/toml.hpp:32:
In file included from /tmp/Havoc/client/external/toml/toml/parser.hpp:14:
In file included from /tmp/Havoc/client/external/toml/toml/value.hpp:8:
/tmp/Havoc/client/external/toml/toml/exception.hpp:44:17: error: cannot initialize a variable of type 'const char *' with an rvalue of type 'int'
    const char* result = strerror_r(errnum, buf.data(), bufsize);
                ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning and 1 error generated.

Then I found this project and found different process here.

# ./toml/exception.hpp
inline std::string str_error(int errnum)
{
    // C++ standard strerror is not thread-safe.
    // C11 provides thread-safe version of this function, `strerror_s`, but it
    // is not available in C++.
    // To avoid using std::strerror, we need to use platform-specific functions.
    // If none of the conditions are met, it calls std::strerror as a fallback.
#ifdef _MSC_VER // MSVC
    constexpr std::size_t bufsize = 256;
    std::array<char, bufsize> buf;
    buf.fill('\0');
    const auto result = strerror_s(buf.data(), bufsize, errnum); # here use auto 
    if(result != 0)
    {
        return std::string("strerror_s failed");
    }
    else
    {
        return std::string(buf.data());
    }
#elif defined(_GNU_SOURCE)
    constexpr std::size_t bufsize = 256;
    std::array<char, bufsize> buf;
    buf.fill('\0');
    const char* result = strerror_r(errnum, buf.data(), bufsize); # here receive as const char*
    return std::string(result);
#elif (defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE >= 200112L) || (defined(_XOPEN_SOURCE) && _XOPEN_SOURCE >= 600)
    constexpr std::size_t bufsize = 256;
    std::array<char, bufsize> buf;
    buf.fill('\0');
    const int result = strerror_r(errnum, buf.data(), bufsize); # but here is int
    if (result != 0)
    {
        return std::string("strerror_r failed");
    }
    else
    {
        return std::string(buf.data());
    }
#else // fallback
    return std::strerror(errnum);
#endif

but my macos c header looks like

# /Library/Developer/CommandLineTools/SDKs/MacOSX13.3.sdk/usr/include/string.h
/* Additional functionality provided by:
 * POSIX.1-2001
 */

#if __DARWIN_C_LEVEL >= 200112L
__BEGIN_DECLS
int	 strerror_r(int __errnum, char *__strerrbuf, size_t __buflen);
char	*strdup(const char *__s1);
void	*memccpy(void *__dst, const void *__src, int __c, size_t __n);
__END_DECLS
#endif /* __DARWIN_C_LEVEL >= 200112L */

current fix

I changed the code like following to patch a little but it's not good I think.


inline std::string str_error(int errnum) {
  // C++ standard strerror is not thread-safe.
  // C11 provides thread-safe version of this function, `strerror_s`, but it
  // is not available in C++.
  // To avoid using std::strerror, we need to use platform-specific functions.
  // If none of the conditions are met, it calls std::strerror as a fallback.
#ifdef _MSC_VER // MSVC
  constexpr std::size_t bufsize = 256;
  std::array<char, bufsize> buf;
  buf.fill('\0');
  const auto result = strerror_s(buf.data(), bufsize, errnum);
  if (result != 0) {
    return std::string("strerror_s failed");
  } else {
    return std::string(buf.data());
  }
#elif defined(_GNU_SOURCE)
  constexpr std::size_t bufsize = 256;
  std::array<char, bufsize> buf;
  buf.fill('\0');
  const int result = strerror_r(errnum, buf.data(), bufsize);
  if (result != 0) {
    return std::string("strerror_r failed");
  } else {
    return std::string(buf.data());
  }
#elif (defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE >= 200112L) ||              \
    (defined(_XOPEN_SOURCE) && _XOPEN_SOURCE >= 600)
  constexpr std::size_t bufsize = 256;
  std::array<char, bufsize> buf;
  buf.fill('\0');
  const int result = strerror_r(errnum, buf.data(), bufsize);
  if (result != 0) {
    return std::string("strerror_r failed");
  } else {
    return std::string(buf.data());
  }
#else // fallback
  return std::strerror(errnum);
#endif
}

It looks like to following the (defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE >= 200112L) || (defined(_XOPEN_SOURCE) && _XOPEN_SOURCE >= 600) condition and compile the code.

Is it a bug or need more classification in macro?

@Esonhugh
Copy link
Contributor Author

Havoc depends on [toml @ c32a20e](https://github.com/ToruNiina/toml11/tree/c32a20e1ee690d6e6bf6f37e6d603402d49b15f0)

@Esonhugh
Copy link
Contributor Author

Esonhugh commented Dec 14, 2023

I guess if change the order of (defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE >= 200112L) || (defined(_XOPEN_SOURCE) && _XOPEN_SOURCE >= 600) and defined(_GNU_SOURCE) may fix this problem.

@ToruNiina
Copy link
Owner

Thank you for finding the problem.
If the patch fixes the problem in your env (and also passes all other CIs), it is worth merging.
Would you be interested in creating a pull request to address this issue?

@Esonhugh
Copy link
Contributor Author

Esonhugh commented Dec 25, 2023

Thank you for finding the problem. If the patch fixes the problem in your env (and also passes all other CIs), it is worth merging. Would you be interested in creating a pull request to address this issue?

HELP NEEDED

I think this is a problem based on difference between arch and compilers. And that will be a little bit hard on testing whether fix works.

So I need more details about such Marco and flags. And then create a new fix in PR. What do u think about that?

@Esonhugh
Copy link
Contributor Author

Esonhugh commented Dec 25, 2023

Bad news is it works well when i clone project directly(not as submodule). It compiled and works well.
and I tried change order of defined and it died in Havoc as well.

@Esonhugh
Copy link
Contributor Author

#233

ToruNiina added a commit that referenced this issue Jan 4, 2024
@ToruNiina
Copy link
Owner

Fixed by #233.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants