[Lldb-commits] Improving the documentation
John Lindal via lldb-commits
lldb-commits at lists.llvm.org
Fri Mar 18 11:41:16 PDT 2016
I added a reference in the class documentation. Should I submit it via a
ticket?
Thanks!
John
On Tue, Mar 15, 2016 at 5:51 PM, Jim Ingham <jingham at apple.com> wrote:
> The only other suggestion I have is that the first time we refer to a
> module in this file
> we should say "(see SBModule)" or something like that to make it clear
> what we mean by a module.
>
> Other than that this looks good to me.
>
> Jim
>
> > On Mar 15, 2016, at 5:05 PM, John Lindal <github at newplanetsoftware.com>
> wrote:
> >
> > Great! Attached is the update.
> >
> > On Thu, Mar 10, 2016 at 6:57 PM, Jim Ingham <jingham at apple.com> wrote:
> > Few more comments...
> >
> > +/// If an address comes from an existing module, then it will be
> resolved
> > +/// into an offset from its containing section in that module. That
> way it
> > +/// can refer to the same logical location as the module that holds it
> even
> >
> > Probably my error, but "location as the module" -> "location in the
> module"
> >
> > You use the terms "file virtual address" and "load virtual address". I
> don't know what virtual means in that context.
> >
> >
> > + //------------------------------------------------------------------
> > + /// Tries to resolve the address within the target. If this fails,
> >
> > "target" -> "target's modules"? That makes it clearer what's going on.
> >
> > + /// assumes the address is absolute, e.g., on the stack or heap.
> The
> > + /// object becomes valid.
> > + ///
> > + /// @param[in] load_addr
> > + /// A new offset value for this object.
> >
> > This isn't right, it isn't the new offset value, for instance if this is
> a load address from a loaded module,
> > the SBAddress will be the containing section + offset, and load_addr !=
> SBAddress.GetOffset(). This is just the
> > load address that will be resolved.
> >
> > + ///
> > + /// @param[in] target
> > + /// The target within which the offset is valid.
> >
> > "The target within which the load address will be resolved" is better.
> This will always return a valid SBAddress, it
> > just might or might not be a section-relative one.
> >
> > + //------------------------------------------------------------------
> > void
> > SetLoadAddress (lldb::addr_t load_addr,
> > lldb::SBTarget &target);
> >
> > And "The target within which the offset is valid." -> "The target within
> which the load address will be resolved"?
> >
> >
> > + //------------------------------------------------------------------
> > + /// Set the offset for this address, relative to the current
> section,
> > + /// if any.
> > + ///
> > + /// @param[in] offset
> > + /// A new offset value for this object.
> > + //------------------------------------------------------------------
> > + // FIXME: Should this be SetOffsetAddress?
> > bool
> > OffsetAddress (addr_t offset);
> >
> > This call actually slides the SBAddress, so new_offset = old_offset +
> offset
> >
> >
> > > On Mar 10, 2016, at 10:46 AM, John Lindal <
> github at newplanetsoftware.com> wrote:
> > >
> > > Thanks for your patience and feedback! Attached is the updated file.
> > >
> > > John
> > >
> > > On Wed, Mar 9, 2016 at 3:10 PM, Jim Ingham <jingham at apple.com> wrote:
> > > The relation between section offsets and files is stronger than you
> are stating here. You say:
> > >
> > > +/// Represents an address. An address may refer to code or data from
> an
> > > +/// existing module, or it may refer to something on the stack or
> heap.
> > > +///
> > >
> > > That part is good, but you should use that in the next paragraph, so
> instead of:
> > >
> > > +/// If an address comes from a file on disk that has section relative
> > > +/// addresses, then it has a virtual address that is relative to a
> unique
> > > +/// section in the object file. Sections get resolved at runtime by
> > > +/// DynamicLoader plug-ins as images (executables and shared
> libraries) get
> > > +/// loaded/unloaded. If a section is loaded, then the load address
> can be
> > > +/// resolved.
> > >
> > > Something like:
> > >
> > > If an address comes from an existing module, then it will be resolved
> into an offset
> > > from its containing section in that module. That way it can refer to
> the same logical
> > > location as the module that holds it is unloaded and loaded at
> different addresses.
> > > Module based SBAddresses are not bound to a particular target or
> process, but you
> > > can ask the SBAddress where/if it has been loaded in a particular
> target.
> > >
> > > I don't think you need to mention the Dynamic loader plugin here, it
> isn't essential to know who tracks the
> > > loads to understand what these do. Also, you use "resolve" in the
> rest of the docs to mean "resolve to
> > > section/offset in a Module. So using it for loading libraries here is
> confusing. Better to define it
> > > here as you are going to use it.
> > >
> > > This bit doesn't seem right to me:
> > >
> > > - // The following queries can lookup symbol information for a
> given address.
> > > - // An address might refer to code or data from an existing
> module, or it
> > > - // might refer to something on the stack or heap. The following
> functions
> > > - // will only return valid values if the address has been resolved
> to a code
> > > - // or data address using "void SBAddress::SetLoadAddress(...)" or
> > > - // "lldb::SBAddress SBTarget::ResolveLoadAddress (...)".
> > > +
> //------------------------------------------------------------------
> > > + /// Lookup symbol information for this address. This function
> will only
> > > + /// return a valid object if the address has been resolved to a
> code or
> > > + /// data address using "void SBAddress::SetLoadAddress(...)" or
> > > + /// "lldb::SBAddress SBTarget::ResolveLoadAddress (...)".
> > > + ///
> > > + /// @param[in] resolve_scope
> > > + /// lldb::SymbolContextItem value specifying the scope in
> which to
> > > + /// resolve this address.
> > > + ///
> > > + /// @return
> > > + /// lldb::SBSymbolContext with the result.
> > > +
> //------------------------------------------------------------------
> > >
> > > You are faithfully representing the original docs, but they aren't
> right. For instance, load a program that has a "main" symbol, but don't
> run it, then do:
> > >
> > > (lldb) script
> > > >>> func = lldb.target.FindFunctions("main")
> > > >>> print len(func)
> > > 1
> > > >>> func = func[0]
> > > >>> addr = func.symbol.GetStartAddress()
> > > >>> sc = addr.GetSymbolContext(lldb.eSymbolContextEverything)
> > > >>> print sc.symbol
> > > id = {0x000004df}, range = [0x0000000100018fa0-0x00000001000191f6),
> name="main"
> > >
> > > So that worked just fine even though there were no load addresses
> around anywhere. ResolveSymbolContext needs an Address that can be
> > > resolved to a Module, but that module doesn't need to be loaded...
> > >
> > > Also this bit:
> > >
> > > + /// @param[in] resolve_scope
> > > + /// lldb::SymbolContextItem value specifying the scope in
> which to
> > > + /// resolve this address.
> > > + ///
> > >
> > > isn't quite right. "The scope in which to resolve the address" makes
> it sound like this specifies some context you are using to resolve the
> address,
> > > but it really determines how much information to fetch in the symbol
> context that the function returns. There should be some general discussion
> of the
> > > SymbolContextItem (maybe where we document FindFunctions?) But in any
> case, here you can just "specifying how much information to fill in in the
> returned
> > > SymbolContext.
> > >
> > > Jim
> > >
> > >
> > >
> > > > On Mar 9, 2016, at 2:24 PM, John Lindal <
> github at newplanetsoftware.com> wrote:
> > > >
> > > > Here is a refinement of SBAddress. The patch also includes updates
> to classes referenced by SBAddress.
> > > >
> > > > Does this read better?
> > > >
> > > > Thanks,
> > > > John
> > > >
> > > > On Wed, Mar 9, 2016 at 11:56 AM, Jim Ingham <jingham at apple.com>
> wrote:
> > > > In the case of something like SBAddress, I think it would be better
> to have a class header that explains file/load addresses and section offset
> addresses. Then you can just use the terms in the function documentation,
> and they can be shorter and more explicit. Trying to define terms inline
> like this makes it harder both to define the term and to say what the
> function does. It also means it is hard to avoid redundantly redefining
> terms as they are used in different functions.
> > > >
> > > > Thanks for starting this process!
> > > >
> > > > Jim
> > > >
> > > >
> > > > > On Mar 9, 2016, at 11:47 AM, John Lindal via lldb-commits <
> lldb-commits at lists.llvm.org> wrote:
> > > > >
> > > > > Having completed my C++ app on top of lldb, I would like to
> improve the function-level documentation so others don't have to blunder
> around in the dark the way I did :)
> > > > >
> > > > > Here is a patch for SBAddress.h
> > > > >
> > > > > If this is the right direction, I will work my way through the
> rest of the SB*.h files.
> > > > >
> > > > > Thanks!
> > > > > John Lindal
> > > > >
> > > > > -----
> > > > >
> > > > > diff --git a/include/lldb/API/SBAddress.h
> b/include/lldb/API/SBAddress.h
> > > > > index 4cbbee9..b324e90 100644
> > > > > --- a/include/lldb/API/SBAddress.h
> > > > > +++ b/include/lldb/API/SBAddress.h
> > > > > @@ -33,27 +33,100 @@ public:
> > > > > const lldb::SBAddress &
> > > > > operator = (const lldb::SBAddress &rhs);
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return
> > > > > + /// true if the object is valid. If the object is
> invalid, it is
> > > > > + /// not safe to call any other methods.
> > > > > +
> //------------------------------------------------------------------
> > > > > bool
> > > > > IsValid () const;
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// Clears the address. The object is no longer valid.
> > > > > +
> //------------------------------------------------------------------
> > > > > void
> > > > > Clear ();
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// Get the file address.
> > > > > + ///
> > > > > + /// If an address comes from a file on disk that has section
> > > > > + /// relative addresses, then it has a virtual address that is
> > > > > + /// relative to a unique section in the object file.
> > > > > + ///
> > > > > + /// @return
> > > > > + /// The valid file virtual address, or
> LLDB_INVALID_ADDRESS if
> > > > > + /// the address doesn't have a file virtual address
> (image is
> > > > > + /// from memory only with no representation on disk).
> > > > > +
> //------------------------------------------------------------------
> > > > > addr_t
> > > > > GetFileAddress () const;
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// Get the load address.
> > > > > + ///
> > > > > + /// If an address comes from a file on disk that has section
> > > > > + /// relative addresses, then it has a virtual address that is
> > > > > + /// relative to a unique section in the object file. Sections
> > > > > + /// get resolved at runtime by DynamicLoader plug-ins as
> images
> > > > > + /// (executables and shared libraries) get loaded/unloaded.
> If a
> > > > > + /// section is loaded, then the load address can be resolved.
> > > > > + ///
> > > > > + /// @param[in] target
> > > > > + /// The target in which to search.
> > > > > + ///
> > > > > + /// @return
> > > > > + /// The valid load virtual address, or
> LLDB_INVALID_ADDRESS if
> > > > > + /// the address is currently not loaded.
> > > > > +
> //------------------------------------------------------------------
> > > > > addr_t
> > > > > GetLoadAddress (const lldb::SBTarget &target) const;
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// Set the section and address within the section. If it
> succeeds,
> > > > > + /// the object becomes valid.
> > > > > + ///
> > > > > + /// @param[in] section
> > > > > + /// A lldb::SBSection object to use as the section base.
> > > > > + ///
> > > > > + /// @param[in] offset
> > > > > + /// A new offset value for this object.
> > > > > +
> //------------------------------------------------------------------
> > > > > void
> > > > > SetAddress (lldb::SBSection section, lldb::addr_t offset);
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// Tries to resolve the address within the target. If this
> fails,
> > > > > + /// assumes the address is absolute, e.g., on the stack or
> heap. If it
> > > > > + /// succeeds, the object becomes valid.
> > > > > + ///
> > > > > + /// @param[in] load_addr
> > > > > + /// A new offset value for this object.
> > > > > + ///
> > > > > + /// @param[in] target
> > > > > + /// The target within which the offset is valid.
> > > > > +
> //------------------------------------------------------------------
> > > > > void
> > > > > SetLoadAddress (lldb::addr_t load_addr,
> > > > > lldb::SBTarget &target);
> > > > > +
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// Set the offset for this address, relative to the current
> section,
> > > > > + /// if any.
> > > > > + ///
> > > > > + /// @param[in] offset
> > > > > + /// A new offset value for this object.
> > > > > +
> //------------------------------------------------------------------
> > > > > + // FIXME: Should this be SetOffsetAddress?
> > > > > bool
> > > > > OffsetAddress (addr_t offset);
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// Dump a description of this object to the given
> lldb::SBStream.
> > > > > + ///
> > > > > + /// @param[in] description
> > > > > + /// The stream to which to dump the object description.
> > > > > +
> //------------------------------------------------------------------
> > > > > bool
> > > > > GetDescription (lldb::SBStream &description);
> > > > >
> > > > > @@ -63,10 +136,25 @@ public:
> > > > > // will only return valid values if the address has been
> resolved to a code
> > > > > // or data address using "void
> SBAddress::SetLoadAddress(...)" or
> > > > > // "lldb::SBAddress SBTarget::ResolveLoadAddress (...)".
> > > > > +
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// Lookup symbol information for this address. An address
> might
> > > > > + /// refer to code or data from an existing module, or it
> might refer to
> > > > > + /// something on the stack or heap. This function will only
> return
> > > > > + /// valid values if the address has been resolved to a code
> or data
> > > > > + /// address using "void SBAddress::SetLoadAddress(...)" or
> > > > > + /// "lldb::SBAddress SBTarget::ResolveLoadAddress (...)".
> > > > > + ///
> > > > > + /// @param[in] resolve_scope
> > > > > + /// lldb::SymbolContextItem value specifying the scope in
> which to
> > > > > + /// resolve this address.
> > > > > + ///
> > > > > + /// @return
> > > > > + /// lldb::SBSymbolContext with the result.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::SBSymbolContext
> > > > > GetSymbolContext (uint32_t resolve_scope);
> > > > >
> > > > > -
> > > > > // The following functions grab individual objects for a
> given address and
> > > > > // are less efficient if you want more than one symbol
> related objects.
> > > > > // Use one of the following when you want multiple debug
> symbol related
> > > > > @@ -76,30 +164,67 @@ public:
> > > > > // One or more bits from the SymbolContextItem enumerations
> can be logically
> > > > > // OR'ed together to more efficiently retrieve multiple
> symbol objects.
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return section
> > > > > + /// The lldb::SBSection containing this address.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::SBSection
> > > > > GetSection ();
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return section
> > > > > + /// The offset for this address, relative to the section.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::addr_t
> > > > > GetOffset ();
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return section
> > > > > + /// The module containing this address.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::SBModule
> > > > > GetModule ();
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return section
> > > > > + /// The compile unit containing this address.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::SBCompileUnit
> > > > > GetCompileUnit ();
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return section
> > > > > + /// The function containing this address.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::SBFunction
> > > > > GetFunction ();
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return section
> > > > > + /// The block containing this address.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::SBBlock
> > > > > GetBlock ();
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return section
> > > > > + /// The symbol at this address.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::SBSymbol
> > > > > GetSymbol ();
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return section
> > > > > + /// The lldb::SBLineEntry specifying the file location
> that
> > > > > + /// corresponds to this address.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::SBLineEntry
> > > > > GetLineEntry ();
> > > > >
> > > > > +
> //------------------------------------------------------------------
> > > > > + /// @return section
> > > > > + /// The classification for this address.
> > > > > +
> //------------------------------------------------------------------
> > > > > lldb::AddressClass
> > > > > GetAddressClass ();
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > lldb-commits mailing list
> > > > > lldb-commits at lists.llvm.org
> > > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> > > >
> > > >
> > > > <diff>
> > >
> > >
> > > <diff>
> >
> >
> > <SBAddress.h>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160318/53bc7613/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SBAddress.h
Type: text/x-chdr
Size: 9446 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160318/53bc7613/attachment-0001.h>
More information about the lldb-commits
mailing list