[PATCH] D79910: [x86][seses] Add clang flag; Use lvi-cfi with seses

2020-05-20 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added a comment. Took a quick look and seems sane -- will look after Craig's comment is addressed and build is passing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79910/new/ https://reviews.llvm.org/D79910 _

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

2020-05-13 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added inline comments. Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:248-249 + def warn_slh_does_not_support_gcc_asm_goto : Warning< +"speculative load hardening does not support use of GCC asm goto. asm goto " +"detected with SLH">, InGroup>

[PATCH] D79743: [clang][asm goto][slh] Warn if asm goto + SLH

2020-05-13 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added a comment. Not sure we want to move this into the Parser -- SLH is a property of code generation, and I think it's possible (through LTO?) to mismatch the flags between parse and codegen. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-05-12 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added a comment. I'm no expert in the pass manager, but given the very targeted applicability of LVI it definitely seems like our goal should be 0% impact for the vast majority of compilations that don't concern themselves with it. Is there a way to require the pass be enabled for the co

[PATCH] D79733: [clang][SLH] Add __has_feature(speculative_load_hardening)

2020-05-11 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added subscribers: rsmith, mattdr. mattdr added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:369 +BENIGN_LANGOPT(SpeculativeLoadHardeningEnabled, 1, 0, + "Speculative load hardening enabled") I've read the descript

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-05-08 Thread Matthew Riley via Phabricator via cfe-commits
mattdr accepted this revision. mattdr marked 2 inline comments as done. mattdr added a comment. The extra comments and the new variable name are all helpful. Thanks again. Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:321-322 + for (auto UseID

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-05-07 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added a comment. Calling special attention to the comment at line 341, since I think it affects the correctness of the pass. Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:287 + // The `Transmitters` map memoizes transmitters found for each def.

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-04-14 Thread Matthew Riley via Phabricator via cfe-commits
mattdr marked an inline comment as done. mattdr added inline comments. Comment at: llvm/lib/Target/X86/ImmutableGraph.h:366 +std::vector TrimmedNodes(TrimNodes.size()); +for (size_type I = 0; I < TrimNodes.size(); ++I) { + TrimmedNodes[I] = TrimmedNodesSoFar; ---

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-04-13 Thread Matthew Riley via Phabricator via cfe-commits
mattdr accepted this revision. mattdr added a comment. This revision is now accepted and ready to land. Accepting modulo some comments to be addressed. Most of my review effort was spent on the data structure and algorithm employed as well as code style and readability. I am least confident abo

[PATCH] D75934: Add Indirect Thunk Support to X86 to mitigate Load Value Injection (LVI) [2/6]

2020-04-10 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added a comment. I accidentally spent time reviewing this, only to cross-reference something in the LLVM code and find another diff (https://reviews.llvm.org/D76812) was written to answer Zola's request and has already been submitted. CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-04-10 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added a comment. @craig.topper can you please update/rebase this stack to remove the early LVI CFI work that ended up landed in https://reviews.llvm.org/D76812 instead? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75936/new/ https://reviews.llvm.org/D75936 __

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-04-07 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added a comment. I'll wait for current comments to be addressed before doing my next round here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75936/new/ https://reviews.llvm.org/D75936 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-04-07 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added a comment. Some more comments. FWIW, I'm doing rounds of review as I can in some evening quiet or during my son's nap. This is a huge change and it's really hard to get any part of it into my head at once in a reasonable amount of time. Comment at: llvm/lib/Targe

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-04-07 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added inline comments. Comment at: llvm/lib/Target/X86/ImmutableGraph.h:318 +} +auto *VertexArray = new Node[VertexSize + 1 /* terminator node */]; +auto *EdgeArray = new Edge[EdgeSize]; sconstab wrote: > mattdr wrote: > > sconstab wrote: > > >

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-04-04 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added inline comments. Comment at: llvm/lib/Target/X86/ImmutableGraph.h:318 +} +auto *VertexArray = new Node[VertexSize + 1 /* terminator node */]; +auto *EdgeArray = new Edge[EdgeSize]; sconstab wrote: > mattdr wrote: > > As a general rule `ne

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-04-03 Thread Matthew Riley via Phabricator via cfe-commits
mattdr added a comment. Adding a few early style notes for the next round, but overall echo @chandlerc that this seems significantly outside of normal LLVM code. Comment at: llvm/lib/Target/X86/ImmutableGraph.h:26 + +#ifndef IMMUTABLEGRAPH_H +#define IMMUTABLEGRAPH_H -