[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

Thu Apr 27 09:51:29 PDT 2023

bulbazord added inline comments.

================
Comment at: lldb/source/Expression/LLVMUserExpression.cpp:339
+      if (stack_frame_size == 0)
+        stack_frame_size = arch == llvm::Triple::msp430 ? 512 : 512 * 1024;

----------------
kuilpd wrote:
> bulbazord wrote:
> > jingham wrote:
> > > This doesn't seem appropriate in generic code.  You should ask the Arch plugin for this.
> > Oh, I missed this in the previous patch. Yeah, we shouldn't be hardcoding `llvm::Triple::msp430` in generic code. You're introducing even more uses of this in this patch, we should abstract this out into a plugin like Jim suggested.
> Do you mean creating a new MSP430 plugin in lldb/source/Plugins/Architecture/, or accessing the existing ABI plugin? Should I create new methods for getting stack frame size in each existing plugin?
You could probably put this in the ABI plugin? The base ABI class could have a virtual method like `GetStackFrameSize` that returns `512 * 1024` and the MSP430 subclass can override it and return `512` instead. That should work, I think.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149262/new/

https://reviews.llvm.org/D149262