Fix HAVE_PTHREAD_CONDATTR_SETCLOCK detection on Android

[MichalStrehovsky]

This is my second attempt. The first attempt was #58701 that got rolled back because it broke gcc build (#58744) and I wasn't in a good enough mental state to investigate (and I didn't really need it anymore). I now had a look and the fix was simple enough.

---

Android puts pthread stuff into libc. Because this wasn't detected, we are using some iOS fallbacks in System.Native. I'm propagating how pthreads are detected in the CoreCLR PAL (that was fixed up for Android in the CoreCLR PAL a couple years ago).

Failure to properly detect it here was causing build breaks in CoreCLR on Android (that's how I found this).

[ghost]

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This is my second attempt. The first attempt was #58701 that got rolled back because it broke gcc build (#58744) and I wasn't in a good enough mental state to investigate (and I didn't really need it anymore). I now had a look and the fix was simple enough.

---

Android puts pthread stuff into libc. Because this wasn't detected, we are using some iOS fallbacks in System.Native. I'm propagating how pthreads are detected in the CoreCLR PAL (that was fixed up for Android in the CoreCLR PAL a couple years ago).

Failure to properly detect it here was causing build breaks in CoreCLR on Android (that's how I found this).

Author:	MichalStrehovsky
Assignees:	MichalStrehovsky
Labels:	area-System.Threading
Milestone:	-

[wfurt]

This change broke my build on macOS (found via binary search)

  -- Performing Test HAVE_THREAD_LOCAL - Success
  CMake Error at gc/unix/configure.cmake:98 (check_library_exists):
    check_library_exists Macro invoked with incorrect arguments for macro
    named: CHECK_LIBRARY_EXISTS
  Call Stack (most recent call first):
    gc/CMakeLists.txt:30 (include)


  CMake Error at gc/unix/configure.cmake:100 (check_library_exists):
    check_library_exists Macro invoked with incorrect arguments for macro
    named: CHECK_LIBRARY_EXISTS
  Call Stack (most recent call first):
    gc/CMakeLists.txt:30 (include)


  -- Looking for process_vm_readv
  -- Looking for process_vm_readv - not found
  -- Configuring incomplete, errors occurred!
  See also "/Users/furt/github/wfurt-runtime/artifacts/obj/coreclr/OSX.x64.Release/CMakeFiles/CMakeOutput.log".
  See also "/Users/furt/github/wfurt-runtime/artifacts/obj/coreclr/OSX.x64.Release/CMakeFiles/CMakeError.log".
  ~/github/wfurt-runtime/src/coreclr
  ~/github/wfurt-runtime/artifacts/obj/coreclr/OSX.x64.Release ~/github/wfurt-runtime/src/coreclr

cmake version 3.21.3

I'm not sure why CI builds.

[MichalStrehovsky]

This change broke my build on macOS (found via binary search)

That sounds like PTHREAD_LIBRARY is ending up being empty on your machine. I'm not sure how this change would cause that.

This change propagated what was already done in the GC's configuration:

runtime/src/coreclr/gc/unix/configure.cmake

Lines 89 to 100 in dff742a

	check_library_exists(pthread pthread_create "" HAVE_LIBPTHREAD)
	check_library_exists(c pthread_create "" HAVE_PTHREAD_IN_LIBC)
	if (HAVE_LIBPTHREAD)
	set(PTHREAD_LIBRARY pthread)
	elseif (HAVE_PTHREAD_IN_LIBC)
	set(PTHREAD_LIBRARY c)
	endif()
	check_library_exists(${PTHREAD_LIBRARY} pthread_condattr_setclock "" HAVE_PTHREAD_CONDATTR_SETCLOCK)
	check_library_exists(${PTHREAD_LIBRARY} pthread_setaffinity_np "" HAVE_PTHREAD_SETAFFINITY_NP)

And PAL's configuration:

runtime/src/coreclr/pal/src/configure.cmake

Lines 87 to 105 in 72d643d

	check_library_exists(pthread pthread_create "" HAVE_LIBPTHREAD)
	check_library_exists(c pthread_create "" HAVE_PTHREAD_IN_LIBC)
	if (HAVE_LIBPTHREAD)
	set(PTHREAD_LIBRARY pthread)
	elseif (HAVE_PTHREAD_IN_LIBC)
	set(PTHREAD_LIBRARY c)
	endif()
	check_library_exists(${PTHREAD_LIBRARY} pthread_suspend "" HAVE_PTHREAD_SUSPEND)
	check_library_exists(${PTHREAD_LIBRARY} pthread_suspend_np "" HAVE_PTHREAD_SUSPEND_NP)
	check_library_exists(${PTHREAD_LIBRARY} pthread_continue "" HAVE_PTHREAD_CONTINUE)
	check_library_exists(${PTHREAD_LIBRARY} pthread_continue_np "" HAVE_PTHREAD_CONTINUE_NP)
	check_library_exists(${PTHREAD_LIBRARY} pthread_resume_np "" HAVE_PTHREAD_RESUME_NP)
	check_library_exists(${PTHREAD_LIBRARY} pthread_attr_get_np "" HAVE_PTHREAD_ATTR_GET_NP)
	check_library_exists(${PTHREAD_LIBRARY} pthread_getattr_np "" HAVE_PTHREAD_GETATTR_NP)
	check_library_exists(${PTHREAD_LIBRARY} pthread_getcpuclockid "" HAVE_PTHREAD_GETCPUCLOCKID)
	check_library_exists(${PTHREAD_LIBRARY} pthread_sigqueue "" HAVE_PTHREAD_SIGQUEUE)
	check_library_exists(${PTHREAD_LIBRARY} pthread_getaffinity_np "" HAVE_PTHREAD_GETAFFINITY_NP)

If those are not failing on your box before this pull request, there might be something odd going on somewhere.

[wfurt]

I think it fails to detect the PTHREAD_LIBRARY because of include issue. It may be worth of add fallback error/break...?

/Users/furt/github/wfurt-ssl/artifacts/obj/coreclr/OSX.x64.Release/CMakeFiles/CMakeError.log:/usr/local/Cellar/cmake/3.21.3/share/cmake/Modules/CheckFunctionExists.c:7:3: error: declaration of built-in function 'pthread_create' requires inclusion of the header <pthread.h> [-Werror,-Wbuiltin-requires-header]

[MichalStrehovsky]

That's odd - the GC configuration script doesn't treat warnings as error. This is just a warning. Warnings as errors is a questionable choice done for src/libraries/native. set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Wno-error=builtin-requires-header") in this pull request covers that.

The all-warnings-as-errors is causing other odd effects such as #58746 (comment).

[wfurt]

For now, I just added set(HAVE_PTHREAD_IN_LIBC 1) and build is running. So it really seems just the detection around pthread_create

[wfurt]

Btw there were 3 places I had to touch. I'm wondering if the GC configuration script does not treat it as error but perhaps other places do...?

[wfurt]

I updated Xcode 11.4 -> 13.2 and everything seems to work again.

[MichalStrehovsky]

I updated Xcode 11.4 -> 13.2 and everything seems to work again.

Great to hear that! This was a mystery issue...