New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 47162 - clang rGab9fc8bae805 crashes when building the Linux kernel
Summary: clang rGab9fc8bae805 crashes when building the Linux kernel
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: -New Bugs (show other bugs)
Version: trunk
Hardware: PC Linux
: P normal
Assignee: David Bolvansky
URL:
Keywords:
Depends on:
Blocks: 4068
  Show dependency tree
 
Reported: 2020-08-13 18:27 PDT by Sami Tolvanen
Modified: 2020-08-14 17:43 PDT (History)
8 users (show)

See Also:
Fixed By Commit(s):


Attachments
tty_io-98ae38 crash reproducer (917.87 KB, application/gzip)
2020-08-13 18:27 PDT, Sami Tolvanen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sami Tolvanen 2020-08-13 18:27:26 PDT
Created attachment 23850 [details]
tty_io-98ae38 crash reproducer

Starting with commit ab9fc8bae805c785066779e76e7846aabad5609e, clang crashes when building the ToT x86 Linux kernel:

  CC      drivers/tty/tty_io.o
fatal error: error in backend: Cannot emit physreg copy instruction
clang-12: error: clang frontend command failed with exit code 70 (use -v to see invocation)
clang version 12.0.0 (https://github.com/llvm/llvm-project.git ab9fc8bae805c785066779e76e7846aabad5609e)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/local/google/home/samitolvanen/src/unified-llvm/build.ab9fc8bae805c785066779e76e7846aabad5609e/bin
clang-12: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-12: note: diagnostic msg: /tmp/tty_io-98ae38.c
clang-12: note: diagnostic msg: /tmp/tty_io-98ae38.sh
clang-12: note: diagnostic msg: 

********************

The previous commit works. The preprocessed source and the run script are attached.
Comment 1 Craig Topper 2020-08-13 19:41:30 PDT
This asserts during InstCombine

PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: ./bin/clang -cc1 -triple x86_64-unknown-linux-gnu -S -disable-free -disable-llvm-verifier -discard-value-names -main-file-name tty_io.c -mrelocation-model static -fno-delete-null-pointer-checks -mllvm -warn-stack-size=2048 -mframe-pointer=none -relaxed-aliasing -fmath-errno -fno-rounding-math -no-integrated-as -mconstructor-aliases -mcmodel=kernel -target-cpu x86-64 -target-feature +retpoline-indirect-calls -target-feature +retpoline-indirect-branches -target-feature -sse -target-feature -mmx -target-feature -sse2 -target-feature -3dnow -target-feature -avx -target-feature -x87 -target-feature +retpoline-external-thunk -disable-red-zone -fno-split-dwarf-inlining -debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb -nostdsysteminc -nobuiltininc -D __KERNEL__ -D CC_USING_FENTRY -D KBUILD_MODFILE="drivers/tty/tty_io" -D KBUILD_BASENAME="tty_io" -D KBUILD_MODNAME="tty_io" -fmacro-prefix-map=./= -O2 -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -Werror=unknown-warning-option -Wno-sign-compare -Wno-frame-address -Wno-address-of-packed-member -Wno-format-invalid-specifier -Wno-gnu -Wno-unused-const-variable -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-array-bounds -Werror=date-time -Werror=incompatible-pointer-types -Wno-initializer-overrides -Wno-format -Wno-sign-compare -Wno-format-zero-length -Wno-pointer-to-enum-cast -Wno-tautological-constant-out-of-range-compare -std=gnu89 -fno-dwarf-directory-asm -ferror-limit 19 -pg -mfentry -fwrapv -stack-protector 2 -mstack-alignment=8 -fcf-protection=none -fwchar-type=short -fno-signed-wchar -fgnuc-version=4.2.1 -fcolor-diagnostics -vectorize-loops -vectorize-slp -x c tty_io-98ae38.c
1.      <eof> parser at end of file
2.      Per-module optimization passes
3.      Running pass 'Function Pass Manager' on module 'tty_io-98ae38.c'.
4.      Running pass 'Combine redundant instructions' on function '@tty_line_name'
Comment 2 Craig Topper 2020-08-13 19:44:29 PDT
Probably because sprintf returns the number of characters written. And strcpy returns a pointer.
Comment 3 Sami Tolvanen 2020-08-14 09:12:46 PDT
This was reverted in commit 48cd5b72b13c1283eedb0f3fac7c14167da7fc2f, so marking as fixed.
Comment 4 David Bolvansky 2020-08-14 15:02:57 PDT
Problematic line:
return sprintf(p, "%s", driver->name);

Transformation was fixed and relanded:
https://reviews.llvm.org/D85963

But as suggested, we could use stpcpy if result is not unused. To avoid this, use -fno-builtin-stpcpy.
Comment 5 Sami Tolvanen 2020-08-14 15:54:22 PDT
> But as suggested, we could use stpcpy if result is not unused. To avoid this, use -fno-builtin-stpcpy.

This breaks kernel builds again, because there's no stpcpy. Having to use a compiler flag to prevent this behavior doesn't seem ideal to me:

ld.lld: error: undefined symbol: stpcpy
>>> referenced by core.c:1712 (/usr/local/google/home/samitolvanen/android/kernel/linux/arch/x86/events/core.c:1712)
>>>               events/core.o:(events_sysfs_show) in archive arch/x86/built-in.a
>>> referenced by core.c:1735 (/usr/local/google/home/samitolvanen/android/kernel/linux/arch/x86/events/core.c:1735)
>>>               events/core.o:(events_ht_sysfs_show) in archive arch/x86/built-in.a
>>> referenced by uncore.c:100 (/usr/local/google/home/samitolvanen/android/kernel/linux/arch/x86/events/intel/uncore.c:100)
>>>               events/intel/uncore.o:(uncore_event_show) in archive arch/x86/built-in.a
>>> referenced 5 more times
>>> did you mean: strcpy
>>> defined in: lib/lib.a(string.o)
Comment 6 David Bolvansky 2020-08-14 16:08:04 PDT
Either provide implementation of stpcpy or use that flag.
Comment 7 Eli Friedman 2020-08-14 16:33:54 PDT
Hmm.  I'm surprised the kernel has never run into issues with gcc generating calls to stpcpy; it does a similar transform for similar constructs.  See bug 47144.

How does the kernel decide what C library functions to provide?

Probably makes sense to revert the change to generate stpcpy while we discuss this with kernel developers.
Comment 8 Nick Desaulniers 2020-08-14 16:44:50 PDT
Proposed diff in https://github.com/ClangBuiltLinux/linux/issues/1126.
Comment 9 David Bolvansky 2020-08-14 16:48:09 PDT
Thanks for kernel fix!
Comment 10 Nick Desaulniers 2020-08-14 16:48:25 PDT
We had the same fire drill last year with lowering calls to bcmp.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f074f3e192f10c9fade898b9b3b8812e3d83342

We can provide an implementation to the kernel and backport it.
Comment 11 Nick Desaulniers 2020-08-14 16:53:32 PDT
Quick question, POSIX.1-2008 defines stpcpy as having it's arguments be `restrict` qualified.  Does snprintf have the same guarantees?

Man page say:
C99 and POSIX.1-2001 specify that the results are undefined if a call to sprintf(), snprintf(), vsprintf(), or vsnprintf() would cause copying to take place between objects that overlap (e.g., if the target string array and one of the supplied input arguments refer to the same buffer).

So I guess that's ok.
Comment 12 Eli Friedman 2020-08-14 16:56:09 PDT
strcpy, stpcpy, and sprintf all require that the buffers don't overlap, as far as I know.
Comment 13 Nick Desaulniers 2020-08-14 17:43:20 PDT
Ok, I've submitted https://lore.kernel.org/lkml/20200815002417.1512973-1-ndesaulniers@google.com/T/#u to the kernel. Re-closing.

(In reply to David Bolvansky from comment #6)
> Either provide implementation of stpcpy or use that flag.

We generally avoid `-f` flags since they typically get dropped during LTO (IIRC)!

(In reply to Eli Friedman from comment #7)
> How does the kernel decide what C library functions to provide?

It doesn't use -ffreestanding for most of the kernel (though I think parts of the tree do), as generally the libcall optimizations are helpful.  It uses the same target triple generally as userspace so it's not easy for the toolchain to know it's targeting a kernel.

The hard part and reason why these kinds of changes are painful is that various lib fns get deprecated or added in glibc, but not necessarily the kernel.  It can take a long time to get the implementations added, and backported to the "stable" tree that most distro's use.  (The max delay can be if you just missed a "merge window" which would reopen every ~2 months).  Meanwhile, CI is red.