[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,0]

Thu Jan 23 10:46:57 PST 2020

peter.smith added a comment.

In D72222#1836703 <https://reviews.llvm.org/D72222#1836703>, @MaskRay wrote:

> In D72222#1836669 <https://reviews.llvm.org/D72222#1836669>, @hans wrote:
>
> > In D72222#1836643 <https://reviews.llvm.org/D72222#1836643>, @peter.smith wrote:
> >
> > > >> If the patchable functions is intended for clang-10 we'll need to make sure any fix is merged to clang-10.
> > > > 
> > > > This commit was made before release/10.x branch. Maybe the easiest thing is to revert the driver change in release/10.x (CC @hans), before we had a better understanding of the problem.
> > > >  (Eventually I think the Linux kernel should have a better configure time test than a simple `whether the compiler accepts -fpatchable-function-entry=2,0?`)
> > > > 
> > > > @peter.smith @nickdesaulniers What do you think?
> > >
> > > Revert on the 10.0 release sounds reasonable to me. That would prevent the kernel from enabling the option and would prevent the build failure.
> >
> >
> > But if trunk is broken, shouldn't it be reverted there first, and then we can merge the revert to 10.x (and then trunk can be fixed eventually)?
>
>
> I don't think the commit is to be blamed. The availability of the driver option -fpatchable-function-entry= enables `CONFIG_DYNAMIC_FTRACE_WITH_REGS` and `CONFIG_LIVEPATCH`, which were not tested before. There could be other issues in the code path.
>
>   % rg fpatchable-function-entry
>   arch/arm64/Kconfig
>   147:            if $(cc-option,-fpatchable-function-entry=2)
>  
>   arch/arm64/Makefile
>   100:  CC_FLAGS_FTRACE := -fpatchable-function-entry=2
>
>
> If we can explicitly disable the options in the CI, that will be very nice.

It definitely won't be this commit, the assert fail looks like it is from the patchable-function attribute pass. The CI just runs allyesconfig and allmodconfig and tells us if there are any new regressions, it won't report this one again so it is unlikely to be worth masking it out.

Assert failure:

  clang-10: /work/llvm-project/llvm/include/llvm/ADT/ilist_iterator.h:139: llvm::ilist_iterator::reference llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>, false, false>::operator*() const [OptionsT = llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>, IsReverse = false, IsConst = false]: Assertion `!NodePtr->isKnownSentinel()' failed.
  Stack dump:
  0.	Program arguments: /work/llvm-project/build/buildclang/bin/clang-10 -cc1 -triple aarch64-unknown-linux-gnu -S -disable-free -main-file-name slab_common.c -mrelocation-model static -mthread-model posix -fno-delete-null-pointer-checks -mllvm -warn-stack-size=2048 -mframe-pointer=non-leaf -relaxed-aliasing -mdisable-tail-calls -fmath-errno -fno-rounding-math -masm-verbose -no-integrated-as -mconstructor-aliases -target-cpu generic -target-feature -fp-armv8 -target-feature -crypto -target-feature -neon -target-feature -sha2 -target-feature -aes -target-abi aapcs -mllvm -aarch64-enable-global-merge=false -fallow-half-arguments-and-returns -dwarf-column-info -fno-split-dwarf-inlining -debugger-tuning=gdb -nostdsysteminc -nobuiltininc -resource-dir /work/llvm-project/build/buildclang/lib/clang/10.0.0 -dependency-file mm/.slab_common.o.d -MT mm/slab_common.o -sys-header-deps -isystem /work/clangmaster/lib/clang/10.0.0/include -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -I ./arch/arm64/include -I ./arch/arm64/include/generated -I ./include -I ./arch/arm64/include/uapi -I ./arch/arm64/include/generated/uapi -I ./include/uapi -I ./include/generated/uapi -D __KERNEL__ -D CC_USING_PATCHABLE_FUNCTION_ENTRY -D KASAN_SHADOW_SCALE_SHIFT=3 -D CONFIG_AS_LSE=1 -D CONFIG_CC_HAS_K_CONSTRAINT=1 -D KASAN_SHADOW_SCALE_SHIFT=3 -D KBUILD_BASENAME="slab_common" -D KBUILD_MODNAME="slab_common" -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-address-of-packed-member -Wno-format-invalid-specifier -Wno-gnu -Wno-tautological-compare -Wno-unused-const-variable -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Werror=date-time -Werror=incompatible-pointer-types -Wno-initializer-overrides -Wno-format -Wno-sign-compare -Wno-format-zero-length -std=gnu89 -fno-dwarf-directory-asm -fdebug-compilation-dir /work/linux/linux -ferror-limit 19 -fmessage-length 0 -fpatchable-function-entry=2 -fwrapv -stack-protector 2 -ftrivial-auto-var-init=pattern -fno-builtin -fno-signed-char -fwchar-type=short -fno-signed-wchar -fgnuc-version=4.2.1 -fobjc-runtime=gcc -fno-common -fdiagnostics-show-option -fcolor-diagnostics -vectorize-loops -vectorize-slp -o /tmp/slab_common-7e60cc.s -x c mm/slab_common.c 
  1.	<eof> parser at end of file
  2.	Code generation
  3.	Running pass 'Function Pass Manager' on module 'mm/slab_common.c'.
  4.	Running pass 'Implement the 'patchable-function' attribute' on function '@rcu_lock_acquire'

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72222/new/

https://reviews.llvm.org/D72222