[lldb][NFC] Switch IRMemoryMap::Malloc to return llvm::Expected #146016
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Igor Kudrin (igorkudrin) ChangesThis will make changes in #145599 a bit nicer. Full diff: https://github.com/llvm/llvm-project/pull/146016.diff 6 Files Affected:
diff --git a/lldb/include/lldb/Expression/IRMemoryMap.h b/lldb/include/lldb/Expression/IRMemoryMap.h
index abec5442793c6..acbffd1e40b94 100644
--- a/lldb/include/lldb/Expression/IRMemoryMap.h
+++ b/lldb/include/lldb/Expression/IRMemoryMap.h
@@ -12,6 +12,7 @@
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/UserID.h"
#include "lldb/lldb-public.h"
+#include "llvm/Support/Error.h"
#include <map>
@@ -50,8 +51,10 @@ class IRMemoryMap {
///only in the process.
};
- lldb::addr_t Malloc(size_t size, uint8_t alignment, uint32_t permissions,
- AllocationPolicy policy, bool zero_memory, Status &error);
+ llvm::Expected<lldb::addr_t> Malloc(size_t size, uint8_t alignment,
+ uint32_t permissions,
+ AllocationPolicy policy,
+ bool zero_memory);
void Leak(lldb::addr_t process_address, Status &error);
void Free(lldb::addr_t process_address, Status &error);
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp
index 06b0cb7769f64..e445fa8833022 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -57,12 +57,14 @@ IRExecutionUnit::IRExecutionUnit(std::unique_ptr<llvm::LLVMContext> &context_up,
lldb::addr_t IRExecutionUnit::WriteNow(const uint8_t *bytes, size_t size,
Status &error) {
const bool zero_memory = false;
- lldb::addr_t allocation_process_addr =
+ auto address_or_error =
Malloc(size, 8, lldb::ePermissionsWritable | lldb::ePermissionsReadable,
- eAllocationPolicyMirror, zero_memory, error);
-
- if (!error.Success())
+ eAllocationPolicyMirror, zero_memory);
+ if (!address_or_error) {
+ error = Status::FromError(address_or_error.takeError());
return LLDB_INVALID_ADDRESS;
+ }
+ lldb::addr_t allocation_process_addr = *address_or_error;
WriteMemory(allocation_process_addr, bytes, size, error);
@@ -1102,9 +1104,12 @@ bool IRExecutionUnit::CommitOneAllocation(lldb::ProcessSP &process_sp,
break;
default:
const bool zero_memory = false;
- record.m_process_address =
- Malloc(record.m_size, record.m_alignment, record.m_permissions,
- eAllocationPolicyProcessOnly, zero_memory, error);
+ if (auto address_or_error =
+ Malloc(record.m_size, record.m_alignment, record.m_permissions,
+ eAllocationPolicyProcessOnly, zero_memory))
+ record.m_process_address = *address_or_error;
+ else
+ error = Status::FromError(address_or_error.takeError());
break;
}
diff --git a/lldb/source/Expression/IRMemoryMap.cpp b/lldb/source/Expression/IRMemoryMap.cpp
index 65b5d11413c85..f500272cfb302 100644
--- a/lldb/source/Expression/IRMemoryMap.cpp
+++ b/lldb/source/Expression/IRMemoryMap.cpp
@@ -319,11 +319,11 @@ IRMemoryMap::Allocation::Allocation(lldb::addr_t process_alloc,
}
}
-lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
- uint32_t permissions, AllocationPolicy policy,
- bool zero_memory, Status &error) {
+llvm::Expected<lldb::addr_t> IRMemoryMap::Malloc(size_t size, uint8_t alignment,
+ uint32_t permissions,
+ AllocationPolicy policy,
+ bool zero_memory) {
lldb_private::Log *log(GetLog(LLDBLog::Expressions));
- error.Clear();
lldb::ProcessSP process_sp;
lldb::addr_t allocation_address = LLDB_INVALID_ADDRESS;
@@ -347,15 +347,14 @@ lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
switch (policy) {
default:
- error =
- Status::FromErrorString("Couldn't malloc: invalid allocation policy");
- return LLDB_INVALID_ADDRESS;
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ "Couldn't malloc: invalid allocation policy");
case eAllocationPolicyHostOnly:
allocation_address = FindSpace(allocation_size);
- if (allocation_address == LLDB_INVALID_ADDRESS) {
- error = Status::FromErrorString("Couldn't malloc: address space is full");
- return LLDB_INVALID_ADDRESS;
- }
+ if (allocation_address == LLDB_INVALID_ADDRESS)
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Couldn't malloc: address space is full");
break;
case eAllocationPolicyMirror:
process_sp = m_process_wp.lock();
@@ -366,6 +365,7 @@ lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
process_sp && process_sp->CanJIT() ? "true" : "false",
process_sp && process_sp->IsAlive() ? "true" : "false");
if (process_sp && process_sp->CanJIT() && process_sp->IsAlive()) {
+ Status error;
if (!zero_memory)
allocation_address =
process_sp->AllocateMemory(allocation_size, permissions, error);
@@ -374,7 +374,7 @@ lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
process_sp->CallocateMemory(allocation_size, permissions, error);
if (!error.Success())
- return LLDB_INVALID_ADDRESS;
+ return error.takeError();
} else {
LLDB_LOGF(log,
"IRMemoryMap::%s switching to eAllocationPolicyHostOnly "
@@ -382,17 +382,17 @@ lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
__FUNCTION__);
policy = eAllocationPolicyHostOnly;
allocation_address = FindSpace(allocation_size);
- if (allocation_address == LLDB_INVALID_ADDRESS) {
- error =
- Status::FromErrorString("Couldn't malloc: address space is full");
- return LLDB_INVALID_ADDRESS;
- }
+ if (allocation_address == LLDB_INVALID_ADDRESS)
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ "Couldn't malloc: address space is full");
}
break;
case eAllocationPolicyProcessOnly:
process_sp = m_process_wp.lock();
if (process_sp) {
if (process_sp->CanJIT() && process_sp->IsAlive()) {
+ Status error;
if (!zero_memory)
allocation_address =
process_sp->AllocateMemory(allocation_size, permissions, error);
@@ -401,17 +401,16 @@ lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
process_sp->CallocateMemory(allocation_size, permissions, error);
if (!error.Success())
- return LLDB_INVALID_ADDRESS;
+ return error.takeError();
} else {
- error = Status::FromErrorString(
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
"Couldn't malloc: process doesn't support allocating memory");
- return LLDB_INVALID_ADDRESS;
}
} else {
- error = Status::FromErrorString(
- "Couldn't malloc: process doesn't exist, and this "
- "memory must be in the process");
- return LLDB_INVALID_ADDRESS;
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Couldn't malloc: process doesn't exist, "
+ "and this memory must be in the process");
}
break;
}
diff --git a/lldb/source/Expression/LLVMUserExpression.cpp b/lldb/source/Expression/LLVMUserExpression.cpp
index fac3ce6f5799d..2d59194027b57 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/lldb/source/Expression/LLVMUserExpression.cpp
@@ -313,25 +313,22 @@ bool LLVMUserExpression::PrepareToExecuteJITExpression(
if (m_jit_start_addr != LLDB_INVALID_ADDRESS || m_can_interpret) {
if (m_materialized_address == LLDB_INVALID_ADDRESS) {
- Status alloc_error;
-
IRMemoryMap::AllocationPolicy policy =
m_can_interpret ? IRMemoryMap::eAllocationPolicyHostOnly
: IRMemoryMap::eAllocationPolicyMirror;
const bool zero_memory = false;
-
- m_materialized_address = m_execution_unit_sp->Malloc(
- m_materializer_up->GetStructByteSize(),
- m_materializer_up->GetStructAlignment(),
- lldb::ePermissionsReadable | lldb::ePermissionsWritable, policy,
- zero_memory, alloc_error);
-
- if (!alloc_error.Success()) {
+ if (auto address_or_error = m_execution_unit_sp->Malloc(
+ m_materializer_up->GetStructByteSize(),
+ m_materializer_up->GetStructAlignment(),
+ lldb::ePermissionsReadable | lldb::ePermissionsWritable, policy,
+ zero_memory)) {
+ m_materialized_address = *address_or_error;
+ } else {
diagnostic_manager.Printf(
lldb::eSeverityError,
"Couldn't allocate space for materialized struct: %s",
- alloc_error.AsCString());
+ toString(address_or_error.takeError()).c_str());
return false;
}
}
@@ -339,8 +336,6 @@ bool LLVMUserExpression::PrepareToExecuteJITExpression(
struct_address = m_materialized_address;
if (m_can_interpret && m_stack_frame_bottom == LLDB_INVALID_ADDRESS) {
- Status alloc_error;
-
size_t stack_frame_size = target->GetExprAllocSize();
if (stack_frame_size == 0) {
ABISP abi_sp;
@@ -351,19 +346,17 @@ bool LLVMUserExpression::PrepareToExecuteJITExpression(
}
const bool zero_memory = false;
-
- m_stack_frame_bottom = m_execution_unit_sp->Malloc(
- stack_frame_size, 8,
- lldb::ePermissionsReadable | lldb::ePermissionsWritable,
- IRMemoryMap::eAllocationPolicyHostOnly, zero_memory, alloc_error);
-
- m_stack_frame_top = m_stack_frame_bottom + stack_frame_size;
-
- if (!alloc_error.Success()) {
+ if (auto address_or_error = m_execution_unit_sp->Malloc(
+ stack_frame_size, 8,
+ lldb::ePermissionsReadable | lldb::ePermissionsWritable,
+ IRMemoryMap::eAllocationPolicyHostOnly, zero_memory)) {
+ m_stack_frame_bottom = *address_or_error;
+ m_stack_frame_top = m_stack_frame_bottom + stack_frame_size;
+ } else {
diagnostic_manager.Printf(
lldb::eSeverityError,
"Couldn't allocate space for the stack frame: %s",
- alloc_error.AsCString());
+ toString(address_or_error.takeError()).c_str());
return false;
}
}
@@ -382,4 +375,3 @@ bool LLVMUserExpression::PrepareToExecuteJITExpression(
}
return true;
}
-
diff --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp
index 79c804c6c4214..96add56f92180 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -74,22 +74,20 @@ class EntityPersistentVariable : public Materializer::Entity {
// Allocate a spare memory area to store the persistent variable's
// contents.
- Status allocate_error;
const bool zero_memory = false;
-
- lldb::addr_t mem = map.Malloc(
+ auto address_or_error = map.Malloc(
llvm::expectedToOptional(m_persistent_variable_sp->GetByteSize())
.value_or(0),
8, lldb::ePermissionsReadable | lldb::ePermissionsWritable,
- IRMemoryMap::eAllocationPolicyMirror, zero_memory, allocate_error);
-
- if (!allocate_error.Success()) {
+ IRMemoryMap::eAllocationPolicyMirror, zero_memory);
+ if (!address_or_error) {
err = Status::FromErrorStringWithFormat(
"couldn't allocate a memory area to store %s: %s",
m_persistent_variable_sp->GetName().GetCString(),
- allocate_error.AsCString());
+ toString(address_or_error.takeError()).c_str());
return;
}
+ lldb::addr_t mem = *address_or_error;
LLDB_LOGF(log, "Allocated %s (0x%" PRIx64 ") successfully",
m_persistent_variable_sp->GetName().GetCString(), mem);
@@ -565,26 +563,25 @@ class EntityVariableBase : public Materializer::Entity {
size_t byte_align = (*opt_bit_align + 7) / 8;
- Status alloc_error;
const bool zero_memory = false;
-
- m_temporary_allocation = map.Malloc(
- data.GetByteSize(), byte_align,
- lldb::ePermissionsReadable | lldb::ePermissionsWritable,
- IRMemoryMap::eAllocationPolicyMirror, zero_memory, alloc_error);
+ if (auto address_or_error = map.Malloc(
+ data.GetByteSize(), byte_align,
+ lldb::ePermissionsReadable | lldb::ePermissionsWritable,
+ IRMemoryMap::eAllocationPolicyMirror, zero_memory)) {
+ m_temporary_allocation = *address_or_error;
+ } else {
+ err = Status::FromErrorStringWithFormat(
+ "couldn't allocate a temporary region for %s: %s",
+ GetName().AsCString(),
+ toString(address_or_error.takeError()).c_str());
+ return;
+ }
m_temporary_allocation_size = data.GetByteSize();
m_original_data = std::make_shared<DataBufferHeap>(data.GetDataStart(),
data.GetByteSize());
- if (!alloc_error.Success()) {
- err = Status::FromErrorStringWithFormat(
- "couldn't allocate a temporary region for %s: %s",
- GetName().AsCString(), alloc_error.AsCString());
- return;
- }
-
Status write_error;
map.WriteMemory(m_temporary_allocation, data.GetDataStart(),
@@ -967,22 +964,21 @@ class EntityResultVariable : public Materializer::Entity {
size_t byte_align = (*opt_bit_align + 7) / 8;
- Status alloc_error;
const bool zero_memory = true;
-
- m_temporary_allocation = map.Malloc(
- byte_size, byte_align,
- lldb::ePermissionsReadable | lldb::ePermissionsWritable,
- IRMemoryMap::eAllocationPolicyMirror, zero_memory, alloc_error);
- m_temporary_allocation_size = byte_size;
-
- if (!alloc_error.Success()) {
+ if (auto address_or_error = map.Malloc(
+ byte_size, byte_align,
+ lldb::ePermissionsReadable | lldb::ePermissionsWritable,
+ IRMemoryMap::eAllocationPolicyMirror, zero_memory)) {
+ m_temporary_allocation = *address_or_error;
+ } else {
err = Status::FromErrorStringWithFormat(
"couldn't allocate a temporary region for the result: %s",
- alloc_error.AsCString());
+ toString(address_or_error.takeError()).c_str());
return;
}
+ m_temporary_allocation_size = byte_size;
+
Status pointer_write_error;
map.WritePointerToMemory(load_addr, m_temporary_allocation,
diff --git a/lldb/tools/lldb-test/lldb-test.cpp b/lldb/tools/lldb-test/lldb-test.cpp
index 6743ce543dc66..3f198d963a93b 100644
--- a/lldb/tools/lldb-test/lldb-test.cpp
+++ b/lldb/tools/lldb-test/lldb-test.cpp
@@ -1112,13 +1112,13 @@ bool opts::irmemorymap::evalMalloc(StringRef Line,
// Issue the malloc in the target process with "-rw" permissions.
const uint32_t Permissions = 0x3;
const bool ZeroMemory = false;
- Status ST;
- addr_t Addr =
- State.Map.Malloc(Size, Alignment, Permissions, AP, ZeroMemory, ST);
- if (ST.Fail()) {
- outs() << formatv("Malloc error: {0}\n", ST);
+ auto AddrOrErr =
+ State.Map.Malloc(Size, Alignment, Permissions, AP, ZeroMemory);
+ if (!AddrOrErr) {
+ outs() << formatv("Malloc error: {0}\n", toString(AddrOrErr.takeError()));
return true;
}
+ addr_t Addr = *AddrOrErr;
// Print the result of the allocation before checking its validity.
outs() << formatv("Malloc: address = {0:x}\n", Addr);
|
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
…#146016) This will make changes in llvm#145599 a bit nicer.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will make changes in #145599 a bit nicer.