[Lldb-commits] Improving the documentation
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 15 17:51:40 PDT 2016
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>
More information about the lldb-commits
mailing list