[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3009-3010 + + if (!EffectiveTriple.isOSLinux()) +return; + emaste wrote: > Is there anything OS-dependent here? > > I plan to add `EffectiveTriple.isOSFreeBSD()` to

[PATCH] D68720: Support -fstack-clash-protection for x86

emaste added inline comments. Herald added subscribers: dexonsmith, pengfei. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3009-3010 + + if (!EffectiveTriple.isOSLinux()) +return; + Is there anything OS-dependent here? I plan to add

[PATCH] D68720: Support -fstack-clash-protection for x86

kees added a comment. Ah! Yes, I see it now. Thanks and sorry for the noise! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list

[PATCH] D68720: Support -fstack-clash-protection for x86

efriedma closed this revision. efriedma added a comment. It was reverted, then re-landed. It's in trunk now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___

[PATCH] D68720: Support -fstack-clash-protection for x86

kees reopened this revision. kees added a comment. This revision is now accepted and ready to land. Sorry if I missed something here, but why is this marked as "Closed"? It seems like the feature has still not landed (i.e. it got reverted). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D68720: Support -fstack-clash-protection for x86

sberg added a comment. I filed https://bugs.llvm.org/show_bug.cgi?id=47139 "Misspelled -fnostack-clash-protection" now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720

[PATCH] D68720: Support -fstack-clash-protection for x86

xbolva00 added inline comments. Comment at: clang/include/clang/Driver/Options.td:1778 + HelpText<"Enable stack clash protection">; +def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, Group, + HelpText<"Disable stack clash protection">;

[PATCH] D68720: Support -fstack-clash-protection for x86

sberg added inline comments. Herald added a subscriber: dang. Comment at: clang/include/clang/Driver/Options.td:1778 + HelpText<"Enable stack clash protection">; +def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, Group, + HelpText<"Disable stack clash

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. For the record: tests have been updated, option handling cleaned up and expansive check failure fixed in commit e67cbac81211d40332a79d98c9d5953624cc1202 . Repository: rG LLVM Github

Re: [PATCH] D68720: Support -fstack-clash-protection for x86

On Sat, Feb 08, 2020 at 01:26:03PM +, Simon Pilgrim via Phabricator wrote: > RKSimon added a comment. > > @serge-sans-paille Please can you fix the issues with EXPENSIVE_CHECKS > builds: > http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-debian/builds/2604 yeah, spotted

[PATCH] D68720: Support -fstack-clash-protection for x86

RKSimon added a comment. @serge-sans-paille Please can you fix the issues with EXPENSIVE_CHECKS builds: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-debian/builds/2604 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

Re: [PATCH] D68720: Support -fstack-clash-protection for x86

On Fri, Feb 07, 2020 at 07:12:39PM +, Nico Weber via Phabricator wrote: > thakis added a comment. > > This breaks check-clang on mac and win: > http://45.33.8.238/mac/7485/step_7.txt http://45.33.8.238/win/7753/step_7.txt > > Please revert and then investigate asynchronously, unless the fix

[PATCH] D68720: Support -fstack-clash-protection for x86

vvereschaka added a comment. Also got failed clang tests on Arm/Aarch64 builders http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/4295 http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64/builds/4277 - FAIL: Clang::stack-clash-protection.c - FAIL:

[PATCH] D68720: Support -fstack-clash-protection for x86

thakis added a comment. Reverted in b03c3d8c620 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720

[PATCH] D68720: Support -fstack-clash-protection for x86

thakis added a comment. This breaks check-clang on mac and win: http://45.33.8.238/mac/7485/step_7.txt http://45.33.8.238/win/7753/step_7.txt Please revert and then investigate asynchronously, unless the fix is obvious. Comment at:

[PATCH] D68720: Support -fstack-clash-protection for x86

hans added a comment. This seems to be breaking the buildbots, e.g. http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/33475/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Astack-clash-protection.c

[PATCH] D68720: Support -fstack-clash-protection for x86

This revision was automatically updated to reflect the committed changes. Closed by commit rG39f50da2a357: Support -fstack-clash-protection for x86 (authored by serge-sans-paille). Changed prior to commit: https://reviews.llvm.org/D68720?vs=240844=243101#toc Repository: rG LLVM Github

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a subscriber: hans. serge-sans-paille added a comment. I would love too, @hans what do you think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720

[PATCH] D68720: Support -fstack-clash-protection for x86

sylvestre.ledru added a comment. In D68720#1862050 , @xbolva00 wrote: > Any plans to merge this feature to 10 release? I would love to have it in -10 for Firefox ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

xbolva00 added a comment. Any plans to merge this feature to 10 release? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list

[PATCH] D68720: Support -fstack-clash-protection for x86

craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @craig.topper up :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68720: Support -fstack-clash-protection for x86

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 62269 tests passed, 0 failed and 827 were skipped. {icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 16 warnings

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 240844. serge-sans-paille added a comment. Make some tests more portable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 Files:

[PATCH] D68720: Support -fstack-clash-protection for x86

merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 62059 tests passed, 1 failed and 783 were skipped. failed: Clang.CodeGen/stack-clash-protection.c {icon question-circle color=gray} clang-tidy: unknown. {icon times-circle color=red} clang-format: fail. Please

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 239257. serge-sans-paille added a comment. Update warning category Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 Files:

[PATCH] D68720: Support -fstack-clash-protection for x86

merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 62035 tests passed, 2 failed and 783 were skipped. failed: Clang.CodeGen/stack-clash-protection.c failed: Clang.Misc/warning-flags.c {icon question-circle color=gray} clang-tidy: unknown. {icon times-circle

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 239166. serge-sans-paille added a comment. - Harmonize esp/rsp usage as hinted by @craig.topper - Fix argument detection Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/

[PATCH] D68720: Support -fstack-clash-protection for x86

craig.topper added inline comments. Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31080 + + BuildMI(blockMBB, DL, TII->get(IsLP64 ? X86::SUB64ri32 : X86::SUB32ri), + physSPReg) serge-sans-paille wrote: > craig.topper wrote: > > This uses

[PATCH] D68720: Support -fstack-clash-protection for x86

merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 61960 tests passed, 3 failed and 783 were skipped. failed: Clang.CodeGen/stack-clash-protection.c failed: Clang.Driver/stack-clash-protection.c failed: Clang.Misc/warning-flags.c {icon question-circle

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 238758. serge-sans-paille marked 5 inline comments as done. serge-sans-paille added a comment. Take into account some of the reviews Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille marked 2 inline comments as done. serge-sans-paille added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2910 + + if (Arg *A = Args.getLastArg(options::OPT_fnostack_clash_protection, +

[PATCH] D68720: Support -fstack-clash-protection for x86

craig.topper added inline comments. Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:243 + + def warn_fe_stack_clash_protection_inline_asm : Warning< + "Unable to protect inline asm that clobbers stack pointer against stack clash">; Remove

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @craig.topper up? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 237486. serge-sans-paille added a comment. - update warn_fe_stack_clash_protection_inline_asm location. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille marked 2 inline comments as done. serge-sans-paille added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:2254 +CGM.getDiags().Report(S.getAsmLoc(), + diag::warn_fe_stack_clash_protection_inline_asm); + }

[PATCH] D68720: Support -fstack-clash-protection for x86

craig.topper added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:2254 +CGM.getDiags().Report(S.getAsmLoc(), + diag::warn_fe_stack_clash_protection_inline_asm); + } serge-sans-paille wrote: > craig.topper

[PATCH] D68720: Support -fstack-clash-protection for x86

sylvestre.ledru added a comment. @craig.topper do you think there is a chance that this change could be part of clang-10 ? Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille marked 9 inline comments as done. serge-sans-paille added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:2254 +CGM.getDiags().Report(S.getAsmLoc(), + diag::warn_fe_stack_clash_protection_inline_asm); + }

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 237298. serge-sans-paille added a comment. Take review into account. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 Files:

[PATCH] D68720: Support -fstack-clash-protection for x86

tstellar added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2575 + default: +return; + case llvm::Triple::ArchType::x86: Do we need to warn (or error) here for arches that don't implement -fstack-clash-protection? Repository: rG

[PATCH] D68720: Support -fstack-clash-protection for x86

craig.topper added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:1903 +.. option:: -fstack-clash-protection + Probably need a -fno-stack-class-protection as well. Looks like gcc has it. You'll need to update the handling in the driver

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @rnk up? the mozilla folks confirmed there's no significant regression (see https://bugzilla.mozilla.org/show_bug.cgi?id=1588710#c12). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @sylvestre.ledru backport for release/9.x branch: https://sergesanspaille.fedorapeople.org/0001-Stack-clash-mir-attempt.patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. For the record : this validates fine on OSX, Linux and Windows, using the github actions setup by @tstellar https://github.com/serge-sans-paille/llvm-project/pull/3 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. In D68720#1755546 , @xbolva00 wrote: > https://reviews.llvm.org/rG397fa687691876de9ff0fbaaf0def3ac5a48899c > > Commited? No, still waiting for @rnk feedback. This commit is just me using the wrong remote when pushing

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 230494. serge-sans-paille added a comment. Added extra test for arch support (warns if the flag is used on unsupported targets, and checks it's effectively unused), cc @sylvestre.ledru Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D68720: Support -fstack-clash-protection for x86

xbolva00 added a comment. https://reviews.llvm.org/rG397fa687691876de9ff0fbaaf0def3ac5a48899c Commited? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @rnk : up ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68720: Support -fstack-clash-protection for x86

sylvestre.ledru added a comment. In D68720#1733839 , @serge-sans-paille wrote: > @sylvestre.ledru did the testing and benchmarking on firefox (see > https://bugzilla.mozilla.org/show_bug.cgi?id=1588710#c12), everything seems > ok, let's move forward?

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @sylvestre.ledru did the testing and benchmarking on firefox (see https://bugzilla.mozilla.org/show_bug.cgi?id=1588710#c12), everything seems ok, let's move forward? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @sylvestre.ledru : quick backport for llvm9: https://sergesanspaille.fedorapeople.org/0001-Stack-clash-mir-attempt.patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @rnk what's your take on that version? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 227194. serge-sans-paille edited the summary of this revision. serge-sans-paille added a comment. Moved to a simpler, easier to review, less error prone implementation. Validates just fines on llvm bootstrap an cpython code base. @sylvestre.ledru

[PATCH] D68720: Support -fstack-clash-protection for x86

sylvestre.ledru added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:1901 + +Instrument stack allocation to prevent stack clash attacks. + Maybe add that it is Linux only? :) CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 226109. serge-sans-paille added a comment. Temporarily comment out call support as free probe, everything else passes validation but a call may have some stack effect I don't handle (yet). CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Some benchmark / instrumentation report: due to the way memory moves are ordered in the entry block, there tend to be relatively few free probes between two stack growth within a function, and a large number after the last stack growth. When recompiling llc,

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 226043. serge-sans-paille added a comment. Update documentation as suggested by @sylvestre.ledru Corner case when a write was touching memory beyond the allocated stack. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 226002. serge-sans-paille added a comment. Better integration with MachineInstr description Handle stacks with multiple stack objects More test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

sylvestre.ledru added a comment. You should also probably document the arg in clang/docs/ClangCommandLineReference.rst we have -fstack-protector-strong & co in the doc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 225580. serge-sans-paille added a comment. Explicilty prefer existing mechanism for windows. Split loop/block allocation strategy to different functions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Extra note: an older version of the patch has been tested by the firefox team without much performance impact, (and no test failure), see https://bugzilla.mozilla.org/show_bug.cgi?id=1588710 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 225462. serge-sans-paille added a comment. Moved the implementation to a specialization of `emitStackProbeInline` that used to be Windows-centric. Provide a generic implementation that generates inline assembly instead. that way we don't clutter

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 225049. serge-sans-paille added a comment. Get rid of static mapping + update test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 Files:

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Another test run on an x264 encoder (source: https://openbenchmarking.org/test/pts/x264) Compiled with -O2 and with or without -fstack-clash-protection; Run without threads (`x265 --pools 1 -F 1 ./Bosphorus_1920x1080_120fps_420_8bit_YUV.y4m /dev/null`)

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. > For maintenance reasons, I'd really prefer it if we could find a way to reuse > the existing code that calls an external stack probe function. What do you > think about taking a look at X86RetpolineThunks.cpp and doing something > similar to that?

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Some benchmarks, running the python performance suite from recompiled cpython build, one built with `-O2` (baseline) and one built with `-O2 -fstack-clash-protection` (protection). Surprisingly enough, the stack protection makes the code faster in various

[PATCH] D68720: Support -fstack-clash-protection for x86

rnk added a comment. For maintenance reasons, I'd really prefer it if we could find a way to reuse the existing code that calls an external stack probe function. What do you think about taking a look at X86RetpolineThunks.cpp and doing something similar to that? Basically, when the user sets

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 224606. serge-sans-paille added a comment. Ensure the distance between two probes is at max PAGE_SIZE. Use Calls as free probes. Fix alignment for dynamic alloca This passes the llvm-test suite, and thanks to the use of calls, no inserted probe

[PATCH] D68720: Support -fstack-clash-protection for x86

tstellar added inline comments. Comment at: clang/test/CodeGen/stack-clash-protection.c:3 +// RUN: %clang -target x86_64 -o %t.out %s -fstack-clash-protection && %t.out + +#include There were concerns[1] raised recently about adding clang tests that were

[PATCH] D68720: Support -fstack-clash-protection for x86

efriedma added inline comments. Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:423 + AbsOffset - CurrentAbsOffset + PageSize); +if (FreeProbeIterator != MBB.end()) { + NumFrameFreeProbe++; Each probe has to

[PATCH] D68720: Support -fstack-clash-protection for x86

xbolva00 added inline comments. Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:479 +} +if (std::any_of(MI.operands_begin(), MI.operands_end(), +[](MachineOperand ) { return MO.isFI(); })) { nit: llvm::any_of Repository: rG

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. Some early stats: on the sqlite amalgamation [0], the free probe reuse allows to skip 123 out of the 474 probes needed during frame lowering. [0] https://www.sqlite.org/download.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 224431. serge-sans-paille edited the summary of this revision. serge-sans-paille added a comment. Added test case, statistics and refactor interactions with existing stack probing mechanism. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille marked 3 inline comments as done. serge-sans-paille added inline comments. Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:400 +!(STI.isOSWindows() && !STI.isTargetMachO()); +if (InlineStackClashProtector && !InEpilogue) { + const uint64_t

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a subscriber: eli.friedman. serge-sans-paille added a comment. @efriedma the free probe algorithm requires more testing, and I'd like to take into account memset and memcpy as free probes too. To showcase this algorithm, consider the following LLVM bitcode: define

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 224254. serge-sans-paille added a comment. Added documentation + release note entry Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 Files:

[PATCH] D68720: Support -fstack-clash-protection for x86

xbolva00 added a comment. Please add info about this new feature to release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list

[PATCH] D68720: Support -fstack-clash-protection for x86

efriedma added a comment. > (b) is an issue, as pointed out in https://lwn.net/Articles/726587/ (grep for > valgrind) : from valgrind point of view, accessing un-allocated stack memory > triggers error, and we probably want to please valgrind > > Doing the call *after* the stack allocation is

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @efriedma alos compared to `probe-stack` with a function, this version has the ability to use existing MOV operations to avoid generating probes, which looks like a big plus to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille updated this revision to Diff 224196. serge-sans-paille added a comment. Move to `stack-probe` compatibility, using a dedicated name to trigger inline assembly. It looks better to me because 1. it leverage existing mechanics 2. it has a finer grain Repository: rG LLVM

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille added a comment. @efriedma : there's indeed an intersection with the `probe-stack` attribute. The `probe-stack` attribute (a) forces a function call, and (b) this function call only happens **before** the stack gets expanded. (a) is probably a performance issue in several

[PATCH] D68720: Support -fstack-clash-protection for x86

efriedma added a comment. Sorry, I meant the "probe-stack" attribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list

[PATCH] D68720: Support -fstack-clash-protection for x86

efriedma added a comment. Is there some reason this isn't using the existing stack-probe-size attribute? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___

[PATCH] D68720: Support -fstack-clash-protection for x86

serge-sans-paille created this revision. Herald added subscribers: llvm-commits, cfe-commits, hiraditya, dschuff. Herald added projects: clang, LLVM. Implement protection against the stack clash attack [0]. Probe stack allocation every PAGE_SIZE during frame lowering or dynamic allocation to