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

[Scott Constable via Phabricator via llvm-commits]

sconstab added inline comments.


================
Comment at: llvm/lib/Target/X86/ImmutableGraph.h:73
+    // The end of this Node's edges is the beginning of the next node's edges.
+    const Edge *edges_end() const { return (this + 1)->Edges; }
+    ArrayRef<Edge> edges() const {
----------------
mattdr wrote:
> sconstab wrote:
> > mattdr wrote:
> > > Seems like you also want to add a comment here that we know we will never be asked for `edges_end` for the last stored node -- that is, we know that `this + 1` always refers to a valid Node (which is presumably a dummy/sentinel)
> > Not sure I agree. I cannot think of a conventional use of this interface that would perform an operation on the sentinel.
> > ```
> > G->nodes_end().edges_end(); // invalid use of any end iterator
> > SomeNode.edges_end(); // invalid if SomeNode == G->nodes_end()
> > ```
> > That is, the way that we "know" that we will never be asked for `edges_end()` for the last stored node is that the ask itself would already violate C++ conventions.
> I believe any operation on the last `Node` in the array will end up accessing the sentinel:
> 
> ```
> Node* LastNode = G->nodes_begin() + (G->nodes_size() - 1);  // valid reference to the last node
> LastNode->edges_end();  // uses `this+1`, accessing the sentinel value in the Nodes array
> ```
> 
`G->nodes_size()` will return the size without the sentinel node, so your example should actually operate on the last data node. Right?


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

https://reviews.llvm.org/D75936