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 <git...@newplanetsoftware.com> wrote:
>
> Great! Attached is the update.
>
> On Thu, Mar 10, 2016 at 6:57 PM, Jim Ingham <jing...@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 <git...@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 <jing...@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 <git...@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 <jing...@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@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@lists.llvm.org
> > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> > >
> > >
> > > <diff>
> >
> >
> > <diff>
>
>
> <SBAddress.h>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
