From 0367a1f6fec5a8d1ee2fb4cfa3004591a5510d1a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Sat, 4 Feb 2017 14:35:48 -0800 Subject: [PATCH] Drop prefix from own memcpy/memset/memcmp C compiler might generate calls to memcpy, memset, memcmp, and memmove as it seem fit (so far we haven't seen memmove being required). That means we need to provide our own versions of it for code which is not linked to a libc. We already have a solution for that in commit bdf6051 ("pie: provide memcpy/memcmp/memset for noglibc case") but we faced another problem of compiler trying to optimize our builtin_memset() by inserting calls to memset() which is just an alias in our case and so it lead to infinite recursion. This was workarounded in commit 8ea0ba7 ("string.h: fix memset over-optimization with clang") but it's not clear that was a proper fix. This patch is considered to be the real solution. As we don't have any other implementations of memset/memcpy/memcmp in non-libc case, we can call ours without any prefixes and avoid using weak aliases. Implementation notes: 1. mem*() functions code had to be moved from .h to .c for the functions to be compatible with their prototypes declared in /usr/include/string.h (i.e. "extern"). 2. FORTIFY_SOURCE needed to be disabled for code not linked to libc, because otherwise memcpy() may be replaced with a macro that expands to __memcpy_chk() which of course can't be resolved during linking. https://travis-ci.org/kolyshkin/criu/builds/198415449 Signed-off-by: Kir Kolyshkin Signed-off-by: Pavel Emelyanov --- criu/arch/aarch64/include/asm/string.h | 7 --- criu/arch/aarch64/include/features.h | 4 ++ criu/arch/aarch64/restorer.c | 1 - criu/arch/aarch64/vdso-pie.c | 1 - criu/arch/arm/include/asm/string.h | 7 --- criu/arch/arm/include/features.h | 4 ++ criu/arch/arm/restorer.c | 1 - criu/arch/ppc64/include/asm/string.h | 28 --------- criu/arch/ppc64/include/features.h | 7 +++ criu/arch/ppc64/memcmp_64.S | 2 +- criu/arch/ppc64/memcpy_power7.S | 1 - criu/arch/ppc64/vdso-pie.c | 4 +- criu/arch/x86/include/asm/string.h | 21 ------- criu/arch/x86/include/features.h | 6 ++ criu/arch/x86/memcpy.S | 2 - criu/arch/x86/vdso-pie.c | 4 +- criu/include/asm-generic/string.h | 82 -------------------------- criu/include/string.h | 3 +- criu/pie/Makefile | 1 + criu/pie/Makefile.library | 3 + criu/pie/restorer.c | 2 +- criu/pie/string.c | 63 ++++++++++++++++++++ criu/pie/util-fd.c | 2 - criu/pie/util-vdso.c | 13 +++- include/common/scm-code.c | 8 +-- scripts/build/Dockerfile.tmpl | 4 +- 26 files changed, 110 insertions(+), 171 deletions(-) delete mode 100644 criu/arch/aarch64/include/asm/string.h create mode 100644 criu/arch/aarch64/include/features.h delete mode 100644 criu/arch/arm/include/asm/string.h create mode 100644 criu/arch/arm/include/features.h delete mode 100644 criu/arch/ppc64/include/asm/string.h create mode 100644 criu/arch/ppc64/include/features.h delete mode 100644 criu/arch/x86/include/asm/string.h create mode 100644 criu/arch/x86/include/features.h delete mode 100644 criu/include/asm-generic/string.h create mode 100644 criu/pie/string.c diff --git a/criu/arch/aarch64/include/asm/string.h b/criu/arch/aarch64/include/asm/string.h deleted file mode 100644 index 020a8eccd..000000000 --- a/criu/arch/aarch64/include/asm/string.h +++ /dev/null @@ -1,7 +0,0 @@ -#ifndef __CR_ASM_STRING_H__ -#define __CR_ASM_STRING_H__ - -#include "common/compiler.h" -#include "asm-generic/string.h" - -#endif /* __CR_ASM_STRING_H__ */ diff --git a/criu/arch/aarch64/include/features.h b/criu/arch/aarch64/include/features.h new file mode 100644 index 000000000..5b51d1892 --- /dev/null +++ b/criu/arch/aarch64/include/features.h @@ -0,0 +1,4 @@ +#ifndef __CRIU_ARCH_FEATURES_H +#define __CRIU_ARCH_FEATURES_H + +#endif /* __CRIU_ARCH_FEATURES_H */ diff --git a/criu/arch/aarch64/restorer.c b/criu/arch/aarch64/restorer.c index 2c61e2d03..c833a8836 100644 --- a/criu/arch/aarch64/restorer.c +++ b/criu/arch/aarch64/restorer.c @@ -2,7 +2,6 @@ #include "restorer.h" #include "asm/restorer.h" -#include "asm/string.h" #include "syscall.h" #include "log.h" diff --git a/criu/arch/aarch64/vdso-pie.c b/criu/arch/aarch64/vdso-pie.c index 6ece3775a..16a05506d 100644 --- a/criu/arch/aarch64/vdso-pie.c +++ b/criu/arch/aarch64/vdso-pie.c @@ -1,6 +1,5 @@ #include -#include "asm/string.h" #include "asm/types.h" #include "syscall.h" diff --git a/criu/arch/arm/include/asm/string.h b/criu/arch/arm/include/asm/string.h deleted file mode 100644 index 020a8eccd..000000000 --- a/criu/arch/arm/include/asm/string.h +++ /dev/null @@ -1,7 +0,0 @@ -#ifndef __CR_ASM_STRING_H__ -#define __CR_ASM_STRING_H__ - -#include "common/compiler.h" -#include "asm-generic/string.h" - -#endif /* __CR_ASM_STRING_H__ */ diff --git a/criu/arch/arm/include/features.h b/criu/arch/arm/include/features.h new file mode 100644 index 000000000..5b51d1892 --- /dev/null +++ b/criu/arch/arm/include/features.h @@ -0,0 +1,4 @@ +#ifndef __CRIU_ARCH_FEATURES_H +#define __CRIU_ARCH_FEATURES_H + +#endif /* __CRIU_ARCH_FEATURES_H */ diff --git a/criu/arch/arm/restorer.c b/criu/arch/arm/restorer.c index 786feeeb3..774a411de 100644 --- a/criu/arch/arm/restorer.c +++ b/criu/arch/arm/restorer.c @@ -2,7 +2,6 @@ #include "restorer.h" #include "asm/restorer.h" -#include "asm/string.h" #include "syscall.h" #include "log.h" diff --git a/criu/arch/ppc64/include/asm/string.h b/criu/arch/ppc64/include/asm/string.h deleted file mode 100644 index 93eb42813..000000000 --- a/criu/arch/ppc64/include/asm/string.h +++ /dev/null @@ -1,28 +0,0 @@ -#ifndef __CR_ASM_STRING_H__ -#define __CR_ASM_STRING_H__ - -#include "common/compiler.h" - -#define HAS_BUILTIN_MEMCPY -#define HAS_BUILTIN_MEMCMP - -#include "asm-generic/string.h" - -#ifdef CR_NOGLIBC -extern void memcpy_power7(void *to, const void *from, unsigned long n); -static inline void *builtin_memcpy(void *to, const void *from, unsigned long n) -{ - if (n) - memcpy_power7(to, from, n); - return to; -} -extern int builtin_memcmp(const void *cs, const void *ct, size_t count); -#else -/* - * When building with the C library, call its services - */ -#define builtin_memcpy memcpy -#define builtin_memcmp memcmp -#endif - -#endif /* __CR_ASM_STRING_H__ */ diff --git a/criu/arch/ppc64/include/features.h b/criu/arch/ppc64/include/features.h new file mode 100644 index 000000000..5b720f8f3 --- /dev/null +++ b/criu/arch/ppc64/include/features.h @@ -0,0 +1,7 @@ +#ifndef __CRIU_ARCH_FEATURES_H +#define __CRIU_ARCH_FEATURES_H + +#define ARCH_HAS_MEMCPY +#define ARCH_HAS_MEMCMP + +#endif /* __CRIU_ARCH_FEATURES_H */ diff --git a/criu/arch/ppc64/memcmp_64.S b/criu/arch/ppc64/memcmp_64.S index fd1bfb97e..7f7fe9158 100644 --- a/criu/arch/ppc64/memcmp_64.S +++ b/criu/arch/ppc64/memcmp_64.S @@ -31,7 +31,7 @@ #define LD ldx #endif -ENTRY(builtin_memcmp) +ENTRY(memcmp) cmpdi cr1,r5,0 /* Use the short loop if both strings are not 8B aligned */ diff --git a/criu/arch/ppc64/memcpy_power7.S b/criu/arch/ppc64/memcpy_power7.S index e66572324..e1afb7e7f 100644 --- a/criu/arch/ppc64/memcpy_power7.S +++ b/criu/arch/ppc64/memcpy_power7.S @@ -28,7 +28,6 @@ * service memcpy to initialise big local variable in the stack. */ ENTRY(memcpy) -ENTRY(memcpy_power7) cmpldi r5,16 std r3,-STACKFRAMESIZE+STK_REG(R31)(r1) blt .Lshort_copy diff --git a/criu/arch/ppc64/vdso-pie.c b/criu/arch/ppc64/vdso-pie.c index b4bdf23aa..c7d8b9b7e 100644 --- a/criu/arch/ppc64/vdso-pie.c +++ b/criu/arch/ppc64/vdso-pie.c @@ -1,6 +1,6 @@ #include +#include -#include "asm/string.h" #include "asm/types.h" #include "syscall.h" @@ -104,7 +104,7 @@ static unsigned long put_trampoline(unsigned long at, struct vdso_symtable *sym) pr_debug("Putting vDSO trampoline in %s at %lx\n", sym->symbols[i].name, trampoline); - builtin_memcpy((void *)trampoline, &vdso_trampoline, + memcpy((void *)trampoline, &vdso_trampoline, size); invalidate_caches(trampoline); } diff --git a/criu/arch/x86/include/asm/string.h b/criu/arch/x86/include/asm/string.h deleted file mode 100644 index 415bf5ce3..000000000 --- a/criu/arch/x86/include/asm/string.h +++ /dev/null @@ -1,21 +0,0 @@ -#ifndef __CR_ASM_STRING_H__ -#define __CR_ASM_STRING_H__ - -#define HAS_BUILTIN_MEMCPY - -#include "common/compiler.h" -#include "asm-generic/string.h" - -#ifdef CR_NOGLIBC -extern void *memcpy_x86(void *to, const void *from, size_t n); -static inline void *builtin_memcpy(void *to, const void *from, size_t n) -{ - if (n) - memcpy_x86(to, from, n); - return to; -} -#else -#define builtin_memcpy memcpy -#endif /* CR_NOGLIBC */ - -#endif /* __CR_ASM_STRING_H__ */ diff --git a/criu/arch/x86/include/features.h b/criu/arch/x86/include/features.h new file mode 100644 index 000000000..d357e99ad --- /dev/null +++ b/criu/arch/x86/include/features.h @@ -0,0 +1,6 @@ +#ifndef __CRIU_ARCH_FEATURES_H +#define __CRIU_ARCH_FEATURES_H + +#define ARCH_HAS_MEMCPY + +#endif /* __CRIU_ARCH_FEATURES_H */ diff --git a/criu/arch/x86/memcpy.S b/criu/arch/x86/memcpy.S index c8f7555fc..2496cb9e5 100644 --- a/criu/arch/x86/memcpy.S +++ b/criu/arch/x86/memcpy.S @@ -16,7 +16,6 @@ * Output: * rax original destination */ -ENTRY(memcpy_x86) ENTRY(memcpy) movq %rdi, %rax movq %rdx, %rcx @@ -27,4 +26,3 @@ ENTRY(memcpy) rep movsb ret END(memcpy) -END(memcpy_x86) diff --git a/criu/arch/x86/vdso-pie.c b/criu/arch/x86/vdso-pie.c index cb47496ef..3aac72e96 100644 --- a/criu/arch/x86/vdso-pie.c +++ b/criu/arch/x86/vdso-pie.c @@ -1,6 +1,6 @@ #include -#include "asm/string.h" +#include "string.h" #include "asm/types.h" #include "syscall.h" @@ -41,7 +41,7 @@ int vdso_redirect_calls(unsigned long base_to, unsigned long base_from, base_to, to->symbols[i].offset, i); jmp.imm64 = base_to + to->symbols[i].offset; - builtin_memcpy((void *)(base_from + from->symbols[i].offset), &jmp, sizeof(jmp)); + memcpy((void *)(base_from + from->symbols[i].offset), &jmp, sizeof(jmp)); } return 0; diff --git a/criu/include/asm-generic/string.h b/criu/include/asm-generic/string.h deleted file mode 100644 index 6e0f659af..000000000 --- a/criu/include/asm-generic/string.h +++ /dev/null @@ -1,82 +0,0 @@ -#ifndef __CR_ASM_GENERIC_STRING_H__ -#define __CR_ASM_GENERIC_STRING_H__ - -#include "common/compiler.h" - -/* C compiler may generate calls to memcmp, memset, memcpy and memmove, - * so it relies on those to be available during linking. - * In case we are not linking our code against glibc, we set CR_NOGLIBC - * and have to provide our own implementations of mem*() functions. - * - * For now, not having memmove() seems OK for both gcc and clang. - */ - -#ifndef HAS_BUILTIN_MEMCPY -static __maybe_unused void *builtin_memcpy(void *to, const void *from, size_t n) -{ - size_t i; - unsigned char *cto = to; - const unsigned char *cfrom = from; - - for (i = 0; i < n; ++i, ++cto, ++cfrom) { - *cto = *cfrom; - } - - return to; -} -#ifdef CR_NOGLIBC -void *memcpy(void *to, const void *from, size_t n) \ - __attribute__ ((weak, alias ("builtin_memcpy"))); -#endif -#endif - -#ifndef HAS_BUILTIN_MEMCMP -static __maybe_unused int builtin_memcmp(const void *cs, const void *ct, size_t count) -{ - const unsigned char *su1, *su2; - int res = 0; - - for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--) - if ((res = *su1 - *su2) != 0) - break; - return res; -} -#ifdef CR_NOGLIBC -int memcmp(const void *cs, const void *ct, size_t count) \ - __attribute__ ((weak, alias ("builtin_memcmp"))); -#endif -#endif - -#ifndef HAS_BUILTIN_STRNCMP -static always_inline int builtin_strncmp(const char *cs, const char *ct, size_t count) -{ - size_t i; - - for (i = 0; i < count; i++) { - if (cs[i] != ct[i]) - return cs[i] < ct[i] ? -1 : 1; - if (!cs[i]) - break; - } - return 0; -} -#endif - -#ifndef HAS_BUILTIN_MEMSET -static __maybe_unused void *builtin_memset(void *s, const int c, size_t count) -{ - volatile char *dest = s; - size_t i = 0; - - while (i < count) - dest[i++] = (char) c; - - return s; -} -#ifdef CR_NOGLIBC -void *memset(void *s, const int c, size_t count) \ - __attribute__ ((weak, alias ("builtin_memset"))); -#endif -#endif - -#endif /* __CR_ASM_GENERIC_STRING_H__ */ diff --git a/criu/include/string.h b/criu/include/string.h index 53c009866..3a87e85ea 100644 --- a/criu/include/string.h +++ b/criu/include/string.h @@ -3,7 +3,6 @@ #include #include -#include "asm/string.h" #ifdef CONFIG_HAS_LIBBSD # include @@ -19,4 +18,6 @@ extern size_t strlcpy(char *dest, const char *src, size_t size); extern size_t strlcat(char *dest, const char *src, size_t count); #endif +extern int builtin_strncmp(const char *cs, const char *ct, size_t count); + #endif /* __CR_STRING_H__ */ diff --git a/criu/pie/Makefile b/criu/pie/Makefile index 655cae1bc..8d593e226 100644 --- a/criu/pie/Makefile +++ b/criu/pie/Makefile @@ -21,6 +21,7 @@ CFLAGS += -iquote $(SRC_DIR)/criu/arch/$(ARCH)/include CFLAGS += -iquote $(SRC_DIR)/criu/include CFLAGS += -iquote $(SRC_DIR)/include CFLAGS += -iquote $(SRC_DIR) +CFLAGS += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 ifneq ($(filter-out ia32,$(ARCH)),) diff --git a/criu/pie/Makefile.library b/criu/pie/Makefile.library index 180985dac..7d6d85992 100644 --- a/criu/pie/Makefile.library +++ b/criu/pie/Makefile.library @@ -1,6 +1,7 @@ lib-y += log-simple.o lib-y += util-fd.o lib-y += util.o +lib-y += string.o ifeq ($(VDSO),y) lib-y += util-vdso.o @@ -47,3 +48,5 @@ ifneq ($(filter-out ia32,$(ARCH)),) else ccflags-y += -DCR_NOGLIBC -fno-pic -Wa,--noexecstack -fno-stack-protector endif + +ccflags-y += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c index 1b136d3ce..ff7b0e640 100644 --- a/criu/pie/restorer.c +++ b/criu/pie/restorer.c @@ -672,7 +672,7 @@ static int restore_aio_ring(struct rst_aio_ring *raio) populate: i = offsetof(struct aio_ring, io_events); - builtin_memcpy((void *)ctx + i, (void *)ring + i, raio->len - i); + memcpy((void *)ctx + i, (void *)ring + i, raio->len - i); /* * If we failed to get the proper nr_req right and diff --git a/criu/pie/string.c b/criu/pie/string.c new file mode 100644 index 000000000..d15419295 --- /dev/null +++ b/criu/pie/string.c @@ -0,0 +1,63 @@ +#include +#include "features.h" + +/* C compiler may generate calls to memcmp, memset, memcpy and memmove, + * so it relies on those to be available during linking. + * In case we are not linking our code against glibc, we set CR_NOGLIBC + * and have to provide our own implementations of mem*() functions. + * + * For now, not having memmove() seems OK for both gcc and clang. + */ + +#ifndef ARCH_HAS_MEMCPY +void *memcpy(void *to, const void *from, size_t n) +{ + size_t i; + unsigned char *cto = to; + const unsigned char *cfrom = from; + + for (i = 0; i < n; ++i, ++cto, ++cfrom) + *cto = *cfrom; + + return to; +} +#endif + +#ifndef ARCH_HAS_MEMCMP +int memcmp(const void *cs, const void *ct, size_t count) +{ + const unsigned char *su1, *su2; + int res = 0; + + for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--) + if ((res = *su1 - *su2) != 0) + break; + return res; +} +#endif + +int builtin_strncmp(const char *cs, const char *ct, size_t count) +{ + size_t i; + + for (i = 0; i < count; i++) { + if (cs[i] != ct[i]) + return cs[i] < ct[i] ? -1 : 1; + if (!cs[i]) + break; + } + return 0; +} + +#ifndef ARCH_HAS_MEMSET +void *memset(void *s, const int c, size_t count) +{ + char *dest = s; + size_t i = 0; + + while (i < count) + dest[i++] = (char) c; + + return s; +} +#endif diff --git a/criu/pie/util-fd.c b/criu/pie/util-fd.c index 0a0a9250e..e753f0b17 100644 --- a/criu/pie/util-fd.c +++ b/criu/pie/util-fd.c @@ -19,8 +19,6 @@ # define __sys_err(ret) (-errno) #endif -#define __memcpy builtin_memcpy - #include "util-pie.h" #include "fcntl.h" diff --git a/criu/pie/util-vdso.c b/criu/pie/util-vdso.c index caea515d5..567d38460 100644 --- a/criu/pie/util-vdso.c +++ b/criu/pie/util-vdso.c @@ -10,13 +10,20 @@ #include #include -#include "string.h" #include "image.h" #include "util-vdso.h" #include "vma.h" #include "log.h" #include "common/bug.h" +#ifdef CR_NOGLIBC +# include "string.h" +#else +# include +# define builtin_strncmp strncmp +#endif + + #ifdef LOG_PREFIX # undef LOG_PREFIX #endif @@ -81,7 +88,7 @@ static int has_elf_identity(Ehdr_t *ehdr) BUILD_BUG_ON(sizeof(elf_ident) != sizeof(ehdr->e_ident)); - if (builtin_memcmp(ehdr->e_ident, elf_ident, sizeof(elf_ident))) { + if (memcmp(ehdr->e_ident, elf_ident, sizeof(elf_ident))) { pr_err("Elf header magic mismatch\n"); return false; } @@ -245,7 +252,7 @@ static void parse_elf_symbols(uintptr_t mem, size_t size, Phdr_t *load, if (builtin_strncmp(name, symbol, vdso_symbol_length)) continue; - builtin_memcpy(t->symbols[i].name, name, vdso_symbol_length); + memcpy(t->symbols[i].name, name, vdso_symbol_length); t->symbols[i].offset = (unsigned long)sym->st_value - load->p_vaddr; break; } diff --git a/include/common/scm-code.c b/include/common/scm-code.c index b36bf68e9..3a88cb46d 100644 --- a/include/common/scm-code.c +++ b/include/common/scm-code.c @@ -2,10 +2,6 @@ #error "The __sys macro is required" #endif -#ifndef __memcpy -#error "The __memcpy macro is required" -#endif - static void scm_fdset_init_chunk(struct scm_fdset *fdset, int nr_fds, void *data, unsigned ch_size) { @@ -62,7 +58,7 @@ int send_fds(int sock, struct sockaddr_un *saddr, int len, for (i = 0; i < nr_fds; i += min_fd) { min_fd = min(CR_SCM_MAX_FD, nr_fds - i); scm_fdset_init_chunk(&fdset, min_fd, data, ch_size); - __memcpy(cmsg_data, &fds[i], sizeof(int) * min_fd); + memcpy(cmsg_data, &fds[i], sizeof(int) * min_fd); ret = __sys(sendmsg)(sock, &fdset.hdr, 0); if (ret <= 0) @@ -113,7 +109,7 @@ int __recv_fds(int sock, int *fds, int nr_fds, void *data, unsigned ch_size, int if (unlikely(min_fd <= 0)) return -EBADFD; - __memcpy(&fds[i], cmsg_data, sizeof(int) * min_fd); + memcpy(&fds[i], cmsg_data, sizeof(int) * min_fd); if (data) data += ch_size * min_fd; } diff --git a/scripts/build/Dockerfile.tmpl b/scripts/build/Dockerfile.tmpl index a42e2d315..6069af25f 100644 --- a/scripts/build/Dockerfile.tmpl +++ b/scripts/build/Dockerfile.tmpl @@ -22,8 +22,8 @@ COPY . /criu WORKDIR /criu RUN make mrproper -RUN make -j $(nproc) CC=$CC criu/parasite-syscall.o -RUN make -j $(nproc) CC=$CC +RUN make -j $(nproc) CC=$CC V=1 criu/parasite-syscall.o +RUN make -j $(nproc) CC=$CC V=1 RUN make mrproper RUN bash -c 'CLEAN="$(git clean -ndx --exclude=scripts/build --exclude=.config)"; echo "${CLEAN}"; test -z "${CLEAN}"; exit $?' RUN make -j $(nproc) CC=$CC -C test/zdtm