[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.
Alex Langford via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Fri Apr 28 13:21:30 PDT 2023
bulbazord requested changes to this revision.
bulbazord added a comment.
This revision now requires changes to proceed.
Mostly looks good to me, just a small thing about an implementation detail.
================
Comment at: lldb/source/Expression/LLVMUserExpression.cpp:38
+using namespace lldb;
using namespace lldb_private;
----------------
What in this file uses something from the lldb namespace now?
================
Comment at: lldb/source/Expression/LLVMUserExpression.cpp:340-348
+ Process *process_sp;
+ ABISP abi_sp;
+ if ((process_sp = exe_ctx.GetProcessPtr()) &&
+ (abi_sp = process_sp->GetABI())) {
+ stack_frame_size = abi_sp->GetStackFrameSize();
+ } else {
+ stack_frame_size = 512 * 1024;
----------------
A few things:
- I don't think you need to re-extract process from `exe_ctx`, the variable `process` at the beginning of this function should have a valid Process pointer in it already (see: `LockAndCheckContext` at the beginning of this function). We already assume `target` is valid and use it in the same way.
- If we can't get an ABI from the Process, do we want to assume a stack frame size of `512 * 1024`? Maybe there is a use case I'm missing, but if we don't have an ABI I'm not convinced that `PrepareToExecuteJITExpression` should succeed. LLDB will need some kind of ABI information to correctly set up the register context to call the JITed code anyway.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149262/new/
https://reviews.llvm.org/D149262
More information about the lldb-commits
mailing list