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>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
