> On Aug 14, 2018, at 6:58 PM, Jason Molenda <jmole...@apple.com> wrote:
>
>
>
>> On Aug 14, 2018, at 6:39 PM, Zachary Turner <ztur...@google.com> wrote:
>>
>> Having bugs also makes the debugger harder to innovate in the future because
>> it’s, not having tests leads to having bugs, and sb api tests leads to not
>> having tests. At the end of the day, it doesn’t matter how stable the tests
>> are if there arent enough of them. There should be about 10x-20x as many
>> tests as there are currently, and that will simply never happen under the
>> current approach. If it means we need to have multiple different styles of
>> test, so be it. The problem we face right now has nothing to do with command
>> output changing, and in fact I don’t that we’ve *ever* had this problem. So
>> we should be focusing on problems we have, not problems we don’t have.
>
>
> I think we've had this discussion many times over the years, so I apologize
> for not reiterating what I've said in the past. I worked on gdb for a decade
> before this, where the entire testsuite was filecheck style tests based on
> gdb's output. It made it easy for them to write, and after a couple decades
> of tests had accumulated, it became a nightmare to change or improve any of
> gdb's commands, we all avoided it like the plague because it was so onerous.
> The tests themselves would accumulate more and more complicated regular
> expressions to handle different output that happened to be seen, so debugging
> WHY a given test was failing required an amazing amount of work.
Yep, I definitely sympathize with this issue. We have this problem of having
large, hard-to-parse pattern-matching tests in the frontends (some specific
IRGen tests come to mind...). Again, I think the solution here is more active
code review and using alternative representations of the input of interest.
It's usually possible to simplify these tests substantially by having an even
more compact, simplified dump of the internal representation to check against.
This is what we did for clang/swift's code coverage IRGen tests and it's worked
really well.
> Yes, lldb does not have these problems -- because we learned from our decades
> working on gdb, and did not repeat that mistake. To be honest, lldb is such
> a young debugger - barely a decade old, depending on how you count it, that
> ANY testsuite approach would be fine at this point. Add a couple more
> decades and we'd be back into the hole that gdb was in. {I have not worked
> on gdb in over a decade, so I don't know how their testing methodology may be
> today}
>
> It's always important to remember that lldb is first and foremost a debugger
> library. It also includes a driver program, lldb, but it is designed as a
> library and it should be tested as a library.
I broadly agree with your points here, but I'm not convinced that the only
acceptable form of testing for lldb is based on sb api tests. I think the only
way to an acceptable level of test coverage is to enable expressive testing on
compact input. Moreover I don't think that's incompatible with having
sufficient API test coverage.
vedant
>
>
>>
>> Note that it is not strictly necessary for a test to check the debuggers
>> command output. There could be a different set of commands whose only
>> purpose is to print information for the purposes of debugging. One idea
>> would be to introduce the notion of a debugger object model, where you print
>> various aspects of the debuggers state with an object like syntax. For
>> example,
>
> This was the whole point of the lit tests, wasn't it? To have a driver
> program, or driver programs, designed explicitly for filecheck, where the
> output IS API and can be relied on. There hasn't been disagreement about
> this.
>
>>
>> (lldb) p debugger.targets
>> ~/foo (running, pid: 123)
>>
>> (lldb) p debugger.targets[0].threads[0].frames[1]
>> int main(int argc=3, char **argv=0x12345678) + 0x72
>>
>> (lldb) p debugger.targets[0].threads[0].frames[1].params[0]
>> int argc=3
>>
>> (lldb) p debugger.targets[0].breakpoints
>> [1] main.cpp:72
>>
>> Etc. you can get arbitrarily granular and dxpose every detail of the
>> debuggers internal state this way, and the output is so simple that you
>> never have to worry about it changing.
>>
>> That said, I think history has shown that limiting ourselves to sb api
>> tests, despite all the theoretical benefits, leads to insufficient test
>> coverage. So while it has benefits, it also has problems for which we need a
>> better solution
>> On Tue, Aug 14, 2018 at 6:19 PM Jason Molenda via lldb-dev
>> <lldb-dev@lists.llvm.org> wrote:
>> It's more verbose, and it does mean test writers need to learn the public
>> API, but it's also much more stable and debuggable in the future. It's a
>> higher up front cost but we're paid back in being able to develop lldb more
>> quickly in the future, where our published API behaviors are being tested
>> directly, and the things that must not be broken. The lldb driver's output
>> isn't a contract, and treating it like one makes the debugger harder to
>> innovate in the future.
>>
>> It's also helpful when adding new features to ensure you've exposed the
>> feature through the API sufficiently. The first thing I thought to try when
>> writing the example below was SBFrame::IsArtificial() (see
>> SBFrame::IsInlined()) which doesn't exist. If a driver / IDE is going to
>> visually indicate artificial frames, they'll need that.
>>
>> J
>>
>>> On Aug 14, 2018, at 5:56 PM, Vedant Kumar <v...@apple.com> wrote:
>>>
>>> It'd be easy to update FileCheck tests when changing the debugger (this
>>> happens all the time in clang/swift). OTOH, the verbosity of the python API
>>> means that fewer tests get written. I see a real need to make expressive
>>> tests easier to write.
>>>
>>> vedant
>>>
>>>> On Aug 14, 2018, at 5:38 PM, Jason Molenda <jmole...@apple.com> wrote:
>>>>
>>>> I'd argue against this approach because it's exactly why the lit tests
>>>> don't run against the lldb driver -- they're hardcoding the output of the
>>>> lldb driver command into the testsuite and these will eventually make it
>>>> much more difficult to change and improve the driver as we've accumulated
>>>> this style of test.
>>>>
>>>> This is a perfect test for a normal SB API. Run to your breakpoints and
>>>> check the stack frames.
>>>>
>>>> f0 = thread.GetFrameAtIndex(0)
>>>> check that f0.GetFunctionName() == sink
>>>> check that f0.IsArtifical() == True
>>>> check that f0.GetLineEntry().GetLine() == expected line number
>>>>
>>>>
>>>> it's more verbose, but it's also much more explicit about what it's
>>>> checking, and easy to see what has changed if there is a failure.
>>>>
>>>>
>>>> J
>>>>
>>>>> On Aug 14, 2018, at 5:31 PM, Vedant Kumar via lldb-dev
>>>>> <lldb-dev@lists.llvm.org> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> I'd like to make FileCheck available within lldb inline tests, in
>>>>> addition to existing helpers like 'runCmd' and 'expect'.
>>>>>
>>>>> My motivation is that several tests I'm working on can't be made as
>>>>> rigorous as they need to be without FileCheck-style checks. In
>>>>> particular, the 'matching', 'substrs', and 'patterns' arguments to
>>>>> runCmd/expect don't allow me to verify the ordering of checked input, to
>>>>> be stringent about line numbers, or to capture & reuse snippets of text
>>>>> from the input stream.
>>>>>
>>>>> I'd curious to know if anyone else is interested or would be willing to
>>>>> review this (https://reviews.llvm.org/D50751).
>>>>>
>>>>> Here's an example of an inline test which benefits from FileCheck-style
>>>>> checking. This test is trying to check that certain frames appear in a
>>>>> backtrace when stopped inside of the "sink" function. Notice that without
>>>>> FileCheck, it's not possible to verify the order in which frames are
>>>>> printed, and that dealing with line numbers would be cumbersome.
>>>>>
>>>>> ```
>>>>> ---
>>>>> a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
>>>>> +++
>>>>> b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
>>>>> @@ -9,16 +9,21 @@
>>>>>
>>>>> volatile int x;
>>>>>
>>>>> +// CHECK: frame #0: {{.*}}sink() at main.cpp:[[@LINE+2]] [opt]
>>>>> void __attribute__((noinline)) sink() {
>>>>> - x++; //% self.expect("bt", substrs = ['main', 'func1', 'func2',
>>>>> 'func3', 'sink'])
>>>>> + x++; //% self.filecheck("bt", "main.cpp")
>>>>> }
>>>>>
>>>>> +// CHECK-NEXT: frame #1: {{.*}}func3() {{.*}}[opt] [artificial]
>>>>> void __attribute__((noinline)) func3() { sink(); /* tail */ }
>>>>>
>>>>> +// CHECK-NEXT: frame #2: {{.*}}func2() at main.cpp:[[@LINE+1]] [opt]
>>>>> void __attribute__((disable_tail_calls, noinline)) func2() { func3(); /*
>>>>> regular */ }
>>>>>
>>>>> +// CHECK-NEXT: frame #3: {{.*}}func1() {{.*}}[opt] [artificial]
>>>>> void __attribute__((noinline)) func1() { func2(); /* tail */ }
>>>>>
>>>>> +// CHECK-NEXT: frame #4: {{.*}}main at main.cpp:[[@LINE+2]] [opt]
>>>>> int __attribute__((disable_tail_calls)) main() {
>>>>> func1(); /* regular */
>>>>> return 0;
>>>>> ```
>>>>>
>>>>> For reference, here's the output of the "bt" command:
>>>>>
>>>>> ```
>>>>> runCmd: bt
>>>>> output: * thread #1, queue = 'com.apple.main-thread', stop reason =
>>>>> breakpoint 1.1
>>>>> * frame #0: 0x000000010c6a6f64 a.out`sink() at main.cpp:14 [opt]
>>>>> frame #1: 0x000000010c6a6f70 a.out`func3() at main.cpp:15 [opt]
>>>>> [artificial]
>>>>> frame #2: 0x000000010c6a6f89 a.out`func2() at main.cpp:21 [opt]
>>>>> frame #3: 0x000000010c6a6f90 a.out`func1() at main.cpp:21 [opt]
>>>>> [artificial]
>>>>> frame #4: 0x000000010c6a6fa9 a.out`main at main.cpp:28 [opt]
>>>>> ```
>>>>>
>>>>> thanks,
>>>>> vedant
>>>>> _______________________________________________
>>>>> lldb-dev mailing list
>>>>> lldb-dev@lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>>>
>>>
>>
>> _______________________________________________
>> lldb-dev mailing list
>> lldb-dev@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
