Page MenuHomePhabricator

compiler-rt: move all __GLIBC_PREREQ into own header file
ClosedPublic

Authored by vitalybuka on Sep 27 2019, 7:19 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

vitalybuka created this revision.Sep 27 2019, 7:19 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 27 2019, 7:19 PM
Herald added subscribers: llvm-commits, Restricted Project, dberris. · View Herald Transcript
eugenis added inline comments.Sep 30 2019, 10:39 AM
compiler-rt/lib/sanitizer_common/sanitizer_glibc_version.h
24 ↗(On Diff #222277)

Why not put this in sanitizer_platform.h directly?

vitalybuka marked an inline comment as done.Sep 30 2019, 10:48 AM
vitalybuka added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_glibc_version.h
24 ↗(On Diff #222277)

looks like it's different from what is defined in platform.h
also it needs to go after the SANITIZER_POSIX definition.

I have no preference here. Should I change?

eugenis accepted this revision.Sep 30 2019, 10:54 AM

LGTM

compiler-rt/lib/sanitizer_common/sanitizer_glibc_version.h
24 ↗(On Diff #222277)

That would put features.h include in sanitizer_platform.h, which might be bad...

This revision is now accepted and ready to land.Sep 30 2019, 10:54 AM
This revision was automatically updated to reflect the committed changes.
ZeGentzy reopened this revision.Oct 1 2019, 1:21 AM
This revision is now accepted and ready to land.Oct 1 2019, 1:21 AM
ZeGentzy requested changes to this revision.EditedOct 1 2019, 1:28 AM
ZeGentzy added a subscriber: ZeGentzy.

Hello folks!

After an hour and a half of git bisecting, this commit appears to have broken my 32-bit LLVM builds.

[1/2530] Generating VCSRevision.h
-- Found Git: /usr/bin/git (found version "2.23.0") 
[2/2530] Building CXX object projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonNoTermination.i386.dir/sanitizer_platform_limits_posix.cpp.o
FAILED: projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonNoTermination.i386.dir/sanitizer_platform_limits_posix.cpp.o 
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++  -DHAVE_RPC_XDR_H=0 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D_LARGEFILE_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/compiler-rt/lib/sanitizer_common -I/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common -I/usr/include/libxml2 -Iinclude -I/build/llvm-git-gentz/src/llvm-project32/llvm/include -I/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/.. -O3 -pipe -ffast-math -funroll-loops -fstack-protector-strong -fno-plt -m32 -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -std=c++14 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -std=c++14 -Wno-unused-parameter -O3 -DNDEBUG    -m32 -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -fno-lto -O3 -gline-tables-only -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -Wno-non-virtual-dtor -fno-rtti -Wframe-larger-than=570 -Wglobal-constructors -MD -MT projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonNoTermination.i386.dir/sanitizer_platform_limits_posix.cpp.o -MF projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonNoTermination.i386.dir/sanitizer_platform_limits_posix.cpp.o.d -o projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonNoTermination.i386.dir/sanitizer_platform_limits_posix.cpp.o -c /build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp:1016:1: error: 'assertion_failed__1016' declared as an array with a negative size
CHECK_SIZE_AND_OFFSET(dirent, d_ino);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h:1438:3: note: expanded from macro 'CHECK_SIZE_AND_OFFSET'
  COMPILER_CHECK(sizeof(((__sanitizer_##CLASS *)NULL)->MEMBER) == \
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:336:30: note: expanded from macro 'COMPILER_CHECK'
#define COMPILER_CHECK(pred) IMPL_COMPILER_ASSERT(pred, __LINE__)
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:342:57: note: expanded from macro 'IMPL_COMPILER_ASSERT'
    typedef char IMPL_PASTE(assertion_failed_##_, line)[2*(int)(pred)-1]
                                                        ^~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp:1022:1: error: 'assertion_failed__1022' declared as an array with a negative size
CHECK_SIZE_AND_OFFSET(dirent, d_off);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h:1438:3: note: expanded from macro 'CHECK_SIZE_AND_OFFSET'
  COMPILER_CHECK(sizeof(((__sanitizer_##CLASS *)NULL)->MEMBER) == \
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:336:30: note: expanded from macro 'COMPILER_CHECK'
#define COMPILER_CHECK(pred) IMPL_COMPILER_ASSERT(pred, __LINE__)
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:342:57: note: expanded from macro 'IMPL_COMPILER_ASSERT'
    typedef char IMPL_PASTE(assertion_failed_##_, line)[2*(int)(pred)-1]
                                                        ^~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp:1022:1: error: 'assertion_failed__1022' declared as an array with a negative size
CHECK_SIZE_AND_OFFSET(dirent, d_off);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h:1440:3: note: expanded from macro 'CHECK_SIZE_AND_OFFSET'
  COMPILER_CHECK(offsetof(__sanitizer_##CLASS, MEMBER) ==         \
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:336:30: note: expanded from macro 'COMPILER_CHECK'
#define COMPILER_CHECK(pred) IMPL_COMPILER_ASSERT(pred, __LINE__)
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:342:57: note: expanded from macro 'IMPL_COMPILER_ASSERT'
    typedef char IMPL_PASTE(assertion_failed_##_, line)[2*(int)(pred)-1]
                                                        ^~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp:1024:1: error: 'assertion_failed__1024' declared as an array with a negative size
CHECK_SIZE_AND_OFFSET(dirent, d_reclen);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h:1440:3: note: expanded from macro 'CHECK_SIZE_AND_OFFSET'
  COMPILER_CHECK(offsetof(__sanitizer_##CLASS, MEMBER) ==         \
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:336:30: note: expanded from macro 'COMPILER_CHECK'
#define COMPILER_CHECK(pred) IMPL_COMPILER_ASSERT(pred, __LINE__)
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/llvm-git-gentz/src/llvm-project32/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:342:57: note: expanded from macro 'IMPL_COMPILER_ASSERT'
    typedef char IMPL_PASTE(assertion_failed_##_, line)[2*(int)(pred)-1]
                                                        ^~~~~~~~~~~~~~~
4 errors generated.
[3/2530] Building CXX object projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonNoTermination.i386.dir/sanitizer_tls_get_addr.cpp.o
[4/2530] Building CXX object projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.i386.dir/sanitizer_stacktrace_printer.cpp.o
[5/2530] Building CXX object projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.i386.dir/sanitizer_stacktrace_libcdep.cpp.o

I'm using this PKGBUILD: https://github.com/ZeGentzy/aur-buildscripts/blob/master/llvm-git-gentz/PKGBUILD#L376

I'm building with the following:

export PKG_CONFIG_PATH="/usr/lib32/pkgconfig"
export CFLAGS="$CFLAGS -m32"
export CXXFLAGS="$CXXFLAGS -m32"
export LDFLAGS="$LDFLAGS -m32"

cmake /build/llvm-git-gentz/src/llvm-project32/llvm -G Ninja \
    -D CMAKE_BUILD_TYPE="Release" \
    -D LLVM_DEFAULT_TARGET_TRIPLE="i686-pc-linux-gnu" \
    -D LLVM_LIBDIR_SUFFIX=32 \
    -D LLVM_TARGET_ARCH:STRING=i686 \
    -D LLVM_CONFIG="/usr/bin/llvm-config32" \
    -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
    -D LLVM_ENABLE_BINDINGS=OFF \
    -D PYTHON_EXECUTABLE=/usr/bin/python-32 \
    -D CMAKE_CXX_COMPILER="/usr/bin/clang++" \
    -D CMAKE_C_COMPILER="/usr/bin/clang" \
    -D LLVM_USE_LINKER="/usr/bin/ld.lld" \
    -D CMAKE_RANLIB="/usr/bin/llvm-ranlib" \
    -D CMAKE_AR="/usr/bin/llvm-ar" \
    -D LLVM_ENABLE_LTO=OFF \
    -D LLVM_BUILD_LLVM_DYLIB=ON \
    -D LLVM_LINK_LLVM_DYLIB=ON \
    -D LLVM_ENABLE_PROJECTS="clang;compiler-rt;lld;lldb;clang-tools-extra" \
    -D LLVM_APPEND_VC_REV=ON \
    -D LLVM_HOST_TRIPLE="x86_64-pc-linux-gnu" \
    -D LLVM_ENABLE_RTTI=ON \
    -D LLVM_ENABLE_FFI=ON \
    -D FFI_INCLUDE_DIR="/usr/lib32/libffi-3.2.1/include" \
    -D LLVM_BUILD_TESTS=ON \
    -D LLVM_VERSION_SUFFIX= \
    -D CMAKE_POLICY_DEFAULT_CMP0075=NEW \
    -D LLVM_OPTIMIZED_TABLEGEN=ON \
    -D LLVM_CCACHE_BUILD=ON \
    -D CMAKE_POLICY_DEFAULT_CMP0075=NEW \
    -D CMAKE_C_FLAGS="-O3 -pipe -ffast-math -funroll-loops -fstack-protector-strong -fno-plt -m32" \
    -D CMAKE_CXX_FLAGS="-O3 -pipe -ffast-math -funroll-loops -fstack-protector-strong -fno-plt -m32" \
    -D LLVM_TARGETS_TO_BUILD="AMDGPU;X86" \
    -D LLVM_BINUTILS_INCDIR=/usr/include \
    -D LLVM_EXTERNAL_LIT="/usr/bin/lit" \
    -D LLVM_BUILD_DOCS=OFF \
    -D LLVM_INCLUDE_DOCS=OFF \
    -D LLVM_ENABLE_SPHINX=OFF \
    -D LLVM_ENABLE_DOXYGEN=OFF \
    -D LLVM_INSTALL_UTILS=ON \
    -D LLVM_PARALLEL_LINK_JOBS=1 \
    -D CMAKE_INSTALL_PREFIX="/opt/llvm32"
ninja -j4 all
DESTDIR=/build/llvm-git-gentz/pkg/lib32-llvm-git-gentz ninja -j4 install

@vitalybuka Could you please revisit this revision so that it doesn't break 32-bit builds and revert this revision until then?

This revision now requires changes to proceed.Oct 1 2019, 1:28 AM
ro added a subscriber: ro.Oct 1 2019, 2:12 AM

This patch also breaks the Solaris builds:

[1097/4181] Building CXX object projects/compiler-rt/lib/ubsan/CMakeFiles/RTUbsan_standalone.i386.dir/ubsan_signals_standalone.cpp.o
FAILED: projects/compiler-rt/lib/ubsan/CMakeFiles/RTUbsan_standalone.i386.dir/ubsan_signals_standalone.cpp.o 
/usr/gcc/7/bin/c++  -D_DEBUG -D_FILE_OFFSET_BITS=64 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/compiler-rt/lib/ubsan -I/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/projects/compiler-rt/lib/ubsan -Iinclude -I/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/include -I/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/include/llvm/Support/Solaris -I/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/projects/compiler-rt/lib/ubsan/.. -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++14 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -std=c++14 -Wno-unused-parameter -O3    -UNDEBUG  -m32 -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fvisibility=hidden -fno-lto -O3 -g -Wno-variadic-macros -Wno-non-virtual-dtor -fno-rtti -DUBSAN_CAN_USE_CXXABI -MD -MT projects/compiler-rt/lib/ubsan/CMakeFiles/RTUbsan_standalone.i386.dir/ubsan_signals_standalone.cpp.o -MF projects/compiler-rt/lib/ubsan/CMakeFiles/RTUbsan_standalone.i386.dir/ubsan_signals_standalone.cpp.o.d -o projects/compiler-rt/lib/ubsan/CMakeFiles/RTUbsan_standalone.i386.dir/ubsan_signals_standalone.cpp.o -c /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/projects/compiler-rt/lib/ubsan/ubsan_signals_standalone.cpp
In file included from /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/projects/compiler-rt/lib/ubsan/../sanitizer_common/sanitizer_platform_interceptors.h:16:0,
                 from /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/projects/compiler-rt/lib/ubsan/../sanitizer_common/sanitizer_signal_interceptors.inc:16,
                 from /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/projects/compiler-rt/lib/ubsan/ubsan_signals_standalone.cpp:38:
/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/projects/compiler-rt/lib/ubsan/../sanitizer_common/sanitizer_glibc_version.h:19:10: fatal error: features.h: No such file or directory
 #include <features.h>
          ^~~~~~~~~~~~
compilation terminated.

Solaris doesn't have <features.h>. It seems that this is a glibc-only header, yet lib/sanitizer_common/sanitizer_glibc_version.h
includes it for

#if (SANITIZER_POSIX && !SANITIZER_MAC) || SANITIZER_FUCHSIA
#include <features.h>
#endif

unlike lib/sanitizer_common/sanitizer_getauxval.h before which had

#if SANITIZER_LINUX || SANITIZER_FUCHSIA
 
# include <features.h>

Why that positive list of targets known to support the header was changed into an incomplete, seemingly random, negative list or, better
yet, a cmake test, is beyond me.

The 32-bit Arm buildbots with compiler-rt are also affected. For example clang-cmake-armv7-full http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/7676/steps/build%20stage%201/logs/stdio

phosek added a subscriber: phosek.Oct 1 2019, 9:14 AM

We're seeing the same issue when building 32-bit compiler-rt runtimes.

sorry, looking

reverted in r373367

vitalybuka updated this revision to Diff 222681.Oct 1 2019, 1:14 PM

fix 32bit and solaris compilation

vitalybuka updated this revision to Diff 222682.Oct 1 2019, 1:17 PM

uncomment the fix

ro added inline comments.Oct 1 2019, 1:51 PM
compiler-rt/lib/sanitizer_common/sanitizer_glibc_version.h
20 ↗(On Diff #222682)

That's exactly the change I've successfully tested on both
amd64-pc-solaris2.11 and sparcv9-sun-solaris2.11
earlier today.

vitalybuka marked an inline comment as done.Oct 1 2019, 2:10 PM
vitalybuka added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_glibc_version.h
20 ↗(On Diff #222682)

This should be this way from begining.
I was confused by __GLIBC_PREREQ sanitizer_linux_libcdep.cpp under
#if SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_NETBSD ||

SANITIZER_OPENBSD || SANITIZER_SOLARIS

but solaris, probably intentionally, was covered by #ifndef __GLIBC_PREREQ check.

eugenis accepted this revision.Oct 2 2019, 10:02 AM

LGTM

ZeGentzy requested changes to this revision.

@ZeGentzy PTAL
"requested changes" in Phabricator requires review by requester.

This revision is now accepted and ready to land.Oct 3 2019, 10:43 AM
This revision was automatically updated to reflect the committed changes.