[Lldb-commits] [PATCH] D136844: [libclang] Expose completion result kind in `CXCompletionResult`

2022-11-14 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid reopened this revision.
omjavaid added a comment.
This revision is now accepted and ready to land.

Hi this broke https://lab.llvm.org/buildbot/#/builders/245/builds/761
I have reverted the change to make the buildbot green.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136844

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


[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory

2022-11-14 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

ping


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

https://reviews.llvm.org/D135631

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


[Lldb-commits] [PATCH] D137682: Change IRMemoryMap's last-resort magic address to an inaddressable address so it doesn't conflict

2022-11-14 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda updated this revision to Diff 475184.
jasonmolenda added a comment.

Update for Jim's feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137682

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/test/API/lang/c/high-mem-global/Makefile
  lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
  lldb/test/API/lang/c/high-mem-global/main.c

Index: lldb/test/API/lang/c/high-mem-global/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/high-mem-global/main.c
@@ -0,0 +1,9 @@
+
+struct mystruct {
+  int c, d, e;
+} global = {1, 2, 3};
+
+int main ()
+{
+  return global.c; // break here
+}
Index: lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
===
--- /dev/null
+++ lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
@@ -0,0 +1,59 @@
+"""Look that lldb can display a global loaded in high memory at an addressable address."""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+
+class TestHighMemGlobal(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessDarwin  # hardcoding of __DATA segment name
+def test_command_line(self):
+"""Test that we can display a global variable loaded in high memory."""
+self.build()
+
+exe = self.getBuildArtifact("a.out")
+err = lldb.SBError()
+
+target = self.dbg.CreateTarget(exe, '', '', False, err)
+self.assertTrue(target.IsValid())
+module = target.GetModuleAtIndex(0)
+self.assertTrue(module.IsValid())
+data_segment = module.FindSection("__DATA")
+self.assertTrue(data_segment.IsValid())
+err.Clear()
+
+self.expect("expr -- global.c", substrs=[' = 1'])
+self.expect("expr -- global.d", substrs=[' = 2'])
+self.expect("expr -- global.e", substrs=[' = 3'])
+
+err = target.SetSectionLoadAddress(data_segment, 0x)
+self.assertTrue(err.Success())
+self.expect("expr -- global.c", substrs=[' = 1'])
+self.expect("expr -- global.d", substrs=[' = 2'])
+self.expect("expr -- global.e", substrs=[' = 3'])
+
+err = target.SetSectionLoadAddress(data_segment, 0x08814000)
+self.assertTrue(err.Success())
+self.expect("expr -- global.c", substrs=[' = 1'])
+self.expect("expr -- global.d", substrs=[' = 2'])
+self.expect("expr -- global.e", substrs=[' = 3'])
+
+# This is an address in IRMemoryMap::FindSpace where it has an 
+# lldb-side buffer of memory that's used in IR interpreters when
+# memory cannot be allocated in the inferior / functions cannot
+# be jitted.
+err = target.SetSectionLoadAddress(data_segment, 0xdead0fff)
+self.assertTrue(err.Success())
+
+# The global variable `global` is now overlayed by this 
+# IRMemoryMap special buffer, and now we cannot see the variable.
+# Testing that we get the incorrect values at this address ensures 
+# that IRMemoryMap::FindSpace and this test stay in sync.
+self.runCmd("expr -- int $global_c = global.c")
+self.runCmd("expr -- int $global_d = global.d")
+self.runCmd("expr -- int $global_e = global.e")
+self.expect("expr -- $global_c != 1 || $global_d != 2 || $global_e != 3", substrs=[' = true'])
Index: lldb/test/API/lang/c/high-mem-global/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/c/high-mem-global/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Expression/IRMemoryMap.cpp
===
--- lldb/source/Expression/IRMemoryMap.cpp
+++ lldb/source/Expression/IRMemoryMap.cpp
@@ -143,7 +143,7 @@
 if (address_byte_size != UINT32_MAX) {
   switch (address_byte_size) {
   case 8:
-ret = 0xull;
+ret = 0xdead0fffull;
 break;
   case 4:
 ret = 0xee00ull;
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -917,9 +917,8 @@
 stack.back().SetValueType(Value::ValueType::FileAddress);
 // Convert the file address to a load address, so subsequent
 // DWARF operators can operate on it.
-if (frame)
-  stack.back().ConvertToLoadAddress(module_sp.get(),
-frame->CalculateTarget().get());
+if (target)
+  stack.back().ConvertToLoadAddress(module_sp.get(), 

[Lldb-commits] [lldb] 53c45df - Change last-ditch magic address in IRMemoryMap::FindSpace

2022-11-14 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2022-11-14T09:54:05-08:00
New Revision: 53c45df5ed1b8ac606f7388cec025aaba0dee9ba

URL: 
https://github.com/llvm/llvm-project/commit/53c45df5ed1b8ac606f7388cec025aaba0dee9ba
DIFF: 
https://github.com/llvm/llvm-project/commit/53c45df5ed1b8ac606f7388cec025aaba0dee9ba.diff

LOG: Change last-ditch magic address in IRMemoryMap::FindSpace

When we cannot allocate memory in the inferior process, the IR
interpreter's IRMemoryMap::FindSpace will create an lldb local
buffer and assign it an address range in the inferior address
space.  When the interpreter sees an address in that range, it
will read/write from the local buffer instead of the target.  If
this magic address overlaps with actual data in the target, the
target cannot be accessed through expressions.

Instead of using a high memory address that is validly addressable,
this patch uses an address that cannot be accessed on 64-bit systems
that don't actually use all 64 bits of the virtual address.

Differential Revision: https://reviews.llvm.org/D137682
rdar://96248287

Added: 
lldb/test/API/lang/c/high-mem-global/Makefile
lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
lldb/test/API/lang/c/high-mem-global/main.c

Modified: 
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Expression/IRMemoryMap.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 3f302e53c00e1..3c36587f63115 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -917,9 +917,8 @@ bool DWARFExpression::Evaluate(
 stack.back().SetValueType(Value::ValueType::FileAddress);
 // Convert the file address to a load address, so subsequent
 // DWARF operators can operate on it.
-if (frame)
-  stack.back().ConvertToLoadAddress(module_sp.get(),
-frame->CalculateTarget().get());
+if (target)
+  stack.back().ConvertToLoadAddress(module_sp.get(), target);
   }
   break;
 

diff  --git a/lldb/source/Expression/IRMemoryMap.cpp 
b/lldb/source/Expression/IRMemoryMap.cpp
index 6b8f7babc6f00..3c102dd4eaef1 100644
--- a/lldb/source/Expression/IRMemoryMap.cpp
+++ b/lldb/source/Expression/IRMemoryMap.cpp
@@ -143,7 +143,7 @@ lldb::addr_t IRMemoryMap::FindSpace(size_t size) {
 if (address_byte_size != UINT32_MAX) {
   switch (address_byte_size) {
   case 8:
-ret = 0xull;
+ret = 0xdead0fffull;
 break;
   case 4:
 ret = 0xee00ull;

diff  --git a/lldb/test/API/lang/c/high-mem-global/Makefile 
b/lldb/test/API/lang/c/high-mem-global/Makefile
new file mode 100644
index 0..10495940055b6
--- /dev/null
+++ b/lldb/test/API/lang/c/high-mem-global/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git a/lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py 
b/lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
new file mode 100644
index 0..b0df19ac690c7
--- /dev/null
+++ b/lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
@@ -0,0 +1,59 @@
+"""Look that lldb can display a global loaded in high memory at an addressable 
address."""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+
+class TestHighMemGlobal(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessDarwin  # hardcoding of __DATA segment name
+def test_command_line(self):
+"""Test that we can display a global variable loaded in high memory."""
+self.build()
+
+exe = self.getBuildArtifact("a.out")
+err = lldb.SBError()
+
+target = self.dbg.CreateTarget(exe, '', '', False, err)
+self.assertTrue(target.IsValid())
+module = target.GetModuleAtIndex(0)
+self.assertTrue(module.IsValid())
+data_segment = module.FindSection("__DATA")
+self.assertTrue(data_segment.IsValid())
+err.Clear()
+
+self.expect("expr -- global.c", substrs=[' = 1'])
+self.expect("expr -- global.d", substrs=[' = 2'])
+self.expect("expr -- global.e", substrs=[' = 3'])
+
+err = target.SetSectionLoadAddress(data_segment, 0x)
+self.assertTrue(err.Success())
+self.expect("expr -- global.c", substrs=[' = 1'])
+self.expect("expr -- global.d", substrs=[' = 2'])
+self.expect("expr -- global.e", substrs=[' = 3'])
+
+err = target.SetSectionLoadAddress(data_segment, 0x08814000)
+self.assertTrue(err.Success())
+self.expect("expr -- global.c", substrs=[' = 1'])
+self.expect("expr -- global.d", substrs=[' = 2'])
+self.expect("expr -- global.e", substrs=[' = 3'])
+
+# This is

[Lldb-commits] [PATCH] D137682: Change IRMemoryMap's last-resort magic address to an inaddressable address so it doesn't conflict

2022-11-14 Thread Jason Molenda via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG53c45df5ed1b: Change last-ditch magic address in 
IRMemoryMap::FindSpace (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137682

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/test/API/lang/c/high-mem-global/Makefile
  lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
  lldb/test/API/lang/c/high-mem-global/main.c

Index: lldb/test/API/lang/c/high-mem-global/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/high-mem-global/main.c
@@ -0,0 +1,9 @@
+
+struct mystruct {
+  int c, d, e;
+} global = {1, 2, 3};
+
+int main ()
+{
+  return global.c; // break here
+}
Index: lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
===
--- /dev/null
+++ lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
@@ -0,0 +1,59 @@
+"""Look that lldb can display a global loaded in high memory at an addressable address."""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+
+class TestHighMemGlobal(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessDarwin  # hardcoding of __DATA segment name
+def test_command_line(self):
+"""Test that we can display a global variable loaded in high memory."""
+self.build()
+
+exe = self.getBuildArtifact("a.out")
+err = lldb.SBError()
+
+target = self.dbg.CreateTarget(exe, '', '', False, err)
+self.assertTrue(target.IsValid())
+module = target.GetModuleAtIndex(0)
+self.assertTrue(module.IsValid())
+data_segment = module.FindSection("__DATA")
+self.assertTrue(data_segment.IsValid())
+err.Clear()
+
+self.expect("expr -- global.c", substrs=[' = 1'])
+self.expect("expr -- global.d", substrs=[' = 2'])
+self.expect("expr -- global.e", substrs=[' = 3'])
+
+err = target.SetSectionLoadAddress(data_segment, 0x)
+self.assertTrue(err.Success())
+self.expect("expr -- global.c", substrs=[' = 1'])
+self.expect("expr -- global.d", substrs=[' = 2'])
+self.expect("expr -- global.e", substrs=[' = 3'])
+
+err = target.SetSectionLoadAddress(data_segment, 0x08814000)
+self.assertTrue(err.Success())
+self.expect("expr -- global.c", substrs=[' = 1'])
+self.expect("expr -- global.d", substrs=[' = 2'])
+self.expect("expr -- global.e", substrs=[' = 3'])
+
+# This is an address in IRMemoryMap::FindSpace where it has an 
+# lldb-side buffer of memory that's used in IR interpreters when
+# memory cannot be allocated in the inferior / functions cannot
+# be jitted.
+err = target.SetSectionLoadAddress(data_segment, 0xdead0fff)
+self.assertTrue(err.Success())
+
+# The global variable `global` is now overlayed by this 
+# IRMemoryMap special buffer, and now we cannot see the variable.
+# Testing that we get the incorrect values at this address ensures 
+# that IRMemoryMap::FindSpace and this test stay in sync.
+self.runCmd("expr -- int $global_c = global.c")
+self.runCmd("expr -- int $global_d = global.d")
+self.runCmd("expr -- int $global_e = global.e")
+self.expect("expr -- $global_c != 1 || $global_d != 2 || $global_e != 3", substrs=[' = true'])
Index: lldb/test/API/lang/c/high-mem-global/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/c/high-mem-global/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Expression/IRMemoryMap.cpp
===
--- lldb/source/Expression/IRMemoryMap.cpp
+++ lldb/source/Expression/IRMemoryMap.cpp
@@ -143,7 +143,7 @@
 if (address_byte_size != UINT32_MAX) {
   switch (address_byte_size) {
   case 8:
-ret = 0xull;
+ret = 0xdead0fffull;
 break;
   case 4:
 ret = 0xee00ull;
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -917,9 +917,8 @@
 stack.back().SetValueType(Value::ValueType::FileAddress);
 // Convert the file address to a load address, so subsequent
 // DWARF operators can operate on it.
-if (frame)
-  stack.back().ConvertToLoadAddress(module_sp.get(),
-

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks added a comment.

I'll hold off on submitting this until that bug is figured out


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

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


[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

2022-11-14 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM!


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

https://reviews.llvm.org/D136650

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


[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-14 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

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


[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

2022-11-14 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Thanks for looking into this. I'm not sure I like how `TypeMatch` is both input 
and output of the type search. What do you think about making an immutable 
"TypeQuery" object that describes the search characteristics and have the API 
return a TypeMap? Apart from making it easier to reason about it would, e.g., 
allow caching the result of a type lookup. I'm not sure what to du about the 
"visited" map in that case. Maybe it could be a `mutable` member of the query 
object. Ideally the TypeQuery object would have only constructors and getters 
and no methods that change its state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137900

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


[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-14 Thread Teresa Johnson via Phabricator via lldb-commits
tejohnson added a comment.

The naming convention used here for COFF is actually very similar to what is 
done for other ThinLTO save-temps output files, which are placed at the input 
module locations. We may want to just do this across the board. @MaskRay wdyt? 
A few other questions/comments below.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104
 
-  auto AddStream = [&](size_t Task) {
+  auto AddStream = [&](size_t Task, Twine File) {
 return std::make_unique(std::move(OS),

I think you might need to mark the new parameter as unused here and in other 
cases where it isn't used via LLVM_ATTRIBUTE_UNUSED, to avoid build warnings - 
I can't recall how strictly that is enforced.



Comment at: lld/COFF/LTO.cpp:229
+StringRef ltoObjName;
+if (bitcodeFilePath == "ld-temp.o") {
+  ltoObjName =

This case should always be i==0 I think?



Comment at: lld/COFF/LTO.cpp:245
+  saveBuffer(buf[i].second, ltoObjName);
 ret.push_back(make(ctx, MemoryBufferRef(objBuf, ltoObjName)));
   }

The above changes affect both the MemoryBufferRef name as well as the saveTemps 
output file name. I assume the change to the former is what is required for 
PDB, is that correct?



Comment at: lld/MachO/LTO.cpp:132
+"ThinLTO", "Thin", config->thinLTOCacheDir,
+[&](size_t task, Twine originFile, std::unique_ptr mb) {
+  files[task] = std::move(mb);

In some files this parameter is called "filed" and in others "originFile". 
Better to be consistent, and perhaps to use a more descriptive name, something 
like moduleName ?



Comment at: llvm/include/llvm/Support/Caching.h:53
 ///
 /// if (AddStreamFn AddStream = Cache(Task, Key))
 ///   ProduceContent(AddStream);

This and possibly other header file descriptions need updates.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

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


[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

2022-11-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D137900#3925775 , @aprantl wrote:

> Thanks for looking into this. I'm not sure I like how `TypeMatch` is both 
> input and output of the type search. What do you think about making an 
> immutable "TypeQuery" object that describes the search characteristics and 
> have the API return a TypeMap?

The main issue is TypeMap has outlawed copy constructors and assignment 
operators for some reason. And the reason to contain all of the results in the 
object itself is because we often call a type search on a specific module first 
and then fall back to other modules. Having the TypeMap be part of the 
TypeMatch means the TypeMatch object can make decisions on if it is done or not 
(found the max number of matches, etc) and we only need to pass one object into 
each call and avoid copying and merging TypeMap objects which will just add 
inefficiency. So think of an API where we have:

  TypeMap Module::FindTypes(TypeMatch &match);

If we want to search a module list for a type using this API, we will need to 
be able to pass the existing TypeMap result into each call to the above API.

  TypeMap ModuleList::FindTypes(TypeMatch &match) {
TypeMap result;
for (auto module: m_modules) {
  TypeMap module_result = module.FindTypes(match);
  result.InsertUnique(module_result); // Copying things from the 
module_result to result will be a bit inefficient here
  // And now to check if the type match is done we need to pass this 
TypeMap into the match object
  if (match.Done(result))
return;
}
  }

Right now the TypeMatch object has all of the state inside of it and it doesn't 
allow for coding mistakes, like if above someone instead of running 
"match.Done(result)" accidentally did "match.Done(module_result)" (using the 
wrong TypeMap to check if the matching is done. This is why I ended up with a 
single object that doesn't allow for any mistakes when using the APIs.

Let me know if you feel strongly about modifying this.

> Apart from making it easier to reason about it would, e.g., allow caching the 
> result of a type lookup. I'm not sure what to du about the "visited" map in 
> that case. Maybe it could be a `mutable` member of the query object.



> Ideally the TypeQuery object would have only constructors and getters and no 
> methods that change its state.

I will try and get rid of the SetMatchContext functions and use only 
constructors and see how it looks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137900

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


[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

2022-11-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

I just got time to run a quick comparison to see how much more efficient this 
patch is over what we currently have when doing lookups. Since we only resolve 
types whose context matches now, we parse many orders of magnitude fewer types. 
I added the following log line into SymbolFileDWARF::ParseType(...):

  LLDB_LOG(GetLog(LLDBLog::Symbols), "ParseType for die @ {0:x} {1} (\"{2}\")", 
die.GetOffset(), die.GetTagAsCString(), die.GetName());

And in this fixed LLDB we see we find resolve 10 types when searching a debug 
version of LLDB when searching for "llvm::StringRef::iterator":

  (lldb) target create "../Debug/bin/lldb"
  Current executable set to 
'/Users/gclayton/Documents/src/lldb/main/Debug/bin/lldb' (arm64).
  (lldb) log enable lldb symbol
  (lldb) type lookup llvm::StringRef::iterator
   ParseType for die @ 0x2265 DW_TAG_typedef ("iterator")
   ParseType for die @ 0x2187 DW_TAG_class_type ("StringRef")
   ParseType for die @ 0x12e DW_TAG_typedef ("iterator")
   ParseType for die @ 0x50 DW_TAG_class_type ("StringRef")
   ParseType for die @ 0x8fb0d DW_TAG_pointer_type ("")
   ParseType for die @ 0x8fb12 DW_TAG_const_type ("")
   ParseType for die @ 0x8fb18 DW_TAG_base_type ("char")
   ParseType for die @ 0x28efa DW_TAG_pointer_type ("")
   ParseType for die @ 0x28eff DW_TAG_const_type ("")
   ParseType for die @ 0x28f05 DW_TAG_base_type ("char")
  const char *
  const char *
  (lldb) quit

So we parse 10 types now, where as before we parsed 2242541 types. I picked the 
typename "llvm::StringRef::iterator" because the previous code would have done 
a lookup on "iterator" and then weed out the resolved types before returning to 
the user. The old LLDB will every single "iterator" in any STL class, or any 
LLVM classes. And along the way it will resolve the containing classes and all 
of the types needed for the methods in the class, etc.

So this type lookup is 5 orders of magnitude better than we used to be!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137900

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


[Lldb-commits] [PATCH] D137983: [lldb] Disable looking at pointee types to find synthetic value for non-ObjC

2022-11-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks created this revision.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

After D134378 , we started seeing crashes 
with incomplete types (in the
context of shared libraries).

When trying to print a `std::vector &` with only debug info for a
declaration, we now try to use the formatter after D134378 
. With an
incomplete type, this somehow goes into infinite recursion with the
frames

  lldb_private::ValueObject::Dereference
  lldb_private::ValueObjectSynthetic::CreateSynthFilter
  lldb_private::ValueObjectSynthetic::ValueObjectSynthetic
  lldb_private::ValueObject::CalculateSyntheticValue
  lldb_private::ValueObject::HasSyntheticValue

The reason this only started appearing after D134378 
 was because
previously with incomplete types, for names with `<`, lldb would attempt
to parse template parameter DIEs, which were empty, then create an empty
`ClassTemplateSpecializationDecl` which overrode the name used to lookup
a formatter in `FormattersMatchData()` to not include template
parameters (e.g. `std::vector<> &`). After D134378 
 we don't create a
`ClassTemplateSpecializationDecl` when there are no template parameters
and the name to lookup a formatter is the original name (e.g.
`std::vector &`).

The code to try harder with incomplete child compiler types was added in
D79554  for ObjC purposes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137983

Files:
  lldb/source/Core/ValueObject.cpp


Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2673,7 +2673,10 @@
 // In case of incomplete child compiler type, use the pointee type and try
 // to recreate a new ValueObjectChild using it.
 if (!m_deref_valobj) {
-  if (HasSyntheticValue()) {
+  // FIXME: C++ stdlib formatters break with incomplete types (e.g.
+  // `std::vector &`). Remove ObjC restriction once that's resolved.
+  if (Language::LanguageIsObjC(GetPreferredDisplayLanguage()) &&
+  HasSyntheticValue()) {
 child_compiler_type = compiler_type.GetPointeeType();
 
 if (child_compiler_type) {


Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2673,7 +2673,10 @@
 // In case of incomplete child compiler type, use the pointee type and try
 // to recreate a new ValueObjectChild using it.
 if (!m_deref_valobj) {
-  if (HasSyntheticValue()) {
+  // FIXME: C++ stdlib formatters break with incomplete types (e.g.
+  // `std::vector &`). Remove ObjC restriction once that's resolved.
+  if (Language::LanguageIsObjC(GetPreferredDisplayLanguage()) &&
+  HasSyntheticValue()) {
 child_compiler_type = compiler_type.GetPointeeType();
 
 if (child_compiler_type) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D137983: [lldb] Disable looking at pointee types to find synthetic value for non-ObjC

2022-11-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks added a comment.
Herald added a subscriber: JDevlieghere.

lldb crash repro

  $ cat /tmp/a.cc
  #include 
  
  void f(std::vector& v) {
  *(volatile int*) nullptr = 0;
  }
  $ cat /tmp/main.cc
  #include 
  
  void f(std::vector& v);
  
  int main() {
  std::vector v ;
  f(v);
  }
  $ ./build/bin/clang++ -g /tmp/a.cc -o /tmp/a.so -shared -stdlib=libc++ 
-fuse-ld=lld -Wl,-rpath,$PWD/build/rel/lib
  $ ./build/bin/clang++ -g /tmp/main.cc /tmp/a.so -o /tmp/a -stdlib=libc++ 
-fuse-ld=lld -Wl,-rpath,$PWD/build/rel/lib
  $ ./build/bin/lldb /tmp/a -b -o run
  # crash

any help with adding a test case for this would be appreciated, I'm not sure if 
there's any testing for shared libraries in lldb?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137983

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


[Lldb-commits] [PATCH] D137983: [lldb] Disable looking at pointee types to find synthetic value for non-ObjC

2022-11-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks added a comment.

also I'm don't understand how this code doesn't infinite recurse on ObjC and 
can't trace it because I don't have a mac

if anybody has an actual way of fixing this I'd be happy with that too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137983

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


[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-14 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 475296.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.

Update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  lld/COFF/LTO.cpp
  lld/COFF/LTO.h
  lld/ELF/LTO.cpp
  lld/MachO/LTO.cpp
  lld/test/COFF/lto-obj-path.ll
  lld/test/COFF/pdb-thinlto.ll
  lld/test/COFF/thinlto.ll
  lld/wasm/LTO.cpp
  lldb/source/Core/DataFileCache.cpp
  llvm/include/llvm/Support/Caching.h
  llvm/lib/Debuginfod/Debuginfod.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/Support/Caching.cpp
  llvm/tools/llvm-lto/llvm-lto.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -411,7 +411,8 @@
   if (HasErrors)
 return 1;
 
-  auto AddStream = [&](size_t Task) -> std::unique_ptr {
+  auto AddStream = [&](size_t Task,
+   Twine ModuleName) -> std::unique_ptr {
 std::string Path = OutputFilename + "." + utostr(Task);
 
 std::error_code EC;
@@ -420,8 +421,9 @@
 return std::make_unique(std::move(S), Path);
   };
 
-  auto AddBuffer = [&](size_t Task, std::unique_ptr MB) {
-*AddStream(Task)->OS << MB->getBuffer();
+  auto AddBuffer = [&](size_t Task, Twine ModuleName,
+   std::unique_ptr MB) {
+*AddStream(Task, ModuleName)->OS << MB->getBuffer();
   };
 
   FileCache Cache;
Index: llvm/tools/llvm-lto/llvm-lto.cpp
===
--- llvm/tools/llvm-lto/llvm-lto.cpp
+++ llvm/tools/llvm-lto/llvm-lto.cpp
@@ -317,11 +317,11 @@
   if (!CurrentActivity.empty())
 OS << ' ' << CurrentActivity;
   OS << ": ";
-  
+
   DiagnosticPrinterRawOStream DP(OS);
   DI.print(DP);
   OS << '\n';
-  
+
   if (DI.getSeverity() == DS_Error)
 exit(1);
   return true;
@@ -1099,7 +1099,9 @@
 error("writing merged module failed.");
 }
 
-auto AddStream = [&](size_t Task) -> std::unique_ptr {
+auto AddStream =
+[&](size_t Task,
+Twine ModuleName) -> std::unique_ptr {
   std::string PartFilename = OutputFilename;
   if (Parallelism != 1)
 PartFilename += "." + utostr(Task);
Index: llvm/lib/Support/Caching.cpp
===
--- llvm/lib/Support/Caching.cpp
+++ llvm/lib/Support/Caching.cpp
@@ -37,7 +37,8 @@
   TempFilePrefixRef.toVector(TempFilePrefix);
   CacheDirectoryPathRef.toVector(CacheDirectoryPath);
 
-  return [=](unsigned Task, StringRef Key) -> Expected {
+  return [=](unsigned Task, StringRef Key,
+ Twine ModuleName) -> Expected {
 // This choice of file name allows the cache to be pruned (see pruneCache()
 // in include/llvm/Support/CachePruning.h).
 SmallString<64> EntryPath;
@@ -54,7 +55,7 @@
 /*RequiresNullTerminator=*/false);
   sys::fs::closeFile(*FDOrErr);
   if (MBOrErr) {
-AddBuffer(Task, std::move(*MBOrErr));
+AddBuffer(Task, ModuleName, std::move(*MBOrErr));
 return AddStreamFn();
   }
   EC = MBOrErr.getError();
@@ -77,14 +78,15 @@
 struct CacheStream : CachedFileStream {
   AddBufferFn AddBuffer;
   sys::fs::TempFile TempFile;
+  std::string ModuleName;
   unsigned Task;
 
   CacheStream(std::unique_ptr OS, AddBufferFn AddBuffer,
   sys::fs::TempFile TempFile, std::string EntryPath,
-  unsigned Task)
+  std::string ModuleName, unsigned Task)
   : CachedFileStream(std::move(OS), std::move(EntryPath)),
 AddBuffer(std::move(AddBuffer)), TempFile(std::move(TempFile)),
-Task(Task) {}
+ModuleName(ModuleName), Task(Task) {}
 
   ~CacheStream() {
 // TODO: Manually commit rather than using non-trivial destructor,
@@ -133,11 +135,12 @@
  TempFile.TmpName + " to " + ObjectPathName + ": " +
  toString(std::move(E)) + "\n");
 
-AddBuffer(Task, std::move(*MBOrErr));
+AddBuffer(Task, ModuleName, std::move(*MBOrErr));
   }
 };
 
-return [=](size_t Task) -> Expected> {
+return [=](size_t Task, Twine ModuleName)
+   -> Expected> {
   // Create the cache directory if not already done. Doing this lazily
   // ensures the filesystem isn't mutated until the cache is.
   if (std::error_code EC = sys::fs::create_directories(
@@ -158,7 +161,8 @@
   // This CacheStream will move the temporary file into the cache when done.
   return std::make_unique(
   std::make_unique(Tem

[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-14 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104
 
-  auto AddStream = [&](size_t Task) {
+  auto AddStream = [&](size_t Task, Twine File) {
 return std::make_unique(std::move(OS),

tejohnson wrote:
> I think you might need to mark the new parameter as unused here and in other 
> cases where it isn't used via LLVM_ATTRIBUTE_UNUSED, to avoid build warnings 
> - I can't recall how strictly that is enforced.
LLVM_ATTRIBUTE_UNUSED is used to suppress unused functions. For unused 
parameter, I don't see any build warnings when compiling with this patch. I 
feel like it's very common to have unused parameter. 



Comment at: lld/COFF/LTO.cpp:229
+StringRef ltoObjName;
+if (bitcodeFilePath == "ld-temp.o") {
+  ltoObjName =

tejohnson wrote:
> This case should always be i==0 I think?
IIUC, "ld-temp.o" is the name of combined module. Do you mean there will be 
only 1 combined module and it will always be the first task?



Comment at: lld/COFF/LTO.cpp:245
+  saveBuffer(buf[i].second, ltoObjName);
 ret.push_back(make(ctx, MemoryBufferRef(objBuf, ltoObjName)));
   }

tejohnson wrote:
> The above changes affect both the MemoryBufferRef name as well as the 
> saveTemps output file name. I assume the change to the former is what is 
> required for PDB, is that correct?
Yes, it changes both the MemoryBufferRef and the saveTemps output file name 
otherwise the saveTemps output file name won't match with the the names in pdb. 
The former is what's written into PDB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

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


[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

2022-11-14 Thread Adrian Vogelsgesang via Phabricator via lldb-commits
avogelsgesang updated this revision to Diff 475327.
avogelsgesang added a comment.

Fix ownership of CompilerType between "scratch typesystem" and "module type 
system"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132624

Files:
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
@@ -43,6 +43,8 @@
   bool is_supported = is_implementation_supported();
   int_generator gen = my_generator_func();
   std::coroutine_handle<> type_erased_hdl = gen.hdl;
+  std::coroutine_handle incorrectly_typed_hdl =
+  std::coroutine_handle::from_address(gen.hdl.address());
   gen.hdl.resume();// Break at initial_suspend
   gen.hdl.resume();// Break after co_yield
   empty_function_so_we_can_set_a_breakpoint(); // Break at final_suspend
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -16,6 +16,7 @@
 def do_test(self, stdlib_type):
 """Test std::coroutine_handle is displayed correctly."""
 self.build(dictionary={stdlib_type: "1"})
+is_clang = self.expectedCompiler(["clang"])
 
 test_generator_func_ptr_re = re.compile(
 r"^\(a.out`my_generator_func\(\) at main.cpp:[0-9]*\)$")
@@ -37,14 +38,31 @@
 ValueCheck(name="current_value", value = "-1"),
 ])
 ])
-# For type-erased `coroutine_handle<>` we are missing the `promise`
-# but still show `resume` and `destroy`.
-self.expect_expr("type_erased_hdl",
-result_summary=re.compile("^coro frame = 0x[0-9a-f]*$"),
-result_children=[
-ValueCheck(name="resume", summary = test_generator_func_ptr_re),
-ValueCheck(name="destroy", summary = test_generator_func_ptr_re),
-])
+if is_clang:
+# For a type-erased `coroutine_handle<>`, we can still devirtualize
+# the promise call and display the correctly typed promise.
+self.expect_expr("type_erased_hdl",
+result_summary=re.compile("^coro frame = 0x[0-9a-f]*$"),
+result_children=[
+ValueCheck(name="resume", summary = test_generator_func_ptr_re),
+ValueCheck(name="destroy", summary = test_generator_func_ptr_re),
+ValueCheck(name="promise", children=[
+ValueCheck(name="current_value", value = "-1"),
+])
+])
+# For an incorrectly typed `coroutine_handle`, we use the user-supplied
+# incorrect type instead of inferring the correct type. Strictly speaking,
+# incorrectly typed coroutine handles are undefined behavior. However,
+# it provides probably a better debugging experience if we display the
+# promise as seen by the program instead of fixing this bug based on
+# the available debug info.
+self.expect_expr("incorrectly_typed_hdl",
+result_summary=re.compile("^coro frame = 0x[0-9a-f]*$"),
+result_children=[
+ValueCheck(name="resume", summary = test_generator_func_ptr_re),
+ValueCheck(name="destroy", summary = test_generator_func_ptr_re),
+ValueCheck(name="promise", value="-1")
+])
 
 # Run until after the `co_yield`
 process = self.process()
@@ -73,6 +91,18 @@
 ValueCheck(name="current_value", value = "42"),
 ])
 ])
+if is_clang:
+# Devirtualization still works, also at the final suspension point, despite
+# the `resume` pointer being reset to a nullptr
+self.expect_expr("type_erased_hdl",
+result_summary=re.compile("^coro frame = 0x[0-9a-f]*$"),
+result_children=[
+ValueCh

[Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

2022-11-14 Thread Adrian Vogelsgesang via Phabricator via lldb-commits
avogelsgesang added a comment.

@aprantl I still had to adjust the code slightly to copy the type from the 
`ASTContext for '.../a.out'` to the `scratch ASTContext` (without this, my code 
was triggering an assert).
See https://reviews.llvm.org/D132624?vs=474793&id=475327 for the related 
changes.
Because I have no prior experience with `ClangASTImporter` and the usage of 
multiple different `TypeSystemClang` instances, I would appreciate if you could 
take another quick look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132624

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


[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-14 Thread Teresa Johnson via Phabricator via lldb-commits
tejohnson added a comment.

@MaskRay wondering if this is a good change to make for ELF as well, wdyt?




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104
 
-  auto AddStream = [&](size_t Task) {
+  auto AddStream = [&](size_t Task, Twine File) {
 return std::make_unique(std::move(OS),

zequanwu wrote:
> tejohnson wrote:
> > I think you might need to mark the new parameter as unused here and in 
> > other cases where it isn't used via LLVM_ATTRIBUTE_UNUSED, to avoid build 
> > warnings - I can't recall how strictly that is enforced.
> LLVM_ATTRIBUTE_UNUSED is used to suppress unused functions. For unused 
> parameter, I don't see any build warnings when compiling with this patch. I 
> feel like it's very common to have unused parameter. 
LLVM_ATTRIBUTE_UNUSED maps to __attribute__((__unused__)) which works for 
either functions or parameters. However, I see where the macro is defined in 
llvm/include/llvm/Support/Compiler.h that using "(void)unused;" is preferred. 
Either one works (see below). However, it must be the case that there are no 
bots building with -Wunused-parameter, since I see Task is also unused here. So 
nevermind this suggestion, since what you have currently is consistent with 
what is already done here.

```
$ cat unused.cc
int foo(int x1, int x2 __attribute__((__unused__)), int x3) {
  (void)x3;
  return 1;
}
$ clang++ -c unused.cc -Wunused-parameter
unused.cc:1:13: warning: unused parameter 'x1' [-Wunused-parameter]
int foo(int x1, int x2 __attribute__((__unused__)), int x3) {
^
1 warning generated.
```



Comment at: lld/COFF/LTO.cpp:229
+StringRef ltoObjName;
+if (bitcodeFilePath == "ld-temp.o") {
+  ltoObjName =

zequanwu wrote:
> tejohnson wrote:
> > This case should always be i==0 I think?
> IIUC, "ld-temp.o" is the name of combined module. Do you mean there will be 
> only 1 combined module and it will always be the first task?
Yes. So you don't need the handling for i==0 in the name in this case (you 
could probably assert that i==0).



Comment at: llvm/include/llvm/Support/Caching.h:53
 ///
 /// if (AddStreamFn AddStream = Cache(Task, Key))
 ///   ProduceContent(AddStream);

tejohnson wrote:
> This and possibly other header file descriptions need updates.
Can you add a note that ModuleName is the unique module identifier for the 
bitcode module the cache is being checked for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

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


[Lldb-commits] [lldb] 15cd237 - [lldb] Re-phase comments in `ScriptedProcess.get_loaded_images` method (NFC)

2022-11-14 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-11-14T23:25:52-08:00
New Revision: 15cd237cc2afc07074c69ff4bd6c63dd3d6e0ebd

URL: 
https://github.com/llvm/llvm-project/commit/15cd237cc2afc07074c69ff4bd6c63dd3d6e0ebd
DIFF: 
https://github.com/llvm/llvm-project/commit/15cd237cc2afc07074c69ff4bd6c63dd3d6e0ebd.diff

LOG: [lldb] Re-phase comments in `ScriptedProcess.get_loaded_images` method 
(NFC)

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/examples/python/scripted_process/scripted_process.py

Removed: 




diff  --git a/lldb/examples/python/scripted_process/scripted_process.py 
b/lldb/examples/python/scripted_process/scripted_process.py
index 29dbd04fd4d56..86ec1db893a27 100644
--- a/lldb/examples/python/scripted_process/scripted_process.py
+++ b/lldb/examples/python/scripted_process/scripted_process.py
@@ -116,17 +116,17 @@ def get_loaded_images(self):
 """ Get the list of loaded images for the scripted process.
 
 ```
-class ScriptedProcessImage:
-def __init__(file_spec, uuid, load_address):
-  self.file_spec = file_spec
-  self.uuid = uuid
-  self.load_address = load_address
+scripted_image = {
+uuid = "c6ea2b64-f77c-3d27-9528-74f507b9078b",
+path = "/usr/lib/dyld"
+load_addr = 0xbadc0ffee
+}
 ```
 
 Returns:
-List[ScriptedProcessImage]: A list of `ScriptedProcessImage`
-containing for each entry, the name of the library, a UUID,
-an `lldb.SBFileSpec` and a load address.
+List[scripted_image]: A list of `scripted_image` dictionaries
+containing for each entry the library UUID or its file path
+and its load address.
 None if the list is empty.
 """
 return self.loaded_images



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


[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type if it has empty debug info and is required to have complete type.

2022-11-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Looks good, but I think you missed one comment.




Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h:90
   void Dump(Stream &stream);
+  static void RequireCompleteType(CompilerType type);
 

labath wrote:
> I forgot about this the first time around, but it'd be nice if this code 
> could be shared between the pdb and dwarf plugins. Maybe by putting it into 
> TypeSystemClang ?
Did you perhaps miss this comment/request?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066

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