[Lldb-commits] Improving the documentation

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Wed Mar 9 15:10:32 PST 2016


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>



More information about the lldb-commits mailing list