[Lldb-commits] [PATCH] D157167: [lldb] Add flag to DynamicLoader::LoadBinaryWithUUIDAndAddress to control whether we fall back to reading the binary out of memory

2023-08-05 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Target/DynamicLoader.h:267
   ///
+  /// \param[in] allow_use_memory_image_last_resort
+  /// If no better binary image can be found, allow reading the binary

Nit: seems like either `allow_memory_image_last_resort` or 
`use_memory_image_last_resort` would convey the same thing. 



Comment at: 
lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py:38-39
 )
+if self.TraceOn():
+self.runCmd("script print('Creating corefile with command %s')" % 
cmd)
 call(cmd, shell=True)

Out of curiosity what's the point of doing `self.runCmd("script print ...` vs 
printing the same thing directly? Does this run in another context or 
something? Do we save the command output somewhere? 



Comment at: 
lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py:122-123
 )
+dwarfdump_cmd_output = subprocess.check_output(
+('/usr/bin/dwarfdump --uuid "%s"' % self.aout_exe), shell=True
+).decode("utf-8")

I know this was moved and this goes beyond the scope of this patch, but we 
should be using the just-built `llvm-dwarfdump` instead of whatever happens to 
be on the system. Obviously no big deal for extracting the `--uuid` but it's 
relatively easy to hook up and what we do for other llvm tools such as 
dsymutil. objdump, etc. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157167

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D157165: [lldb] [darwin kernel debug] When looking for a Darwin kernel symbol file, call GetSharedModules before DownloadObjectAndSymbolFile

2023-08-05 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157165

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

2023-08-05 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5454-5456
+if (log)
+  log->Printf("LC_NOTE 'kern ver str' found with text '%s'",
+  result.c_str());

Please use `LLDB_LOG(F)`:

```
LLDB_LOGF(log, "LC_NOTE 'kern ver str' found with text '%s'", result.c_str());
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157160

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits