clayborg added inline comments.
================
Comment at: lldb/include/lldb/Symbol/Symbol.h:23
+struct JSONSymbol {
+ uint64_t value;
+ std::optional<uint64_t> size;
----------------
JDevlieghere wrote:
> clayborg wrote:
> > clayborg wrote:
> > > JDevlieghere wrote:
> > > > clayborg wrote:
> > > > > Do we something that says "value is an address"? Or are we inferring
> > > > > that from the lldb::SymbolType?
> > > > In the Symbolc constructor that takes a JSONSymbol, I assume the value
> > > > is an address if there's no section_id and an offset otherwise. We can
> > > > definitely tweak that or add a sanity check based on the symbol type.
> > > Remember there are some symbols that are not addresses. Think of trying
> > > to create a N_OSO symbol where the value isn't an address, but it is a
> > > time stamp integer.
> > >
> > > Symbols in the symbol table for mach-o files can have a section ID that
> > > is valid and the value is not an offset, it is still an address. Not sure
> > > if that matters.
> > >
> > > One way to make this clear is if we have a "section_id" then "value" is
> > > an address, and if not, then "value" is just an integer? It does require
> > > that we always specify section IDs though. Also, section IDs are not
> > > human readable, we could change "std::optional<uint64_t> section_id;" to
> > > be "std::optional<std::string> section;" and thid becomes the fully
> > > qualified section name like "__TEXT.__text" or "PT_LOAD[0]..text". Much
> > > more readable. And easy to dig up the section from the Module's section
> > > list.
> > Another way to do this would be to have JSONSymbol know if it has an
> > address or just an integer value by defining this as:
> > ```
> > std::optional<uint64_t> address;
> > std::optional<uint64_t> value;
> > ```
> > The deserializer would verify that only one of these is valid and fail if
> > both or neither were specified
> Sure, that sounds reasonable.
And we can add tests that verify we get an error back if neither or both are
specified.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145180/new/
https://reviews.llvm.org/D145180
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
