Skip to content

[CMake] Remove some always-true HAVE_XXX_H #123087

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.

Sign up for GitHub

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
MaskRay merged 2 commits into from
Jan 15, 2025
Merged

[CMake] Remove some always-true HAVE_XXX_H #123087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffbc2f6
[𝘀𝗽𝗿] initial version
MaskRay Jan 15, 2025
b25902a
remove HAVE_FCNTL_H
MaskRay Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions llvm/cmake/config-ix.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ endif()

# include checks
check_include_file(dlfcn.h HAVE_DLFCN_H)
check_include_file(errno.h HAVE_ERRNO_H)
check_include_file(fcntl.h HAVE_FCNTL_H)
check_include_file(malloc/malloc.h HAVE_MALLOC_MALLOC_H)
if( NOT PURE_WINDOWS )
check_include_file(pthread.h HAVE_PTHREAD_H)
Expand All @@ -69,13 +67,11 @@ check_include_file(signal.h HAVE_SIGNAL_H)
check_include_file(sys/ioctl.h HAVE_SYS_IOCTL_H)
check_include_file(sys/mman.h HAVE_SYS_MMAN_H)
check_include_file(sys/resource.h HAVE_SYS_RESOURCE_H)
check_include_file(sys/stat.h HAVE_SYS_STAT_H)
check_include_file(sys/time.h HAVE_SYS_TIME_H)
check_include_file(sysexits.h HAVE_SYSEXITS_H)
check_include_file(termios.h HAVE_TERMIOS_H)
check_include_file(unistd.h HAVE_UNISTD_H)
check_include_file(valgrind/valgrind.h HAVE_VALGRIND_VALGRIND_H)
check_include_file(fenv.h HAVE_FENV_H)
check_symbol_exists(FE_ALL_EXCEPT "fenv.h" HAVE_DECL_FE_ALL_EXCEPT)
check_symbol_exists(FE_INEXACT "fenv.h" HAVE_DECL_FE_INEXACT)
check_c_source_compiles("
Expand Down
12 changes: 0 additions & 12 deletions llvm/include/llvm/Config/config.h.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,6 @@
/* Define if __unw_add_dynamic_fde() is available on this platform. */
#cmakedefine HAVE_UNW_ADD_DYNAMIC_FDE ${HAVE_UNW_ADD_DYNAMIC_FDE}

/* Define to 1 if you have the <errno.h> header file. */
#cmakedefine HAVE_ERRNO_H ${HAVE_ERRNO_H}

/* Define to 1 if you have the <fcntl.h> header file. */
#cmakedefine HAVE_FCNTL_H ${HAVE_FCNTL_H}

/* Define to 1 if you have the <fenv.h> header file. */
#cmakedefine HAVE_FENV_H ${HAVE_FENV_H}

/* Define if libffi is available on this platform. */
#cmakedefine HAVE_FFI_CALL ${HAVE_FFI_CALL}

Expand Down Expand Up @@ -198,9 +189,6 @@
/* Define to 1 if you have the <sys/resource.h> header file. */
#cmakedefine HAVE_SYS_RESOURCE_H ${HAVE_SYS_RESOURCE_H}

/* Define to 1 if you have the <sys/stat.h> header file. */
#cmakedefine HAVE_SYS_STAT_H ${HAVE_SYS_STAT_H}

/* Define to 1 if you have the <sys/time.h> header file. */
#cmakedefine HAVE_SYS_TIME_H ${HAVE_SYS_TIME_H}

Expand Down
2 changes: 0 additions & 2 deletions llvm/include/llvm/Support/FileSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@
#include <system_error>
#include <vector>

#ifdef HAVE_SYS_STAT_H
#include <sys/stat.h>
Copy link
Member

@mstorsjo mstorsjo Jan 16, 2025

Choose a reason for hiding this comment

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

This change, quite surprisingly, broke building LLDB for a mingw target: https://github.com/mstorsjo/llvm-mingw/actions/runs/12799882284/job/35692046639

The culprit seems to be this particular change; FileSystem.h doesn't include config.h (only llvm-config.h), so in many (but not all) cases of including this header, HAVE_SYS_STAT_H was not defined, and thus <sys/stat.h> wasn't included.

It seems like the <sys/stat.h> include here is (mostly?) unnecessary - I've successfully rebuilt LLVM (llvm+clang+lld+lldb+clang-tools-extra) for both macOS and Linux, with this include entirely removed.

The reason for why it broke LLDB for mingw targets, is that LLDB contains a structure with a field named fstat: https://github.com/llvm/llvm-project/blob/llvmorg-19.1.7/lldb/source/Plugins/Process/Utility/RegisterContext_x86.h#L271
The mingw-w64 sys/stat.h header contains a #define fstat _fstat64: https://github.com/mingw-w64/mingw-w64/blob/v12.0.0/mingw-w64-headers/crt/sys/stat.h#L280

This define would work fine if it would be included before the LLDB struct declaration, but not when it gets included between the struct declaration and use.

I guess the most straightforward fix is to just drop the include of <sys/stat.h> here which doesn't seem to be needed in practice?

Copy link
Contributor

@Endilll Endilll Jan 16, 2025

Choose a reason for hiding this comment

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

Dropping sys/stat.h sounds good to me, but I'd wait for Fangrui to come online if time permits.

#endif

namespace llvm {
namespace sys {
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Analysis/ConstantFolding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1885,7 +1885,7 @@ Constant *GetConstantFoldFPValue128(float128 V, Type *Ty) {

/// Clear the floating-point exception state.
inline void llvm_fenv_clearexcept() {
#if defined(HAVE_FENV_H) && HAVE_DECL_FE_ALL_EXCEPT
#if HAVE_DECL_FE_ALL_EXCEPT
feclearexcept(FE_ALL_EXCEPT);
#endif
errno = 0;
Expand All @@ -1896,7 +1896,7 @@ inline bool llvm_fenv_testexcept() {
int errno_val = errno;
if (errno_val == ERANGE || errno_val == EDOM)
return true;
#if defined(HAVE_FENV_H) && HAVE_DECL_FE_ALL_EXCEPT && HAVE_DECL_FE_INEXACT
#if HAVE_DECL_FE_ALL_EXCEPT && HAVE_DECL_FE_INEXACT
if (fetestexcept(FE_ALL_EXCEPT & ~FE_INEXACT))
return true;
#endif
Expand Down
8 changes: 3 additions & 5 deletions llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@
#ifdef __linux__
// These includes used by RTDyldMemoryManager::getPointerToNamedFunction()
// for Glibc trickery. See comments in this function for more information.
#ifdef HAVE_SYS_STAT_H
#include <sys/stat.h>
#endif
#include <fcntl.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <unistd.h>
#endif

namespace llvm {
Expand Down
5 changes: 0 additions & 5 deletions llvm/lib/Support/Errno.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@
#include "llvm/Support/Errno.h"
#include "llvm/Config/config.h"
#include <cstring>

#if HAVE_ERRNO_H
#include <errno.h>
#endif

//===----------------------------------------------------------------------===//
//=== WARNING: Implementation here must contain only TRULY operating system
Expand All @@ -26,11 +23,9 @@
namespace llvm {
namespace sys {

#if HAVE_ERRNO_H
std::string StrError() {
return StrError(errno);
}
#endif // HAVE_ERRNO_H

std::string StrError(int errnum) {
std::string str;
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/Support/Unix/Path.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,8 @@
#include "Unix.h"
#include <limits.h>
#include <stdio.h>
#if HAVE_SYS_STAT_H
#include <sys/stat.h>
#endif
#if HAVE_FCNTL_H
#include <fcntl.h>
#endif
#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/Support/Unix/Process.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,14 @@
#include "llvm/Config/llvm-config.h" // for LLVM_ENABLE_THREADS
#include <mutex>
#include <optional>
#if HAVE_FCNTL_H
#include <fcntl.h>
#endif
#ifdef HAVE_SYS_TIME_H
#include <sys/time.h>
#endif
#ifdef HAVE_SYS_RESOURCE_H
#include <sys/resource.h>
#endif
#ifdef HAVE_SYS_STAT_H
#include <sys/stat.h>
#endif
#if HAVE_SIGNAL_H
#include <signal.h>
#endif
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/Support/Unix/Program.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,14 @@
#include "llvm/Support/StringSaver.h"
#include "llvm/Support/SystemZ/zOSSupport.h"
#include "llvm/Support/raw_ostream.h"
#if HAVE_SYS_STAT_H
#include <sys/stat.h>
#endif
#if HAVE_SYS_RESOURCE_H
#include <sys/resource.h>
#endif
#if HAVE_SIGNAL_H
#include <signal.h>
#endif
#if HAVE_FCNTL_H
#include <fcntl.h>
#endif
#if HAVE_UNISTD_H
#include <unistd.h>
#endif
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Support/Unix/Signals.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@
#if HAVE_SIGNAL_H
#include <signal.h>
#endif
#if HAVE_SYS_STAT_H
#include <sys/stat.h>
#endif
#if HAVE_DLFCN_H
#include <dlfcn.h>
#endif
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Support/Unix/Unix.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@
# include <dlfcn.h>
#endif

#ifdef HAVE_FCNTL_H
# include <fcntl.h>
#endif

/// This function builds an error message into \p ErrMsg using the \p prefix
/// string and the Unix error number given by \p errnum. If errnum is -1, the
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Support/raw_ostream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@
#include <sys/stat.h>

// <fcntl.h> may provide O_BINARY.
#if defined(HAVE_FCNTL_H)
# include <fcntl.h>
#endif

#if defined(HAVE_UNISTD_H)
# include <unistd.h>
Expand Down
4 changes: 0 additions & 4 deletions llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ write_cmake_config("config") {
"HAVE_DECL_FE_ALL_EXCEPT=1",
"HAVE_DECL_FE_INEXACT=1",
"LLVM_ENABLE_CRASH_DUMPS=",
"HAVE_ERRNO_H=1",
"HAVE_FCNTL_H=1",
"HAVE_FENV_H=1",
"HAVE_FFI_CALL=",
"HAVE_FFI_FFI_H=",
"HAVE_FFI_H=",
Expand All @@ -101,7 +98,6 @@ write_cmake_config("config") {
"HAVE_PTHREAD_GET_NAME_NP=",
"HAVE_PTHREAD_SET_NAME_NP=",
"HAVE_SIGNAL_H=1",
"HAVE_SYS_STAT_H=1",
"HAVE_VALGRIND_VALGRIND_H=",
"HAVE__ALLOCA=",
"HAVE___ALLOCA=",
Expand Down
12 changes: 0 additions & 12 deletions utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,6 @@
/* Define if __unw_add_dynamic_fde() is available on this platform. */
/* HAVE_UNW_ADD_DYNAMIC_FDE defined in Bazel */

/* Define to 1 if you have the <errno.h> header file. */
#define HAVE_ERRNO_H 1

/* Define to 1 if you have the <fcntl.h> header file. */
#define HAVE_FCNTL_H 1

/* Define to 1 if you have the <fenv.h> header file. */
#define HAVE_FENV_H 1

/* Define if libffi is available on this platform. */
/* #undef HAVE_FFI_CALL */

Expand Down Expand Up @@ -201,9 +192,6 @@
/* Define to 1 if you have the <sys/resource.h> header file. */
#define HAVE_SYS_RESOURCE_H 1

/* Define to 1 if you have the <sys/stat.h> header file. */
#define HAVE_SYS_STAT_H 1

/* Define to 1 if you have the <sys/time.h> header file. */
#define HAVE_SYS_TIME_H 1

Expand Down
12 changes: 0 additions & 12 deletions utils/bazel/llvm_configs/config.h.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,6 @@
/* Define if __unw_add_dynamic_fde() is available on this platform. */
#cmakedefine HAVE_UNW_ADD_DYNAMIC_FDE ${HAVE_UNW_ADD_DYNAMIC_FDE}

/* Define to 1 if you have the <errno.h> header file. */
#cmakedefine HAVE_ERRNO_H ${HAVE_ERRNO_H}

/* Define to 1 if you have the <fcntl.h> header file. */
#cmakedefine HAVE_FCNTL_H ${HAVE_FCNTL_H}

/* Define to 1 if you have the <fenv.h> header file. */
#cmakedefine HAVE_FENV_H ${HAVE_FENV_H}

/* Define if libffi is available on this platform. */
#cmakedefine HAVE_FFI_CALL ${HAVE_FFI_CALL}

Expand Down Expand Up @@ -198,9 +189,6 @@
/* Define to 1 if you have the <sys/resource.h> header file. */
#cmakedefine HAVE_SYS_RESOURCE_H ${HAVE_SYS_RESOURCE_H}

/* Define to 1 if you have the <sys/stat.h> header file. */
#cmakedefine HAVE_SYS_STAT_H ${HAVE_SYS_STAT_H}

/* Define to 1 if you have the <sys/time.h> header file. */
#cmakedefine HAVE_SYS_TIME_H ${HAVE_SYS_TIME_H}

Expand Down
Loading