Page MenuHomePhabricator

Remove `bugreport` command
ClosedPublic

Authored by JDevlieghere on Jul 30 2019, 12:12 PM.

Details

Summary

The bugreport command exists to create domain-specific bug reports. Currently it has one implementation for filing bugs on the unwinder. As far as we can tell, it has never been of use. Although not exactly the same as the reproducers, it's a bit confusing to have two parallel command trees for (kind of) the same thing.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

JDevlieghere created this revision.Jul 30 2019, 12:12 PM
JDevlieghere added a reviewer: Restricted Project.Jul 30 2019, 12:16 PM
davide added a subscriber: davide.Jul 30 2019, 12:22 PM

I wanted to remove it a while ago, but Jason told me he found this useful, so I would wait for his opinion.

Other than that, I have no objections.

IMHO: we should keep this command and expand its abilities to help report stepping issues, expression issues and more. Why? Getting good bug reports from users is quite hard. Allowing them to type "bugreport step --step-in" or "bugreport step --step-over" would be really nice. Right now I send people a python script that enables logging, dumps the code in and around the source line with disassembly, and creates a zip file with the current object file and optional debug info file (dSYM or external symbol file).

Happy to hear what others have to say though.

IMHO: we should keep this command and expand its abilities to help report stepping issues, expression issues and more. Why? Getting good bug reports from users is quite hard. Allowing them to type "bugreport step --step-in" or "bugreport step --step-over" would be really nice. Right now I send people a python script that enables logging, dumps the code in and around the source line with disassembly, and creates a zip file with the current object file and optional debug info file (dSYM or external symbol file).

What's the benefit of having this instead of having the user generate a reproducer?

IMHO: we should keep this command and expand its abilities to help report stepping issues, expression issues and more. Why? Getting good bug reports from users is quite hard. Allowing them to type "bugreport step --step-in" or "bugreport step --step-over" would be really nice. Right now I send people a python script that enables logging, dumps the code in and around the source line with disassembly, and creates a zip file with the current object file and optional debug info file (dSYM or external symbol file).

What's the benefit of having this instead of having the user generate a reproducer?

I am guessing the reproducer wouldn't help us with any of the user files. We often need to see the binaries, or enable some logging when the user repros the bug. Users don't often attach all of the files that are needed, nor do they know what logs to enable and how to attach them. I am not up to date on exactly what reproducers do, but I would guess that they don't save off just the required files or enable any logging and attach logs?

IMHO: we should keep this command and expand its abilities to help report stepping issues, expression issues and more. Why? Getting good bug reports from users is quite hard. Allowing them to type "bugreport step --step-in" or "bugreport step --step-over" would be really nice. Right now I send people a python script that enables logging, dumps the code in and around the source line with disassembly, and creates a zip file with the current object file and optional debug info file (dSYM or external symbol file).

What's the benefit of having this instead of having the user generate a reproducer?

I am guessing the reproducer wouldn't help us with any of the user files. We often need to see the binaries, or enable some logging when the user repros the bug. Users don't often attach all of the files that are needed, nor do they know what logs to enable and how to attach them. I am not up to date on exactly what reproducers do, but I would guess that they don't save off just the required files or enable any logging and attach logs?

That's exactly what the reproducers do actually. They capture user interaction, the GDB remote packets and any files used by the debugger. They currently don't enable logs, because you can debug LLDB during reproducer replay, but that would be trivial to include as well.

The thing the "bugreport unwind" adds is that it runs a handful of commands that gather data useful in diagnosing unwind problems. That's useful when the information you need might not be output by the session in which the bug appeared.

The same thing could be achieved by having "reproducer generate" take arguments that the user can specify to automate gathering more data that would be relevant to a particular kind of bug. So "bugreport unwind" -> "reproducer generate unwind" which would run the same extra commands that "bugreport unwind" does (the results will get captured by the same mechanism that's catching all the other session output) and then the reproducer would be generated.

If we don't want to forget that there was specific useful info, we could gate this patch on implementing the "reproducer generate <FEATURE>" feature. That should be easy to add if we're clear this this is the right design for reproducers. But I don't think that "bugreport unwind" was all that much used, so it might be better to design this at leisure.

If we don't want to forget that there was specific useful info, we could gate this patch on implementing the "reproducer generate <FEATURE>" feature. That should be easy to add if we're clear this this is the right design for reproducers. But I don't think that "bugreport unwind" was all that much used, so it might be better to design this at leisure.

Until the reproducers prove to be insufficient to capture a particular bug, I don't plan to invest in this.

What's the state of this? Because thi is in my review queue and it has no associated test, so either this moves forward and we remove bugreport or we keep bugreport and I'll put it on my list of things that need tests :)

jasonmolenda accepted this revision.Sep 5 2019, 2:12 PM

We don't need to keep this command around. It was originated by Tamas in 2015,
https://reviews.llvm.org/D10868
but never got used for anything - Tamas was working on unwind things back then iirc, and may it looks like he thought it would be useful for that. On Darwin systems we do ship with a python command for diagnosing unwind problems ("script import lldb.diagnose"; "diagnose-unwind") which can be used when the current backtrace looks like it may be missing stack frames, or truncate early. The unwinder hasn't seen a lot of active development for years, so we don't have cause to use this very often.

I think removing this is fine. The reproducers will be a great way to include everything needed to repro a bug that a user sees. Not everyone can produce a bug report with all of their binaries etc, but the bugreport command suffers from the obvious problem that it can only gather state after something bad has happened. (as Jim notes in the original phab)

This revision is now accepted and ready to land.Sep 5 2019, 2:12 PM
This revision was automatically updated to reflect the committed changes.