Skip to content

[lldb] Add DWARFExpressionEntry and GetExpressionEntryAtAddress() to … #144238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JDevlieghere merged 15 commits into from
Jul 12, 2025
Merged

[lldb] Add DWARFExpressionEntry and GetExpressionEntryAtAddress() to … #144238

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8ed8c54
[lldb] Add DWARFExpressionEntry and GetExpressionEntryAtAddress() to …
UltimateForce21 Jun 11, 2025
1db5002
Update lldb/include/lldb/Expression/DWARFExpressionList.h
UltimateForce21 Jun 19, 2025
a26010b
Update lldb/include/lldb/Expression/DWARFExpressionList.h
UltimateForce21 Jun 19, 2025
72237b7
Update lldb/source/Expression/DWARFExpressionList.cpp
UltimateForce21 Jun 19, 2025
94e4951
Update DWARFExpressionList.h
UltimateForce21 Jun 24, 2025
e8142da
Update DWARFExpressionList.cpp
UltimateForce21 Jun 24, 2025
7e8741e
Update DWARFExpressionList.h
UltimateForce21 Jun 28, 2025
c4cd77f
Update DWARFExpressionList.cpp
UltimateForce21 Jun 28, 2025
62c02a9
Change GetExpressionEntryAtAddress to return std::optional instead of…
UltimateForce21 Jun 29, 2025
d015971
Update DWARFExpressionList.cpp
UltimateForce21 Jul 2, 2025
60898ea
Add underflow/overflow checks to GetExpressionEntryAtAddressi
UltimateForce21 Jul 3, 2025
3462165
Make file_range optional in DWARFExpressionEntry for always-valid expr
UltimateForce21 Jul 8, 2025
c01a747
Merge branch 'main' into add-dwarfexprentry-api
UltimateForce21 Jul 9, 2025
ec3ac93
Merge branch 'main' into add-dwarfexprentry-api
UltimateForce21 Jul 10, 2025
38baa90
Fixed formating issues
UltimateForce21 Jul 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lldb/include/lldb/Expression/DWARFExpressionList.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#ifndef LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
#define LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H

#include "lldb/Core/AddressRange.h"
#include "lldb/Core/Value.h"
#include "lldb/Expression/DWARFExpression.h"
#include "lldb/Utility/RangeMap.h"
Expand Down Expand Up @@ -58,6 +59,19 @@ class DWARFExpressionList {
}

lldb::addr_t GetFuncFileAddress() { return m_func_file_addr; }

/// Represents an entry in the DWARFExpressionList with all needed metadata.
struct DWARFExpressionEntry {
AddressRange file_range; /// Represents a DWARF location range in the DWARF unit’s file‐address space
Copy link
Collaborator

@adrian-prantl adrian-prantl Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either:

AddressRange file_range; ///< Represents a DWARF location range in the DWARF unit’s file‐address space.

or (preferred)

 /// Represents a DWARF location range in the DWARF unit’s file‐address space.
 AddressRange file_range;

const DWARFExpression *expr;
};

/// Returns a DWARFExpressionEntry whose file_range contains the given
/// load‐address. `func_load_addr` is the load‐address of the function
/// start; `load_addr` is the full runtime PC. On success, `expr` is non-null.
llvm::Expected<DWARFExpressionEntry>
GetExpressionEntryAtAddress(lldb::addr_t func_load_addr,
lldb::addr_t load_addr) const;

const DWARFExpression *GetExpressionAtAddress(lldb::addr_t func_load_addr,
lldb::addr_t load_addr) const;
Expand Down
24 changes: 24 additions & 0 deletions lldb/source/Expression/DWARFExpressionList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//

#include "lldb/Core/AddressRange.h"
#include "lldb/Expression/DWARFExpressionList.h"
#include "lldb/Symbol/Function.h"
#include "lldb/Target/RegisterContext.h"
Expand Down Expand Up @@ -53,6 +54,29 @@ bool DWARFExpressionList::ContainsAddress(lldb::addr_t func_load_addr,
return GetExpressionAtAddress(func_load_addr, addr) != nullptr;
}

