[Lldb-commits] [PATCH] D116195: [LLDB] Allows overwriting the existing line entry at given file address when inserting

2021-12-23 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

How are you planning to make use of this functionality?

I'm asking because I'm wondering if it wouldn't be better to do this kind of 
processing in the PDB code, and then hand this class a finished list of line 
entries. Inserting entries into the middle of a vector is expensive, which is 
why our dwarf code no longer uses this function (it uses the 
vector constructor instead). If we could get pdb to do something 
similar, then we could get rid of this function altogether.




Comment at: lldb/source/Symbol/LineTable.cpp:56-57
+if (pos->file_addr == file_addr) {
+  uint32_t idx = std::distance(m_entries.begin(), pos);
+  m_entries[idx] = entry;
+  return;

`*pos = entry` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116195

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

[Lldb-commits] [PATCH] D106837: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-12-23 Thread PoYao Chang via Phabricator via lldb-commits 
rZhBoYao added a comment.

Hi all, I found this patch causing PR52702  in that 
the parent of this commit 
 and LLDB 12 worked fine.
When disassembling a hello world C program on Linux, LLDB used to show
 `callq  0x401030  ; symbol stub for: puts` 
instead of
`callq  0x401030  ; symbol stub for: ___lldb_unnamed_symbol36`.
Examining the symbol table by running `lldb -b -o 'image dump symtab' a.out` 
used to show:

  [   18] 20   X Undefined   0x
0x 0x0012 puts@GLIBC_2.2.5
   
  [   33] 35   X Code0x00401000
0x001b 0x0212 _init
  [   34] 36  S  Trampoline  0x00401030
0x0010 0x puts
  [   35] 37  SX Code0x00401020
0x0010 0x ___lldb_unnamed_symbol1$$a.out

and now (ToT and LLDB 13) it's:

  [   18] 20   X Undefined   0x
0x 0x0012 puts@GLIBC_2.2.5
   
  [   33] 35   X Code0x00401000
0x001b 0x0212 _init
  [   34] 36  S  Trampoline  0x00401030
0x0010 0x ___lldb_unnamed_symbol36
  [   35] 37  SX Code0x00401020
0x0010 0x ___lldb_unnamed_symbol37

`image dump symtab libc.so.6` gives similar result.
Before ec1a4917  :

  [ 2366]   2367  S  Trampoline  0x00025010 0x77df2010 
0x0010 0x realloc
  [ 2367]   2368  S  Trampoline  0x00025020 0x77df2020 
0x0010 0x __tls_get_addr
  [ 2368]   2369  S  Trampoline  0x00025030 0x77df2030 
0x0010 0x memalign
  [ 2369]   2370  S  Trampoline  0x00025040 0x77df2040 
0x0010 0x _dl_exception_create
  [ 2370]   2371  S  Trampoline  0x00025050 0x77df2050 
0x0010 0x __tunable_get_val
  [ 2371]   2372  S  Trampoline  0x00025060 0x77df2060 
0x0010 0x _dl_find_dso_for_object
  [ 2372]   2373  S  Trampoline  0x00025070 0x77df2070 
0x0010 0x calloc
  [ 2373]   2373  SX Code0x00025000 0x77df2000 
0x0010 0x ___lldb_unnamed_symbol1$$libc.so.6
  [ 2374]   2373  SX Code0x00025300 0x77df2300 
0x0040 0x ___lldb_unnamed_symbol2$$libc.so.6
  [ 2375]   2373  SX Code0x00025340 0x77df2340 
0x02f0 0x ___lldb_unnamed_symbol3$$libc.so.6
  [ 2376]   2373  SX Code0x00025630 0x77df2630 
0x000c 0x ___lldb_unnamed_symbol4$$libc.so.6

After:

  [ 2366]   2367  S  Trampoline  0x00025010 0x77df2010 
0x0010 0x ___lldb_unnamed_symbol2367
  [ 2367]   2368  S  Trampoline  0x00025020 0x77df2020 
0x0010 0x ___lldb_unnamed_symbol2368
  [ 2368]   2369  S  Trampoline  0x00025030 0x77df2030 
0x0010 0x ___lldb_unnamed_symbol2369
  [ 2369]   2370  S  Trampoline  0x00025040 0x77df2040 
0x0010 0x ___lldb_unnamed_symbol2370
  [ 2370]   2371  S  Trampoline  0x00025050 0x77df2050 
0x0010 0x ___lldb_unnamed_symbol2371
  [ 2371]   2372  S  Trampoline  0x00025060 0x77df2060 
0x0010 0x ___lldb_unnamed_symbol2372
  [ 2372]   2373  S  Trampoline  0x00025070 0x77df2070 
0x0010 0x ___lldb_unnamed_symbol2373
  [ 2373]   2374  SX Code0x00025000 0x77df2000 
0x0010 0x ___lldb_unnamed_symbol2374
  [ 2374]   2375  SX Code0x00025300 0x77df2300 
0x0040 0x ___lldb_unnamed_symbol2375
  [ 2375]   2376  SX Code0x00025340 0x77df2340 
0x02f0 0x ___lldb_unnamed_symbol2376
  [ 2376]   2377  SX Code0x00025630 0x77df2630 
0x000c 0x ___lldb_unnamed_symbol2377

Is this intended for the performance boost? It seems to me that "`S  
Trampoline`" symbols should be handled differently.

FWIW, I also found that this doesn't affect macOS tho as `puts` is not even a 
synthetic symbol:

  Index   UserID DSX TypeFile Address/Value Load Address   Size 
  Flags  Name
  --- -- --- --- ---

[Lldb-commits] [PATCH] D116211: With firmware debug sess, if gdb stub knows the UUID/address of binary, try to load it

2021-12-23 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda created this revision.
jasonmolenda added a reviewer: JDevlieghere.
jasonmolenda added a project: LLDB.
jasonmolenda requested review of this revision.

We have a gdb stub that can get the UUID and address/slide of the firmware 
running on a device, and I'm adding support for sending that information up to 
lldb, so lldb can try to find & load the binary at the correct address/offset.

I'd discussed this once in the past, long ago, trying to decide how we would 
handle this - a new packet? - and I think it was Greg who suggested qHostInfo 
could get a couple of new key-value pairs to do it.  I think that idea is the 
right one, but it makes more sense in qProcessInfo to me, so that's where I've 
implemented it.

I haven't thought up a way to test this artificially yet.  While writing the 
patch, I hardcoded the new fields in debugserver's qProcessInfo reply, and I 
removed the darwin dynamic loader plugins in lldb, so they wouldn't get control 
and wipe the image list I manually loaded, hah.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116211

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -561,6 +561,87 @@
 }
   }
 
+  // The remote stub may know about the "main binary" in
+  // the context of a firmware debug session, and can
+  // give us a UUID and an address/slide of where the
+  // binary is loaded in memory.
+  UUID standalone_uuid;
+  addr_t standalone_value;
+  bool standalone_value_is_offset;
+  if (m_gdb_comm.GetProcessStandaloneBinary(
+  standalone_uuid, standalone_value, standalone_value_is_offset)) {
+ModuleSP module_sp;
+
+if (standalone_uuid.IsValid()) {
+  ModuleSpec module_spec;
+  module_spec.GetUUID() = standalone_uuid;
+
+  // Look up UUID in global module cache before attempting
+  // a more expensive search.
+  Status error = ModuleList::GetSharedModule(module_spec, module_sp,
+ nullptr, nullptr, nullptr);
+
+  if (!module_sp.get()) {
+// Force a an external lookup, if that tool is available.
+if (!module_spec.GetSymbolFileSpec())
+  Symbols::DownloadObjectAndSymbolFile(module_spec, true);
+
+if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+  module_sp = std::make_shared(module_spec);
+}
+  }
+
+  // If we couldn't find the binary anywhere else, as a last resort,
+  // read it out of memory.
+  if (!module_sp.get() && standalone_value != LLDB_INVALID_ADDRESS &&
+  !standalone_value_is_offset) {
+char namebuf[80];
+snprintf(namebuf, sizeof(namebuf), "mem-image-0x%" PRIx64,
+ standalone_value);
+module_sp =
+ReadModuleFromMemory(FileSpec(namebuf), standalone_value);
+  }
+
+  if (module_sp.get()) {
+target.GetImages().AppendIfNeeded(module_sp, false);
+
+bool changed = false;
+Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(
+LIBLLDB_LOG_DYNAMIC_LOADER));
+if (module_sp->GetObjectFile()) {
+  if (standalone_value != LLDB_INVALID_ADDRESS) {
+if (log)
+  log->Printf("Loading binary UUID %s at %s 0x%" PRIx64,
+  standalone_uuid.GetAsString().c_str(),
+  standalone_value_is_offset ? "offset" : "address",
+  standalone_value);
+module_sp->SetLoadAddress(target, standalone_value,
+  standalone_value_is_offset, changed);
+  } else {
+// No address/offset/slide, load the binary at file address,
+// offset 0.
+if (log)
+  log->Printf("Loading binary UUID %s at file address",
+  standalone_uuid.GetAsString().c_str());
+const bool value_is_slide = true;
+module_sp->SetLoadAddress(target, 0, value_is_slide, changed);
+  }
+} else {
+  // In-memory image, load at its true address, offset 0.
+  if (log)
+log->Printf("Loading binary UUID %s from memory",
+standalone_uuid.GetAsString().c_str());
+  const bool value_is_slide = true;
+

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

2021-12-23 Thread PoYao Chang via Phabricator via lldb-commits 
rZhBoYao created this revision.
rZhBoYao added reviewers: clayborg, wallace, JDevlieghere.
rZhBoYao added a project: LLDB.
rZhBoYao requested review of this revision.
Herald added a subscriber: lldb-commits.

Mangled::operator! claimes to be true if the object has an empty mangled and 
unmangled name, false otherwise, but it was actually true if the object has an 
empty mangled name. Luckily, this operator doesn't have a lot of users.

The broken logical not operator causes PR52702  as 
https://reviews.llvm.org/D106837 used Mangled::operator! in 
Symbol::SynthesizeNameIfNeeded. For example, consider the symbol "puts" in a 
hello world C program:

  // Inside Symbol::SynthesizeNameIfNeeded
  (lldb) p m_mangled
  (lldb_private::Mangled) $0 = (m_mangled = None, m_demangled = "puts")
  (lldb) p !m_mangled
  (bool) $1 = true  # should be false!!

This leads to Symbol::SynthesizeNameIfNeeded overwriting m_demangled part of 
Mangled (in this case "puts").

In conclusion, this patch turns
`callq  0x401030  ; symbol stub for: ___lldb_unnamed_symbol36`
back into
 `callq  0x401030  ; symbol stub for: puts` .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116217

Files:
  lldb/source/Core/Mangled.cpp


Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -86,7 +86,7 @@
 //  Mangled mangled(...);
 //  if (!file_spec)
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {


Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -86,7 +86,7 @@
 //  Mangled mangled(...);
 //  if (!file_spec)
 //  { ...
-bool Mangled::operator!() const { return !m_mangled; }
+bool Mangled::operator!() const { return !m_mangled && !m_demangled; }
 
 // Clear the mangled and demangled values.
 void Mangled::Clear() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116217: [lldb] Fix PR52702 by fixing Mangled::operator!

2021-12-23 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

We should add a unit test for this to verify this doesn't regress in 
lldb/unittests/Core/MangledTest.cpp for both the operator bool and the ! 
operator




Comment at: lldb/source/Core/Mangled.cpp:79-81
 Mangled::operator void *() const {
   return (m_mangled) ? const_cast(this) : nullptr;
 }

Looks like there is a similar bug here in this function as well. We should 
convert the convert to pointer function here to "operator bool" and add a unit 
test.

```
explicit operator bool() const { return m_mangled || m_demangled; }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116217

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

[Lldb-commits] [PATCH] D106837: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-12-23 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

In D106837#3207676 , @rZhBoYao wrote:

> Hi all, I found this patch causing PR52702  in that 
> the parent of this commit 
>  and LLDB 12 worked 
> fine.
> When disassembling a hello world C program on Linux, LLDB used to show
>  `callq  0x401030  ; symbol stub for: puts` 
> instead of
> `callq  0x401030  ; symbol stub for: 
> ___lldb_unnamed_symbol36`.
> Examining the symbol table by running `lldb -b -o 'image dump symtab' a.out` 
> used to show:
>
>   [   18] 20   X Undefined   0x
> 0x 0x0012 puts@GLIBC_2.2.5
>
>   [   33] 35   X Code0x00401000
> 0x001b 0x0212 _init
>   [   34] 36  S  Trampoline  0x00401030
> 0x0010 0x puts
>   [   35] 37  SX Code0x00401020
> 0x0010 0x ___lldb_unnamed_symbol1$$a.out
>
> and now (ToT and LLDB 13) it's:
>
>   [   18] 20   X Undefined   0x
> 0x 0x0012 puts@GLIBC_2.2.5
>
>   [   33] 35   X Code0x00401000
> 0x001b 0x0212 _init
>   [   34] 36  S  Trampoline  0x00401030
> 0x0010 0x ___lldb_unnamed_symbol36
>   [   35] 37  SX Code0x00401020
> 0x0010 0x ___lldb_unnamed_symbol37
>
> `image dump symtab libc.so.6` gives similar result.
> Before ec1a4917  :
>
>   [ 2366]   2367  S  Trampoline  0x00025010 0x77df2010 
> 0x0010 0x realloc
>   [ 2367]   2368  S  Trampoline  0x00025020 0x77df2020 
> 0x0010 0x __tls_get_addr
>   [ 2368]   2369  S  Trampoline  0x00025030 0x77df2030 
> 0x0010 0x memalign
>   [ 2369]   2370  S  Trampoline  0x00025040 0x77df2040 
> 0x0010 0x _dl_exception_create
>   [ 2370]   2371  S  Trampoline  0x00025050 0x77df2050 
> 0x0010 0x __tunable_get_val
>   [ 2371]   2372  S  Trampoline  0x00025060 0x77df2060 
> 0x0010 0x _dl_find_dso_for_object
>   [ 2372]   2373  S  Trampoline  0x00025070 0x77df2070 
> 0x0010 0x calloc
>   [ 2373]   2373  SX Code0x00025000 0x77df2000 
> 0x0010 0x ___lldb_unnamed_symbol1$$libc.so.6
>   [ 2374]   2373  SX Code0x00025300 0x77df2300 
> 0x0040 0x ___lldb_unnamed_symbol2$$libc.so.6
>   [ 2375]   2373  SX Code0x00025340 0x77df2340 
> 0x02f0 0x ___lldb_unnamed_symbol3$$libc.so.6
>   [ 2376]   2373  SX Code0x00025630 0x77df2630 
> 0x000c 0x ___lldb_unnamed_symbol4$$libc.so.6
>
> After:
>
>   [ 2366]   2367  S  Trampoline  0x00025010 0x77df2010 
> 0x0010 0x ___lldb_unnamed_symbol2367
>   [ 2367]   2368  S  Trampoline  0x00025020 0x77df2020 
> 0x0010 0x ___lldb_unnamed_symbol2368
>   [ 2368]   2369  S  Trampoline  0x00025030 0x77df2030 
> 0x0010 0x ___lldb_unnamed_symbol2369
>   [ 2369]   2370  S  Trampoline  0x00025040 0x77df2040 
> 0x0010 0x ___lldb_unnamed_symbol2370
>   [ 2370]   2371  S  Trampoline  0x00025050 0x77df2050 
> 0x0010 0x ___lldb_unnamed_symbol2371
>   [ 2371]   2372  S  Trampoline  0x00025060 0x77df2060 
> 0x0010 0x ___lldb_unnamed_symbol2372
>   [ 2372]   2373  S  Trampoline  0x00025070 0x77df2070 
> 0x0010 0x ___lldb_unnamed_symbol2373
>   [ 2373]   2374  SX Code0x00025000 0x77df2000 
> 0x0010 0x ___lldb_unnamed_symbol2374
>   [ 2374]   2375  SX Code0x00025300 0x77df2300 
> 0x0040 0x ___lldb_unnamed_symbol2375
>   [ 2375]   2376  SX Code0x00025340 0x77df2340 
> 0x02f0 0x ___lldb_unnamed_symbol2376
>   [ 2376]   2377  SX Code0x00025630 0x77df2630 
> 0x000c 0x ___lldb_unnamed_symbol2377
>
> Is this intended for the performance boost? It seems to me that "`S  
> Trampoline`" symbols should be handled differently.

It i

[Lldb-commits] [PATCH] D115662: [lldb][DWARF] Remove duplicate DIE type assignment

2021-12-23 Thread Luís Ferreira via Phabricator via lldb-commits 
ljmf00 added a comment.

In D115662#3192735 , @labath wrote:

> In D115662#3192651 , @ljmf00 wrote:
>
>> In D115662#3192064 , @labath wrote:
>>
>>> I don't have any issues with this per se, but you may want to sync up with 
>>> @zequanwu, as his D115308  tries to 
>>> delete the second instance.
>>
>> Maybe he can reach out here, but how is this related to D115308 
>>  ?
>
> You're right -- it isn't as related as I originally thought -- you're 
> deduplicating the dietotype map, and he's doing it for the type list.
>
> But it still is related at least on a conceptual level, as both patches deal 
> with recording information about the type, and would be good if those things 
> could happen in a central place (and you seem to have picked different 
> centralization points). Maybe there is a reason it has to be that way (I 
> honestly don't know) but it at least seems like to be aware of each other's 
> efforts.

In the meanwhile, D115308  got merged. What 
are your final thoughts on this? I'm going to rebase and test locally but I'm 
confident that the merged changes doesn't have side effects here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115662

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

[Lldb-commits] [PATCH] D116211: With firmware debug sess, if gdb stub knows the UUID/address of binary, try to load it

2021-12-23 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.

LGMT




Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:584
+
+  if (!module_sp.get()) {
+// Force a an external lookup, if that tool is available.

The `.get()` is redundant.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:613-617
+if (log)
+  log->Printf("Loading binary UUID %s at %s 0x%" PRIx64,
+  standalone_uuid.GetAsString().c_str(),
+  standalone_value_is_offset ? "offset" : 
"address",
+  standalone_value);

Use `LLDB_LOG` which has the `if(log)` inlined in the macro. 

```
LLDB_LOG(log, "Loading binary UUID %s at %s 0x%" PRIx64,
  standalone_uuid.GetAsString().c_str(),
  standalone_value_is_offset ? "offset" : "address",
  standalone_value);
```

Same below.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:626-627
+  standalone_uuid.GetAsString().c_str());
+const bool value_is_slide = true;
+module_sp->SetLoadAddress(target, 0, value_is_slide, changed);
+  }

Same comment as in D116094, but since you didn't change it there, let's keep it 
consistent. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116211

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

[Lldb-commits] [PATCH] D116195: [LLDB] Allows overwriting the existing line entry at given file address when inserting

2021-12-23 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu added a comment.

In D116195#3207672 , @labath wrote:

> How are you planning to make use of this functionality?
>
> I'm asking because I'm wondering if it wouldn't be better to do this kind of 
> processing in the PDB code, and then hand this class a finished list of line 
> entries. Inserting entries into the middle of a vector is expensive, which is 
> why our dwarf code no longer uses this function (it uses the 
> vector constructor instead). If we could get pdb to do 
> something similar, then we could get rid of this function altogether.

My plan was to insert entries when parsing inlined call site(`S_INLINESITE `) 
in ParseBlocksRecursive, which usually happens after ParseLineTable. In PDB, 
line entries in inlined call site often have same file addresses as line 
entries in line table, and the former is better since it describes lines inside 
inlined function rather than lines in caller. And we could have nested inlined 
call sites. The line entries from inner call sites would always overwrite the 
line entries from the outer call sites if they have the same file address.

Maybe it's better to use a set to store the line entries, ordering just by the 
file address so that insertion is cheaper? Currently, it compares other fields 
if two lines have same file address 
(https://github.com/llvm/llvm-project/blob/main/lldb/source/Symbol/LineTable.cpp#L150).
 Is it necessary? I think line entries in line table always have distinct file 
address.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116195

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

[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

2021-12-23 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added a subscriber: arichardson.
mgorny requested review of this revision.

Include the complete list of threads of all running processes
in the FreeBSDKernel plugin.  This makes it possible to inspect
the states (including partial register dumps from PCB) of all kernel
and userspace threads at the time of crash, or at the time of reading
/dev/mem first.


https://reviews.llvm.org/D116255

Files:
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h

Index: lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
===
--- lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
+++ lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
@@ -14,7 +14,7 @@
 class ThreadFreeBSDKernel : public lldb_private::Thread {
 public:
   ThreadFreeBSDKernel(lldb_private::Process &process, lldb::tid_t tid,
-  lldb::addr_t pcb_addr);
+  lldb::addr_t pcb_addr, std::string thread_name);
 
   ~ThreadFreeBSDKernel() override;
 
@@ -25,10 +25,24 @@
   lldb::RegisterContextSP
   CreateRegisterContextForFrame(lldb_private::StackFrame *frame) override;
 
+  const char *GetName() override {
+if (m_thread_name.empty())
+  return nullptr;
+return m_thread_name.c_str();
+  }
+
+  void SetName(const char *name) override {
+if (name && name[0])
+  m_thread_name.assign(name);
+else
+  m_thread_name.clear();
+  }
+
 protected:
   bool CalculateStopInfo() override;
 
 private:
+  std::string m_thread_name;
   lldb::RegisterContextSP m_thread_reg_ctx_sp;
   lldb::addr_t m_pcb_addr;
 };
Index: lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
===
--- lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
+++ lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
@@ -24,8 +24,9 @@
 using namespace lldb_private;
 
 ThreadFreeBSDKernel::ThreadFreeBSDKernel(Process &process, lldb::tid_t tid,
- lldb::addr_t pcb_addr)
-: Thread(process, tid), m_pcb_addr(pcb_addr) {}
+ lldb::addr_t pcb_addr,
+ std::string thread_name)
+: Thread(process, tid), m_thread_name(thread_name), m_pcb_addr(pcb_addr) {}
 
 ThreadFreeBSDKernel::~ThreadFreeBSDKernel() {}
 
@@ -61,9 +62,8 @@
   m_pcb_addr);
   break;
 case llvm::Triple::x86:
-  m_thread_reg_ctx_sp =
-  std::make_shared(
-  *this, new RegisterContextFreeBSD_i386(arch), m_pcb_addr);
+  m_thread_reg_ctx_sp = std::make_shared(
+  *this, new RegisterContextFreeBSD_i386(arch), m_pcb_addr);
   break;
 case llvm::Triple::x86_64:
   m_thread_reg_ctx_sp =
Index: lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
===
--- lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
+++ lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
@@ -46,6 +46,8 @@
 protected:
   bool DoUpdateThreadList(lldb_private::ThreadList &old_thread_list,
   lldb_private::ThreadList &new_thread_list) override;
+
+  lldb::addr_t FindSymbol(const char* name);
 };
 
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_FREEBSDKERNEL_PROCESSFREEBSDKERNEL_H
Index: lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
===
--- lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
+++ lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
@@ -137,12 +137,92 @@
   return false;
 }
 
-const Symbol *pcb_sym =
-GetTarget().GetExecutableModule()->FindFirstSymbolWithNameAndType(
-ConstString("dumppcb"));
-ThreadSP thread_sp(new ThreadFreeBSDKernel(
-*this, 1, pcb_sym ? pcb_sym->GetFileAddress() : LLDB_INVALID_ADDRESS));
-new_thread_list.AddThread(thread_sp);
+Status error;
+
+// struct field offsets are written as symbols so that we don't have
+// to figure them out ourselves
+int32_t offset_p_list = ReadSignedIntegerFromMemory(
+FindSymbol("proc_off_p_list"), 4, -1, error);
+int32_t offset_p_pid =
+ReadSignedIntegerFromMemory(FindSymbol("proc_off_p_pid"), 4, -1, error);
+int32_t offset_p_threads = ReadSignedIntegerFromMemory(
+FindSymbol("proc_off_p_threads"), 4, -1, error);
+int32_t offset_p_comm = ReadSignedIntegerFromMemory(
+FindSymbol("proc_off_p_comm"), 4, -1, error);
+
+int32_t offset_td_tid =

[Lldb-commits] [PATCH] D116211: With firmware debug sess, if gdb stub knows the UUID/address of binary, try to load it

2021-12-23 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda updated this revision to Diff 396111.
jasonmolenda added a comment.

Update patch to incorporate Jonas' feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116211

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -561,6 +561,94 @@
 }
   }
 
+  // The remote stub may know about the "main binary" in
+  // the context of a firmware debug session, and can
+  // give us a UUID and an address/slide of where the
+  // binary is loaded in memory.
+  UUID standalone_uuid;
+  addr_t standalone_value;
+  bool standalone_value_is_offset;
+  if (m_gdb_comm.GetProcessStandaloneBinary(
+  standalone_uuid, standalone_value, standalone_value_is_offset)) {
+ModuleSP module_sp;
+
+if (standalone_uuid.IsValid()) {
+  ModuleSpec module_spec;
+  module_spec.GetUUID() = standalone_uuid;
+
+  // Look up UUID in global module cache before attempting
+  // a more expensive search.
+  Status error = ModuleList::GetSharedModule(module_spec, module_sp,
+ nullptr, nullptr, nullptr);
+
+  if (!module_sp) {
+// Force a an external lookup, if that tool is available.
+if (!module_spec.GetSymbolFileSpec())
+  Symbols::DownloadObjectAndSymbolFile(module_spec, true);
+
+if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+  module_sp = std::make_shared(module_spec);
+}
+  }
+
+  // If we couldn't find the binary anywhere else, as a last resort,
+  // read it out of memory.
+  if (!module_sp.get() && standalone_value != LLDB_INVALID_ADDRESS &&
+  !standalone_value_is_offset) {
+char namebuf[80];
+snprintf(namebuf, sizeof(namebuf), "mem-image-0x%" PRIx64,
+ standalone_value);
+module_sp =
+ReadModuleFromMemory(FileSpec(namebuf), standalone_value);
+  }
+
+  Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(
+  LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (module_sp.get()) {
+target.GetImages().AppendIfNeeded(module_sp, false);
+
+bool changed = false;
+if (module_sp->GetObjectFile()) {
+  if (standalone_value != LLDB_INVALID_ADDRESS) {
+if (log)
+  log->Printf("Loading binary UUID %s at %s 0x%" PRIx64,
+  standalone_uuid.GetAsString().c_str(),
+  standalone_value_is_offset ? "offset" : "address",
+  standalone_value);
+module_sp->SetLoadAddress(target, standalone_value,
+  standalone_value_is_offset, changed);
+  } else {
+// No address/offset/slide, load the binary at file address,
+// offset 0.
+if (log)
+  log->Printf("Loading binary UUID %s at file address",
+  standalone_uuid.GetAsString().c_str());
+const bool value_is_slide = true;
+module_sp->SetLoadAddress(target, 0, value_is_slide, changed);
+  }
+} else {
+  // In-memory image, load at its true address, offset 0.
+  if (log)
+log->Printf("Loading binary UUID %s from memory",
+standalone_uuid.GetAsString().c_str());
+  const bool value_is_slide = true;
+  module_sp->SetLoadAddress(target, 0, value_is_slide, changed);
+}
+
+ModuleList added_module;
+added_module.Append(module_sp, false);
+target.ModulesDidLoad(added_module);
+  } else {
+if (log)
+  log->Printf("Unable to find binary with UUID %s and load it at "
+  "%s 0x%" PRIx64,
+  standalone_uuid.GetAsString().c_str(),
+  standalone_value_is_offset ? "offset" : "address",
+  standalone_value);
+  }
+}
+  }
+
   const StateType state = SetThreadStopInfo(response);
   if (state != eStateInvalid) {
 SetPrivateState(state);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
=

[Lldb-commits] [lldb] 8a26ba6 - Load binary by UUID from qProcessInfo packet fields

2021-12-23 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2021-12-23T15:20:50-08:00
New Revision: 8a26ba6a02f1319ddaca017bbda81c2b82fb4050

URL: 
https://github.com/llvm/llvm-project/commit/8a26ba6a02f1319ddaca017bbda81c2b82fb4050
DIFF: 
https://github.com/llvm/llvm-project/commit/8a26ba6a02f1319ddaca017bbda81c2b82fb4050.diff

LOG: Load binary by UUID from qProcessInfo packet fields

Support three new keys in the qProcessInfo response from the remote
gdb stub to handle the case of attaching to a core running some type
of standalone/firmware code and the stub knows the UUID and load
address-or-slide for the binary.  There will be no proper DynamicLoader
plugin in this scenario, but we can try to locate and load the binary
into lldb at the correct offset.

Differential Revision: https://reviews.llvm.org/D116211
rdar://75191077

Added: 


Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 17588021e3139..980dc77c86f53 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -1003,6 +1003,9 @@ vendor: is a string that represents the vendor (apple)
 endian: is one of "little", "big", or "pdp"
 ptrsize: is a number that represents how big pointers are in bytes
 
+main-binary-uuid: is the UUID of a firmware type binary that the gdb stub 
knows about
+main-binary-address: is the load address of the firmware type binary
+main-binary-slide: is the slide of the firmware type binary, if address isn't 
known
 
 //--
 // "qShlibInfoAddr"

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 07dfa5e04ee57..b5b105351de5d 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1006,6 +1006,23 @@ GDBRemoteCommunicationClient::GetProcessArchitecture() {
   return m_process_arch;
 }
 
+bool GDBRemoteCommunicationClient::GetProcessStandaloneBinary(
+UUID &uuid, addr_t &value, bool &value_is_offset) {
+  if (m_qProcessInfo_is_valid == eLazyBoolCalculate)
+GetCurrentProcessInfo();
+
+  // Return true if we have a UUID or an address/offset of the
+  // main standalone / firmware binary being used.
+  if (!m_process_standalone_uuid.IsValid() &&
+  m_process_standalone_value == LLDB_INVALID_ADDRESS)
+return false;
+
+  uuid = m_process_standalone_uuid;
+  value = m_process_standalone_value;
+  value_is_offset = m_process_standalone_value_is_offset;
+  return true;
+}
+
 bool GDBRemoteCommunicationClient::GetGDBServerVersion() {
   if (m_qGDBServerVersion_is_valid == eLazyBoolCalculate) {
 m_gdb_server_name.clear();
@@ -2147,6 +2164,25 @@ bool 
GDBRemoteCommunicationClient::GetCurrentProcessInfo(bool allow_lazy) {
 } else if (name.equals("elf_abi")) {
   elf_abi = std::string(value);
   ++num_keys_decoded;
+} else if (name.equals("main-binary-uuid")) {
+  m_process_standalone_uuid.SetFromStringRef(value);
+  ++num_keys_decoded;
+} else if (name.equals("main-binary-slide")) {
+  StringExtractor extractor(value);
+  m_process_standalone_value =
+  extractor.GetU64(LLDB_INVALID_ADDRESS, 16);
+  if (m_process_standalone_value != LLDB_INVALID_ADDRESS) {
+m_process_standalone_value_is_offset = true;
+++num_keys_decoded;
+  }
+} else if (name.equals("main-binary-address")) {
+  StringExtractor extractor(value);
+  m_process_standalone_value =
+  extractor.GetU64(LLDB_INVALID_ADDRESS, 16);
+  if (m_process_standalone_value != LLDB_INVALID_ADDRESS) {
+m_process_standalone_value_is_offset = false;
+++num_keys_decoded;
+  }
 }
   }
   if (num_keys_decoded > 0)

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 6765372ce1245..c69c33bb1c153 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -217,6 +217,9 @@ class GDBRemoteCommunicationClient : public 
GDBRemoteClientBase {
 
   const ArchSpec &GetProcessArchitecture();
 
+  bool GetProcessStandaloneBinary(UUID &uuid, lldb::addr_t &value,
+  bool &value_is_offset);
+
   void GetRemoteQSupported();
 
   bool GetVContSupported(char flavor);
@@ -5

[Lldb-commits] [PATCH] D116211: With firmware debug sess, if gdb stub knows the UUID/address of binary, try to load it

2021-12-23 Thread Jason Molenda via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a26ba6a02f1: Load binary by UUID from qProcessInfo packet 
fields (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116211

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -561,6 +561,94 @@
 }
   }
 
+  // The remote stub may know about the "main binary" in
+  // the context of a firmware debug session, and can
+  // give us a UUID and an address/slide of where the
+  // binary is loaded in memory.
+  UUID standalone_uuid;
+  addr_t standalone_value;
+  bool standalone_value_is_offset;
+  if (m_gdb_comm.GetProcessStandaloneBinary(
+  standalone_uuid, standalone_value, standalone_value_is_offset)) {
+ModuleSP module_sp;
+
+if (standalone_uuid.IsValid()) {
+  ModuleSpec module_spec;
+  module_spec.GetUUID() = standalone_uuid;
+
+  // Look up UUID in global module cache before attempting
+  // a more expensive search.
+  Status error = ModuleList::GetSharedModule(module_spec, module_sp,
+ nullptr, nullptr, nullptr);
+
+  if (!module_sp) {
+// Force a an external lookup, if that tool is available.
+if (!module_spec.GetSymbolFileSpec())
+  Symbols::DownloadObjectAndSymbolFile(module_spec, true);
+
+if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+  module_sp = std::make_shared(module_spec);
+}
+  }
+
+  // If we couldn't find the binary anywhere else, as a last resort,
+  // read it out of memory.
+  if (!module_sp.get() && standalone_value != LLDB_INVALID_ADDRESS &&
+  !standalone_value_is_offset) {
+char namebuf[80];
+snprintf(namebuf, sizeof(namebuf), "mem-image-0x%" PRIx64,
+ standalone_value);
+module_sp =
+ReadModuleFromMemory(FileSpec(namebuf), standalone_value);
+  }
+
+  Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(
+  LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (module_sp.get()) {
+target.GetImages().AppendIfNeeded(module_sp, false);
+
+bool changed = false;
+if (module_sp->GetObjectFile()) {
+  if (standalone_value != LLDB_INVALID_ADDRESS) {
+if (log)
+  log->Printf("Loading binary UUID %s at %s 0x%" PRIx64,
+  standalone_uuid.GetAsString().c_str(),
+  standalone_value_is_offset ? "offset" : "address",
+  standalone_value);
+module_sp->SetLoadAddress(target, standalone_value,
+  standalone_value_is_offset, changed);
+  } else {
+// No address/offset/slide, load the binary at file address,
+// offset 0.
+if (log)
+  log->Printf("Loading binary UUID %s at file address",
+  standalone_uuid.GetAsString().c_str());
+const bool value_is_slide = true;
+module_sp->SetLoadAddress(target, 0, value_is_slide, changed);
+  }
+} else {
+  // In-memory image, load at its true address, offset 0.
+  if (log)
+log->Printf("Loading binary UUID %s from memory",
+standalone_uuid.GetAsString().c_str());
+  const bool value_is_slide = true;
+  module_sp->SetLoadAddress(target, 0, value_is_slide, changed);
+}
+
+ModuleList added_module;
+added_module.Append(module_sp, false);
+target.ModulesDidLoad(added_module);
+  } else {
+if (log)
+  log->Printf("Unable to find binary with UUID %s and load it at "
+  "%s 0x%" PRIx64,
+  standalone_uuid.GetAsString().c_str(),
+  standalone_value_is_offset ? "offset" : "address",
+  standalone_value);
+  }
+}
+  }
+
   const StateType state = SetThreadStopInfo(response);
   if (state != eStateInvalid) {
 SetPrivateState(state);
Index: lldb/source/Plugins

[Lldb-commits] [PATCH] D116162: [lldb/python] Fix dangling Event and CommandReturnObject references

2021-12-23 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

This patch looks good to me, however, may be it would be good to have a 
`IsInitialized`/ `IsCleared` method for all SB-types to make sure the object 
hasn't been cleared after going out-of-scope ?

I'm suggesting this because not all SB-types have a `IsValid` method and for 
those who have it, the implementation varies a lot from class to class. So 
comparing the SB object against its default constructed value could be a way to 
check if the object was well-formed.

Having the object reseted by SWIG if it goes out-of-scope could collide with 
this approach. What do you think ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116162

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