Enhance stack protector for calling no return function
I find current stack protector didn't work for calling no return function.
This is an defect for "stack protector", so let's refine it.
Differential D138774
Enhance stack protector xiangzhangllvm on Nov 27 2022, 11:19 PM. Authored by
Details Enhance stack protector for calling no return function I find current stack protector didn't work for calling no return function.
Diff Detail Event TimelineComment Actions I reproduce according to https://buildkite.com/llvm-project/premerge-checks/builds/123514#0184bd1c-b61e-4f3f-b814-f4698f5d7b1b guide The 2 failed tests Timed Out Tests (1): libFuzzer :: minimize_crash.test Failed Tests (1): LLVM :: Examples/OrcV2Examples/lljit-with-thinlto-summaries.test are all passed in my local: PASS: libFuzzer :: minimize_crash.test (80256 of 82179) PASS: LLVM :: Examples/OrcV2Examples/lljit-with-thinlto-summaries.test (48357 of 82179) (they don't have noreturn info) Comment Actions I check the following pull request https://reviews.llvm.org/D138778 LLVM :: Examples/OrcV2Examples/lljit-with-thinlto-summaries.test So this fail is not cause by my patch. (And my local is pass) Comment Actions This pass operates on IR, so it should have IR-based tests (it should be fine to also have codegen tests, since that is a early-codegen ir based pass) Comment Actions Do you mean we should only check IR? I think choose 1 target (e.g X86) to check the asm code may be better. Thanks for review! Comment Actions Did i not literally answer that question in my original comment? Comment Actions Sorry again, let me make more clear, did your "IR-producing tests" means we should check the IR not asm code ? Comment Actions Yes.
For this pass - apparently. In general - no. Comment Actions Hello, I noticed that if the compiler is built with EXPENSIVE_CHECKS on, the stack-protector-no-return.ll testcase fails with DominatorTree is different than a freshly computed one! Current: =============================-------------------------------- Inorder Dominator Tree: DFSNumbers invalid: 0 slow queries. [1] %entry {4294967295,4294967295} [0] [2] %unreachable {4294967295,4294967295} [1] [2] %lpad {4294967295,4294967295} [1] [3] %invoke.cont {4294967295,4294967295} [2] [4] %invoke.cont2 {4294967295,4294967295} [3] [4] %SP_return3 {4294967295,4294967295} [3] [4] %CallStackCheckFailBlk2 {4294967295,4294967295} [3] [3] %lpad1 {4294967295,4294967295} [2] [4] %eh.resume {4294967295,4294967295} [3] [5] %SP_return6 {4294967295,4294967295} [4] [5] %CallStackCheckFailBlk5 {4294967295,4294967295} [4] [4] %terminate.lpad {4294967295,4294967295} [3] [5] %SP_return9 {4294967295,4294967295} [4] [5] %CallStackCheckFailBlk8 {4294967295,4294967295} [4] [2] %SP_return {4294967295,4294967295} [1] [2] %CallStackCheckFailBlk {4294967295,4294967295} [1] Roots: %entry Freshly computed tree: =============================-------------------------------- Inorder Dominator Tree: DFSNumbers invalid: 0 slow queries. [1] %entry {4294967295,4294967295} [0] [2] %SP_return {4294967295,4294967295} [1] [3] %unreachable {4294967295,4294967295} [2] [3] %lpad {4294967295,4294967295} [2] [4] %invoke.cont {4294967295,4294967295} [3] [5] %SP_return3 {4294967295,4294967295} [4] [6] %invoke.cont2 {4294967295,4294967295} [5] [5] %CallStackCheckFailBlk2 {4294967295,4294967295} [4] [4] %lpad1 {4294967295,4294967295} [3] [5] %eh.resume {4294967295,4294967295} [4] [6] %SP_return6 {4294967295,4294967295} [5] [6] %CallStackCheckFailBlk5 {4294967295,4294967295} [5] [5] %terminate.lpad {4294967295,4294967295} [4] [6] %SP_return9 {4294967295,4294967295} [5] [6] %CallStackCheckFailBlk8 {4294967295,4294967295} [5] [2] %CallStackCheckFailBlk {4294967295,4294967295} [1] Roots: %entry This can also be reproduced on a non-EXPENSIVE_CHECKS build just by adding "-verify-dom-info" to the command line like llc test/CodeGen/X86/stack-protector-no-return.ll -mtriple=x86_64-unknown-linux-gnu -o - -verify-dom-info Comment Actions Seems not the patch problem, I find the stack protector is strange, it can't be independently print by -print-after-all, and just show in "Module Verifier". Comment Actions This is causing some bots with EXPENSIVE_CHECKS enabled to fail so I reverted the patch to get the bots back to green: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/24249/testReport/junit/LLVM/CodeGen_X86/stack_protector_no_return_ll/ It looks like the StackProtector pass claims to preserve the DT, but with this patch it probably runs into a case where it is not preserved properly, hence the verification failure. Comment Actions Yes, after we enhance it, the old logic in stack protector is not enough to update the DT. Let me fix it. Thanks!
Comment Actions There's another problem with this patch. Please ensure it's fixed before recommitting. Clang hangs while compiling the following short snippet reduced from an open-source library (compiled on x86-64, linux): $ cat q.c char *texsubexpr(char *expression, char *subexpr) { char *texchar(); if (*expression) return texchar(expression, subexpr); } void __stack_chk_fail (void) { __builtin_trap (); } $ clang -O1 -w -fstack-protector-all -c q.c -o q.o |
Thanks, after we enhance the stack protector, the simple DT update is not enough to cover the DT change.