[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.


================
Comment at: lit/Modules/compressed-sections.yaml:20
+  - Name:            .bogus
+# CHECK-NOT: .bogus
+    Type:            SHT_PROGBITS
----------------
labath wrote:
> zturner wrote:
> > labath wrote:
> > > zturner wrote:
> > > > You should probably put this as the very first check statement.  Each successfully matching `CHECK` line will update an internal position and subsequent checks will only start from that position, so here you're only checking that after `.bogus` does not occur after `.hello_elf`, but this test would pass if `.bogus` occurred before `.hello_elf`.  But putting the `CHECK-NOT` first, both will fail  (this is also a good reason not to intersperse the check lines).
> > > Putting CHECK-NOT first will just make sure that .bogus does not appear *before* the first CHECK match. I put it last as it this is the place it is likely to be if it did we did end up outputting it, but if we want to be safe, I guess we have two options:
> > > - put it both at the front *and* back
> > > - have two FileCheck invocations
> > > I chose the latter.
> > I don't believe this is correct, and if it is then someone has introduced a bug in `FileCheck`.  matches do not succeed or fail based on what check lines come after.  They only succeed or fail based on the current file position.  If the file position is 0, and you say `CHECK-NOT`, then you are checking that it does not appear anywhere in the file (i.e. anywhere starting at position 0).  Assuming the test passes (i.e. it does not find it), the file position is not updated and then the CHECK line continues by making sure that it does appear.  And so on and so forth.
> I don't know what you're basing your claim on, but this behavior is consistent with FileCheck documentation here <https://llvm.org/docs/CommandGuide/FileCheck.html>:
> 
> ```
> The “CHECK-NOT:” directive is used to verify that a string doesn’t occur between two matches (or before the first match, or after the last match).
> ```
Well I guess the best way to be sure is to test it, and...  you're right.  Weird.  I've been using it wrong all this time.

I almost feel like we need a `CHECK-NOT-DAG` or something.  Anyway, your solution looks fine.


https://reviews.llvm.org/D40616