This is an archive of the discontinued LLVM Phabricator instance.

Differential D59702

Unbreak the build of compiler-rt on Linux/mips64el
AbandonedPublic

Authored by sylvestre.ledru on Mar 22 2019, 9:53 AM.

Details

Reviewers
atanasyan
Summary

Fails with:

/<<BUILDDIR>>/llvm-toolchain-8-8~+rc5/projects/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:343:72: error: size of array 'assertion_failed__73' is negative

typedef char IMPL_PASTE(assertion_failed_##_, line)[2*(int)(pred)-1]
                                                                   ^

/<<BUILDDIR>>/llvm-toolchain-8-8~+rc5/projects/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:337:30: note: in expansion of macro 'IMPL_COMPILER_ASSERT'

#define COMPILER_CHECK(pred) IMPL_COMPILER_ASSERT(pred, __LINE__)
                            ^~~~~~~~~~~~~~~~~~~~

/<<BUILDDIR>>/llvm-toolchain-8-8~+rc5/projects/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_linux.cc:73:1: note: in expansion of macro 'COMPILER_CHECK'

COMPILER_CHECK(struct_kernel_stat_sz == sizeof(struct stat));

Diff Detail

Event Timeline

sylvestre.ledru created this revision.Mar 22 2019, 9:53 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 22 2019, 9:53 AM
Herald added subscribers: llvm-commits, Restricted Project, jdoerfert and 5 others. · View Herald Transcript
sylvestre.ledru added a comment.Mar 22 2019, 9:53 AM

Not sure what I am doing but it fixes the issue

Harbormaster completed remote builds in B29502: Diff 191901.Mar 22 2019, 9:55 AM
sylvestre.ledru added a comment.Mar 25 2019, 6:45 AM

@atanasyan ok with you?

atanasyan added inline comments.Mar 25 2019, 7:38 AM
lib/sanitizer_common/sanitizer_platform_limits_linux.cc
31

Why do you need this change?

lib/sanitizer_common/sanitizer_platform_limits_posix.h
85

These numbers are sizes of the struct stat for 32 / 64-bit cases. Due the rL301171 32-bit size depends on definitions of the _LARGEFILE_SOURCE and FILE_OFFSET_BITS=64 macros.

I run the following test case on Debian 9 mipsel 64-bit:

$ cat test.c
#include <stdio.h>
#include <sys/stat.h>

int main() {
  printf("%u\n", sizeof(struct stat));
}

$ gcc -mabi=64 test.c && ./a.out
216

$ gcc -mabi=32 test.c -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 && ./a.out
160

Both numbers are the same as in the original version of the sanitizer_platform_limits_posix.h.

I guess that the 144 is sizeof(struct stat) in 32-bit case without proper _LARGEFILE_SOURCE and _FILE_OFFSET_BITS=64 definitions. It looks like the code introduced by the rL301171 does not work in your case.

The 104 looks strange. It means that the size of struct stat in 32-bit case is larger than in the 64-bit one.

sylvestre.ledru added a comment.Mar 25 2019, 7:47 AM

I agree that 104 looks strange... but it worked.
As I said, I am no expert in mips64el. :/

atanasyan added a comment.Mar 25 2019, 7:51 AM

How do you build compiler-rt? What OS do yo use?

sylvestre.ledru added a comment.Mar 25 2019, 7:53 AM

Debian testing
you can see the full log here:
https://buildd.debian.org/status/fetch.php?pkg=llvm-toolchain-8&arch=mips64el&ver=1%3A8~%2Brc5-1&stamp=1552412165&raw=0

atanasyan added a comment.Apr 23 2019, 1:58 PM

I'm sorry for delay with handling this issue.

The 160 constant in the FIRST_32_SECOND_64(144, 104) depends on the following fragment from the cmake/modules/HandleLLVMOptions.cmake file introduced by the rL301171. If _LARGEFILE_SOURCE and _FILE_OFFSET_BITS=64 are defined, sizeof(struct stat) equals to 160. This constant was configured by the rL301307. Unfortunately, in case of building on MIPS 64-bit CMAKE_SIZEOF_VOID_P equals to 8 and these definitions are not provided to the compiler. It's a separate problem. Probably it needs to be handled by supporting the LLVM_BUILD_32_BITS option for MIPS targets. These option is used to build 32-bit binaries on 64-bit host. But now it's supported on limited set of architectures.

if( CMAKE_SIZEOF_VOID_P EQUAL 4 AND NOT LLVM_FORCE_SMALLFILE_FOR_ANDROID)
  # FIXME: It isn't handled in LLVM_BUILD_32_BITS.
  add_definitions( -D_LARGEFILE_SOURCE )
  add_definitions( -D_FILE_OFFSET_BITS=64 )
endif()

Meanwhile, could you try to workaround the issue by explicitly providing -mabi=32 (for building MIPS 32-bit binaries) and -mabi=64 (for building MIPS 64-bit binaries) options to the cmake? Something like that:

-DCMAKE_C_FLAGS="-mabi=32"
-DCMAKE_CXX_FLAGS="-mabi=32"
sylvestre.ledru added a comment.May 1 2019, 5:07 AM

@atanasyan, I tried with

-DCMAKE_C_FLAGS="-mabi=32"
-DCMAKE_CXX_FLAGS="-mabi=32"

but it still fails with:

cd /<<PKGBUILDDIR>>/build-llvm/projects/compiler-rt/lib/sanitizer_common && /usr/bin/g++-8  -DHAVE_RPC_XDR_H=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/<<PKGBUILDDIR>>/build-llvm/projects/compiler-rt/lib/sanitizer_common -I/<<PKGBUILDDIR>>/projects/compiler-rt/lib/sanitizer_common -I/<<PKGBUILDDIR>>/build-llvm/include -I/<<PKGBUILDDIR>>/include -I/<<PKGBUILDDIR>>/projects/compiler-rt/lib/sanitizer_common/..  -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -ffunction-sections -fdata-sections -Wall -std=c++11 -Wno-unused-parameter -O2 -DNDEBUG -g1    -mips32r2 -mabi=32 -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 -o CMakeFiles/RTSanitizerCommon.mipsel.dir/sanitizer_platform_limits_linux.cc.o -c /<<PKGBUILDDIR>>/projects/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_linux.cc
In file included from /<<PKGBUILDDIR>>/projects/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_linux.cc:22:
/<<PKGBUILDDIR>>/projects/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:343:72: error: size of array 'assertion_failed__73' is negative
     typedef char IMPL_PASTE(assertion_failed_##_, line)[2*(int)(pred)-1]
                                                                        ^
/<<PKGBUILDDIR>>/projects/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:337:30: note: in expansion of macro 'IMPL_COMPILER_ASSERT'
 #define COMPILER_CHECK(pred) IMPL_COMPILER_ASSERT(pred, __LINE__)
                              ^~~~~~~~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/projects/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_linux.cc:73:1: note: in expansion of macro 'COMPILER_CHECK'
 COMPILER_CHECK(struct_kernel_stat_sz == sizeof(struct stat));
 ^~~~~~~~~~~~~~

Full log:
https://buildd.debian.org/status/fetch.php?pkg=llvm-toolchain-8&arch=mips64el&ver=1%3A8-4&stamp=1556705174&raw=0
better idea? :)

atanasyan added a comment.May 15 2019, 3:28 PM

I hope the rL360825 fixes the problem so this patch can be switch to the abandoned state.

sylvestre.ledru abandoned this revision.Jun 8 2019, 7:43 AM

indeed, closing

Revision Contents

Diff 191901

lib/sanitizer_common/sanitizer_platform_limits_linux.cc

Show All 22 Lines
// For offsetof -> __builtin_offsetof definition. // For offsetof -> __builtin_offsetof definition.
#include <stddef.h> #include <stddef.h>
// With old kernels (and even new kernels on powerpc) asm/stat.h uses types that // With old kernels (and even new kernels on powerpc) asm/stat.h uses types that
// are not defined anywhere in userspace headers. Fake them. This seems to work // are not defined anywhere in userspace headers. Fake them. This seems to work
// fine with newer headers, too. // fine with newer headers, too.
#include <linux/posix_types.h> #include <linux/posix_types.h>
#if defined(__x86_64__) || defined(__mips__) #if defined(__x86_64__)
atanasyanUnsubmitted
Not Done
ReplyInline Actions

Why do you need this change?

atanasyan: Why do you need this change?
#include <sys/stat.h> #include <sys/stat.h>
#else #else
#define ino_t __kernel_ino_t #define ino_t __kernel_ino_t
#define mode_t __kernel_mode_t #define mode_t __kernel_mode_t
#define nlink_t __kernel_nlink_t #define nlink_t __kernel_nlink_t
#define uid_t __kernel_uid_t #define uid_t __kernel_uid_t
#define gid_t __kernel_gid_t #define gid_t __kernel_gid_t
#define off_t __kernel_off_t #define off_t __kernel_off_t
▲ Show 20 Lines • Show All 69 Lines • Show Last 20 Lines

lib/sanitizer_common/sanitizer_platform_limits_posix.h

Show First 20 Lines • Show All 75 Lines • ▼ Show 20 Lines#elif defined(__powerpc__) && !defined(__powerpc64__)
const unsigned struct_kernel_stat_sz = 72; const unsigned struct_kernel_stat_sz = 72;
const unsigned struct_kernel_stat64_sz = 104; const unsigned struct_kernel_stat64_sz = 104;
#elif defined(__powerpc64__) #elif defined(__powerpc64__)
const unsigned struct_kernel_stat_sz = 144; const unsigned struct_kernel_stat_sz = 144;
const unsigned struct_kernel_stat64_sz = 104; const unsigned struct_kernel_stat64_sz = 104;
#elif defined(__mips__) #elif defined(__mips__)
const unsigned struct_kernel_stat_sz = const unsigned struct_kernel_stat_sz =
SANITIZER_ANDROID ? FIRST_32_SECOND_64(104, 128) : SANITIZER_ANDROID ? FIRST_32_SECOND_64(104, 128) :
FIRST_32_SECOND_64(160, 216); FIRST_32_SECOND_64(144, 104);
const unsigned struct_kernel_stat64_sz = 104; const unsigned struct_kernel_stat64_sz = 104;
atanasyanUnsubmitted
Not Done
ReplyInline Actions

These numbers are sizes of the struct stat for 32 / 64-bit cases. Due the rL301171 32-bit size depends on definitions of the _LARGEFILE_SOURCE and FILE_OFFSET_BITS=64 macros.

I run the following test case on Debian 9 mipsel 64-bit:

$ cat test.c
#include <stdio.h>
#include <sys/stat.h>

int main() {
  printf("%u\n", sizeof(struct stat));
}

$ gcc -mabi=64 test.c && ./a.out
216

$ gcc -mabi=32 test.c -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 && ./a.out
160

Both numbers are the same as in the original version of the sanitizer_platform_limits_posix.h.

I guess that the 144 is sizeof(struct stat) in 32-bit case without proper _LARGEFILE_SOURCE and _FILE_OFFSET_BITS=64 definitions. It looks like the code introduced by the rL301171 does not work in your case.

The 104 looks strange. It means that the size of struct stat in 32-bit case is larger than in the 64-bit one.

atanasyan: These numbers are sizes of the `struct stat` for 32 / 64-bit cases. Due the rL301171 32-bit…
#elif defined(__s390__) && !defined(__s390x__) #elif defined(__s390__) && !defined(__s390x__)
const unsigned struct_kernel_stat_sz = 64; const unsigned struct_kernel_stat_sz = 64;
const unsigned struct_kernel_stat64_sz = 104; const unsigned struct_kernel_stat64_sz = 104;
#elif defined(__s390x__) #elif defined(__s390x__)
const unsigned struct_kernel_stat_sz = 144; const unsigned struct_kernel_stat_sz = 144;
const unsigned struct_kernel_stat64_sz = 0; const unsigned struct_kernel_stat64_sz = 0;
#elif defined(__sparc__) && defined(__arch64__) #elif defined(__sparc__) && defined(__arch64__)
const unsigned struct___old_kernel_stat_sz = 0; const unsigned struct___old_kernel_stat_sz = 0;
▲ Show 20 Lines • Show All 1,361 Lines • Show Last 20 Lines