[all-commits] [llvm/llvm-project] c2b01c: Make ValueObjectPrinter's handling of its ValueObj...

[jimingham via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: c2b01c87dcc3ab59e7d466cbec795310a3d43fde
      https://github.com/llvm/llvm-project/commit/c2b01c87dcc3ab59e7d466cbec795310a3d43fde
  Author: jimingham <jingham at apple.com>
  Date:   2024-02-12 (Mon, 12 Feb 2024)

  Changed paths:
    M lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
    M lldb/source/Commands/CommandObjectFrame.cpp
    M lldb/source/Core/ValueObject.cpp
    M lldb/source/DataFormatters/TypeSummary.cpp
    M lldb/source/DataFormatters/ValueObjectPrinter.cpp

  Log Message:
  -----------
  Make ValueObjectPrinter's handling of its ValueObject pointers more principled (NFC) (#81314)

I get a small but fairly steady stream of crash reports which I can only
explain by ValueObjectPrinter trying to access its m_valobj field, and
finding it NULL. I have never been able to reproduce any of these, and
the reports show a state too long after the fact to know what went
wrong.

I've read through this section of lldb a bunch of times trying to figure
out how this could happen, but haven't ever found anything actually
wrong that could cause this. OTOH, ValueObjectPrinter is somewhat sloppy
about how it handles the ValueObject it is printing.

a) lldb allows you to make a ValueObjectPrinter with a Null incoming
ValueObject. However, there's no affordance to set the ValueObject in
the Printer after the fact, and it doesn't really make sense to do that.
So I change the ValueObjectPrinter API's to take a ValueObject
reference, rather than a pointer. All the places that make
ValueObjectPrinters already check the non-null status of their
ValueObject's before making the ValueObjectPrinter, so sadly, I didn't
find the bug, but this will enforce the intent.
b) The next step in printing the ValueObject is deciding which of the
associated DynamicValue/SyntheticValue we are actually printing (based
on the use_dynamic and use_synthetic settings in the original
ValueObject. This was put in a pointer by GetMostSpecializedValue, but
most of the printer code just accessed the pointer, and it was hard to
reason out whether we were guaranteed to always call this before using
m_valobj. So far as I could see we always do (sigh, didn't find the bug
there either) but this was way too hard to reason about.

In fact, we figure out once which ValueObject we're going to print and
don't change that through the life of the printer. So I changed this to
both set the "most specialized value" in the constructor, and then to
always access it through GetMostSpecializedValue(). That makes it easier
to reason about the use of this ValueObject as well.

This is an NFC change, all it does is make the code easier to reason
about.