llvm::Expected<DWARFExpressionList::DWARFExpressionEntry>
DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr,
lldb::addr_t load_addr) const {
if (const DWARFExpression *always = GetAlwaysValidExpr()) {
AddressRange full_range(m_func_file_addr, /*size=*/LLDB_INVALID_ADDRESS);
Copy link
Member

@JDevlieghere JDevlieghere Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of the argument is byte_size so let's match that.

Suggested change
AddressRange full_range(m_func_file_addr, /*size=*/LLDB_INVALID_ADDRESS);
AddressRange full_range(m_func_file_addr, /*byte_size=*/LLDB_INVALID_ADDRESS);

I'm somewhat skeptical of LLDB_INVALID_ADDRESS as the size. Is there precedent for using that to indicate we don't know the size? I'm worried about whether the existing code is resilient to that. Would it make sense to make the AddressRange optional in the struct for the always-valid-single-expression case?

Copy link
Contributor Author

@UltimateForce21 UltimateForce21 Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe using an optional AddressRange the way you said is a safer option and probably more clear for users, I have committed the change.

return DWARFExpressionEntry{full_range, always};
}

if (func_load_addr == LLDB_INVALID_ADDRESS)
func_load_addr = m_func_file_addr;
lldb::addr_t file_pc = load_addr - func_load_addr + m_func_file_addr;
Copy link
Collaborator

@adrian-prantl adrian-prantl Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be using subOverflow (https://llvm.org/doxygen/MathExtras_8h.html) here to guard against wrap-around.

Copy link
Contributor Author

@UltimateForce21 UltimateForce21 Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dug into MathExtras and found that its AddOverflow and SubOverflow helpers are only defined for signed types. I they rely on two’s-complement signed overflow semantics and return the truncated signed result. Since lldb::addr_t is an unsigned address, I think using those helpers would either fail to compile or silently convert to signed semantics. So I tried to implement the signed overflow check into the function. Please let me know if you think it is alright.


uint32_t idx = m_exprs.FindEntryIndexThatContains(file_pc);
Copy link
Collaborator

@adrian-prantl adrian-prantl Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use

  const Entry *FindEntryThatContains(B addr) const {
```?

Copy link
Contributor Author

@UltimateForce21 UltimateForce21 Jun 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for sure, seems like it would be easier for implementation, I was just following the same logic, as

const DWARFExpression *
DWARFExpressionList::GetExpressionAtAddress(lldb::addr_t func_load_addr,
                                            lldb::addr_t load_addr) const ()

will update to use const Entry *FindEntryThatContains(B addr) const () instead

if (idx == UINT32_MAX)
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
"no DWARF location list entry for PC 0x%" PRIx64, load_addr);

const auto &entry = *m_exprs.GetEntryAtIndex(idx);
AddressRange range_in_file(entry.base, entry.GetRangeEnd() - entry.base);
return DWARFExpressionEntry{range_in_file, &entry.data};
}

const DWARFExpression *
DWARFExpressionList::GetExpressionAtAddress(lldb::addr_t func_load_addr,
Copy link
Collaborator

@adrian-prantl adrian-prantl Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: Do we still need this API, or is it just a subset of the function above?

Copy link
Contributor Author

@UltimateForce21 UltimateForce21 Jun 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you know the new function GetExpressionEntryAtAddress, does almost exact same thing as this one. I don't see this function being used anywhere else in the lldb code base, so maybe it can be removed, but to be safe I did not want to alter it. But from what I can tell, it should be okay to remove it. We can also choose to keep it and maybe refactor it to just call the GetExpressionEntryAtAddress and just return the DWARFExpression *expr.

Copy link
Collaborator

@adrian-prantl adrian-prantl Jun 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's completely unused, and there is no indication that was added in support of any downstream clients (either as a comment or in the commit message) we could consider deleting it in a separate PR. Not urgent.

lldb::addr_t load_addr) const {
Expand Down