Skip to content
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.

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

ld.lld: error: undefined symbol: memcpy.inline #1466

Closed
nickdesaulniers opened this issue Sep 29, 2021 · 15 comments
Closed

ld.lld: error: undefined symbol: memcpy.inline #1466

nickdesaulniers opened this issue Sep 29, 2021 · 15 comments
Assignees
Labels
[ARCH] x86_64 This bug impacts ARCH=x86_64 [BUG] llvm (main) A bug in an unreleased version of LLVM (this label is appropriate for regressions) [CONFIG] allmodconfig Issue affects allmodconfig on certain architectures [FIXED][LLVM] main This bug was only present and fixed in an unreleased version of LLVM

Comments

@nickdesaulniers
Copy link
Member

nickdesaulniers commented Sep 29, 2021

cc @kees @nathanchance @serge-sans-paille, I see mainline x86_64 allmodconfig failing to link with ToT LLVM:

ld.lld: error: undefined symbol: memcpy.inline

I'm curious if this is related to recent upstream changes to fortify routines? @nathanchance noticed our CI is testing llvm 3 days out of date (@gctucker mentioned that debian builds of llvm were broken/unavailable cc @sylvestre ).

@nickdesaulniers nickdesaulniers added [BUG] Untriaged Something isn't working [CONFIG] allmodconfig Issue affects allmodconfig on certain architectures [ARCH] x86_64 This bug impacts ARCH=x86_64 [TOOL] lld The issue is relevant to LLD linker labels Sep 29, 2021
@nickdesaulniers
Copy link
Member Author

last known good: 5aa4c74c9a2ee3c4e6e87adfa2ef218c5aeed1d6
first known bad: 3d6f49a56995b845c40be5827ded5d1e3f692cec

@nickdesaulniers
Copy link
Member Author

defconfig seems fine. something in the allmodconfig is tickling this

$ make LLVM=1 -j72 allmodconfig vmlinux
...
  LD      .tmp_vmlinux.kallsyms1
ld.lld: error: undefined symbol: memcpy.inline
>>> referenced by module.c
>>>               kernel/module.o:(apply_relocate_add) in archive arch/x86/built-in.a

@nickdesaulniers
Copy link
Member Author

I'm not sure what's up with the warning from lld, the offending file is arch/x86/kernel/module.o (apply_relocate_add has a relocation/reference to memcpy.inline when CONFIG_FORTIFY_SOURCE=y is set).

for f in $(find . -name \*.o); do llvm-readelf -s $f | grep -q memcpy.inline && echo $f ; done
./arch/x86/kernel/module.o
./vmlinux.o

The .ll file contains:

