Page MenuHomePhabricator

[libstdc++ data-formatters] Remove size limits and loop detector.
AbandonedPublic

Authored by sivachandra on Oct 12 2015, 3:06 PM.

Details

Reviewers
granata.enrico

Diff Detail

Event Timeline

sivachandra retitled this revision from to [libstdc++ data-formatters] Remove size limits and loop detector..
sivachandra updated this object.
sivachandra added a reviewer: granata.enrico.
sivachandra added a subscriber: lldb-commits.

Why are you removing the loop detection?

As for the size capping, yes, if you remove the loop detection it will be less of a problem because downstream commands (e.g. frame variable) will actually cap how many elements get fetched unless the user overrides that determination (or the API is used directly to retrieve values)

How can there be loops in an std::list?

  1. If the implementation is flawed.
  2. If someone messes with its implementation.

Should we protect for such cases at all? If #1, don't use that library in the first place. If #2, that is an unsupported use case :)

Possibility number 3 (and the true reason why the check is there): if you stop at a place where the variable is not fully initialized/being torn down, and as a result, something is pointing back inside the list. For a list traversal, that is a deadly outcome.

This is (partially) the same reason why there's a cap on string summaries. It's fairly unlikely that a user will truly have a 3GB string (if they did, we might still want to cap it, but it's unlikely to begin with): an uninitialized string that claims to have a 3GB buffer.

I think there are ways to make this code smarter. For instance, if you have to get node #10, you could only check for a loop up until node #10, and so on..
If you store the state of the loop-checker as you get out, you might get away without having to check the same nodes multiple times as you progress deeper into the list

Drawback is that you would have already handed out at least some data by the time you find out the list is flawed. You can decide for yourself if that is a problem or not (the list might be partially valid, and returning some of the data is actually a good thing, maybe?)

I don't know about the std::list implementation in specific, but in the debugger you have to be aware of the possibility that you are looking at an entity that is in the middle of being modified. Note, it may not be obvious to the user that that is being done since it may be happening on another thread. Anyway, depending on how that is done the entity might be in a weird or weirder state. So it is good never to allow yourself to run off into the weeds if you can easily avoid it.

Jim

Is eliminating the size limits alone acceptable then?

Removing the size limit would be acceptable, yes

It is a remnant of a time when LLDB lacked ways to actually cap data at the command-line level and the Xcode UI did not do lazy fetching of values

Given that these things now work properly, the ability to generate *all* children for large lists should be safe

sivachandra abandoned this revision.Oct 12 2015, 4:58 PM

Abandoning this in favor of http://reviews.llvm.org/D13682. It was easy to create a new diff than to edit the existing one.