This is an archive of the discontinued LLVM Phabricator instance.

Differential D92057

[libc++] Do not include sysctl.h when building with musl libc
AbandonedPublic

Authored by ldionne on Nov 24 2020, 1:48 PM.

Download Raw Diff

Details

Reviewers

bcain
pzheng

Group Reviewers

Restricted Project

Summary

The header file sysctl.h uses BEGIN_DECLS/END_DECLS which are not defined by musl.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

pzheng created this revision.Nov 24 2020, 1:48 PM

Herald added a project: Restricted Project. · View Herald TranscriptNov 24 2020, 1:48 PM

Herald added a reviewer: Restricted Project. · View Herald Transcript

Herald added a subscriber: libcxx-commits. · View Herald Transcript

pzheng requested review of this revision.Nov 24 2020, 1:48 PM

Harbormaster completed remote builds in B80011: Diff 307443.Nov 24 2020, 8:08 PM

The header file sysctl.h uses BEGIN_DECLS/END_DECLS which are not defined by musl.

I don't understand. What's the issue?

This revision now requires changes to proceed.Nov 25 2020, 12:06 PM

The issue here is that the header file "sysctl.h" uses some macros (BEGIN_DECLS/END_DECLS) which are only defined by glibc and not by musl libc (refer to https://wiki.musl-libc.org/faq.html). Therefore, when we build with musl libc, we are seeing the following build failures.

usr/include/sys/sysctl.h:65:1: error: unknown type name '__BEGIN_DECLS'
__BEGIN_DECLS
^

This patch tries to guard the include of sysctl.h with !defined(_LIBCPP_HAS_MUSL_LIBC) so that the header file is not included when musl libc is used. We started to see the libcxx build failure above after 477a687 landed. 477a687 removed some #if guards which seem to have exposed the issue. Before 477a687, those guards actually prevented sysctl.h from being included in our build. However, I think the root cause is not 477a687, but the unconditional include of sysctl.h. I hope this gives a little bit more background on what this patch is trying to fix.

Can you please let me know if https://reviews.llvm.org/D92135 solves your issue? I believe https://reviews.llvm.org/D92135 is superior, since it also simplifies our implementation to use POSIX functionality.

Thanks, @ldionne! D92135 does also fix the issue.

Great!

Herald added a subscriber: jkorous. · View Herald TranscriptNov 26 2020, 9:16 AM

Resolved by D92135 instead.

Revision Contents

Path

Size

libcxx/

src/

thread.cpp

2 lines

Diff 307443

libcxx/src/thread.cpp

	Show All 17 Lines
	#if __has_include(<sys/types.h>)			#if __has_include(<sys/types.h>)
	# include <sys/types.h>			# include <sys/types.h>
	#endif			#endif

	#if __has_include(<sys/param.h>)			#if __has_include(<sys/param.h>)
	# include <sys/param.h>			# include <sys/param.h>
	#endif			#endif

	#if __has_include(<sys/sysctl.h>)			#if !defined(_LIBCPP_HAS_MUSL_LIBC) && __has_include(<sys/sysctl.h>)
	# include <sys/sysctl.h>			# include <sys/sysctl.h>
	#endif			#endif

	#if __has_include(<unistd.h>)			#if __has_include(<unistd.h>)
	# include <unistd.h>			# include <unistd.h>
	#endif			#endif

	#if defined(__NetBSD__)			#if defined(__NetBSD__)
	▲ Show 20 Lines • Show All 195 Lines • Show Last 20 Lines