%write.0 = phi i8* (i8*, i8*, i64)* [ @memcpy.inline, %entry ], [ @text_poke, %if.then ]
...
declare dso_local i8* @memcpy.inline(i8*, i8*, i64) local_unnamed_addr #4
// arch/x86/kernel/module.c
229   void *(*write)(void *, const void *, size_t) = memcpy;                                                                                                                    
230                                                                                                                                                                             
231   if (!early) {                                                                                                                                                             
232     write = text_poke;

@sylvestre
Copy link

(@gctucker mentioned that debian builds of llvm were broken/unavailable cc @sylvestre ).

I haven't seen this issue, mine were caused by various things related to projects vs runtimes + some defaults changes + the triple changes + their revert by LebedevRI

@nickdesaulniers
Copy link
Member Author

nickdesaulniers commented Sep 30, 2021

cvise hacked up:

// clang -O2 foo.c -c && llvm-readelf -s foo.o | grep memcpy.inline
extern inline __attribute__((__always_inline__)) void *memcpy() {}

was enough to produce an undefined reference to memcpy.inline.

// arch/x86/kernel/module.i
extern void *memcpy(void *to, const void *from, size_t len);
extern void *__memcpy(void *to, const void *from, size_t len);
...
extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __asm__("memcpy");
...
// strlcpy...
  __underlying_memcpy(p, q, len);
...
// strncat...
  __underlying_memcpy(p + p_len, q, copy_len);
...
extern inline __attribute__((__gnu_inline__)) __attribute__((__unused__)) __attribute__((no_instrument_function)) __attribute__((__always_inline__)) __attribute__((gnu_inline)) void *memcpy(void *p, const void *q, __kernel_size_t size)                                                                                                         
{
  ...
  return __underlying_memcpy(p, q, size);

@serge-sans-paille
Copy link

That's on me. The always inliner pass should have made this one disappeared, how does it come it doesn't...

@serge-sans-paille
Copy link

serge-sans-paille commented Sep 30, 2021

Basically if one redefines a builtin with the inline + always_inline attributes, the redefinition is renamed with the .inline suffix. This is needed to support builtins redefinition from the glibc headers under _FORTIFY_SOURCE

@serge-sans-paille
Copy link

serge-sans-paille commented Sep 30, 2021

Some more info, with top-of-tree:

typedef unsigned long size_t;
typedef unsigned long __kernel_size_t;
extern void *memcpy(void *to, const void *from, size_t len);
extern void *__underlying_memcpy(void *p, const void *q,
                                 __kernel_size_t size) __asm__("memcpy");
extern inline __attribute__((__gnu_inline__)) __attribute__((__unused__))
__attribute__((no_instrument_function)) __attribute__((__always_inline__))
__attribute__((gnu_inline)) void *
memcpy(void *p, const void *q, __kernel_size_t size) {
  asm("int3"); // just a marker
  return __underlying_memcpy(p, q, size);
}

int main(int argc, char ** argv) {
  char* buffer[10];
  memcpy(&buffer[0], argv, argc * sizeof(char*));
  return buffer[0];
}

as test.c

clang -S -o- test.c -Xclang -disable-llvm-passes

produces

main:                                   # @main
	.cfi_startproc
# %bb.0:
	pushq	%rbp
	.cfi_def_cfa_offset 16
	.cfi_offset %rbp, -16
	movq	%rsp, %rbp
	.cfi_def_cfa_register %rbp
	subq	$96, %rsp
	movl	$0, -4(%rbp)
	movl	%edi, -8(%rbp)
	movq	%rsi, -16(%rbp)
	leaq	-96(%rbp), %rdi
	movq	-16(%rbp), %rsi
	movslq	-8(%rbp), %rdx
	shlq	$3, %rdx
	callq	memcpy.inline
	movq	-96(%rbp), %rax
                                        # kill: def $eax killed $eax killed $rax
	addq	$96, %rsp
	popq	%rbp
	.cfi_def_cfa %rsp, 8
	retq

which is expected, as the always inliner is skipped.

Otherwise we have

./bin/clang -S -o- test.c

that produces

main:                                   # @main
	.cfi_startproc
# %bb.0:
	pushq	%rbp
	.cfi_def_cfa_offset 16
	.cfi_offset %rbp, -16
	movq	%rsp, %rbp
	.cfi_def_cfa_register %rbp
	subq	$128, %rsp
	movl	$0, -28(%rbp)
	movl	%edi, -32(%rbp)
	movq	%rsi, -40(%rbp)
	leaq	-128(%rbp), %rdx
	movq	-40(%rbp), %rcx
	movslq	-32(%rbp), %rax
	shlq	$3, %rax
	movq	%rdx, -8(%rbp)
	movq	%rcx, -16(%rbp)
	movq	%rax, -24(%rbp)
	#APP
	int3
	#NO_APP
	movq	-8(%rbp), %rdi
	movq	-16(%rbp), %rsi
	movq	-24(%rbp), %rdx
	callq	memcpy
	movq	-128(%rbp), %rax
                                        # kill: def $eax killed $eax killed $rax
	addq	$128, %rsp
	popq	%rbp
	.cfi_def_cfa %rsp, 8
	retq

which is expected.

@nickdesaulniers
Copy link
Member Author

@serge-sans-paille that test case doesn't refer to memcpy via fn ptr.

@@ -11,8 +11,16 @@
   return __underlying_memcpy(p, q, size);
 }
 
+void *text_poke(void *, const void *, size_t);
+
 int main(int argc, char ** argv) {
+  void *(*write)(void *, const void *, size_t);
+  if (argc)
+    write = memcpy;
+  else
+    write = text_poke;
+
   char* buffer[10];
-  memcpy(&buffer[0], argv, argc * sizeof(char*));
+  write(buffer, argv, argc * sizeof(char));
   return buffer[0];
 }

clang -O2 produces undefined references to memcpy.inline, regardless of -disable-llvm-passes. Is that expected?

@serge-sans-paille
Copy link

serge-sans-paille commented Sep 30, 2021

Thanks for the details. Quoting the GCC info page about gnu_inline:

If the function is declared 'extern', then this definition of the
function is used only for inlining. In no case is the function
compiled as a standalone function, not even if you take its address
explicitly.

So What I should implement there is if an inline builtin has its address taken, use the builtin instead. Note that in that case no protection will be offered, but that's consistent with GCC behavior.

@nickdesaulniers
Copy link
Member Author

So What I should implement there is if an inline builtin has its address taken, use the builtin instead. Note that in that case no protection will be offered, but that's consistent with GCC behavior.

Yes, I think so.

@nickdesaulniers
Copy link
Member Author

@serge-sans-paille this is a build breakage for us, and is now frustrating efforts to debug other failures (#1471). If this can't be fixed forward quickly, please revert.

@nickdesaulniers nickdesaulniers added [BUG] llvm (main) A bug in an unreleased version of LLVM (this label is appropriate for regressions) and removed [BUG] Untriaged Something isn't working [TOOL] lld The issue is relevant to LLD linker labels Oct 1, 2021
@serge-sans-paille
Copy link

ack. I'm on it and plan to release a fix during the week end.

@serge-sans-paille
Copy link

I've tested https://reviews.llvm.org/D111009 and the patch should fix the issue.

@nickdesaulniers nickdesaulniers added the [PATCH] Submitted A patch has been submitted for review label Oct 3, 2021
@nickdesaulniers nickdesaulniers added [PATCH] Accepted A submitted patch has been accepted upstream and removed [PATCH] Submitted A patch has been submitted for review labels Oct 4, 2021
@nickdesaulniers
Copy link
Member Author

Fixed: https://reviews.llvm.org/rG0f0e31cf511def3e92244e615b2646c1fd0df0cd

closing, but #1477 looks related. (perhaps it's a false positive from LLVM being older than 0f0e31cf511def3e92244e615b2646c1fd0df0cd).

@nickdesaulniers nickdesaulniers added [FIXED][LLVM] main This bug was only present and fixed in an unreleased version of LLVM and removed [PATCH] Accepted A submitted patch has been accepted upstream labels Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ARCH] x86_64 This bug impacts ARCH=x86_64 [BUG] llvm (main) A bug in an unreleased version of LLVM (this label is appropriate for regressions) [CONFIG] allmodconfig Issue affects allmodconfig on certain architectures [FIXED][LLVM] main This bug was only present and fixed in an unreleased version of LLVM
Projects
None yet
Development

No branches or pull requests

3 participants