[Lldb-commits] [PATCH] D121036: Fix target.save-jit-objects when the CWD is not writeable

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: clayborg, labath, JDevlieghere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

One of the diagnostic outputs for the expression parser is the jit objects.  The way you used to produce these was to set "target.save-jit-objects" to true and the files would get dumped into the CWD.  That's not going to work if the CWD is not writeable (which for GUI apps on macOS is almost always true).

It's also a little annoying to dump them unconditionally in the CWD rather than letting the user specify a directory.

So this patch changes `target.save-jit-objects` to `target.save-jit-objects-dir`.  If this is empty we do nothing and if it has a path we put the files there.

That part seems straightforward, but I also try to validate that the path you provided is good.  The checking part again is easy, but there are three tricky bits, one of which I resolved and two of which I punted.

1. If you want to add a ValueChanged callback to a setting, you need to put the callback into the global properties so you can check when the setting is done before you have a target.  But you also need to insert it into the copy that the target gets, however the callback captures the TargetProperties object it is going to inspect, that's how it knows what to work on, so the callback has to be different.  That's actually fine, except that we have an assert if you try to overwrite a callback.   That assert has been there forever, but I don't know why, and in this case it is something you need to do.  So I removed the assert.

2. When we find an incorrect save directory I would like to inform the user that something is wrong.  That works for the Target's local copy, because I can get the Debugger and use its ErrorOutput.  But the Global copy of the TargetProperty objects doesn't have links back to anything that can be informed.  I don't have a good way to solve this currently.  You can't use the Debugger that caused the creation of the global properties since it might no longer be around.

I could add the hack of "If there's only one debugger, tell it" which would work for command line lldb.  I didn't do that yet in this patch but if there aren't any better ideas I am willing to add that.  It seem unfriendly to spray it across all the debuggers...

3. A better way to fix this would probably be to allow the ValueChanged callbacks to report an error back up to whoever it trying to change the value, which in the end would result in a "settings set" error.  But that's way more infrastructure than I can invest in right now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121036

Files:
  lldb/include/lldb/Interpreter/OptionValue.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/test/API/commands/expression/save_jit_objects/TestSaveJITObjects.py

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D121036.413179.patch
Type: text/x-patch
Size: 8301 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220305/eca09e56/attachment.bin>