Skip to content

Commit 062028f

Browse files
igorkudringithub-actions[bot]
igorkudrin
authored and
github-actions[bot]
committed
Automerge: [lldb][NFC] Switch IRMemoryMap::Malloc to return llvm::Expected (#146016)
This will make changes in #145599 a bit nicer.
2 parents 4c59dda + 21993f0 commit 062028f

File tree

6 files changed

+87
-92
lines changed

6 files changed

+87
-92
lines changed

‎lldb/include/lldb/Expression/IRMemoryMap.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "lldb/Utility/DataBufferHeap.h"
1313
#include "lldb/Utility/UserID.h"
1414
#include "lldb/lldb-public.h"
15+
#include "llvm/Support/Error.h"
1516

1617
#include <map>
1718

@@ -50,8 +51,10 @@ class IRMemoryMap {
5051
///only in the process.
5152
};
5253

53-
lldb::addr_t Malloc(size_t size, uint8_t alignment, uint32_t permissions,
54-
AllocationPolicy policy, bool zero_memory, Status &error);
54+
llvm::Expected<lldb::addr_t> Malloc(size_t size, uint8_t alignment,
55+
uint32_t permissions,
56+
AllocationPolicy policy,
57+
bool zero_memory);
5558
void Leak(lldb::addr_t process_address, Status &error);
5659
void Free(lldb::addr_t process_address, Status &error);
5760

‎lldb/source/Expression/IRExecutionUnit.cpp‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,14 @@ IRExecutionUnit::IRExecutionUnit(std::unique_ptr<llvm::LLVMContext> &context_up,
5757
lldb::addr_t IRExecutionUnit::WriteNow(const uint8_t *bytes, size_t size,
5858
Status &error) {
5959
const bool zero_memory = false;
60-
lldb::addr_t allocation_process_addr =
60+
auto address_or_error =
6161
Malloc(size, 8, lldb::ePermissionsWritable | lldb::ePermissionsReadable,
62-
eAllocationPolicyMirror, zero_memory, error);
63-
64-
if (!error.Success())
62+
eAllocationPolicyMirror, zero_memory);
63+
if (!address_or_error) {
64+
error = Status::FromError(address_or_error.takeError());
6565
return LLDB_INVALID_ADDRESS;
66+
}
67+
lldb::addr_t allocation_process_addr = *address_or_error;
6668

6769
WriteMemory(allocation_process_addr, bytes, size, error);
6870

@@ -1102,9 +1104,12 @@ bool IRExecutionUnit::CommitOneAllocation(lldb::ProcessSP &process_sp,
11021104
break;
11031105
default:
11041106
const bool zero_memory = false;
1105-
record.m_process_address =
1106-
Malloc(record.m_size, record.m_alignment, record.m_permissions,
1107-
eAllocationPolicyProcessOnly, zero_memory, error);
1107+
if (auto address_or_error =
1108+
Malloc(record.m_size, record.m_alignment, record.m_permissions,
1109+
eAllocationPolicyProcessOnly, zero_memory))
1110+
record.m_process_address = *address_or_error;
1111+
else
1112+
error = Status::FromError(address_or_error.takeError());
11081113
break;
11091114
}
11101115

‎lldb/source/Expression/IRMemoryMap.cpp‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -319,11 +319,11 @@ IRMemoryMap::Allocation::Allocation(lldb::addr_t process_alloc,
319319
}
320320
}
321321

322-
lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
323-
uint32_t permissions, AllocationPolicy policy,
324-
bool zero_memory, Status &error) {
322+
llvm::Expected<lldb::addr_t> IRMemoryMap::Malloc(size_t size, uint8_t alignment,
323+
uint32_t permissions,
324+
AllocationPolicy policy,
325+
bool zero_memory) {
325326
lldb_private::Log *log(GetLog(LLDBLog::Expressions));
326-
error.Clear();
327327

328328
lldb::ProcessSP process_sp;
329329
lldb::addr_t allocation_address = LLDB_INVALID_ADDRESS;
@@ -347,15 +347,14 @@ lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
347347

348348
switch (policy) {
349349
default:
350-
error =
351-
Status::FromErrorString("Couldn't malloc: invalid allocation policy");
352-
return LLDB_INVALID_ADDRESS;
350+
return llvm::createStringError(
351+
llvm::inconvertibleErrorCode(),
352+
"Couldn't malloc: invalid allocation policy");
353353
case eAllocationPolicyHostOnly:
354354
allocation_address = FindSpace(allocation_size);
355-
if (allocation_address == LLDB_INVALID_ADDRESS) {
356-
error = Status::FromErrorString("Couldn't malloc: address space is full");
357-
return LLDB_INVALID_ADDRESS;
358-
}
355+
if (allocation_address == LLDB_INVALID_ADDRESS)
356+
return llvm::createStringError(llvm::inconvertibleErrorCode(),
357+
"Couldn't malloc: address space is full");
359358
break;
360359
case eAllocationPolicyMirror:
361360
process_sp = m_process_wp.lock();
@@ -366,6 +365,7 @@ lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
366365
process_sp && process_sp->CanJIT() ? "true" : "false",
367366
process_sp && process_sp->IsAlive() ? "true" : "false");
368367
if (process_sp && process_sp->CanJIT() && process_sp->IsAlive()) {
368+
Status error;
369369
if (!zero_memory)
370370
allocation_address =
371371
process_sp->AllocateMemory(allocation_size, permissions, error);
@@ -374,25 +374,25 @@ lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
374374
process_sp->CallocateMemory(allocation_size, permissions, error);
375375

376376
if (!error.Success())
377-
return LLDB_INVALID_ADDRESS;
377+
return error.takeError();
378378
} else {
379379
LLDB_LOGF(log,
380380
"IRMemoryMap::%s switching to eAllocationPolicyHostOnly "
381381
"due to failed condition (see previous expr log message)",
382382
__FUNCTION__);
383383
policy = eAllocationPolicyHostOnly;
384384
allocation_address = FindSpace(allocation_size);
385-
if (allocation_address == LLDB_INVALID_ADDRESS) {
386-
error =
387-
Status::FromErrorString("Couldn't malloc: address space is full");
388-
return LLDB_INVALID_ADDRESS;
389-
}
385+
if (allocation_address == LLDB_INVALID_ADDRESS)
386+
return llvm::createStringError(
387+
llvm::inconvertibleErrorCode(),
388+
"Couldn't malloc: address space is full");
390389
}
391390
break;
392391
case eAllocationPolicyProcessOnly:
393392
process_sp = m_process_wp.lock();
394393
if (process_sp) {
395394
if (process_sp->CanJIT() && process_sp->IsAlive()) {
395+
Status error;
396396
if (!zero_memory)
397397
allocation_address =
398398
process_sp->AllocateMemory(allocation_size, permissions, error);
@@ -401,17 +401,16 @@ lldb::addr_t IRMemoryMap::Malloc(size_t size, uint8_t alignment,
401401
process_sp->CallocateMemory(allocation_size, permissions, error);
402402

403403
if (!error.Success())
404-
return LLDB_INVALID_ADDRESS;
404+
return error.takeError();
405405
} else {
406-
error = Status::FromErrorString(
406+
return llvm::createStringError(
407+
llvm::inconvertibleErrorCode(),
407408
"Couldn't malloc: process doesn't support allocating memory");
408-
return LLDB_INVALID_ADDRESS;
409409
}
410410
} else {
411-
error = Status::FromErrorString(
412-
"Couldn't malloc: process doesn't exist, and this "
413-
"memory must be in the process");
414-
return LLDB_INVALID_ADDRESS;
411+
return llvm::createStringError(llvm::inconvertibleErrorCode(),
412+
"Couldn't malloc: process doesn't exist, "
413+
"and this memory must be in the process");
415414
}
416415
break;
417416
}

‎lldb/source/Expression/LLVMUserExpression.cpp‎

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -313,34 +313,29 @@ bool LLVMUserExpression::PrepareToExecuteJITExpression(
313313

314314
if (m_jit_start_addr != LLDB_INVALID_ADDRESS || m_can_interpret) {
315315
if (m_materialized_address == LLDB_INVALID_ADDRESS) {
316-
Status alloc_error;
317-
318316
IRMemoryMap::AllocationPolicy policy =
319317
m_can_interpret ? IRMemoryMap::eAllocationPolicyHostOnly
320318
: IRMemoryMap::eAllocationPolicyMirror;
321319

322320
const bool zero_memory = false;
323-
324-
m_materialized_address = m_execution_unit_sp->Malloc(
325-
m_materializer_up->GetStructByteSize(),
326-
m_materializer_up->GetStructAlignment(),
327-
lldb::ePermissionsReadable | lldb::ePermissionsWritable, policy,
328-
zero_memory, alloc_error);
329-
330-
if (!alloc_error.Success()) {
321+
if (auto address_or_error = m_execution_unit_sp->Malloc(
322+
m_materializer_up->GetStructByteSize(),
323+
m_materializer_up->GetStructAlignment(),
324+
lldb::ePermissionsReadable | lldb::ePermissionsWritable, policy,
325+
zero_memory)) {
326+
m_materialized_address = *address_or_error;
327+
} else {
331328
diagnostic_manager.Printf(
332329
lldb::eSeverityError,
333330
"Couldn't allocate space for materialized struct: %s",
334-
alloc_error.AsCString());
331+
toString(address_or_error.takeError()).c_str());
335332
return false;
336333
}
337334
}
338335

339336
struct_address = m_materialized_address;
340337

341338
if (m_can_interpret && m_stack_frame_bottom == LLDB_INVALID_ADDRESS) {
342-
Status alloc_error;
343-
344339
size_t stack_frame_size = target->GetExprAllocSize();
345340
if (stack_frame_size == 0) {
346341
ABISP abi_sp;
@@ -351,19 +346,17 @@ bool LLVMUserExpression::PrepareToExecuteJITExpression(
351346
}
352347

353348
const bool zero_memory = false;
354-
355-
m_stack_frame_bottom = m_execution_unit_sp->Malloc(
356-
stack_frame_size, 8,
357-
lldb::ePermissionsReadable | lldb::ePermissionsWritable,
358-
IRMemoryMap::eAllocationPolicyHostOnly, zero_memory, alloc_error);
359-
360-
m_stack_frame_top = m_stack_frame_bottom + stack_frame_size;
361-
362-
if (!alloc_error.Success()) {
349+
if (auto address_or_error = m_execution_unit_sp->Malloc(
350+
stack_frame_size, 8,
351+
lldb::ePermissionsReadable | lldb::ePermissionsWritable,
352+
IRMemoryMap::eAllocationPolicyHostOnly, zero_memory)) {
353+
m_stack_frame_bottom = *address_or_error;
354+
m_stack_frame_top = m_stack_frame_bottom + stack_frame_size;
355+
} else {
363356
diagnostic_manager.Printf(
364357
lldb::eSeverityError,
365358
"Couldn't allocate space for the stack frame: %s",
366-
alloc_error.AsCString());
359+
toString(address_or_error.takeError()).c_str());
367360
return false;
368361
}
369362
}
@@ -382,4 +375,3 @@ bool LLVMUserExpression::PrepareToExecuteJITExpression(
382375
}
383376
return true;
384377
}
385-

‎lldb/source/Expression/Materializer.cpp‎

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -74,22 +74,20 @@ class EntityPersistentVariable : public Materializer::Entity {
7474
// Allocate a spare memory area to store the persistent variable's
7575
// contents.
7676

77-
Status allocate_error;
7877
const bool zero_memory = false;
79-
80-
lldb::addr_t mem = map.Malloc(
78+
auto address_or_error = map.Malloc(
8179
llvm::expectedToOptional(m_persistent_variable_sp->GetByteSize())
8280
.value_or(0),
8381
8, lldb::ePermissionsReadable | lldb::ePermissionsWritable,
84-
IRMemoryMap::eAllocationPolicyMirror, zero_memory, allocate_error);
85-
86-
if (!allocate_error.Success()) {
82+
IRMemoryMap::eAllocationPolicyMirror, zero_memory);
83+
if (!address_or_error) {
8784
err = Status::FromErrorStringWithFormat(
8885
"couldn't allocate a memory area to store %s: %s",
8986
m_persistent_variable_sp->GetName().GetCString(),
90-
allocate_error.AsCString());
87+
toString(address_or_error.takeError()).c_str());
9188
return;
9289
}
90+
lldb::addr_t mem = *address_or_error;
9391

9492
LLDB_LOGF(log, "Allocated %s (0x%" PRIx64 ") successfully",
9593
m_persistent_variable_sp->GetName().GetCString(), mem);
@@ -565,26 +563,25 @@ class EntityVariableBase : public Materializer::Entity {
565563

566564
size_t byte_align = (*opt_bit_align + 7) / 8;
567565

568-
Status alloc_error;
569566
const bool zero_memory = false;
570-
571-
m_temporary_allocation = map.Malloc(
572-
data.GetByteSize(), byte_align,
573-
lldb::ePermissionsReadable | lldb::ePermissionsWritable,
574-
IRMemoryMap::eAllocationPolicyMirror, zero_memory, alloc_error);
567+
if (auto address_or_error = map.Malloc(
568+
data.GetByteSize(), byte_align,
569+
lldb::ePermissionsReadable | lldb::ePermissionsWritable,
570+
IRMemoryMap::eAllocationPolicyMirror, zero_memory)) {
571+
m_temporary_allocation = *address_or_error;
572+
} else {
573+
err = Status::FromErrorStringWithFormat(
574+
"couldn't allocate a temporary region for %s: %s",
575+
GetName().AsCString(),
576+
toString(address_or_error.takeError()).c_str());
577+
return;
578+
}
575579

576580
m_temporary_allocation_size = data.GetByteSize();
577581

578582
m_original_data = std::make_shared<DataBufferHeap>(data.GetDataStart(),
579583
data.GetByteSize());
580584

581-
if (!alloc_error.Success()) {
582-
err = Status::FromErrorStringWithFormat(
583-
"couldn't allocate a temporary region for %s: %s",
584-
GetName().AsCString(), alloc_error.AsCString());
585-
return;
586-
}
587-
588585
Status write_error;
589586

590587
map.WriteMemory(m_temporary_allocation, data.GetDataStart(),
@@ -967,22 +964,21 @@ class EntityResultVariable : public Materializer::Entity {
967964

968965
size_t byte_align = (*opt_bit_align + 7) / 8;
969966

970-
Status alloc_error;
971967
const bool zero_memory = true;
972-
973-
m_temporary_allocation = map.Malloc(
974-
byte_size, byte_align,
975-
lldb::ePermissionsReadable | lldb::ePermissionsWritable,
976-
IRMemoryMap::eAllocationPolicyMirror, zero_memory, alloc_error);
977-
m_temporary_allocation_size = byte_size;
978-
979-
if (!alloc_error.Success()) {
968+
if (auto address_or_error = map.Malloc(
969+
byte_size, byte_align,
970+
lldb::ePermissionsReadable | lldb::ePermissionsWritable,
971+
IRMemoryMap::eAllocationPolicyMirror, zero_memory)) {
972+
m_temporary_allocation = *address_or_error;
973+
} else {
980974
err = Status::FromErrorStringWithFormat(
981975
"couldn't allocate a temporary region for the result: %s",
982-
alloc_error.AsCString());
976+
toString(address_or_error.takeError()).c_str());
983977
return;
984978
}
985979

980+
m_temporary_allocation_size = byte_size;
981+
986982
Status pointer_write_error;
987983

988984
map.WritePointerToMemory(load_addr, m_temporary_allocation,

‎lldb/tools/lldb-test/lldb-test.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,13 +1112,13 @@ bool opts::irmemorymap::evalMalloc(StringRef Line,
11121112
// Issue the malloc in the target process with "-rw" permissions.
11131113
const uint32_t Permissions = 0x3;
11141114
const bool ZeroMemory = false;
1115-
Status ST;
1116-
addr_t Addr =
1117-
State.Map.Malloc(Size, Alignment, Permissions, AP, ZeroMemory, ST);
1118-
if (ST.Fail()) {
1119-
outs() << formatv("Malloc error: {0}\n", ST);
1115+
auto AddrOrErr =
1116+
State.Map.Malloc(Size, Alignment, Permissions, AP, ZeroMemory);
1117+
if (!AddrOrErr) {
1118+
outs() << formatv("Malloc error: {0}\n", toString(AddrOrErr.takeError()));
11201119
return true;
11211120
}
1121+
addr_t Addr = *AddrOrErr;
11221122

11231123
// Print the result of the allocation before checking its validity.
11241124
outs() << formatv("Malloc: address = {0:x}\n", Addr);

0 commit comments

Comments
 (0)