[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So it looks like there are bugs in the "llvm-dwarfdump --verify". The blurb I 
posted above clearly has the function's address range contained in it, sorry 
for the false alarm. I have been quite a few problems with DWARF with LTO and 
other post production linkers. The llvm-dwarfdump might be assuming that the 
address ranges in DW_AT_ranges are sorted. I will work on a fix for this if my 
llvm-dwarfdump was from top of tree.

I am worried about performance with this patch. Prior to this we would:
1 - check for DW_AT_ranges, and use that if present
2 - go through all DWARF and look for DW_TAG_subprogram DIEs with valid 
DW_AT_low_pc/DW_AT_high_pc attributes
3 - _only_ parse the line tables as a last resort

This seems like the right way to go to get max performance right?

With this patch we:
1 - check for DW_AT_ranges, and use that if present
2 - _always_ parse the line tables and try to glean address range information 
from this

This seems like it will be slower, though I have not benchmarked this. Should 
be easy to test with a large binary and just comment out the code that checks 
for DW_AT_ranges.

One thing to think about that my flawed example above did show: dead stripped 
functions cause problems for debug info. Both in DW_AT_ranges on the compile 
unit and in all of the DWARF below this. We might want to check any and all 
address ranges to ensure that they are in sections that read + execute 
permissions and if they don't throw them out. It is easy to come up with a 
minimal address range from the main object file prior to parsing any address 
information and use that to weed out the bad entries. If functions have had 
their address set to zero, then we will usually correctly weed these out as 
zero is often part of an image base load address, but not in a section (not 
program header or top level section in LLDB's ObjectFileELF, but in a section 
header section) with r+x permissions. It would be nice to be able to weed these 
addresses out so they don't cause problems possibly as a follow up patch.

So I will be ok with this patch if we can verify that line table parsing is 
faster than checking DIEs for DW_AT_high/low pc attributes. If there is a 
regression, we should keep the code as is.

Sorry for the false alarms. That'll teach me to check patches at the end of a 
long day...




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:58-67
-  // We don't have a DW_AT_ranges attribute, so we need to parse the DWARF
-
-  // If the DIEs weren't parsed, then we don't want all dies for all compile
-  // units to stay loaded when they weren't needed. So we can end up parsing
-  // the DWARF and then throwing them all away to keep memory usage down.
-  ScopedExtractDIEs clear_dies(ExtractDIEsScoped());
-

So this is clearly much more efficient than parsing all the line tables for all 
functions right? Why would we stop doing this? With the old code, if we didn't 
have DW_AT_ranges, and we did have DW_TAG_subprogram DIEs with valid address 
ranges, then we would not go through all of line tables. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78489



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

[Lldb-commits] [PATCH] D72698: [lldb] Add method decls to a CXXRecordDecl only after all their properties are defined

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

> The only code that is currently after addDecl is changing whether the special 
> members are
>  defaulted/trivial. I'm not sure if this actually fixes anything but it's 
> more correct than what we
>  did before.

But at least you return immediately after calling `addDecl`.

When trying to see what `VerifyDecl(cxx_method_decl)` does, I noticed that I 
don't have a `ClangASTContext.cpp` in my source tree. Even grepping for the 
file in the last 400 revisions didn't show it: `git log --stat -n400 | grep 
ClangASTContext.cpp`. No luck with github as well: 
https://github.com/llvm/llvm-project/search?q=ClangASTContext.cpp_q=ClangASTContext.cpp.
 Is that expected?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D72698



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

[Lldb-commits] [PATCH] D78242: [lldb/Docs] Add some more info about the test suite layout

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

I've marked some mistakes that were not addressed yet but are marked as "Done".




Comment at: lldb/docs/resources/test.rst:86
+
+API tests are located under ``lldb/test/API``. Thy are run with the
+``dotest.py``. Tests are written in Python and test binaries (inferiors) are

kwk wrote:
> s/Thy/They
Not done yet.



Comment at: lldb/docs/resources/test.rst:94
+The test directory will always contain a python file, starting with ``Test``.
+Most of the tests are structured as a binary begin debugged, so there will be
+one or more sources file and a Makefile.

kwk wrote:
> s/begin/being
Not done yet.



Comment at: lldb/docs/resources/test.rst:95
+Most of the tests are structured as a binary begin debugged, so there will be
+one or more sources file and a Makefile.
 

kwk wrote:
> s/sources/source
> s/file/files
Not done yet.


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

https://reviews.llvm.org/D78242



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

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

And also it is worth noting that just because new compilers might generate this 
information correctly, it doesn't mean LLDB won't be used to try and load older 
debug info from compilers that don't. LTO can also greatly affect the 
reliability of this information. And many other code post production tools 
often ruin this information when they move things around. The tools know to 
modify the information for the DW_TAG_subprogram DIE, but they very often don't 
even try to update the compile unit's DW_AT_ranges.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78489



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

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

And this one binary had 115 examples of this:

  $ llvm-dwarfdump --verify libIGL.so | grep "DIE address ranges are not 
contained in its parent" | wc -l
  115


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78489



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

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So if llvm is relying on this, we should change llvm. Symbolication will fail 
using LLVM's DWARF parser if it is using this information for anything real.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78489



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

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Sorry I might have not understood this patch's actual implementation. I thought 
we were switching to trusting DW_AT_ranges, which seems we are already doing. 
Let me read the patch code a bit more carefully, not just quickly reading the 
description...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78489



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

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

I see plenty of examples daily, from clang version 7, where this happens. The 
first binary I tried to verify after seeing this patch, I found this error. 
Dump is below. This compile unit also shows what happens when the linker sets 
every function that was dead stripped to address zero: we end up with many 
functions at address zero. What happens if address zero is a real address? Then 
we end up with the first function that got dead stripped that was large enough 
to contain the address we are trying to lookup claiming to own this address. So 
I would vote to NOT change LLDB.

  error: DIE has overlapping address ranges: [0x, 
0x0008) and [0x, 0x001e)
  error: DIE address ranges are not contained in its parent's ranges:
  0x0002022e: DW_TAG_compile_unit
DW_AT_producer  ("Facebook clang version 7.0.0 (llvm: 
fa3433ee8def2a61195b46a32b44800c59bd21a1, cfe: 
e8d08cb2bc1f92443c1e64977d78ec6454370e01, compiler-rt: 
9439182a2f29f42bd22126a1e0d967f9eba34616, lld: 
d83a49d30bca97c359a7d7995da6cc1468041cb1 
e8d08cb2bc1f92443c1e64977d78ec6454370e01) 
(ssh://git-ro.vip.facebook.com/data/gitrepos/osmeta/external/llvm 
fa3433ee8def2a61195b46a32b44800c59bd21a1) (based on LLVM 7.0.0)")
DW_AT_language  (DW_LANG_C_plus_plus)
DW_AT_name  ("xplat/IGL/src/igl/opengl/Device.cpp")
DW_AT_stmt_list (0x4f60)
DW_AT_comp_dir  (".")
DW_AT_GNU_pubnames  (true)
DW_AT_low_pc(0x)
DW_AT_ranges(0x10b0
   [0x45c4, 0x45e8)
   [0x45e8, 0x4610)
   [0x4610, 0x4612)
   [0x, 0x001e)
   [0x, 0x0008)
   [0x, 0x0004)
   [0x4612, 0x4660)
   [0x4660, 0x466a)
   [0x466a, 0x469e)
   [0x469e, 0x46a6)
   [0x46a6, 0x46da)
   [0x46da, 0x4772)
   [0x4774, 0x47a0)
   [0x47a0, 0x47c8)
   [0x47c8, 0x47d8)
   [0x47d8, 0x4826)
   [0x4826, 0x4830)
   [0x4830, 0x487e)
   [0x487e, 0x4888)
   [0x4888, 0x4890)
   [0x4890, 0x48f0)
   [0x48f0, 0x48fc)
   [0x48fc, 0x490e)
   [0x490e, 0x4932)
   [0x3388, 0x3398)
   [0x3398, 0x3410)
   [0x3340, 0x3350)
   [0x3350, 0x3388)
   [0x4932, 0x495c)
   [0x495c, 0x4990)
   [0x3ea0, 0x3eec)
   [0x4990, 0x4996)
   [0x4996, 0x49a8)
   [0x49a8, 0x49ac)
   [0x49ac, 0x49b8)
   [0x49b8, 0x49bc)
   [0x49bc, 0x49c0)
   [0x49c0, 0x4a08)
   [0x4a08, 0x4a0e)
   [0x4a10, 0x4a40)
   [0x4a40, 0x4a5c)
   [0x4a5c, 0x4a62)
   [0x4a64, 0x4a80)
   [0x4a80, 0x4a82)
   [0x4a82, 0x4a86)
   [0x4a86, 0x4a8e)
   [0x4a8e, 0x4a92)
   [0x4a92, 0x4a96)
   [0x4a96, 0x4aa6)
   [0x4288, 0x42b0)
   [0x42be, 0x42d4)
   [0x42d4, 0x42e4)
   [0x42e4, 0x42fa)
   [0x4aa6, 0x4ace)
   [0x4ace, 0x4ade)
   [0x4ade, 0x4b08)
   [0x4b08, 0x4b3c)
   [0x4b3c, 0x4b42)
   [0x4b42, 0x4b54)
   [0x4b54, 0x4b56)
   [0x4b56, 0x4b5a)
   [0x4b5a, 0x4b66)
   [0x4b66, 0x4b6a)
   [0x4b6a, 0x4b6e)
   [0x4b6e, 0x4ba4)
   [0x4ba4, 0x4baa)
   [0x4bac, 0x4bdc)
   [0x4bdc, 0x4bf8)
   [0x4bf8, 0x4bfe)
   [0x4c00, 0x4c1c)
   [0x4c1c, 0x4c1e)
   [0x4c1e, 0x4c22)
   [0x4c22, 0x4c2a)
   [0x4c2a, 0x4c2e)
   

[Lldb-commits] [PATCH] D76955: [lldb/Test] Decode stdout and stderr in case it contains UTF-8

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Lit's `to_string` will just return the string when it's a `str` instance, which 
in Python can still contain UTF-8 characters:

  >>> foo = ""
  >>> isinstance(foo, str)
  True
  >>> foo.encode('utf-8')
  Traceback (most recent call last):
File "", line 1, in 
  UnicodeDecodeError: 'ascii' codec can't decode byte 0xf0 in position 0: 
ordinal not in range(128)


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

https://reviews.llvm.org/D76955



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

[Lldb-commits] [PATCH] D76955: [lldb/Test] Decode stdout and stderr in case it contains UTF-8

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 258857.

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

https://reviews.llvm.org/D76955

Files:
  lldb/test/API/lldbtest.py


Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -110,6 +110,11 @@
 timeoutInfo = 'Reached timeout of {} seconds'.format(
 litConfig.maxIndividualTestTime)
 
+if sys.version_info.major == 2:
+# In Python 2, string objects can contain Unicode characters.
+out = out.decode('utf-8')
+err = err.decode('utf-8')
+
 output = """Script:\n--\n%s\n--\nExit Code: %d\n""" % (
 ' '.join(cmd), exitCode)
 if timeoutInfo is not None:


Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -110,6 +110,11 @@
 timeoutInfo = 'Reached timeout of {} seconds'.format(
 litConfig.maxIndividualTestTime)
 
+if sys.version_info.major == 2:
+# In Python 2, string objects can contain Unicode characters.
+out = out.decode('utf-8')
+err = err.decode('utf-8')
+
 output = """Script:\n--\n%s\n--\nExit Code: %d\n""" % (
 ' '.join(cmd), exitCode)
 if timeoutInfo is not None:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D78396#1993286 , @davide wrote:

> This looks fine to me but please give Pavel another chance to look before 
> committing.


Will wait for @labath approval :) !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This looks fine to me but please give Pavel another chance to look before 
committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Below are the results for the situation you suggested:  release (no-asserts) 
lldb, debug clang (no debug names). Results were collected in a Ubuntu 18.04 
docker image running on CentOS.

  $ for i in {1..10}; do  time ./build-ra/bin/lldb ./build-debug/bin/clang -o 
"br set -n nonexisting_function" -b > /dev/null ;done
  
  -
  before
  -
  real0m11.613s
  real0m11.527s
  real0m11.547s
  real0m11.677s
  real0m11.614s
  real0m11.627s
  real0m11.882s
  real0m11.468s
  real0m11.682s
  real0m11.562s
  -
  avg 0m11.620s
  
  -
  after
  -
  real0m11.887s
  real0m11.614s
  real0m11.812s
  real0m11.723s
  real0m11.605s
  real0m12.080s
  real0m11.673s
  real0m11.751s
  real0m11.660s
  real0m11.647s
  -
  avg 0m11.745s


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D78337



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

[Lldb-commits] [PATCH] D78462: get rid of PythonInteger::GetInteger()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 258831.
lawrence_danna added a comment.

fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78462

Files:
  lldb/bindings/python/python-typemaps.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -123,13 +123,11 @@
   EXPECT_TRUE(major_version_field.IsAllocated());
   EXPECT_TRUE(minor_version_field.IsAllocated());
 
-  PythonInteger major_version_value =
-  major_version_field.AsType();
-  PythonInteger minor_version_value =
-  minor_version_field.AsType();
+  auto major_version_value = As(major_version_field);
+  auto minor_version_value = As(minor_version_field);
 
-  EXPECT_EQ(PY_MAJOR_VERSION, major_version_value.GetInteger());
-  EXPECT_EQ(PY_MINOR_VERSION, minor_version_value.GetInteger());
+  EXPECT_THAT_EXPECTED(major_version_value, llvm::HasValue(PY_MAJOR_VERSION));
+  EXPECT_THAT_EXPECTED(minor_version_value, llvm::HasValue(PY_MINOR_VERSION));
 }
 
 TEST_F(PythonDataObjectsTest, TestGlobalNameResolutionWithDot) {
@@ -137,16 +135,14 @@
   EXPECT_TRUE(sys_path.IsAllocated());
   EXPECT_TRUE(PythonList::Check(sys_path.get()));
 
-  PythonInteger version_major =
-  m_main_module.ResolveName("sys.version_info.major")
-  .AsType();
-  PythonInteger version_minor =
-  m_main_module.ResolveName("sys.version_info.minor")
-  .AsType();
-  EXPECT_TRUE(version_major.IsAllocated());
-  EXPECT_TRUE(version_minor.IsAllocated());
-  EXPECT_EQ(PY_MAJOR_VERSION, version_major.GetInteger());
-  EXPECT_EQ(PY_MINOR_VERSION, version_minor.GetInteger());
+  auto version_major =
+  As(m_main_module.ResolveName("sys.version_info.major"));
+
+  auto version_minor =
+  As(m_main_module.ResolveName("sys.version_info.minor"));
+
+  EXPECT_THAT_EXPECTED(version_major, llvm::HasValue(PY_MAJOR_VERSION));
+  EXPECT_THAT_EXPECTED(version_minor, llvm::HasValue(PY_MINOR_VERSION));
 }
 
 TEST_F(PythonDataObjectsTest, TestDictionaryResolutionWithDot) {
@@ -155,14 +151,14 @@
   dict.SetItemForKey(PythonString("sys"), m_sys_module);
 
   // Now use that dictionary to resolve `sys.version_info.major`
-  PythonInteger version_major =
-  PythonObject::ResolveNameWithDictionary("sys.version_info.major", dict)
-  .AsType();
-  PythonInteger version_minor =
-  PythonObject::ResolveNameWithDictionary("sys.version_info.minor", dict)
-  .AsType();
-  EXPECT_EQ(PY_MAJOR_VERSION, version_major.GetInteger());
-  EXPECT_EQ(PY_MINOR_VERSION, version_minor.GetInteger());
+  auto version_major = As(
+  PythonObject::ResolveNameWithDictionary("sys.version_info.major", dict));
+
+  auto version_minor = As(
+  PythonObject::ResolveNameWithDictionary("sys.version_info.minor", dict));
+
+  EXPECT_THAT_EXPECTED(version_major, llvm::HasValue(PY_MAJOR_VERSION));
+  EXPECT_THAT_EXPECTED(version_minor, llvm::HasValue(PY_MINOR_VERSION));
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonInteger) {
@@ -176,7 +172,8 @@
   PythonInteger python_int(PyRefType::Owned, py_int);
 
   EXPECT_EQ(PyObjectType::Integer, python_int.GetObjectType());
-  EXPECT_EQ(12, python_int.GetInteger());
+  auto python_int_value = As(python_int);
+  EXPECT_THAT_EXPECTED(python_int_value, llvm::HasValue(12));
 #endif
 
   // Verify that `PythonInteger` works correctly when given a PyLong object.
@@ -187,12 +184,14 @@
 
   // Verify that you can reset the value and that it is reflected properly.
   python_long.SetInteger(40);
-  EXPECT_EQ(40, python_long.GetInteger());
+  auto e = As(python_long);
+  EXPECT_THAT_EXPECTED(e, llvm::HasValue(40));
 
   // Test that creating a `PythonInteger` object works correctly with the
   // int constructor.
   PythonInteger constructed_int(7);
-  EXPECT_EQ(7, constructed_int.GetInteger());
+  auto value = As(constructed_int);
+  EXPECT_THAT_EXPECTED(value, llvm::HasValue(7));
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonBoolean) {
@@ -339,7 +338,8 @@
   PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get());
   PythonString chk_str(PyRefType::Borrowed, chk_value2.get());
 
-  EXPECT_EQ(long_value0, chk_int.GetInteger());
+  auto chkint = As(chk_value1);
+  ASSERT_THAT_EXPECTED(chkint, llvm::HasValue(long_value0));
   EXPECT_EQ(string_value1, chk_str.GetString());
 }
 
@@ -367,7 +367,8 @@
   PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get());
   PythonString chk_str(PyRefType::Borrowed, 

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe687aa828263: [lldb/Reproducers] Fix passive replay for 
(char*, size_t) functions. (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D77759?vs=258172=258830#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77759

Files:
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBFileSpec.cpp
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBThread.cpp
  lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
===
--- lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -134,7 +134,7 @@
   int C(float *c);
   float GetC();
   int D(const char *d) const;
-  void GetD(char *buffer, size_t length);
+  size_t GetD(char *buffer, size_t length);
   static void E(double e);
   double GetE();
   static int F();
@@ -257,10 +257,11 @@
   return 2;
 }
 
-void InstrumentedFoo::GetD(char *buffer, size_t length) {
-  LLDB_RECORD_METHOD(void, InstrumentedFoo, GetD, (char *, size_t), buffer,
- length);
+size_t InstrumentedFoo::GetD(char *buffer, size_t length) {
+  LLDB_RECORD_CHAR_PTR_METHOD(size_t, InstrumentedFoo, GetD, (char *, size_t),
+  buffer, "", length);
   ::snprintf(buffer, length, "%s", m_d.c_str());
+  return m_d.size();
 }
 
 void InstrumentedFoo::E(double e) {
@@ -374,7 +375,7 @@
   LLDB_REGISTER_METHOD(int, InstrumentedFoo, GetA, ());
   LLDB_REGISTER_METHOD(int &, InstrumentedFoo, GetB, ());
   LLDB_REGISTER_METHOD(float, InstrumentedFoo, GetC, ());
-  LLDB_REGISTER_METHOD(void, InstrumentedFoo, GetD, (char *, size_t));
+  LLDB_REGISTER_METHOD(size_t, InstrumentedFoo, GetD, (char *, size_t));
   LLDB_REGISTER_METHOD(double, InstrumentedFoo, GetE, ());
   LLDB_REGISTER_METHOD(bool, InstrumentedFoo, GetF, ());
 }
@@ -814,6 +815,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
   }
@@ -839,6 +843,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
   }
@@ -920,6 +927,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -951,6 +961,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -985,6 +998,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -1016,6 +1032,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -1050,6 +1069,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -1081,6 +1103,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
Index: lldb/source/API/SBThread.cpp
===
--- lldb/source/API/SBThread.cpp
+++ lldb/source/API/SBThread.cpp
@@ -312,8 +312,8 @@
 }
 
 size_t SBThread::GetStopDescription(char *dst, size_t dst_len) {
-  LLDB_RECORD_METHOD(size_t, SBThread, GetStopDescription, (char *, size_t), "",
- dst_len);
+  LLDB_RECORD_CHAR_PTR_METHOD(size_t, SBThread, 

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 258827.
mib added a comment.

Fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396

Files:
  lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp
  lldb/source/Plugins/Language/ObjC/CFBasicHash.h
  lldb/source/Plugins/Language/ObjC/CMakeLists.txt
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.h
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
@@ -376,12 +376,16 @@
 	[newMutableDictionary setObject:@"foo" forKey:@"bar19"];
 	[newMutableDictionary setObject:@"foo" forKey:@"bar20"];
 
-	id cfKeys[2] = { @"foo", @"bar", @"baz", @"quux" };
-	id cfValues[2] = { @"foo", @"bar", @"baz", @"quux" };
-	NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
-	CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
-
-	NSAttributedString* attrString = [[NSAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
+id cfKeys[4] = {@"foo", @"bar", @"baz", @"quux"};
+id cfValues[4] = {@"foo", @"bar", @"baz", @"quux"};
+NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(
+nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
+NSDictionary *nscfDictionary = CFBridgingRelease(CFDictionaryCreate(
+nil, (void *)cfKeys, (void *)cfValues, 4, nil, nil));
+CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(
+nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
+
+NSAttributedString* attrString = [[NSAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
 	[attrString isEqual:nil];
 	NSAttributedString* mutableAttrString = [[NSMutableAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
 	[mutableAttrString isEqual:nil];
@@ -412,10 +416,13 @@
 	NSSet* nsset = [[NSSet alloc] initWithObjects:str1,str2,str3,nil];
 	NSSet *nsmutableset = [[NSMutableSet alloc] initWithObjects:str1,str2,str3,nil];
 	[nsmutableset addObject:str4];
+NSSet *nscfSet =
+CFBridgingRelease(CFSetCreate(nil, (void *)cfValues, 2, nil));
 
-	CFDataRef data_ref = CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
+CFDataRef data_ref =
+CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
 
-	CFMutableDataRef mutable_data_ref = CFDataCreateMutable(kCFAllocatorDefault, 8);
+CFMutableDataRef mutable_data_ref = CFDataCreateMutable(kCFAllocatorDefault, 8);
 	CFDataAppendBytes(mutable_data_ref, [mutableData bytes], 5);
 
 	CFMutableStringRef mutable_string_ref = CFStringCreateMutable(NULL,100);
Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
@@ -29,6 +29,8 @@
 ' 2 key/value pairs',
 '(NSDictionary *) newDictionary = ',
 ' 12 key/value pairs',
+'(NSDictionary *) nscfDictionary = ',
+' 4 key/value pairs',
 '(CFDictionaryRef) cfDictionaryRef = ',
 ' 3 key/value pairs',
 '(NSDictionary *) newMutableDictionary = ',
@@ -39,6 +41,36 @@
 ' @"11 elements"',
 ])
 
+self.expect(
+'frame variable -d run-target *nscfDictionary',
+patterns=[
+'\(__NSCFDictionary\) \*nscfDictionary =',
+'key = 0x.* @"foo"',
+'value = 0x.* @"foo"',
+'key = 0x.* @"bar"',
+'value = 0x.* @"bar"',
+'key = 0x.* @"baz"',
+'value = 0x.* @"baz"',
+'key = 0x.* @"quux"',
+'value = 0x.* @"quux"',
+])
+
+
+self.expect(
+  'frame var nscfSet',
+  substrs=[
+  '(NSSet *) nscfSet = ',
+  '2 elements',
+  

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 258826.
mib marked 14 inline comments as done.
mib added a comment.

Address Davide's & Shafik's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396

Files:
  lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp
  lldb/source/Plugins/Language/ObjC/CFBasicHash.h
  lldb/source/Plugins/Language/ObjC/CMakeLists.txt
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.h
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
@@ -376,12 +376,16 @@
 	[newMutableDictionary setObject:@"foo" forKey:@"bar19"];
 	[newMutableDictionary setObject:@"foo" forKey:@"bar20"];
 
-	id cfKeys[2] = { @"foo", @"bar", @"baz", @"quux" };
-	id cfValues[2] = { @"foo", @"bar", @"baz", @"quux" };
-	NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
-	CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
-
-	NSAttributedString* attrString = [[NSAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
+id cfKeys[4] = {@"foo", @"bar", @"baz", @"quux"};
+id cfValues[4] = {@"foo", @"bar", @"baz", @"quux"};
+NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(
+nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
+NSDictionary *nscfDictionary = CFBridgingRelease(CFDictionaryCreate(
+nil, (void *)cfKeys, (void *)cfValues, 4, nil, nil));
+CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(
+nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
+
+NSAttributedString* attrString = [[NSAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
 	[attrString isEqual:nil];
 	NSAttributedString* mutableAttrString = [[NSMutableAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
 	[mutableAttrString isEqual:nil];
@@ -412,10 +416,13 @@
 	NSSet* nsset = [[NSSet alloc] initWithObjects:str1,str2,str3,nil];
 	NSSet *nsmutableset = [[NSMutableSet alloc] initWithObjects:str1,str2,str3,nil];
 	[nsmutableset addObject:str4];
+NSSet *nscfSet =
+CFBridgingRelease(CFSetCreate(nil, (void *)cfValues, 2, nil));
 
-	CFDataRef data_ref = CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
+CFDataRef data_ref =
+CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
 
-	CFMutableDataRef mutable_data_ref = CFDataCreateMutable(kCFAllocatorDefault, 8);
+CFMutableDataRef mutable_data_ref = CFDataCreateMutable(kCFAllocatorDefault, 8);
 	CFDataAppendBytes(mutable_data_ref, [mutableData bytes], 5);
 
 	CFMutableStringRef mutable_string_ref = CFStringCreateMutable(NULL,100);
Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
@@ -29,6 +29,8 @@
 ' 2 key/value pairs',
 '(NSDictionary *) newDictionary = ',
 ' 12 key/value pairs',
+'(NSDictionary *) nscfDictionary = ',
+' 4 key/value pairs',
 '(CFDictionaryRef) cfDictionaryRef = ',
 ' 3 key/value pairs',
 '(NSDictionary *) newMutableDictionary = ',
@@ -39,6 +41,36 @@
 ' @"11 elements"',
 ])
 
+self.expect(
+'frame variable -d run-target *nscfDictionary',
+patterns=[
+'\(__NSCFDictionary\) \*nscfDictionary =',
+'key = 0x.* @"foo"',
+'value = 0x.* @"foo"',
+'key = 0x.* @"bar"',
+'value = 0x.* @"bar"',
+'key = 0x.* @"baz"',
+'value = 0x.* @"baz"',
+'key = 0x.* @"quux"',
+'value = 0x.* @"quux"',
+])
+
+
+self.expect(
+  'frame var nscfSet',
+  substrs=[
+   

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Vedant Kumar via Phabricator via lldb-commits]

vsk updated this revision to Diff 258825.
vsk added a comment.

Address review feedback:

- Make 'escape_style' a regular parameter, use raw string literals


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843

Files:
  lldb/include/lldb/DataFormatters/StringPrinter.h
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Plugins/Language/ObjC/NSString.cpp
  lldb/source/Target/Language.cpp
  lldb/unittests/DataFormatter/CMakeLists.txt
  lldb/unittests/DataFormatter/StringPrinterTests.cpp

Index: lldb/unittests/DataFormatter/StringPrinterTests.cpp
===
--- /dev/null
+++ lldb/unittests/DataFormatter/StringPrinterTests.cpp
@@ -0,0 +1,159 @@
+//===-- StringPrinterTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/DataFormatters/StringPrinter.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/Endian.h"
+#include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+using lldb_private::formatters::StringPrinter;
+using llvm::Optional;
+using llvm::StringRef;
+
+#define QUOTE(x) std::string("\"" x "\"")
+
+/// Format \p input according to the specified string encoding and special char
+/// escape style.
+template 
+static Optional format(StringRef input,
+StringPrinter::EscapeStyle escape_style) {
+  StreamString out;
+  StringPrinter::ReadBufferAndDumpToStreamOptions opts;
+  opts.SetStream();
+  opts.SetSourceSize(input.size());
+  opts.SetNeedsZeroTermination(true);
+  opts.SetEscapeNonPrintables(true);
+  opts.SetIgnoreMaxLength(false);
+  opts.SetEscapeStyle(escape_style);
+  DataExtractor extractor(input.data(), input.size(),
+  endian::InlHostByteOrder(), sizeof(void *));
+  opts.SetData(extractor);
+  const bool success = StringPrinter::ReadBufferAndDumpToStream(opts);
+  if (!success)
+return llvm::None;
+  return out.GetString().str();
+}
+
+// Test ASCII formatting for C++. This behaves exactly like UTF8 formatting for
+// C++, although that's questionable (see FIXME in StringPrinter.cpp).
+TEST(StringPrinterTests, CxxASCII) {
+  auto fmt = [](StringRef str) {
+return format(
+str, StringPrinter::EscapeStyle::CXX);
+  };
+
+  // Special escapes.
+  EXPECT_EQ(fmt({"\0", 1}), QUOTE(""));
+  EXPECT_EQ(fmt("\a"), QUOTE(R"(\a)"));
+  EXPECT_EQ(fmt("\b"), QUOTE(R"(\b)"));
+  EXPECT_EQ(fmt("\f"), QUOTE(R"(\f)"));
+  EXPECT_EQ(fmt("\n"), QUOTE(R"(\n)"));
+  EXPECT_EQ(fmt("\r"), QUOTE(R"(\r)"));
+  EXPECT_EQ(fmt("\t"), QUOTE(R"(\t)"));
+  EXPECT_EQ(fmt("\v"), QUOTE(R"(\v)"));
+  EXPECT_EQ(fmt("\""), QUOTE(R"(\")"));
+  EXPECT_EQ(fmt("\'"), QUOTE(R"(')"));
+  EXPECT_EQ(fmt("\\"), QUOTE(R"(\\)"));
+
+  // Printable characters.
+  EXPECT_EQ(fmt("'"), QUOTE("'"));
+  EXPECT_EQ(fmt("a"), QUOTE("a"));
+  EXPECT_EQ(fmt("Z"), QUOTE("Z"));
+  EXPECT_EQ(fmt("陋"), QUOTE("陋"));
+
+  // Octal (\nnn), hex (\xnn), extended octal (\u or \U).
+  EXPECT_EQ(fmt("\uD55C"), QUOTE("한"));
+  EXPECT_EQ(fmt("\U00010348"), QUOTE("͈"));
+
+  // FIXME: These strings are all rejected, but shouldn't be AFAICT. LLDB finds
+  // that these are not valid utf8 sequences, but that's OK, the raw values
+  // should still be printed out.
+  EXPECT_NE(fmt("\376"), QUOTE(R"(\xfe)")); // \376 is 254 in decimal.
+  EXPECT_NE(fmt("\xfe"), QUOTE(R"(\xfe)")); // \xfe is 254 in decimal.
+}
+
+// Test UTF8 formatting for C++.
+TEST(StringPrinterTests, CxxUTF8) {
+  auto fmt = [](StringRef str) {
+return format(
+str, StringPrinter::EscapeStyle::CXX);
+  };
+
+  // Special escapes.
+  EXPECT_EQ(fmt({"\0", 1}), QUOTE(""));
+  EXPECT_EQ(fmt("\a"), QUOTE(R"(\a)"));
+  EXPECT_EQ(fmt("\b"), QUOTE(R"(\b)"));
+  EXPECT_EQ(fmt("\f"), QUOTE(R"(\f)"));
+  EXPECT_EQ(fmt("\n"), QUOTE(R"(\n)"));
+  EXPECT_EQ(fmt("\r"), QUOTE(R"(\r)"));
+  EXPECT_EQ(fmt("\t"), QUOTE(R"(\t)"));
+  EXPECT_EQ(fmt("\v"), QUOTE(R"(\v)"));
+  EXPECT_EQ(fmt("\""), QUOTE(R"(\")"));
+  EXPECT_EQ(fmt("\'"), QUOTE(R"(')"));
+  EXPECT_EQ(fmt("\\"), QUOTE(R"(\\)"));
+
+  // Printable characters.
+  EXPECT_EQ(fmt("'"), QUOTE("'"));
+  EXPECT_EQ(fmt("a"), QUOTE("a"));
+  EXPECT_EQ(fmt("Z"), QUOTE("Z"));
+  EXPECT_EQ(fmt("陋"), QUOTE("陋"));
+
+  // Octal (\nnn), hex (\xnn), extended octal (\u or \U).
+  EXPECT_EQ(fmt("\uD55C"), QUOTE("한"));
+  EXPECT_EQ(fmt("\U00010348"), QUOTE("͈"));
+
+  // FIXME: 

[Lldb-commits] [PATCH] D76906: [lldb] Fixing the bug that the "log timer" has no tab completion

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D76906#1972413 , @gedatsu217 wrote:

>   [39/575] Linking CXX shared library lib/libc++abi.1.0.dylib
>   FAILED: lib/libc++abi.1.0.dylib 
>   : && /Library/Developer/CommandLineTools/usr/bin/c++ -fPIC 
> -fvisibility-inlines-hidden -Werror=date-time 
> -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
> -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
> -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default 
> -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor 
> -Wstring-conversion -fdiagnostics-color  -g  -dynamiclib 
> -Wl,-headerpad_max_install_names -nodefaultlibs  -compatibility_version 1.0.0 
> -current_version 1.0.0 -o lib/libc++abi.1.0.dylib -install_name 
> @rpath/libc++abi.1.dylib 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_aux_runtime.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_default_handlers.cpp.o
>  projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_demangle.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_exception_storage.cpp.o
>  projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_guard.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_handlers.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_unexpected.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_vector.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_virtual.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/stdlib_exception.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/stdlib_stdexcept.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/stdlib_typeinfo.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/abort_message.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/fallback_malloc.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/private_typeinfo.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/stdlib_new_delete.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_exception.cpp.o 
> projects/libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_personality.cpp.o  
> -Wl,-rpath,@loader_path/../lib  -lSystem  
> -Wl,-exported_symbols_list,/Users/shu/Documents/llvm-project/libcxxabi/src/../lib/itanium-base.exp
>   
> -Wl,-exported_symbols_list,/Users/shu/Documents/llvm-project/libcxxabi/src/../lib/new-delete.exp
>   
> -Wl,-exported_symbols_list,/Users/shu/Documents/llvm-project/libcxxabi/src/../lib/personality-v0.exp
>  && :
>   Undefined symbols for architecture x86_64:
> "__ZTIDu", referenced from:
>-exported_symbol[s_list] command line option
> "__ZTIPDu", referenced from:
>-exported_symbol[s_list] command line option
> "__ZTIPKDu", referenced from:
>-exported_symbol[s_list] command line option
> "__ZTSDu", referenced from:
>-exported_symbol[s_list] command line option
> "__ZTSPDu", referenced from:
>-exported_symbol[s_list] command line option
> "__ZTSPKDu", referenced from:
>-exported_symbol[s_list] command line option
>   ld: symbol(s) not found for architecture x86_64
>   clang: error: linker command failed with exit code 1 (use -v to see 
> invocation)
>
>
> This is an error message. I know it is caused around libc++, but I don't know 
> where parts should I fix. 
>  I did below to solve it.
>
> - delete build directory and make build directory again (cmake -G Ninja 
> -DLLVM_ENABLE_PROJECTS="clang;lldb;libcxx;libcxxabi" ../llvm)
> - build each project(ninja lldb, ninja clang, ninja cxx, ninja cxxabi)
>   - but when I executed "ninja cxx" and "ninja cxxabi", the same error 
> occured.
>
> I sometimes tried to solve this problem this week, but I could not solve 
> it. (I could not go to my laboratory because of coronavirus spreading, and I 
> was busy dealing with that, so I could not take much time.)


I don't think I've seen this error before. Are you on the latest Xcode?

In any case, you should be able to just have "clang;lldb" in the project list 
which should get your check-lldb working. You won't get libc++ tests but at 
least the testsuit itself should work this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76906



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

[Lldb-commits] [lldb] e687aa8 - [lldb/Reproducers] Fix passive replay for (char*, size_t) functions.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-20T13:26:11-07:00
New Revision: e687aa82826385454a3d3a192a46b7a9d4eddae1

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

LOG: [lldb/Reproducers] Fix passive replay for (char*, size_t) functions.

Several SB API functions return strings using (char*, size_t) output
arguments. During capture, we serialize an empty string for the char*
because the memory can be uninitialized.

During active replay, we have custom replay redirects that ensure that
we don't override the buffer from which we're reading, but rather write
to a buffer on the heap with the given length. This is sufficient for
the active reproducer use case, where we only care about the side
effects of the API calls, not the values actually returned.

This approach does not not work for passive replay because here we
ignore all the incoming arguments, and re-execute the current function
with the arguments deserialized from the reproducer. This means that
these function will update the deserialized copy of the arguments,
rather than whatever was passed in by the SWIG wrapper.

To solve this problem, this patch extends the reproducer instrumentation
to handle this special case for passive replay. We nog ignore the
replayer in the registry and the incoming char pointer, and instead
reinvoke the current method on the deserialized class, and populate the
output argument.

Differential revision: https://reviews.llvm.org/D77759

Added: 


Modified: 
lldb/include/lldb/Utility/ReproducerInstrumentation.h
lldb/source/API/SBDebugger.cpp
lldb/source/API/SBFileSpec.cpp
lldb/source/API/SBProcess.cpp
lldb/source/API/SBStructuredData.cpp
lldb/source/API/SBThread.cpp
lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h 
b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
index 1e027c3952fe..346eac52501a 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -97,25 +97,25 @@ template  inline std::string 
stringify_args(const Ts &... ts) {
   R.Register(::method<(::Method)>::record,   
\
  #Result, #Class, #Method, #Signature)
 
-#define LLDB_REGISTER_CHAR_PTR_REDIRECT_STATIC(Result, Class, Method)  
\
+#define LLDB_REGISTER_CHAR_PTR_METHOD_STATIC(Result, Class, Method)
\
   R.Register(  
\
   ::method<(::Method)>::record,   
\
-  _ptr_redirect::method<( 
\
-  ::Method)>::record,
\
+  _char_ptr::method<(::Method)>::record,  
\
   #Result, #Class, #Method, "(char*, size_t");
 
-#define LLDB_REGISTER_CHAR_PTR_REDIRECT(Result, Class, Method) 
\
+#define LLDB_REGISTER_CHAR_PTR_METHOD(Result, Class, Method)   
\
   R.Register(::method<(  
\
  ::Method)>::record, 
\
- _ptr_redirect::method<(   
\
+ _char_ptr::method<( 
\
  ::Method)>::record, 
\
  #Result, #Class, #Method, "(char*, size_t");
 
-#define LLDB_REGISTER_CHAR_PTR_REDIRECT_CONST(Result, Class, Method)   
\
+#define LLDB_REGISTER_CHAR_PTR_METHOD_CONST(Result, Class, Method) 
\
   R.Register(::method<(::Method)>::record, 
\
- _ptr_redirect::method<(::Method)>::record,  
\
+ _char_ptr::method<(::Method)>::record,
\
  #Result, #Class, #Method, "(char*, size_t");
 
 #define LLDB_CONSTRUCT_(T, Class, ...) 
\
@@ -167,6 +167,40 @@ template  inline std::string 
stringify_args(const Ts &... ts) {
 #define LLDB_RECORD_STATIC_METHOD_NO_ARGS(Result, Class, Method)   
\
   LLDB_RECORD_(Result (*)(), (::Method), lldb_private::repro::EmptyArg())
 
+#define LLDB_RECORD_CHAR_PTR_(T1, T2, StrOut, ...) 
\
+  lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,
\
+  stringify_args(__VA_ARGS__));
\
+  if (lldb_private::repro::InstrumentationData _data = 
\
+  LLDB_GET_INSTRUMENTATION_DATA()) {   
\
+if (lldb_private::repro::Serializer *_serializer = 
\
+_data.GetSerializer()) {   
\
+  _recorder.Record(*_serializer, _data.GetRegistry(),  
\
+   _private::repro::invoke::method<(T2)>::record, 
\
+   

[Lldb-commits] [PATCH] D78421: Fix out of sync source code/executable when debugging

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I can see the cases where this would help, but the way you are doing it could 
lead to some odd behavior that might be confusing.

Suppose I have two shared libraries, libBar.dylib and libNotBar.dylib in my 
project.  I debug the process, find a bug in FileFromNotBar.c.  So I change 
FileFromNotBar.c and then rebuild libNotBar.dylib.  Then I go back to lldb and 
hit "run" again.

Suppose the next thing to happen is that I hit a breakpoint in foo.c in 
libBar.dylib.  If I'm filled with doubt about the change I made in 
FileNotFromBar.c, so I then do:

(lldb) source list -f FileFromNotBar.c -l 10

So you check the currently selected frame's module - which is libbar.dylib and 
find that it was built before FileFromNotBar.c and would show me the old 
version.

Showing me the latest version of the file is not great, but totally explicable. 
 Whereas this error, when it happens, would be confusing and hard to understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78421



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

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:49
+  size_t size = 0;
+  size += sizeof(m_ht->base);
+  size += sizeof(m_ht->bits);

Shouldn't we check that `m_ht` is actually managing an object before attempting 
to access it's value?



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:50
+  size += sizeof(m_ht->base);
+  size += sizeof(m_ht->bits);
+

These `sizeof` calls feel like the should just be consolidated into the 
initialization of `size`.



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:698
+  const char *item_name = name.GetCString();
+  uint32_t idx = ExtractIndexFromString(item_name);
+  if (idx < UINT32_MAX && idx >= CalculateNumChildren())

`const`



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:738
+
+  uint32_t num_children = CalculateNumChildren();
+

`const`



Comment at: lldb/source/Plugins/Language/ObjC/NSSet.cpp:601
+
+  uint32_t num_children = CalculateNumChildren();
+

`const` if a value is not supposed to change make it `const` always. This 
prevents bugs where a "const" is modified by mistake.



Comment at: lldb/source/Plugins/Language/ObjC/NSSet.cpp:647
+case 4:
+  *((uint32_t *)buffer.GetBytes()) = (uint32_t)set_item.item_ptr;
+  break;

`reinterpret_cast` and `static_cast` respectively. Same below. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D78462: get rid of PythonInteger::GetInteger()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:3153-3159
+  long long py_return = unwrapOrSetPythonException(
+  As(implementor.CallMethod(callee_name)));
 
   // if it fails, print the error but otherwise go on
   if (PyErr_Occurred()) {
 PyErr_Print();
 PyErr_Clear();

labath wrote:
> This converts the Expected into a python exception, only to clear (and print) 
> it at the next line. Is there a more direct way of doing it?
I don't know, what did you have in mind?   It could just call the Python C API 
directly and not bother converting it into an Expected, but would that be 
better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78462



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

[Lldb-commits] [PATCH] D78462: get rid of PythonInteger::GetInteger()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 258810.
lawrence_danna marked 4 inline comments as done.
lawrence_danna added a comment.

review fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78462

Files:
  lldb/bindings/python/python-typemaps.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -123,13 +123,11 @@
   EXPECT_TRUE(major_version_field.IsAllocated());
   EXPECT_TRUE(minor_version_field.IsAllocated());
 
-  PythonInteger major_version_value =
-  major_version_field.AsType();
-  PythonInteger minor_version_value =
-  minor_version_field.AsType();
+  auto major_version_value = As(major_version_field);
+  auto minor_version_value = As(minor_version_field);
 
-  EXPECT_EQ(PY_MAJOR_VERSION, major_version_value.GetInteger());
-  EXPECT_EQ(PY_MINOR_VERSION, minor_version_value.GetInteger());
+  EXPECT_THAT_EXPECTED(major_version_value, llvm::HasValue(PY_MAJOR_VERSION));
+  EXPECT_THAT_EXPECTED(minor_version_value, llvm::HasValue(PY_MINOR_VERSION));
 }
 
 TEST_F(PythonDataObjectsTest, TestGlobalNameResolutionWithDot) {
@@ -137,16 +135,14 @@
   EXPECT_TRUE(sys_path.IsAllocated());
   EXPECT_TRUE(PythonList::Check(sys_path.get()));
 
-  PythonInteger version_major =
-  m_main_module.ResolveName("sys.version_info.major")
-  .AsType();
-  PythonInteger version_minor =
-  m_main_module.ResolveName("sys.version_info.minor")
-  .AsType();
-  EXPECT_TRUE(version_major.IsAllocated());
-  EXPECT_TRUE(version_minor.IsAllocated());
-  EXPECT_EQ(PY_MAJOR_VERSION, version_major.GetInteger());
-  EXPECT_EQ(PY_MINOR_VERSION, version_minor.GetInteger());
+  auto version_major =
+  As(m_main_module.ResolveName("sys.version_info.major"));
+
+  auto version_minor =
+  As(m_main_module.ResolveName("sys.version_info.minor"));
+
+  EXPECT_THAT_EXPECTED(version_major, llvm::HasValue(PY_MAJOR_VERSION));
+  EXPECT_THAT_EXPECTED(version_minor, llvm::HasValue(PY_MINOR_VERSION));
 }
 
 TEST_F(PythonDataObjectsTest, TestDictionaryResolutionWithDot) {
@@ -155,14 +151,14 @@
   dict.SetItemForKey(PythonString("sys"), m_sys_module);
 
   // Now use that dictionary to resolve `sys.version_info.major`
-  PythonInteger version_major =
-  PythonObject::ResolveNameWithDictionary("sys.version_info.major", dict)
-  .AsType();
-  PythonInteger version_minor =
-  PythonObject::ResolveNameWithDictionary("sys.version_info.minor", dict)
-  .AsType();
-  EXPECT_EQ(PY_MAJOR_VERSION, version_major.GetInteger());
-  EXPECT_EQ(PY_MINOR_VERSION, version_minor.GetInteger());
+  auto version_major = As(
+  PythonObject::ResolveNameWithDictionary("sys.version_info.major", dict));
+
+  auto version_minor = As(
+  PythonObject::ResolveNameWithDictionary("sys.version_info.minor", dict));
+
+  EXPECT_THAT_EXPECTED(version_major, llvm::HasValue(PY_MAJOR_VERSION));
+  EXPECT_THAT_EXPECTED(version_minor, llvm::HasValue(PY_MINOR_VERSION));
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonInteger) {
@@ -176,7 +172,8 @@
   PythonInteger python_int(PyRefType::Owned, py_int);
 
   EXPECT_EQ(PyObjectType::Integer, python_int.GetObjectType());
-  EXPECT_EQ(12, python_int.GetInteger());
+  auto python_int_value = As(python_int);
+  EXPECT_THAT_EXPECTED(python_int_value, llvm::HasValue(12));
 #endif
 
   // Verify that `PythonInteger` works correctly when given a PyLong object.
@@ -187,12 +184,15 @@
 
   // Verify that you can reset the value and that it is reflected properly.
   python_long.SetInteger(40);
-  EXPECT_EQ(40, python_long.GetInteger());
+  auto e = As(python_long);
+  EXPECT_THAT_EXPECTED(e, llvm::Succeeded());
+  EXPECT_EQ(40, e.get());
 
   // Test that creating a `PythonInteger` object works correctly with the
   // int constructor.
   PythonInteger constructed_int(7);
-  EXPECT_EQ(7, constructed_int.GetInteger());
+  auto value = As(constructed_int);
+  EXPECT_THAT_EXPECTED(value, llvm::HasValue(7));
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonBoolean) {
@@ -339,7 +339,10 @@
   PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get());
   PythonString chk_str(PyRefType::Borrowed, chk_value2.get());
 
-  EXPECT_EQ(long_value0, chk_int.GetInteger());
+  auto chkint = As(chk_value1);
+  ASSERT_THAT_EXPECTED(chkint, llvm::Succeeded());
+
+  EXPECT_EQ(long_value0, chkint.get());
   EXPECT_EQ(string_value1, chk_str.GetString());
 }
 
@@ -367,7 

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:8
+
+CFBasicHash::CFBasicHash()
+: m_ptr_size(UINT32_MAX), m_byte_order(eByteOrderInvalid),

Use in class member initializers please then you can use `=default` for the 
constructor this is more idiomatic modern C++



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.h:64
+private:
+  uint32_t m_ptr_size;
+  lldb::ByteOrder m_byte_order;

`uint32_t m_ptr_size = UINT32_MAX;`



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.h:65
+  uint32_t m_ptr_size;
+  lldb::ByteOrder m_byte_order;
+

`lldb::ByteOrder m_byte_order = eByteOrderInvalid;`



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.h:67
+
+  Address m_address;
+

` Address m_address = LLDB_INVALID_ADDRESS;`



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.h:74
+
+  bool m_mutable;
+  bool m_multi;

These `bool` should have default values?

I don't see them initialized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

If I had to guess what this might've related to is the fact that LLVM puts a 
DW_AT_low_pc on the CU even if the CU uses discontiguous ranges - and in that 
case the low_pc has the constant value 0. So that all address values are 
resolved "relative" to that zero, making them absolute. There's some support in 
the DWARF spec for this being a right/good thing.

It's /possible/ that at some point LLVM didn't emit CU level address range info 
(it's redundant with aranges after all - though these days we err on the other 
direction of skipping aranges and just emitting CU ranges) - and just emitted 
the zero low_pc which might've been confusing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78489



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

[Lldb-commits] [lldb] e128d53 - [lldb/Test] Don't friend std::make_unique

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-20T11:48:52-07:00
New Revision: e128d5389547571abe04045e14c679517d01d1f6

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

LOG: [lldb/Test] Don't friend std::make_unique

This wasn't a great idea to begin with, as you can't really rely on the
implementation, but since it also doesn't work with MSVC I've just made
the ctors public.

Added: 


Modified: 
lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Removed: 




diff  --git a/lldb/unittests/Utility/ReproducerInstrumentationTest.cpp 
b/lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
index 1a7f21ded9a5..02e4e7e3420b 100644
--- a/lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ b/lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -73,7 +73,7 @@ inline TestInstrumentationData GetTestInstrumentationData() {
 }
 
 class TestInstrumentationDataRAII {
-private:
+public:
   TestInstrumentationDataRAII(llvm::raw_string_ostream ) {
 g_registry.emplace();
 g_serializer.emplace(os);
@@ -86,12 +86,6 @@ class TestInstrumentationDataRAII {
 g_deserializer.emplace(buffer);
   }
 
-  friend std::unique_ptr
-  std::make_unique(llvm::raw_string_ostream );
-  friend std::unique_ptr
-  std::make_unique(llvm::StringRef );
-
-public:
   ~TestInstrumentationDataRAII() { Reset(); }
 
   void Reset() {



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

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This is almost ready. After we're done with this round of cosmetics, I'll take 
another look a the algorithm and sign off.




Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:18-19
+  return true;
+else if (m_ptr_size == 8 && m_ht_64)
+  return true;
+  }

`else` after return



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:34-38
+  if (m_ptr_size == 4) {
+return UpdateFor(m_ht_32);
+  } else if (m_ptr_size == 8) {
+return UpdateFor(m_ht_64);
+  }

else after return. no need for `{}`



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:753-776
+
+while (tries < num_children) {
+  key_at_idx = m_keys_ptr + (test_idx * m_ptr_size);
+  val_at_idx = m_values_ptr + (test_idx * m_ptr_size);
+
+  key_at_idx = process_sp->ReadPointerFromMemory(key_at_idx, error);
+  if (error.Fail())

Can you add a comment explaining what this loop does?



Comment at: lldb/source/Plugins/Language/ObjC/NSSet.cpp:653
+default:
+  assert(false && "pointer size is not 4 nor 8 - get out of here ASAP");
+}

lldbassert. Also no need for the second part of the comment.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:23
 def nscontainers_data_formatter_commands(self):
+
 self.expect(

stray line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D78333: Add Objective-C property accessors loaded from Clang module DWARF to lookup

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> I'm not sure I understand why are we not adding the property to the lookup 
> ptr? I would assume we would call addDecl to it which should make it visible 
> and I don't see any ObjCPropertyDecl (?) check in that code?

Thanks! This sounds like exactly the kind of feedback I was hoping for, just to 
be sure, could you please clarify what you mean specifically? The problem is 
that the ObjCMethodDecl for the getter that comes from the `@synthesize` 
declaration is missing in the LookupPtr and therefore this check:

  clang::ObjCMethodDecl *getter = nullptr;
  if (!getter_sel.isNull())
getter = isInstance ? class_interface_decl->lookupInstanceMethod(getter_sel)
: class_interface_decl->lookupClassMethod(getter_sel);

fails and so `TypeSystemClang::AddObjCClassProperty()` synthesizes a second 
getter. Where are you suggesting to add the ObjCPropertyDecl to the lookup ptr 
and how would that help?


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

https://reviews.llvm.org/D78333



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

[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG950a8aa165eb: [lldb/Reproducers] Support new replay mode: 
passive replay (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D77602?vs=258390=258784#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77602

Files:
  lldb/include/lldb/Utility/Reproducer.h
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SBReproducerPrivate.h
  lldb/source/Utility/Reproducer.cpp
  lldb/source/Utility/ReproducerInstrumentation.cpp
  lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
===
--- lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -52,23 +52,62 @@
 
 static llvm::Optional g_registry;
 static llvm::Optional g_serializer;
+static llvm::Optional g_deserializer;
 
-inline InstrumentationData GetTestInstrumentationData() {
+class TestInstrumentationData : public InstrumentationData {
+public:
+  TestInstrumentationData() : InstrumentationData() {}
+  TestInstrumentationData(Serializer , Registry )
+  : InstrumentationData(serializer, registry) {}
+  TestInstrumentationData(Deserializer , Registry )
+  : InstrumentationData(deserializer, registry) {}
+};
+
+inline TestInstrumentationData GetTestInstrumentationData() {
+  assert(!(g_serializer && g_deserializer));
   if (g_serializer)
-return InstrumentationData(*g_serializer, *g_registry);
-  return InstrumentationData();
+return TestInstrumentationData(*g_serializer, *g_registry);
+  if (g_deserializer)
+return TestInstrumentationData(*g_deserializer, *g_registry);
+  return TestInstrumentationData();
 }
 
 class TestInstrumentationDataRAII {
-public:
+private:
   TestInstrumentationDataRAII(llvm::raw_string_ostream ) {
 g_registry.emplace();
 g_serializer.emplace(os);
+g_deserializer.reset();
   }
 
-  ~TestInstrumentationDataRAII() {
+  TestInstrumentationDataRAII(llvm::StringRef buffer) {
+g_registry.emplace();
+g_serializer.reset();
+g_deserializer.emplace(buffer);
+  }
+
+  friend std::unique_ptr
+  std::make_unique(llvm::raw_string_ostream );
+  friend std::unique_ptr
+  std::make_unique(llvm::StringRef );
+
+public:
+  ~TestInstrumentationDataRAII() { Reset(); }
+
+  void Reset() {
 g_registry.reset();
 g_serializer.reset();
+g_deserializer.reset();
+  }
+
+  static std::unique_ptr
+  GetRecordingData(llvm::raw_string_ostream ) {
+return std::make_unique(os);
+  }
+
+  static std::unique_ptr
+  GetReplayData(llvm::StringRef buffer) {
+return std::make_unique(buffer);
   }
 };
 
@@ -95,11 +134,17 @@
   InstrumentedFoo(const InstrumentedFoo );
   InstrumentedFoo =(const InstrumentedFoo );
   void A(int a);
+  int GetA();
   void B(int ) const;
+  int ();
   int C(float *c);
+  float GetC();
   int D(const char *d) const;
+  void GetD(char *buffer, size_t length);
   static void E(double e);
+  double GetE();
   static int F();
+  bool GetF();
   void Validate() override;
    }
   virtual bool IsA(Class c) override { return c == Class::Foo; }
@@ -182,35 +227,71 @@
   m_a = a;
 }
 
+int InstrumentedFoo::GetA() {
+  LLDB_RECORD_METHOD_NO_ARGS(int, InstrumentedFoo, GetA);
+
+  return m_a;
+}
+
 void InstrumentedFoo::B(int ) const {
   LLDB_RECORD_METHOD_CONST(void, InstrumentedFoo, B, (int &), b);
   m_called++;
   m_b = b;
 }
 
+int ::GetB() {
+  LLDB_RECORD_METHOD_NO_ARGS(int &, InstrumentedFoo, GetB);
+
+  return m_b;
+}
+
 int InstrumentedFoo::C(float *c) {
   LLDB_RECORD_METHOD(int, InstrumentedFoo, C, (float *), c);
   m_c = *c;
   return 1;
 }
 
+float InstrumentedFoo::GetC() {
+  LLDB_RECORD_METHOD_NO_ARGS(float, InstrumentedFoo, GetC);
+
+  return m_c;
+}
+
 int InstrumentedFoo::D(const char *d) const {
   LLDB_RECORD_METHOD_CONST(int, InstrumentedFoo, D, (const char *), d);
   m_d = std::string(d);
   return 2;
 }
 
+void InstrumentedFoo::GetD(char *buffer, size_t length) {
+  LLDB_RECORD_METHOD(void, InstrumentedFoo, GetD, (char *, size_t), buffer,
+ length);
+  ::snprintf(buffer, length, "%s", m_d.c_str());
+}
+
 void InstrumentedFoo::E(double e) {
   LLDB_RECORD_STATIC_METHOD(void, InstrumentedFoo, E, (double), e);
   g_e = e;
 }
 
+double InstrumentedFoo::GetE() {
+  LLDB_RECORD_METHOD_NO_ARGS(double, InstrumentedFoo, GetE);
+
+  return g_e;
+}
+
 int InstrumentedFoo::F() {
   LLDB_RECORD_STATIC_METHOD_NO_ARGS(int, InstrumentedFoo, F);
   g_f = true;
   return 3;
 }
 
+bool InstrumentedFoo::GetF() {
+  LLDB_RECORD_METHOD_NO_ARGS(bool, InstrumentedFoo, GetF);
+
+  return g_f;
+}
+
 void InstrumentedFoo::Validate() {
   

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D77759#1992252 , @labath wrote:

> In D77759#1988419 , @labath wrote:
>
> > Hmm... well, I like how this (active) is unified at the template 
> > level. It still feels like there's too much code to implement this thing 
> > (coming from the duplication for const, non-const and static variants), but 
> > I'm not sure what can be done about that. I'll need to ponder this a bit 
> > more
>
>
> After sleeping on this a couple of times, I've come to believe that the 
> problem here is that we've let the redirection functions inherit the c++ 
> function pointer weirdness, which resulted in a need to write template goo 
> for const, non-const and static methods (and maybe sometimes even 
> constructors). The goo is (more or less) unavoidable if we want to have 
> compiler-generated "identifiers" for all these functions, but there's a 
> difference between writing that once, and once for each redirection we need 
> to make. Particularly as it doesn't seem like there's a compelling reason for 
> that. There no reason that the "redirected" function in the replayer 
> machinery needs be globally unique in the same way as the "original" 
> functions do. In fact I think it doesn't even have to be a template at all -- 
> it could just take the function-to-wrap as a regular argument instead of a 
> templated one.


Yeah, I've come to the same conclusion while working on this. Hindsight is 
20/20 and changing all that might be a significant amount of work.

> I believe `char_ptr_redirect::method<>::record` could have been 
> written as:
> 
>   // this handles const and non-const methods
>   template // Maybe "Result" not even needed, 
> if this is always the same type.
>   Result char_ptr_redirect(Result (*f)(This *, char *, size_t), This *this_, 
> char *ptr, size_t len) {
> char *buffer = reinterpret_cast(calloc(len, sizeof(char)));
> return f(this_, buffer, len)
>   }
>   // and another overload without This for static functions
> 
> 
> Similar thing could be applied to the "replay" functions introduced here. 
> Reduction from three overloads to two may not sound like much, but given that 
> it would also remove the need for a lot of template goo (and reduce code 
> size), I still find it interesting. It might be possible to also merge the 
> two remaining overloads if we try hard enough.

Agreed and while it would be a small thing it would also help compile times: 
this file is by far the slowest to compile in lldb. That and the templates have 
a significant cognitive overhead.

> Nonetheless, as long as we have only one kind of redirects (char_ptr thingy), 
> it's benefit is not very clear, so I'm not going to insist on that. However, 
> if more of these crop up, I think we should definitely revisit this.

Thanks, I totally share your concerns and really appreciate the pragmatism.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77759



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

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

The one "compiler" (DWARF generator) missing from this list is dsymutil, but I 
would expect it to produce reliable information, too. Given your thorough 
analysis it sounds like we should do this. @clayborg probably remembers why 
LLDB didn't trust the info best. Assuming that the original reason does no 
longer exist, this LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78489



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

[Lldb-commits] [lldb] 950a8aa - [lldb/Reproducers] Support new replay mode: passive replay

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-20T09:41:40-07:00
New Revision: 950a8aa165ebe7c7b8b8b83b2c6c60af5fad89d7

URL: 
https://github.com/llvm/llvm-project/commit/950a8aa165ebe7c7b8b8b83b2c6c60af5fad89d7
DIFF: 
https://github.com/llvm/llvm-project/commit/950a8aa165ebe7c7b8b8b83b2c6c60af5fad89d7.diff

LOG: [lldb/Reproducers] Support new replay mode: passive replay

Support passive replay as proposed in the RFC [1] on lldb-dev and
described in more detail on the lldb website [2].

This patch extends the LLDB_RECORD macros to re-invoke the current
function with arguments deserialized from the reproducer. This relies on
the function being called in the exact same order as during replay. It
uses the same mechanism to toggle the API boundary as during recording,
which guarantees that only boundary crossing calls are replayed.

Another major change is that before this patch we could ignore the
result of an API call, because we only cared about the observable
behavior. Now we need to be able to return the replayed result to the
SWIG bindings.

We reuse a lot of the recording infrastructure, which can be a little
confusing. We kept the existing naming to limit the amount of churn, but
might revisit that in a future patch.

[1] http://lists.llvm.org/pipermail/lldb-dev/2020-April/016100.html
[2] https://lldb.llvm.org/resources/reproducers.html

Differential revision: https://reviews.llvm.org/D77602

Added: 


Modified: 
lldb/include/lldb/Utility/Reproducer.h
lldb/include/lldb/Utility/ReproducerInstrumentation.h
lldb/source/API/SBDebugger.cpp
lldb/source/API/SBReproducer.cpp
lldb/source/API/SBReproducerPrivate.h
lldb/source/Utility/Reproducer.cpp
lldb/source/Utility/ReproducerInstrumentation.cpp
lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Reproducer.h 
b/lldb/include/lldb/Utility/Reproducer.h
index 10d2c801bc95..499243b41202 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/lldb/include/lldb/Utility/Reproducer.h
@@ -27,6 +27,7 @@ class Reproducer;
 enum class ReproducerMode {
   Capture,
   Replay,
+  PassiveReplay,
   Off,
 };
 
@@ -287,7 +288,7 @@ class Generator final {
 
 class Loader final {
 public:
-  Loader(FileSpec root);
+  Loader(FileSpec root, bool passive = false);
 
   template  FileSpec GetFile() {
 if (!HasFile(T::file))
@@ -309,12 +310,15 @@ class Loader final {
 
   const FileSpec () const { return m_root; }
 
+  bool IsPassiveReplay() const { return m_passive_replay; }
+
 private:
   bool HasFile(llvm::StringRef file);
 
   FileSpec m_root;
   std::vector m_files;
   bool m_loaded;
+  bool m_passive_replay;
 };
 
 /// The reproducer enables clients to obtain access to the Generator and
@@ -342,7 +346,7 @@ class Reproducer {
 
 protected:
   llvm::Error SetCapture(llvm::Optional root);
-  llvm::Error SetReplay(llvm::Optional root);
+  llvm::Error SetReplay(llvm::Optional root, bool passive = false);
 
 private:
   static llvm::Optional ();

diff  --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h 
b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
index 3b5dde3d2e2a..1e027c3952fe 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -80,68 +80,72 @@ template  inline std::string 
stringify_args(const Ts &... ts) {
 // #define LLDB_REPRO_INSTR_TRACE
 
 #define LLDB_REGISTER_CONSTRUCTOR(Class, Signature)
\
-  R.Register(::doit, "", #Class, 
\
-#Class, #Signature)
+  R.Register(::record, "",   
\
+#Class, #Class, #Signature)
 
 #define LLDB_REGISTER_METHOD(Result, Class, Method, Signature) 
\
   R.Register(  
\
-  ::method<(::Method)>::doit, 
\
+  ::method<(::Method)>::record,   
\
   #Result, #Class, #Method, #Signature)
 
 #define LLDB_REGISTER_METHOD_CONST(Result, Class, Method, Signature)   
\
   R.Register(::method<(::Method)>::doit, 
\
+ Signature const>::method<(::Method)>::record,   
\
  #Result, #Class, #Method, #Signature)
 
 #define LLDB_REGISTER_STATIC_METHOD(Result, Class, Method, Signature)  
\
-  R.Register(::method<(::Method)>::doit, 
\
+  R.Register(::method<(::Method)>::record,   
\
  #Result, #Class, #Method, #Signature)
 
 #define LLDB_REGISTER_CHAR_PTR_REDIRECT_STATIC(Result, Class, Method)  
\
   R.Register(  
\
-  ::method<(::Method)>::doit, 
\
-  _ptr_redirect::method<(::Method)>::doit,  
\
+  ::method<(::Method)>::record,   
\
+  _ptr_redirect::method<( 
\
+  ::Method)>::record, 

[Lldb-commits] [PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Beside Gabors comment I think I'm happy with this. Thanks!




Comment at: clang/unittests/AST/ASTImporterTest.cpp:5939
+
+/// An ExternalASTSource that keeps track of the tags is completed.
+struct SourceWithCompletedTagList : clang::ExternalASTSource {

"is completed" -> "it completed"



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5954
+   SmallVectorImpl ) override {
+DC->setHasExternalLexicalStorage(true);
+  }

You can remove this as you changed the check in the ASTImporter to only check 
for the existence of an ExternalASTSource. 


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

https://reviews.llvm.org/D78000



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

[Lldb-commits] [lldb] 4cfb71a - [lldb/Scripts] Add verbose and failure only mode to replay script.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-20T09:03:48-07:00
New Revision: 4cfb71adba06037438e37dac84dbd9c3ac2b4319

URL: 
https://github.com/llvm/llvm-project/commit/4cfb71adba06037438e37dac84dbd9c3ac2b4319
DIFF: 
https://github.com/llvm/llvm-project/commit/4cfb71adba06037438e37dac84dbd9c3ac2b4319.diff

LOG: [lldb/Scripts] Add verbose and failure only mode to replay script.

Add two modes to the reproducer replay script that make debugging a
little easier. Verbose mode prints stdout and stderr, regardless of
whether replay was successful. When --failure-only is passed, output is
limited to tests that failed to replay.

Added: 


Modified: 
lldb/scripts/reproducer-replay.py

Removed: 




diff  --git a/lldb/scripts/reproducer-replay.py 
b/lldb/scripts/reproducer-replay.py
index 5e9fab176ab6..f16d6e8b146c 100755
--- a/lldb/scripts/reproducer-replay.py
+++ b/lldb/scripts/reproducer-replay.py
@@ -15,9 +15,10 @@ def run_reproducer(path):
 stderr=subprocess.PIPE)
 reason = None
 try:
+success = proc.returncode == 0
 outs, errs = proc.communicate(timeout=TIMEOUT)
-result = 'PASSED' if proc.returncode == 0 else 'FAILED'
-if proc.returncode != 0:
+result = 'PASSED' if success else 'FAILED'
+if not success:
 outs = outs.decode()
 errs = errs.decode()
 # Do some pattern matching to find out the cause of the failure.
@@ -35,11 +36,18 @@ def run_reproducer(path):
 reason = f'Exit code {proc.returncode}'
 except subprocess.TimeoutExpired:
 proc.kill()
+success = False
 outs, errs = proc.communicate()
 result = 'TIMEOUT'
 
-reason_str = f' ({reason})' if reason else ''
-print(f'{result}: {path}{reason_str}')
+if not FAILURE_ONLY or not success:
+reason_str = f' ({reason})' if reason else ''
+print(f'{result}: {path}{reason_str}')
+if VERBOSE:
+if outs:
+print(outs)
+if errs:
+print(errs)
 
 
 def find_reproducers(path):
@@ -82,12 +90,23 @@ def find_reproducers(path):
 type=str,
 required=True,
 help='Path to the LLDB command line driver')
+parser.add_argument('-v',
+'--verbose',
+help='Print replay output.',
+action='store_true')
+parser.add_argument('--failure-only',
+help='Only log failures.',
+action='store_true')
 args = parser.parse_args()
 
 global LLDB
 global TIMEOUT
+global VERBOSE
+global FAILURE_ONLY
 LLDB = args.lldb
 TIMEOUT = args.timeout
+VERBOSE = args.verbose
+FAILURE_ONLY = args.failure_only
 
 print(
 f'Replaying reproducers in {args.path} with {args.threads} threads and 
a {args.timeout} seconds timeout'



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

[Lldb-commits] [PATCH] D77970: 2/2: [nfc] [lldb] DWARF callbacks: DIERef -> DWARFDIE

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks pretty good, I just think the std::function solution is too smart for its 
own good.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h:82
+  /// The object will remain valid during the whole call statement:
+  /// Function(name, DIERefCallback({callback, name}));
+  struct DIERefCallbackArgs {

jankratochvil wrote:
> This calling with curly brackets is a bit tricky but I found it the least 
> worst option.
I think it would be much simpler to make this a callable object instead (though 
I'm not sure we need to worry about optimizing to this level of detail):
```
class ResultProcessor (?) {
  ResultProcessor(callback, name);
  bool operator()(DIERef ref);
};
```



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:186
 
+  auto dierefcallback = DIERefCallback({callback, class_name.GetStringRef()});
   for (DIERef ref : incomplete_types)

dangling `DIERefCallbackArgs` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77970



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

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG865996ddf626: [lldb] Remove m_last_file_sp from 
SourceManager (authored by emrekultursay, committed by labath).

Changed prior to commit:
  https://reviews.llvm.org/D76806?vs=256416=258738#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,616 @@
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:
+//   ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+#include 
+
+int calc(int x0, int x1, int x2);
+
+int main(int argc, char const *argv[]) {
+  fprintf(stderr, "Hello, world => %d\n", calc(0, 1, 2));
+  return 0;
+}
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 + x111;
+  int x115 = x114 * x113 + x112;
+  int x116 = x115 * x114 + x113;
+  int x117 = x116 * x115 + x114;
+  int x118 = x117 * x116 + x115;
+  int x119 = x118 * x117 + x116;
+  int x120 = x119 * x118 + x117;
+  int x121 = x120 * x119 + x118;
+  int x122 = 

[Lldb-commits] [PATCH] D76804: Add new LLDB setting: use-source-cache

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGacae69d08c88: [lldb] Add new LLDB setting: use-source-cache 
(authored by emrekultursay, committed by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76804

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/SourceManager.cpp

Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -72,7 +72,7 @@
   FileSP file_sp;
   if (same_as_previous)
 file_sp = m_last_file_sp;
-  else if (debugger_sp)
+  else if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -95,7 +95,7 @@
 else
   file_sp = std::make_shared(file_spec, debugger_sp);
 
-if (debugger_sp)
+if (debugger_sp && debugger_sp->GetUseSourceCache())
   debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
   }
   return file_sp;
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -212,6 +212,11 @@
   // use-color changed. Ping the prompt so it can reset the ansi terminal
   // codes.
   SetPrompt(GetPrompt());
+} else if (property_path == g_debugger_properties[ePropertyUseSourceCache].name) {
+  // use-source-cache changed. Wipe out the cache contents if it was disabled.
+  if (!GetUseSourceCache()) {
+m_source_file_cache.Clear();
+  }
 } else if (is_load_script && target_sp &&
load_script_old_value == eLoadScriptFromSymFileWarn) {
   if (target_sp->TargetProperties::GetLoadScriptFromSymbolFile() ==
@@ -338,6 +343,20 @@
   return ret;
 }
 
+bool Debugger::GetUseSourceCache() const {
+  const uint32_t idx = ePropertyUseSourceCache;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
+}
+
+bool Debugger::SetUseSourceCache(bool b) {
+  const uint32_t idx = ePropertyUseSourceCache;
+  bool ret = m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
+  if (!ret) {
+m_source_file_cache.Clear();
+  }
+  return ret;
+}
 bool Debugger::GetHighlightSource() const {
   const uint32_t idx = ePropertyHighlightSource;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
Index: lldb/source/Core/CoreProperties.td
===
--- lldb/source/Core/CoreProperties.td
+++ lldb/source/Core/CoreProperties.td
@@ -103,6 +103,10 @@
 Global,
 DefaultTrue,
 Desc<"Whether to use Ansi color codes or not.">;
+  def UseSourceCache: Property<"use-source-cache", "Boolean">,
+Global,
+DefaultTrue,
+Desc<"Whether to cache source files in memory or not.">;
   def AutoOneLineSummaries: Property<"auto-one-line-summaries", "Boolean">,
 Global,
 DefaultTrue,
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -1374,6 +1374,18 @@
   return (m_opaque_sp ? m_opaque_sp->GetUseColor() : false);
 }
 
+bool SBDebugger::SetUseSourceCache(bool value) {
+  LLDB_RECORD_METHOD(bool, SBDebugger, SetUseSourceCache, (bool), value);
+
+  return (m_opaque_sp ? m_opaque_sp->SetUseSourceCache(value) : false);
+}
+
+bool SBDebugger::GetUseSourceCache() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBDebugger, GetUseSourceCache);
+
+  return (m_opaque_sp ? m_opaque_sp->GetUseSourceCache() : false);
+}
+
 bool SBDebugger::GetDescription(SBStream ) {
   LLDB_RECORD_METHOD(bool, SBDebugger, GetDescription, (lldb::SBStream &),
  description);
Index: lldb/include/lldb/Core/SourceManager.h
===
--- lldb/include/lldb/Core/SourceManager.h
+++ lldb/include/lldb/Core/SourceManager.h
@@ -101,6 +101,9 @@
 void AddSourceFile(const FileSP _sp);
 FileSP FindSourceFile(const FileSpec _spec) const;
 
+// Removes all elements from the cache.
+void Clear() { m_file_cache.clear(); }
+
   protected:
 typedef std::map FileCache;
 FileCache m_file_cache;
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -273,6 +273,10 @@
 
   bool SetUseColor(bool use_color);
 
+  bool GetUseSourceCache() const;
+
+  bool SetUseSourceCache(bool use_source_cache);
+
   bool 

[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6afa5c407c13: [lldb] Prefer executable files from sysroot 
over files from local filesystem (authored by yuri, committed by labath).

Changed prior to commit:
  https://reviews.llvm.org/D77529?vs=258034=258735#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529

Files:
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64.core


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -210,6 +210,44 @@
 self.dbg.DeleteTarget(target)
 
 @skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# Copy wrong executable to the location outside of sysroot
+exe_outside = os.path.join(self.getBuildDir(), "bin", "a.out")
+lldbutil.mkdir_p(os.path.dirname(exe_outside))
+shutil.copyfile("altmain.out", exe_outside)
+
+# Copy correct executable to the location inside sysroot
+tmp_sysroot = os.path.join(self.getBuildDir(), "mock_sysroot")
+exe_inside = os.path.join(tmp_sysroot, os.path.relpath(exe_outside, 
"/"))
+lldbutil.mkdir_p(os.path.dirname(exe_inside))
+shutil.copyfile("linux-x86_64.out", exe_inside)
+
+# Prepare patched core file
+core_file = os.path.join(self.getBuildDir(), "patched.core")
+with open("linux-x86_64.core", "rb") as f:
+core = f.read()
+core = replace_path(core, "/test" * 817 + "/a.out", exe_outside)
+with open(core_file, "wb") as f:
+f.write(core)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % 
tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore(core_file)
+
+# Check that we found executable from the sysroot
+mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
+self.assertEqual(mod_path, exe_inside)
+self.check_all(process, self._x86_64_pid, self._x86_64_regions, 
"a.out")
+
+self.dbg.DeleteTarget(target)
+
+@skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("ARM")
 def test_arm_core(self):
 # check 32 bit ARM core file
@@ -369,3 +407,9 @@
 self.check_all(process, pid, region_count, thread_name)
 
 self.dbg.DeleteTarget(target)
+
+def replace_path(binary, replace_from, replace_to):
+src = replace_from.encode()
+dst = replace_to.encode()
+dst += b"\0" * (len(src) - len(dst))
+return binary.replace(src, dst)
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -21,7 +21,7 @@
 return m_remote_platform_sp->GetModuleSpec(module_file_spec, arch,
module_spec);
 
-  return Platform::GetModuleSpec(module_file_spec, arch, module_spec);
+  return false;
 }
 
 Status RemoteAwarePlatform::RunShellCommand(


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -210,6 +210,44 @@
 self.dbg.DeleteTarget(target)
 
 @skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# Copy wrong executable to the location outside of sysroot
+exe_outside = os.path.join(self.getBuildDir(), "bin", "a.out")
+lldbutil.mkdir_p(os.path.dirname(exe_outside))
+shutil.copyfile("altmain.out", exe_outside)
+
+# Copy correct executable to the location inside sysroot
+tmp_sysroot = os.path.join(self.getBuildDir(), "mock_sysroot")
+exe_inside = os.path.join(tmp_sysroot, os.path.relpath(exe_outside, "/"))
+lldbutil.mkdir_p(os.path.dirname(exe_inside))
+shutil.copyfile("linux-x86_64.out", exe_inside)
+
+# Prepare patched core file
+core_file = os.path.join(self.getBuildDir(), "patched.core")
+with open("linux-x86_64.core", "rb") as f:
+core = f.read()
+core = replace_path(core, "/test" * 817 + 

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f820fa4feda: [lldb] Fix SourceManager::SourceFileCache 
insertion (authored by emrekultursay, committed by labath).

Changed prior to commit:
  https://reviews.llvm.org/D76805?vs=252881=258737#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/SourceManagerTest.cpp


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,48 @@
+//===-- SourceManagerTest.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo, expect found.
+  FileSpec another_foo_file_spec("foo");
+  ASSERT_EQ(cache.FindSourceFile(another_foo_file_spec), foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: bar, expect not found.
+  FileSpec bar_file_spec("bar");
+  ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   CommunicationTest.cpp
   MangledTest.cpp
   RichManglingContextTest.cpp
+  SourceManagerTest.cpp
   StreamCallbackTest.cpp
   UniqueCStringMapTest.cpp
 
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP _sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,48 @@
+//===-- SourceManagerTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo, expect found.
+  FileSpec another_foo_file_spec("foo");
+  ASSERT_EQ(cache.FindSourceFile(another_foo_file_spec), foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: bar, expect not found.
+  FileSpec bar_file_spec("bar");
+  ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ 

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In D76806#1973035 , @emrekultursay 
wrote:

> Thanks for noticing the test breakages Pavel.  I fixed the bug, and verified 
> that the tests you mentioned pass. PTAL.


Everything looks fine now. Thanks for fixing that (it would be nice to mention 
what the bug was so one doesn't have to dig through the patch history to figure 
it out). I'm going to commit this shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806



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

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In D77759#1988419 , @labath wrote:

> Hmm... well, I like how this (active) is unified at the template 
> level. It still feels like there's too much code to implement this thing 
> (coming from the duplication for const, non-const and static variants), but 
> I'm not sure what can be done about that. I'll need to ponder this a bit more


After sleeping on this a couple of times, I've come to believe that the problem 
here is that we've let the redirection functions inherit the c++ function 
pointer weirdness, which resulted in a need to write template goo for const, 
non-const and static methods (and maybe sometimes even constructors). The goo 
is (more or less) unavoidable if we want to have compiler-generated 
"identifiers" for all these functions, but there's a difference between writing 
that once, and once for each redirection we need to make. Particularly as it 
doesn't seem like there's a compelling reason for that. There no reason that 
the "redirected" function in the replayer machinery needs be globally unique in 
the same way as the "original" functions do. In fact I think it doesn't even 
have to be a template at all -- it could just take the function-to-wrap as a 
regular argument instead of a templated one. I believe 
`char_ptr_redirect::method<>::record` could have been written as:

  // this handles const and non-const methods
  template // Maybe "Result" not even needed, 
if this is always the same type.
  Result char_ptr_redirect(Result (*f)(This *, char *, size_t), This *this_, 
char *ptr, size_t len) {
char *buffer = reinterpret_cast(calloc(len, sizeof(char)));
return f(this_, buffer, len)
  }
  // and another overload without This for static functions

Similar thing could be applied to the "replay" functions introduced here. 
Reduction from three overloads to two may not sound like much, but given that 
it would also remove the need for a lot of template goo (and reduce code size), 
I still find it interesting. It might be possible to also merge the two 
remaining overloads if we try hard enough.

Nonetheless, as long as we have only one kind of redirects (char_ptr thingy), 
it's benefit is not very clear, so I'm not going to insist on that. However, if 
more of these crop up, I think we should definitely revisit this.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77759



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

[Lldb-commits] [lldb] acae69d - [lldb] Add new LLDB setting: use-source-cache

[Pavel Labath via lldb-commits]

Author: Emre Kultursay
Date: 2020-04-20T16:24:25+02:00
New Revision: acae69d08c88b701e25318f8d4100f5542c44407

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

LOG: [lldb] Add new LLDB setting: use-source-cache

Summary:
LLDB memory-maps large source files, and at the same time, caches
all source files in the Source Cache.

On Windows, memory-mapped source files are not writeable, causing
bad user experience in IDEs (such as errors when saving edited files).
IDEs should have the ability to disable the Source Cache at LLDB
startup, so that users can edit source files while debugging.

Bug: llvm.org/PR45310

Reviewers: labath, JDevlieghere, jingham

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D76804

Added: 


Modified: 
lldb/include/lldb/API/SBDebugger.h
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Core/SourceManager.h
lldb/source/API/SBDebugger.cpp
lldb/source/Core/CoreProperties.td
lldb/source/Core/Debugger.cpp
lldb/source/Core/SourceManager.cpp

Removed: 




diff  --git a/lldb/include/lldb/API/SBDebugger.h 
b/lldb/include/lldb/API/SBDebugger.h
index 84879be5e0db..21fe77fa4f15 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -199,6 +199,10 @@ class LLDB_API SBDebugger {
 
   bool GetUseColor() const;
 
+  bool SetUseSourceCache(bool use_source_cache);
+
+  bool GetUseSourceCache() const;
+
   static bool GetDefaultArchitecture(char *arch_name, size_t arch_name_len);
 
   static bool SetDefaultArchitecture(const char *arch_name);

diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 4340f32fd6f3..0524e63beb59 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -273,6 +273,10 @@ class Debugger : public 
std::enable_shared_from_this,
 
   bool SetUseColor(bool use_color);
 
+  bool GetUseSourceCache() const;
+
+  bool SetUseSourceCache(bool use_source_cache);
+
   bool GetHighlightSource() const;
 
   lldb::StopShowColumn GetStopShowColumn() const;

diff  --git a/lldb/include/lldb/Core/SourceManager.h 
b/lldb/include/lldb/Core/SourceManager.h
index c039200ca442..90ee59402d98 100644
--- a/lldb/include/lldb/Core/SourceManager.h
+++ b/lldb/include/lldb/Core/SourceManager.h
@@ -101,6 +101,9 @@ class SourceManager {
 void AddSourceFile(const FileSP _sp);
 FileSP FindSourceFile(const FileSpec _spec) const;
 
+// Removes all elements from the cache.
+void Clear() { m_file_cache.clear(); }
+
   protected:
 typedef std::map FileCache;
 FileCache m_file_cache;

diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 1aeb5e2ca9ff..414d85e64363 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1374,6 +1374,18 @@ bool SBDebugger::GetUseColor() const {
   return (m_opaque_sp ? m_opaque_sp->GetUseColor() : false);
 }
 
+bool SBDebugger::SetUseSourceCache(bool value) {
+  LLDB_RECORD_METHOD(bool, SBDebugger, SetUseSourceCache, (bool), value);
+
+  return (m_opaque_sp ? m_opaque_sp->SetUseSourceCache(value) : false);
+}
+
+bool SBDebugger::GetUseSourceCache() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBDebugger, GetUseSourceCache);
+
+  return (m_opaque_sp ? m_opaque_sp->GetUseSourceCache() : false);
+}
+
 bool SBDebugger::GetDescription(SBStream ) {
   LLDB_RECORD_METHOD(bool, SBDebugger, GetDescription, (lldb::SBStream &),
  description);

diff  --git a/lldb/source/Core/CoreProperties.td 
b/lldb/source/Core/CoreProperties.td
index 73294349438a..b04738175f34 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -103,6 +103,10 @@ let Definition = "debugger" in {
 Global,
 DefaultTrue,
 Desc<"Whether to use Ansi color codes or not.">;
+  def UseSourceCache: Property<"use-source-cache", "Boolean">,
+Global,
+DefaultTrue,
+Desc<"Whether to cache source files in memory or not.">;
   def AutoOneLineSummaries: Property<"auto-one-line-summaries", "Boolean">,
 Global,
 DefaultTrue,

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 68eb856b85f0..1d4bf0dafb72 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -212,6 +212,11 @@ Status Debugger::SetPropertyValue(const ExecutionContext 
*exe_ctx,
   // use-color changed. Ping the prompt so it can reset the ansi terminal
   // codes.
   SetPrompt(GetPrompt());
+} else if (property_path == 
g_debugger_properties[ePropertyUseSourceCache].name) {
+  // use-source-cache changed. Wipe out the cache contents if it was 
disabled.
+  if (!GetUseSourceCache()) {
+

[Lldb-commits] [lldb] 1f820fa - [lldb] Fix SourceManager::SourceFileCache insertion

[Pavel Labath via lldb-commits]

Author: Emre Kultursay
Date: 2020-04-20T16:25:54+02:00
New Revision: 1f820fa4feda34d25e62762392c50049e5282330

URL: 
https://github.com/llvm/llvm-project/commit/1f820fa4feda34d25e62762392c50049e5282330
DIFF: 
https://github.com/llvm/llvm-project/commit/1f820fa4feda34d25e62762392c50049e5282330.diff

LOG: [lldb] Fix SourceManager::SourceFileCache insertion

Summary:
Lookup and subsequent insert was done using uninitialized
FileSpec object, which caused the cache to be a no-op.

Bug: llvm.org/PR45310

Depends on D76804.

Reviewers: labath, JDevlieghere

Reviewed By: labath

Subscribers: mgorny, jingham, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D76805

Added: 
lldb/unittests/Core/SourceManagerTest.cpp

Modified: 
lldb/source/Core/SourceManager.cpp
lldb/unittests/Core/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Core/SourceManager.cpp 
b/lldb/source/Core/SourceManager.cpp
index 593459ba4509..4edc11c99bf1 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@ bool SourceManager::File::GetLine(uint32_t line_no, 
std::string ) {
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP _sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;

diff  --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index 6393c6fe38c2..a2cc5a7f1f6d 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@ add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
   MangledTest.cpp
   RichManglingContextTest.cpp
+  SourceManagerTest.cpp
   StreamCallbackTest.cpp
   UniqueCStringMapTest.cpp
 

diff  --git a/lldb/unittests/Core/SourceManagerTest.cpp 
b/lldb/unittests/Core/SourceManagerTest.cpp
new file mode 100644
index ..9dcd048ce3fb
--- /dev/null
+++ b/lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,48 @@
+//===-- SourceManagerTest.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo, expect found.
+  FileSpec another_foo_file_spec("foo");
+  ASSERT_EQ(cache.FindSourceFile(another_foo_file_spec), foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: bar, expect not found.
+  FileSpec bar_file_spec("bar");
+  ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
+}



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

[Lldb-commits] [lldb] 865996d - [lldb] Remove m_last_file_sp from SourceManager

[Pavel Labath via lldb-commits]

Author: Emre Kultursay
Date: 2020-04-20T16:27:19+02:00
New Revision: 865996ddf626a4d6e2a9c401b1fffc731a1946aa

URL: 
https://github.com/llvm/llvm-project/commit/865996ddf626a4d6e2a9c401b1fffc731a1946aa
DIFF: 
https://github.com/llvm/llvm-project/commit/865996ddf626a4d6e2a9c401b1fffc731a1946aa.diff

LOG: [lldb] Remove m_last_file_sp from SourceManager

Summary:
...and replace it with m_last_file_spec instead.

When Source Cache is enabled, the value stored in m_last_file_sp is
already in the Source Cache, and caching it again in SourceManager
brings no extra benefit. All we need is to "remember" the last used
file, and FileSpec can serve the same purpose.

When Source Cache is disabled, the user explicitly requested no caching
of source files, and therefore, m_last_file_sp should NOT be used.

Bug: llvm.org/PR45310

Depends on D76805.

Reviewers: labath, jingham

Reviewed By: jingham

Subscribers: labath, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D76806

Added: 
lldb/test/API/commands/settings/use_source_cache/Makefile
lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
lldb/test/API/commands/settings/use_source_cache/main.cpp

Modified: 
lldb/include/lldb/Core/SourceManager.h
lldb/source/Core/SourceManager.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/SourceManager.h 
b/lldb/include/lldb/Core/SourceManager.h
index 90ee59402d98..2bbb36e50456 100644
--- a/lldb/include/lldb/Core/SourceManager.h
+++ b/lldb/include/lldb/Core/SourceManager.h
@@ -119,7 +119,7 @@ class SourceManager {
 
   ~SourceManager();
 
-  FileSP GetLastFile() { return m_last_file_sp; }
+  FileSP GetLastFile() { return GetFile(m_last_file_spec); }
 
   size_t
   DisplaySourceLinesWithLineNumbers(const FileSpec , uint32_t line,
@@ -141,7 +141,9 @@ class SourceManager {
 
   bool GetDefaultFileAndLine(FileSpec _spec, uint32_t );
 
-  bool DefaultFileAndLineSet() { return (m_last_file_sp.get() != nullptr); }
+  bool DefaultFileAndLineSet() {
+return (GetFile(m_last_file_spec).get() != nullptr);
+  }
 
   void FindLinesMatchingRegex(FileSpec _spec, RegularExpression ,
   uint32_t start_line, uint32_t end_line,
@@ -150,7 +152,7 @@ class SourceManager {
   FileSP GetFile(const FileSpec _spec);
 
 protected:
-  FileSP m_last_file_sp;
+  FileSpec m_last_file_spec;
   uint32_t m_last_line;
   uint32_t m_last_count;
   bool m_default_set;

diff  --git a/lldb/source/Core/SourceManager.cpp 
b/lldb/source/Core/SourceManager.cpp
index 4edc11c99bf1..7414dd281d43 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -52,27 +52,24 @@ static inline bool is_newline_char(char ch) { return ch == 
'\n' || ch == '\r'; }
 
 // SourceManager constructor
 SourceManager::SourceManager(const TargetSP _sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(target_sp),
   m_debugger_wp(target_sp->GetDebugger().shared_from_this()) {}
 
 SourceManager::SourceManager(const DebuggerSP _sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
-  m_target_wp(), m_debugger_wp(debugger_sp) {}
+: m_last_line(0), m_last_count(0), m_default_set(false), m_target_wp(),
+  m_debugger_wp(debugger_sp) {}
 
 // Destructor
 SourceManager::~SourceManager() {}
 
 SourceManager::FileSP SourceManager::GetFile(const FileSpec _spec) {
-  bool same_as_previous =
-  m_last_file_sp &&
-  FileSpec::Match(file_spec, m_last_file_sp->GetFileSpec());
+  if (!file_spec)
+return nullptr;
 
   DebuggerSP debugger_sp(m_debugger_wp.lock());
   FileSP file_sp;
-  if (same_as_previous)
-file_sp = m_last_file_sp;
-  else if (debugger_sp && debugger_sp->GetUseSourceCache())
+  if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -178,10 +175,10 @@ size_t 
SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile(
   m_last_line = start_line;
   m_last_count = count;
 
-  if (m_last_file_sp.get()) {
+  if (FileSP last_file_sp = GetLastFile()) {
 const uint32_t end_line = start_line + count - 1;
 for (uint32_t line = start_line; line <= end_line; ++line) {
-  if (!m_last_file_sp->LineIsValid(line)) {
+  if (!last_file_sp->LineIsValid(line)) {
 m_last_line = UINT32_MAX;
 break;
   }
@@ -219,12 +216,12 @@ size_t 
SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile(
 columnToHighlight = column - 1;
 
   size_t this_line_size =
-  m_last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
+  last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
   if (column != 0 && line == curr_line &&
 

[Lldb-commits] [lldb] 9cd9f3f - [lldb] Fix gcc warnings in TypeCategory.cpp

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-04-20T16:12:51+02:00
New Revision: 9cd9f3f1b8b7bfd30f7140c4c0006be5cead3e9a

URL: 
https://github.com/llvm/llvm-project/commit/9cd9f3f1b8b7bfd30f7140c4c0006be5cead3e9a
DIFF: 
https://github.com/llvm/llvm-project/commit/9cd9f3f1b8b7bfd30f7140c4c0006be5cead3e9a.diff

LOG: [lldb] Fix gcc warnings in TypeCategory.cpp

The cleanup in 3e3701f8a0bf left these variable unused.

Added: 


Modified: 
lldb/source/DataFormatters/TypeCategory.cpp

Removed: 




diff  --git a/lldb/source/DataFormatters/TypeCategory.cpp 
b/lldb/source/DataFormatters/TypeCategory.cpp
index 0cfa8d7eff78..8368c91a57f1 100644
--- a/lldb/source/DataFormatters/TypeCategory.cpp
+++ b/lldb/source/DataFormatters/TypeCategory.cpp
@@ -112,17 +112,15 @@ bool TypeCategoryImpl::Get(lldb::LanguageType lang,
   if (!IsEnabled() || !IsApplicable(lang))
 return false;
   TypeFilterImpl::SharedPointer filter_sp;
-  bool regex_filter = false;
   // first find both Filter and Synth, and then check which is most recent
 
   if (!GetTypeFiltersContainer()->Get(candidates, filter_sp))
-regex_filter = GetRegexTypeFiltersContainer()->Get(candidates, filter_sp);
+GetRegexTypeFiltersContainer()->Get(candidates, filter_sp);
 
-  bool regex_synth = false;
   bool pick_synth = false;
   ScriptedSyntheticChildren::SharedPointer synth;
   if (!GetTypeSyntheticsContainer()->Get(candidates, synth))
-regex_synth = GetRegexTypeSyntheticsContainer()->Get(candidates, synth);
+GetRegexTypeSyntheticsContainer()->Get(candidates, synth);
   if (!filter_sp.get() && !synth.get())
 return false;
   else if (!filter_sp.get() && synth.get())



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

[Lldb-commits] [lldb] 6afa5c4 - [lldb] Prefer executable files from sysroot over files from local filesystem

[Pavel Labath via lldb-commits]

Author: Yuri Per
Date: 2020-04-20T16:12:51+02:00
New Revision: 6afa5c407c1330efcadb75b8f85993660bf275a7

URL: 
https://github.com/llvm/llvm-project/commit/6afa5c407c1330efcadb75b8f85993660bf275a7
DIFF: 
https://github.com/llvm/llvm-project/commit/6afa5c407c1330efcadb75b8f85993660bf275a7.diff

LOG: [lldb] Prefer executable files from sysroot over files from local 
filesystem

Summary:
In D49685 sysroot behaviour was partially fixed. But files from local 
filesystem with same path still has priority over files from sysroot.

This patch fixes it by removing fallback to local filesystem from 
RemoteAwarePlatform::GetModuleSpec(). It is not actually required because 
higher level code do such fallback itself. See, for example, resolver in 
Platform::GetSharedModule().

Reviewers: labath, clayborg, EugeneBi

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D77529

Added: 


Modified: 
lldb/source/Target/RemoteAwarePlatform.cpp
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64.core

Removed: 




diff  --git a/lldb/source/Target/RemoteAwarePlatform.cpp 
b/lldb/source/Target/RemoteAwarePlatform.cpp
index 86f6b9045f70..d45e8e644a71 100644
--- a/lldb/source/Target/RemoteAwarePlatform.cpp
+++ b/lldb/source/Target/RemoteAwarePlatform.cpp
@@ -21,7 +21,7 @@ bool RemoteAwarePlatform::GetModuleSpec(const FileSpec 
_file_spec,
 return m_remote_platform_sp->GetModuleSpec(module_file_spec, arch,
module_spec);
 
-  return Platform::GetModuleSpec(module_file_spec, arch, module_spec);
+  return false;
 }
 
 Status RemoteAwarePlatform::RunShellCommand(

diff  --git 
a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py 
b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
index 5a1eab3f9018..435d3358b030 100644
--- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -209,6 +209,44 @@ def test_i386_sysroot(self):
 
 self.dbg.DeleteTarget(target)
 
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# Copy wrong executable to the location outside of sysroot
+exe_outside = os.path.join(self.getBuildDir(), "bin", "a.out")
+lldbutil.mkdir_p(os.path.dirname(exe_outside))
+shutil.copyfile("altmain.out", exe_outside)
+
+# Copy correct executable to the location inside sysroot
+tmp_sysroot = os.path.join(self.getBuildDir(), "mock_sysroot")
+exe_inside = os.path.join(tmp_sysroot, os.path.relpath(exe_outside, 
"/"))
+lldbutil.mkdir_p(os.path.dirname(exe_inside))
+shutil.copyfile("linux-x86_64.out", exe_inside)
+
+# Prepare patched core file
+core_file = os.path.join(self.getBuildDir(), "patched.core")
+with open("linux-x86_64.core", "rb") as f:
+core = f.read()
+core = replace_path(core, "/test" * 817 + "/a.out", exe_outside)
+with open(core_file, "wb") as f:
+f.write(core)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % 
tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore(core_file)
+
+# Check that we found executable from the sysroot
+mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
+self.assertEqual(mod_path, exe_inside)
+self.check_all(process, self._x86_64_pid, self._x86_64_regions, 
"a.out")
+
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("ARM")
 def test_arm_core(self):
@@ -369,3 +407,9 @@ def do_test(self, filename, pid, region_count, thread_name):
 self.check_all(process, pid, region_count, thread_name)
 
 self.dbg.DeleteTarget(target)
+
+def replace_path(binary, replace_from, replace_to):
+src = replace_from.encode()
+dst = replace_to.encode()
+dst += b"\0" * (len(src) - len(dst))
+return binary.replace(src, dst)

diff  --git 
a/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64.core 
b/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64.core
index e2fa69e4558e..2675eadd6a7f 100644
Binary files 
a/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64.core and 
b/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64.core 
diff er



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

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, aprantl, clayborg.
Herald added a project: LLDB.

The code in DWARFCompileUnit::BuildAddressRangeTable tries hard to avoid
relying on DW_AT_low/high_pc for compile unit range information, and
this logic is a big cause of llvm/lldb divergence in the lowest layers
of dwarf parsing code.

The implicit assumption in that code is that this information (as opposed to
DW_AT_ranges) is unreliable. However, I have not been able to verify
that assumption. It is definitely not true for all present-day
compilers (gcc, clang, icc), and it was also not the case for the
historic compilers that I have been able to get a hold of (thanks Matt
Godbolt).

All compiler included in my research either produced correct
DW_AT_ranges or .debug_aranges entries, or they produced no DW_AT_hi/lo
pc at all. The detailed findings are:

- gcc >= 4.4: produces DW_AT_ranges and .debug_aranges
- 4.1 <= gcc < 4.4: no DW_AT_ranges, no DW_AT_high_pc, .debug_aranges present. 
The upper version range here is uncertain as godbolt.org does not have 
intermediate versions.
- gcc < 4.1: no versions on godbolt.org
- clang >= 3.5: produces DW_AT_ranges, and (optionally) .debug_aranges
- 3.4 <= clang < 3.5: no DW_AT_ranges, no DW_AT_high_pc, .debug_aranges present.
- clang <= 3.3: no DW_AT_ranges, no DW_AT_high_pc, no .debug_aranges
- icc >= 16.0.1: produces DW_AT_ranges
- icc < 16.0.1: no functional versions on godbolt.org (some are present but 
fail to compile)

Based on this analysis, I believe it is safe to start trusting
DW_AT_low/high_pc information in dwarf as well as remove the code for
manually reconstructing range information by traversing the DIE
structure, and just keep the line table fallback. The only compilers
where this will change behavior are pre-3.4 clangs, which are almost 7
years old now. However, the functionality should remain unchanged
because we will be able to reconstruct this information from the line
table, which seems to be needed for some line-tables-only scenarios
anyway (haven't researched this too much, but at least some compilers
seem to emit DW_AT_ranges even in these situations).

In return, we simplify the code base, remove some untested code (the
only test changes are recent tests with overly reduced synthetic dwarf),
and increase llvm convergence.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78489

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_loclists_base.s
  lldb/test/Shell/SymbolFile/DWARF/debug_loc.s

Index: lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
===
--- lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
+++ lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
@@ -153,6 +153,10 @@
 .byte   14  # DW_FORM_strp
 .byte   19  # DW_AT_language
 .byte   5   # DW_FORM_data2
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
 .byte   0   # EOM(1)
 .byte   0   # EOM(2)
 .byte   2   # Abbreviation Code
@@ -210,6 +214,8 @@
 .byte   1   # Abbrev [1] 0xb:0x50 DW_TAG_compile_unit
 .long   .Linfo_string0  # DW_AT_producer
 .short  12  # DW_AT_language
+.quad   .Lfunc_begin0   # DW_AT_low_pc
+.long   .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc
 .byte   2   # Abbrev [2] 0x2a:0x29 DW_TAG_subprogram
 .quad   .Lfunc_begin0   # DW_AT_low_pc
 .long   .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc
Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_loclists_base.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_loclists_base.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_loclists_base.s
@@ -58,6 +58,10 @@
 .byte   5   # DW_FORM_data2
 .uleb128 0x8c   # DW_AT_loclists_base
 .byte   0x17# DW_FORM_sec_offset
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
 .byte   0   # EOM(1)
 .byte   0   # EOM(2)
 .byte   2   # Abbreviation Code
@@ -109,6 +113,8 @@
 .asciz  "Hand-written DWARF"# DW_AT_producer
 .short  12 

[Lldb-commits] [PATCH] D78333: Add Objective-C property accessors loaded from Clang module DWARF to lookup

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I'm not sure I understand why are we not adding the property to the lookup ptr? 
I would assume we would call addDecl to it which should make it visible and I 
don't see any ObjCPropertyDecl (?) check in that code?




Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:348
+  if (auto *nd = llvm::dyn_cast(member))
+if (auto *dc = llvm::dyn_cast(parent)) {
+  // This triggers ExternalASTSource::FindExternalVisibleDeclsByName() to 
be

SetMemberOwningModule is called really often and these two conditions are 
always met in our code, so we are now constantly resetting 
setHasExternalVisibleStorage to true?



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:351
+  // called when searching for members.
+  dc->setHasExternalLexicalStorage(true);
+  dc->setHasExternalVisibleStorage(true);

You shouldn't need this to get FindExternalVisibleDeclsByName and all this 
seems to work without it?


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

https://reviews.llvm.org/D78333



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

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 258710.
mib marked 2 inline comments as done.
mib added a comment.

Address Pavel's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396

Files:
  lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp
  lldb/source/Plugins/Language/ObjC/CFBasicHash.h
  lldb/source/Plugins/Language/ObjC/CMakeLists.txt
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.h
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
@@ -376,10 +376,12 @@
 	[newMutableDictionary setObject:@"foo" forKey:@"bar19"];
 	[newMutableDictionary setObject:@"foo" forKey:@"bar20"];
 
-	id cfKeys[2] = { @"foo", @"bar", @"baz", @"quux" };
-	id cfValues[2] = { @"foo", @"bar", @"baz", @"quux" };
-	NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
-	CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
+id cfKeys[4] = {@"foo", @"bar", @"baz", @"quux"};
+id cfValues[4] = {@"foo", @"bar", @"baz", @"quux"};
+NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
+NSDictionary *nscfDictionary = CFBridgingRelease(CFDictionaryCreate(
+nil, (void *)cfKeys, (void *)cfValues, 4, nil, nil));
+CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
 
 	NSAttributedString* attrString = [[NSAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
 	[attrString isEqual:nil];
@@ -412,8 +414,10 @@
 	NSSet* nsset = [[NSSet alloc] initWithObjects:str1,str2,str3,nil];
 	NSSet *nsmutableset = [[NSMutableSet alloc] initWithObjects:str1,str2,str3,nil];
 	[nsmutableset addObject:str4];
+NSSet *nscfSet =
+CFBridgingRelease(CFSetCreate(nil, (void *)cfValues, 2, nil));
 
-	CFDataRef data_ref = CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
+CFDataRef data_ref = CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
 
 	CFMutableDataRef mutable_data_ref = CFDataCreateMutable(kCFAllocatorDefault, 8);
 	CFDataAppendBytes(mutable_data_ref, [mutableData bytes], 5);
Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
@@ -20,6 +20,7 @@
 self.appkit_tester_impl(self.nscontainers_data_formatter_commands)
 
 def nscontainers_data_formatter_commands(self):
+
 self.expect(
 'frame variable newArray nsDictionary newDictionary nscfDictionary cfDictionaryRef newMutableDictionary cfarray_ref mutable_array_ref',
 substrs=[
@@ -29,6 +30,8 @@
 ' 2 key/value pairs',
 '(NSDictionary *) newDictionary = ',
 ' 12 key/value pairs',
+'(NSDictionary *) nscfDictionary = ',
+' 4 key/value pairs',
 '(CFDictionaryRef) cfDictionaryRef = ',
 ' 3 key/value pairs',
 '(NSDictionary *) newMutableDictionary = ',
@@ -39,6 +42,36 @@
 ' @"11 elements"',
 ])
 
+self.expect(
+'frame variable -d run-target *nscfDictionary',
+patterns=[
+'\(__NSCFDictionary\) \*nscfDictionary =',
+'key = 0x.* @"foo"',
+'value = 0x.* @"foo"',
+'key = 0x.* @"bar"',
+'value = 0x.* @"bar"',
+'key = 0x.* @"baz"',
+'value = 0x.* @"baz"',
+'key = 0x.* @"quux"',
+'value = 0x.* @"quux"',
+])
+
+
+self.expect(
+  'frame var nscfSet',
+  substrs=[
+  '(NSSet *) nscfSet = ',
+  '2 elements',
+  ])
+  
+self.expect(
+  'frame variable -d run-target *nscfSet',
+  patterns=[
+  

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked 5 inline comments as done.
mib added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:35-71
+
+size = sizeof(__CFBasicHash::RuntimeBase);
+size += sizeof(__CFBasicHash::Bits);
+
+DataBufferHeap buffer(size, 0);
+m_exe_ctx_ref.GetProcessSP()->ReadMemory(addr, buffer.GetBytes(), size,
+ error);

labath wrote:
> mib wrote:
> > davide wrote:
> > > These two pieces of code, `m_ptr_size == 4` and `m_ptr_size == 8` are 
> > > surprisingly similar. I'm really worried we might have a bug in one of 
> > > them and miss the other. Is there anything we can do to share the code? 
> > > [e.g. templatize].
> > Indeed, they're very similar and I already tried using templates (and 
> > SFINAE) to make it more generic, however I couldn't achieve that.
> > 
> > Since the remote architecture might be different from lldb's, we can't use 
> > macros to generate the underlying struct with the right size. So, I decided 
> > to template the structure used by CF, and have one of each architecture as 
> > a class attribute (look at CFBasicHash.h:114). 
> > 
> > Basically it's a tradeoff I chose voluntarily: I preferred having the 
> > CFBasicHash class handle the architecture to only expose one CFBasicHash 
> > object in the CFDictionary and CFSet data-formatter, rather than having two 
> > CFBasicHash objects templated for each ptr_size and have all the logic 
> > duplicated for each different architecture AND each data formatters.
> > 
> > If you can see a better way to do it, please let me know :) 
> ```
> template updateFor(std::unique_ptr<__CFBasicHash> _ht, ...)
> 
> if (m_ptr_size == 4)
>   updateFor(m_ht_4, ...);
> else if (m_ptr_size == 8)
>   updateFor(m_ht_8, ...)
> ```
> ?
> 
> Or the entire class could be a template, inheriting from a common 
> (non-templatized) interface...
I didn't think about that --' ... Thanks for the suggestion ^^



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:178-188
+private:
+  // Prime numbers. Values above 100 have been adjusted up so that the
+  // malloced block size will be just below a multiple of 512; values
+  // above 1200 have been adjusted up to just below a multiple of 4096.
+  constexpr static const uintptr_t NSDictionaryCapacities[] = {
+  0,3,6, 11,19,32,   52,
+  85,   118,  155,   237,   390,   672,  1065,

labath wrote:
> mib wrote:
> > davide wrote:
> > > Maybe a reference to the foundation header where these are defined, if 
> > > public.
> > It is not in a public header, that's why I copied the explanation.
> Are these actually used anywhere?
This is a left-over used in the multi variant implement that I'm working on ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D78421: Fix out of sync source code/executable when debugging

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 updated this revision to Diff 258705.
n1tram1 added a comment.

Fix the latest patch.

(I hope I got it right this time, sorry it's my first time doing this)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78421

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp

Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -21,7 +21,10 @@
 #include "lldb/Symbol/LineEntry.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/PathMappingList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
 #include "lldb/Utility/AnsiTerminal.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBuffer.h"
@@ -436,7 +439,8 @@
 sc_list.GetContextAtIndex(0, sc);
 if (sc.comp_unit)
   m_file_spec = sc.comp_unit->GetPrimaryFile();
-m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+m_mod_time =
+FileSystem::Instance().GetModificationTime(m_file_spec);
   }
 }
   }
@@ -531,12 +535,64 @@
   // For now we check each time we want to display info for the file.
   auto curr_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
 
+  // If the source file is newer than the executable don't update,
+  // otherwise the source file being displayed will be different from
+  // the executable being ran.
+  // (We only want to update if the executable has been recompiled)
+  if (IsNewerThanCurrentModule(curr_mod_time))
+// TODO: maybe issue a:
+//   'warning: Source file is more recent than executable.' ?
+return;
+
   if (curr_mod_time != llvm::sys::TimePoint<>() &&
   m_mod_time != curr_mod_time) {
-m_mod_time = curr_mod_time;
-m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec);
-m_offsets.clear();
+Update(curr_mod_time);
+  }
+}
+
+bool SourceManager::File::IsNewerThanCurrentModule(
+llvm::sys::TimePoint<> time) {
+  DebuggerSP debugger_sp(m_debugger_wp.lock());
+  if (!debugger_sp)
+return false;
+
+  lldb::TargetSP target_sp(debugger_sp->GetSelectedTarget());
+  if (!target_sp)
+return false;
+
+  lldb::ProcessSP process_sp(target_sp->CalculateProcess());
+  if (!process_sp)
+return false;
+
+  ThreadList _list(process_sp->GetThreadList());
+
+  lldb::ThreadSP thread_sp(thread_list.GetSelectedThread());
+  if (!thread_sp)
+return false;
+
+  lldb::StackFrameSP stackframe_sp(thread_sp->GetSelectedFrame());
+  if (!stackframe_sp)
+return false;
+
+  const Address pc_addr(stackframe_sp->GetFrameCodeAddress());
+
+  lldb::ModuleSP current_module(pc_addr.GetModule());
+  if (!current_module)
+return false;
+
+  auto current_module_mod_time = current_module->GetModificationTime();
+
+  if (time <= current_module_mod_time) {
+return false;
   }
+
+  return true;
+}
+
+void SourceManager::File::Update(llvm::sys::TimePoint<> modification_time) {
+  m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec);
+  m_offsets.clear();
+  m_mod_time = modification_time;
 }
 
 size_t SourceManager::File::DisplaySourceLines(uint32_t line,
Index: lldb/include/lldb/Core/SourceManager.h
===
--- lldb/include/lldb/Core/SourceManager.h
+++ lldb/include/lldb/Core/SourceManager.h
@@ -86,6 +86,9 @@
 
   private:
 void CommonInitializer(const FileSpec _spec, Target *target);
+
+bool IsNewerThanCurrentModule(llvm::sys::TimePoint<> time);
+void Update(llvm::sys::TimePoint<> mod_time);
   };
 
   typedef std::shared_ptr FileSP;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

Thanks for the test Shafik, that is pretty self explanatory!




Comment at: clang/lib/AST/ASTImporter.cpp:8161
   if (auto *ToRecord = dyn_cast(ToDC)) {
 auto *FromRecord = cast(FromDC);
 if (ToRecord->isCompleteDefinition()) {

What if we did this a bit differently? We could simply complete the From type 
if it is not completed, before getting into `ImportDefinition`.
```
if (ToRecord->isCompleteDefinition())
  return ToDC;
auto *FromRecord = cast(FromDC);
if (FromRecord->hasExternalLexicalStorage() &&
  !FromRecord->isCompleteDefinition())
FromRecord->getASTContext().getExternalSource()->CompleteType(
FromRecord);
```

This way we could get rid of the redundant calls of `ImportDefinition`. As we 
have now a call for the case when we don't have external storage and for the 
case when we have.


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

https://reviews.llvm.org/D78000



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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D75750#1988669 , @kwk wrote:

> In D75750#1988330 , @labath wrote:
>
> > The situation around finding symbols is messy enough already
>
>
> The way in which I integrated debuginfod for now is just to find source files 
> and not yet symbols. That being said. I don't fear the status quo so much. 
> The status quo is probably worse for symbols than it is for source files, 
> don't you think? So with *all* the CMake integration, the hosting inside 
> `lldb/include/lldb/Host/DebugInfoD.h` and your beloved test case,
>
> 
>
> I think it is fair to say that at least some work is there that can be taken 
> into LLDB. **As long** as I fix the retrieval of the module in 
> `SourceManager::File::CommonInitializer`. As suggested by @jankratochvil 
> either here or on IRC, I would like to give it a shot and try to pass down 
> the correct module to this function. I'd say, let's see if this function can 
> be passed a Module and if the changes are worth it. The whole part for 
> retrieving debug information can come when the architectural changes are done.


That all sounds reasonable, and I would not really have a big problem with 
integrating debuginfod in this way (through @clayborg might -- you will also 
want to check with him). However, I have doubts about how long will it take to 
do the architectural changes to get symbol downloading to work (or even if it 
will happen). I don't want to demean the work you have done so far, but I think 
that stuff will be much more complicated than this.

In that light, it's not clear to me whether having the ability to download the 
source files without being able to get the executable and symbol files is 
particularly useful. It does not seem very useful to me, but maybe I am missing 
something.

Do you find that useful? If not, I think the changes can just as easily sit in 
this patch instead of in the tree. This isn't touching any areas under active 
development, so its not like this functionality will rot quickly.

> But then it's a piece of cake to extend `lldb/include/lldb/Host/DebugInfoD.h` 
> with the right methods to call the debuginfod client lib.
> 
>> because one needs to understand the funky mac symbol searching mechanism, 
>> which is pretty much impossible without a mac machine.
> 
> I'm setting up my old mac to compile LLDB and I guess @jankratochvil might 
> soon also have his own Mac. This at least puts us in a position where we can 
> verify some of our changes.

That's great to hear. (though it's sad that is necessary)

> 
> 
>> After debuginfod, one will need to understand both, and have a linux machine 
>> with some debuginfod setup. The set of such people is likely to be empty of 
>> a long time...
> 
> I'm not sure if I understand you correctly but to me the *setup* is just to 
> point to a machine with *your* or a hosted server. At least for OS binaries 
> @fche2 @fche  (which is the correct one?) is making some effort to have those 
> debuginfos and source files available and setup. That is a great start for 
> most embedded systems with not much disk space to install all debug 
> information I guess. Correct my if this is a wrong anticipation. Sure, I mean 
> it will take a while before LLDB with debuginfod will make it into a 
> distribution but hey, time flies by.

I'm not worried about having symbols for all the system binaries. But just the 
effort to setup a environment with a foreign operating system and learn enough 
about it to be able to make changes to it are enough to dissuade a lot of 
potential developers (this is to your credit). After you start playing around 
with a mac, I think you'll find that a lot of things work differently there -- 
it took me about four years to understand how dsyms and debug maps work (I 
wasn't trying to learnt it all of that time, but still...).

In D75750#1988694 , @jankratochvil 
wrote:

> The current plan discussed with @kwk is to create the new `SymbolServer` 
> abstract superclass and some its inherited implementation and move there the 
> appropriate parts of existing `lldb/source/Symbol/LocateSymbolFile.cpp`. 
> Current `SymbolVendor` implementations would then iterate new `SymbolServer`s 
> by the existing `LocateExecutableSymbolFile` function. That may be enough for 
> a patch of its own.


I'll have to see the actual patch for a definitive opinion, but I have to say 
that a priori I am sceptical of this direction. And yes, that should definitely 
be a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D78462: get rid of PythonInteger::GetInteger()

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:55
+return obj.takeError();
+  return obj.get().AsUnsignedLongLong();
+}

`obj->AsUnsignedLongLong()` ?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:3153-3159
+  long long py_return = unwrapOrSetPythonException(
+  As(implementor.CallMethod(callee_name)));
 
   // if it fails, print the error but otherwise go on
   if (PyErr_Occurred()) {
 PyErr_Print();
 PyErr_Clear();

This converts the Expected into a python exception, only to clear (and print) 
it at the next line. Is there a more direct way of doing it?



Comment at: 
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp:126-132
+  auto major_version_value = As(major_version_field);
+  auto minor_version_value = As(minor_version_field);
 
-  EXPECT_EQ(PY_MAJOR_VERSION, major_version_value.GetInteger());
-  EXPECT_EQ(PY_MINOR_VERSION, minor_version_value.GetInteger());
+  EXPECT_THAT_EXPECTED(major_version_value, llvm::Succeeded());
+  EXPECT_THAT_EXPECTED(minor_version_value, llvm::Succeeded());
+  EXPECT_EQ(PY_MAJOR_VERSION, major_version_value.get());
+  EXPECT_EQ(PY_MINOR_VERSION, minor_version_value.get());

`EXPECT_THAT_EXPECTED(As(major_version_field), 
llvm::HasValue(PY_MAJOR_VERSION))` (and similarly in other tests too)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78462



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

[Lldb-commits] [PATCH] D78421: Fix out of sync source code/executable when debugging

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This looks like a bad upload. Each time you update the patch it should include 
the full list of changes, not just your recent additions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78421



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

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:102-107
+m_ht_64 = new __CFBasicHash();
+size_t copied = data_extractor.CopyData(0, size, m_ht_64);
+
+if (copied != size) {
+  delete m_ht_64;
+  m_ht_64 = nullptr;

No manual memory management, please.



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:35-71
+
+size = sizeof(__CFBasicHash::RuntimeBase);
+size += sizeof(__CFBasicHash::Bits);
+
+DataBufferHeap buffer(size, 0);
+m_exe_ctx_ref.GetProcessSP()->ReadMemory(addr, buffer.GetBytes(), size,
+ error);

mib wrote:
> davide wrote:
> > These two pieces of code, `m_ptr_size == 4` and `m_ptr_size == 8` are 
> > surprisingly similar. I'm really worried we might have a bug in one of them 
> > and miss the other. Is there anything we can do to share the code? [e.g. 
> > templatize].
> Indeed, they're very similar and I already tried using templates (and SFINAE) 
> to make it more generic, however I couldn't achieve that.
> 
> Since the remote architecture might be different from lldb's, we can't use 
> macros to generate the underlying struct with the right size. So, I decided 
> to template the structure used by CF, and have one of each architecture as a 
> class attribute (look at CFBasicHash.h:114). 
> 
> Basically it's a tradeoff I chose voluntarily: I preferred having the 
> CFBasicHash class handle the architecture to only expose one CFBasicHash 
> object in the CFDictionary and CFSet data-formatter, rather than having two 
> CFBasicHash objects templated for each ptr_size and have all the logic 
> duplicated for each different architecture AND each data formatters.
> 
> If you can see a better way to do it, please let me know :) 
```
template updateFor(std::unique_ptr<__CFBasicHash> _ht, ...)

if (m_ptr_size == 4)
  updateFor(m_ht_4, ...);
else if (m_ptr_size == 8)
  updateFor(m_ht_8, ...)
```
?

Or the entire class could be a template, inheriting from a common 
(non-templatized) interface...



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:178-188
+private:
+  // Prime numbers. Values above 100 have been adjusted up so that the
+  // malloced block size will be just below a multiple of 512; values
+  // above 1200 have been adjusted up to just below a multiple of 4096.
+  constexpr static const uintptr_t NSDictionaryCapacities[] = {
+  0,3,6, 11,19,32,   52,
+  85,   118,  155,   237,   390,   672,  1065,

mib wrote:
> davide wrote:
> > Maybe a reference to the foundation header where these are defined, if 
> > public.
> It is not in a public header, that's why I copied the explanation.
Are these actually used anywhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The test stuff looks good to me.




Comment at: lldb/unittests/DataFormatter/StringPrinterTests.cpp:109
+  EXPECT_TRUE(matches({"\0", 1}, QUOTE("")));
+  EXPECT_TRUE(matches("\a", QUOTE("\\a")));
+  EXPECT_TRUE(matches("\b", QUOTE("\\u{8}")));

vsk wrote:
> shafik wrote:
> > We have [raw string 
> > literals](https://en.cppreference.com/w/cpp/language/string_literal) since 
> > C++11:
> > 
> > ```
> > EXPECT_TRUE(matches("\a", R"("\a")"));
> > ```
> I personally find the current version much clearer, but can change this if 
> others feel strongly.
I think the raw strings are cleaner, particularly when it comes to strings like 
`"\\\""` and `""`



Comment at: lldb/unittests/DataFormatter/StringPrinterTests.cpp:31
+template 
+static Optional format(StringRef input) {

it looks like `escape_style` could be a regular argument instead of a template.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843



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

[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Looks good to me.




Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:206-207
   template  T *GetObjectForIndex(unsigned idx) {
-assert(idx != 0 && "Cannot get object for sentinel");
+assert(idx != 0 && "Cannot get object for "
+   "sentinel");
 void *object = GetObjectForIndexImpl(idx);

bad formatting ?


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

https://reviews.llvm.org/D77602



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