From 8c78ca2e8f14ba15dbfcee13259806fff0649735 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Fri, 7 Apr 2017 16:35:09 +0000 Subject: [PATCH] [builtins] Get the builtins tests passing on Windows Many things were broken: - We stopped building most builtins on Windows in r261432 for reasons that are not at all clear to me. This essentially reverts that patch. - Fix %librt to expand to clang_rt.builtins-$arch.lib on Windows instead of libclang_rt.builtins-$arch.a. - Fix memory protection tests (trampoline, enable executable, clear cache) on Windows. One issue was that the MSVC incremental linker generates ILT thunks for functions with external linkage, so memcpying the functions into the executable stack buffer wasn't working. You can't memcpy an RIP-relative jump without fixing up the offset. - Disable tests that rely on C99 complex library functions when using the MSVC CRT, which isn't compatible with clang's C99 _Complex. In theory, these could all be separate patches, but it would not green the tests, so let's try for it all at once. Hopefully this fixes the clang-x64-ninja-win7 bot. llvm-svn: 299780 --- compiler-rt/lib/builtins/CMakeLists.txt | 13 ++------- compiler-rt/test/builtins/CMakeLists.txt | 2 ++ compiler-rt/test/builtins/Unit/clear_cache_test.c | 21 +++----------- compiler-rt/test/builtins/Unit/divdc3_test.c | 2 ++ compiler-rt/test/builtins/Unit/divsc3_test.c | 2 ++ compiler-rt/test/builtins/Unit/divtc3_test.c | 2 ++ compiler-rt/test/builtins/Unit/divxc3_test.c | 2 ++ .../test/builtins/Unit/enable_execute_stack_test.c | 32 +++------------------- compiler-rt/test/builtins/Unit/lit.cfg | 14 ++++++++-- compiler-rt/test/builtins/Unit/lit.site.cfg.in | 2 +- compiler-rt/test/builtins/Unit/muldc3_test.c | 2 ++ compiler-rt/test/builtins/Unit/mulsc3_test.c | 2 ++ compiler-rt/test/builtins/Unit/mulxc3_test.c | 2 ++ .../test/builtins/Unit/trampoline_setup_test.c | 1 - 14 files changed, 38 insertions(+), 61 deletions(-) diff --git a/compiler-rt/lib/builtins/CMakeLists.txt b/compiler-rt/lib/builtins/CMakeLists.txt index 2890103..161487e 100644 --- a/compiler-rt/lib/builtins/CMakeLists.txt +++ b/compiler-rt/lib/builtins/CMakeLists.txt @@ -176,15 +176,6 @@ if(COMPILER_RT_HAS_ATOMIC_KEYWORD AND NOT COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN) atomic.c) endif() -set(MSVC_SOURCES - divsc3.c - divdc3.c - divxc3.c - mulsc3.c - muldc3.c - mulxc3.c) - - if(APPLE) set(GENERIC_SOURCES ${GENERIC_SOURCES} @@ -264,9 +255,9 @@ else () # MSVC x86_64/floatdidf.c x86_64/floatdisf.c x86_64/floatdixf.c - ${MSVC_SOURCES}) + ${GENERIC_SOURCES}) set(x86_64h_SOURCES ${x86_64_SOURCES}) - set(i386_SOURCES ${MSVC_SOURCES}) + set(i386_SOURCES ${GENERIC_SOURCES}) set(i686_SOURCES ${i386_SOURCES}) endif () # if (NOT MSVC) diff --git a/compiler-rt/test/builtins/CMakeLists.txt b/compiler-rt/test/builtins/CMakeLists.txt index f37e46d..cabf767 100644 --- a/compiler-rt/test/builtins/CMakeLists.txt +++ b/compiler-rt/test/builtins/CMakeLists.txt @@ -13,6 +13,8 @@ configure_lit_site_cfg( include(builtin-config-ix) +pythonize_bool(MSVC) + #TODO: Add support for Apple. if (NOT APPLE) foreach(arch ${BUILTIN_SUPPORTED_ARCH}) diff --git a/compiler-rt/test/builtins/Unit/clear_cache_test.c b/compiler-rt/test/builtins/Unit/clear_cache_test.c index 590be7e..58960ce 100644 --- a/compiler-rt/test/builtins/Unit/clear_cache_test.c +++ b/compiler-rt/test/builtins/Unit/clear_cache_test.c @@ -16,12 +16,6 @@ #include #if defined(_WIN32) #include -void __clear_cache(void* start, void* end) -{ - if (!FlushInstructionCache(GetCurrentProcess(), start, end-start)) - exit(1); -} - static uintptr_t get_page_size() { SYSTEM_INFO si; GetSystemInfo(&si); @@ -30,27 +24,20 @@ static uintptr_t get_page_size() { #else #include #include -extern void __clear_cache(void* start, void* end); static uintptr_t get_page_size() { return sysconf(_SC_PAGE_SIZE); } #endif - +extern void __clear_cache(void* start, void* end); typedef int (*pfunc)(void); -int func1() -{ - return 1; -} - -int func2() -{ - return 2; -} +// Make these static to avoid ILT jumps for incremental linking on Windows. +static int func1() { return 1; } +static int func2() { return 2; } void *__attribute__((noinline)) memcpy_f(void *dst, const void *src, size_t n) { diff --git a/compiler-rt/test/builtins/Unit/divdc3_test.c b/compiler-rt/test/builtins/Unit/divdc3_test.c index 3c69cf3..042fd23 100644 --- a/compiler-rt/test/builtins/Unit/divdc3_test.c +++ b/compiler-rt/test/builtins/Unit/divdc3_test.c @@ -17,6 +17,8 @@ #include #include +// REQUIRES: c99-complex + // Returns: the quotient of (a + ib) / (c + id) COMPILER_RT_ABI double _Complex diff --git a/compiler-rt/test/builtins/Unit/divsc3_test.c b/compiler-rt/test/builtins/Unit/divsc3_test.c index 4243015..daa2218 100644 --- a/compiler-rt/test/builtins/Unit/divsc3_test.c +++ b/compiler-rt/test/builtins/Unit/divsc3_test.c @@ -17,6 +17,8 @@ #include #include +// REQUIRES: c99-complex + // Returns: the quotient of (a + ib) / (c + id) COMPILER_RT_ABI float _Complex diff --git a/compiler-rt/test/builtins/Unit/divtc3_test.c b/compiler-rt/test/builtins/Unit/divtc3_test.c index f561a96..7474330 100644 --- a/compiler-rt/test/builtins/Unit/divtc3_test.c +++ b/compiler-rt/test/builtins/Unit/divtc3_test.c @@ -18,6 +18,8 @@ #include #include +// REQUIRES: c99-complex + // Returns: the quotient of (a + ib) / (c + id) COMPILER_RT_ABI long double _Complex diff --git a/compiler-rt/test/builtins/Unit/divxc3_test.c b/compiler-rt/test/builtins/Unit/divxc3_test.c index d71660b..d0cdb01 100644 --- a/compiler-rt/test/builtins/Unit/divxc3_test.c +++ b/compiler-rt/test/builtins/Unit/divxc3_test.c @@ -19,6 +19,8 @@ #include #include +// REQUIRES: c99-complex + // Returns: the quotient of (a + ib) / (c + id) COMPILER_RT_ABI long double _Complex diff --git a/compiler-rt/test/builtins/Unit/enable_execute_stack_test.c b/compiler-rt/test/builtins/Unit/enable_execute_stack_test.c index 24165ed..72fc042 100644 --- a/compiler-rt/test/builtins/Unit/enable_execute_stack_test.c +++ b/compiler-rt/test/builtins/Unit/enable_execute_stack_test.c @@ -13,39 +13,14 @@ #include #include #include -#if defined(_WIN32) -#include -void __clear_cache(void* start, void* end) -{ - if (!FlushInstructionCache(GetCurrentProcess(), start, end-start)) - exit(1); -} -void __enable_execute_stack(void *addr) -{ - MEMORY_BASIC_INFORMATION b; - - if (!VirtualQuery(addr, &b, sizeof(b))) - exit(1); - if (!VirtualProtect(b.BaseAddress, b.RegionSize, PAGE_EXECUTE_READWRITE, &b.Protect)) - exit(1); -} -#else -#include extern void __clear_cache(void* start, void* end); extern void __enable_execute_stack(void* addr); -#endif typedef int (*pfunc)(void); -int func1() -{ - return 1; -} - -int func2() -{ - return 2; -} +// Make these static to avoid ILT jumps for incremental linking on Windows. +static int func1() { return 1; } +static int func2() { return 2; } void *__attribute__((noinline)) memcpy_f(void *dst, const void *src, size_t n) { @@ -69,6 +44,7 @@ int main() // verify you can copy and execute a function pfunc f1 = (pfunc)memcpy_f(execution_buffer, func1, 128); __clear_cache(execution_buffer, &execution_buffer[128]); + printf("f1: %p\n", f1); if ((*f1)() != 1) return 1; diff --git a/compiler-rt/test/builtins/Unit/lit.cfg b/compiler-rt/test/builtins/Unit/lit.cfg index f29f7e0..9e3a0d6 100644 --- a/compiler-rt/test/builtins/Unit/lit.cfg +++ b/compiler-rt/test/builtins/Unit/lit.cfg @@ -24,15 +24,21 @@ default_builtins_opts = '' config.test_source_root = os.path.dirname(__file__) # Path to the static library -base_lib = os.path.join(config.compiler_rt_libdir, "libclang_rt.builtins-%s.a " - % config.target_arch) +is_msvc = get_required_attr(config, "builtins_is_msvc") +if is_msvc: + base_lib = os.path.join(config.compiler_rt_libdir, "clang_rt.builtins-%s.lib " + % config.target_arch) + config.substitutions.append( ("%librt ", base_lib) ) +else: + base_lib = os.path.join(config.compiler_rt_libdir, "libclang_rt.builtins-%s.a" + % config.target_arch) + config.substitutions.append( ("%librt ", base_lib + ' -lc -lm ') ) builtins_source_dir = os.path.join( get_required_attr(config, "compiler_rt_src_root"), "lib", "builtins") builtins_lit_source_dir = get_required_attr(config, "builtins_lit_source_dir") extra_link_flags = ["-nodefaultlibs"] -config.substitutions.append( ("%librt ", base_lib + ' -lc -lm ') ) target_cflags = [get_required_attr(config, "target_cflags")] target_cflags += ['-fno-builtin', '-I', builtins_source_dir] @@ -46,6 +52,8 @@ clang_builtins_static_cxxflags = config.cxx_mode_flags + \ clang_builtins_cflags = clang_builtins_static_cflags clang_builtins_cxxflags = clang_builtins_static_cxxflags +if not is_msvc: + config.available_features.add('c99-complex') config.available_features.add('not-android') clang_wrapper = "" diff --git a/compiler-rt/test/builtins/Unit/lit.site.cfg.in b/compiler-rt/test/builtins/Unit/lit.site.cfg.in index 4b4009d..16dc5ab 100644 --- a/compiler-rt/test/builtins/Unit/lit.site.cfg.in +++ b/compiler-rt/test/builtins/Unit/lit.site.cfg.in @@ -4,7 +4,7 @@ config.name_suffix = "@BUILTINS_TEST_CONFIG_SUFFIX@" config.builtins_lit_source_dir = "@BUILTINS_LIT_SOURCE_DIR@/Unit" config.target_cflags = "@BUILTINS_TEST_TARGET_CFLAGS@" config.target_arch = "@BUILTINS_TEST_TARGET_ARCH@" - +config.builtins_is_msvc = "@MSVC_PYBOOL@" # Load common config for all compiler-rt lit tests. lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configured") diff --git a/compiler-rt/test/builtins/Unit/muldc3_test.c b/compiler-rt/test/builtins/Unit/muldc3_test.c index add09f4..5a856bc 100644 --- a/compiler-rt/test/builtins/Unit/muldc3_test.c +++ b/compiler-rt/test/builtins/Unit/muldc3_test.c @@ -17,6 +17,8 @@ #include #include +// REQUIRES: c99-complex + // Returns: the product of a + ib and c + id COMPILER_RT_ABI double _Complex diff --git a/compiler-rt/test/builtins/Unit/mulsc3_test.c b/compiler-rt/test/builtins/Unit/mulsc3_test.c index b14e237..46309c3 100644 --- a/compiler-rt/test/builtins/Unit/mulsc3_test.c +++ b/compiler-rt/test/builtins/Unit/mulsc3_test.c @@ -19,6 +19,8 @@ #include #include +// REQUIRES: c99-complex + // Returns: the product of a + ib and c + id COMPILER_RT_ABI float _Complex diff --git a/compiler-rt/test/builtins/Unit/mulxc3_test.c b/compiler-rt/test/builtins/Unit/mulxc3_test.c index 384c6ee..3260b75 100644 --- a/compiler-rt/test/builtins/Unit/mulxc3_test.c +++ b/compiler-rt/test/builtins/Unit/mulxc3_test.c @@ -19,6 +19,8 @@ #include #include +// REQUIRES: c99-complex + // Returns: the product of a + ib and c + id COMPILER_RT_ABI long double _Complex diff --git a/compiler-rt/test/builtins/Unit/trampoline_setup_test.c b/compiler-rt/test/builtins/Unit/trampoline_setup_test.c index 5a75724f..b8c3eae 100644 --- a/compiler-rt/test/builtins/Unit/trampoline_setup_test.c +++ b/compiler-rt/test/builtins/Unit/trampoline_setup_test.c @@ -13,7 +13,6 @@ #include #include #include -#include /* * Tests nested functions -- 2.7.4