date:20200324

[Lldb-commits] [PATCH] D76736: [LLDB] Fix parsing of IPv6 host:port inside brackets

2020-03-24 Thread Emre Kultursay via Phabricator via lldb-commits

emrekultursay updated this revision to Diff 252499.
emrekultursay edited the summary of this revision.
emrekultursay added a comment.

Added unit tests.


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

https://reviews.llvm.org/D76736

Files:
  lldb/source/Utility/UriParser.cpp
  lldb/unittests/Utility/UriParserTest.cpp


Index: lldb/unittests/Utility/UriParserTest.cpp
===
--- lldb/unittests/Utility/UriParserTest.cpp
+++ lldb/unittests/Utility/UriParserTest.cpp
@@ -74,12 +74,18 @@
   VALIDATE
 }
 
-TEST(UriParserTest, TypicalPortPath) {
+TEST(UriParserTest, TypicalPortPathIPv4) {
   const UriTestCase testCase("connect://192.168.100.132:5432/", "connect",
  "192.168.100.132", 5432, "/");
   VALIDATE;
 }
 
+TEST(UriParserTest, TypicalPortPathIPv6) {
+  const UriTestCase 
testCase("connect://[2601:600:107f:db64:a42b:4faa:284:3082]:5432/", "connect",
+ "2601:600:107f:db64:a42b:4faa:284:3082", 5432, 
"/");
+  VALIDATE;
+}
+
 TEST(UriParserTest, BracketedHostnamePort) {
   const UriTestCase testCase("connect://[192.168.100.132]:5432/", "connect",
  "192.168.100.132", 5432, "/");
@@ -102,6 +108,18 @@
   VALIDATE
 }
 
+TEST(UriParserTest, BracketedHostnameWithPortIPv4) {
+  const UriTestCase testCase("connect://[192.168.100.132:1234]", "connect",
+ "192.168.100.132:1234", -1, "/");
+  VALIDATE
+}
+
+TEST(UriParserTest, BracketedHostnameWithPortIPv6) {
+  const UriTestCase 
testCase("connect://[[2601:600:107f:db64:a42b:4faa:284]:1234]", "connect",
+ "[2601:600:107f:db64:a42b:4faa:284]:1234", -1, 
"/");
+  VALIDATE
+}
+
 TEST(UriParserTest, BracketedHostnameWithColon) {
   const UriTestCase testCase("connect://[192.168.100.132:]:1234", 
"connect",
  "192.168.100.132:", 1234, "/");
Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 


Index: lldb/unittests/Utility/UriParserTest.cpp
===
--- lldb/unittests/Utility/UriParserTest.cpp
+++ lldb/unittests/Utility/UriParserTest.cpp
@@ -74,12 +74,18 @@
   VALIDATE
 }
 
-TEST(UriParserTest, TypicalPortPath) {
+TEST(UriParserTest, TypicalPortPathIPv4) {
   const UriTestCase testCase("connect://192.168.100.132:5432/", "connect",
  "192.168.100.132", 5432, "/");
   VALIDATE;
 }
 
+TEST(UriParserTest, TypicalPortPathIPv6) {
+  const UriTestCase testCase("connect://[2601:600:107f:db64:a42b:4faa:284:3082]:5432/", "connect",
+ "2601:600:107f:db64:a42b:4faa:284:3082", 5432, "/");
+  VALIDATE;
+}
+
 TEST(UriParserTest, BracketedHostnamePort) {
   const UriTestCase testCase("connect://[192.168.100.132]:5432/", "connect",
  "192.168.100.132", 5432, "/");
@@ -102,6 +108,18 @@
   VALIDATE
 }
 
+TEST(UriParserTest, BracketedHostnameWithPortIPv4) {
+  const UriTestCase testCase("connect://[192.168.100.132:1234]", "connect",
+ "192.168.100.132:1234", -1, "/");
+  VALIDATE
+}
+
+TEST(UriParserTest, BracketedHostnameWithPortIPv6) {
+  const UriTestCase testCase("connect://[[2601:600:107f:db64:a42b:4faa:284]:1234]", "connect",
+ "[2601:600:107f:db64:a42b:4faa:284]:1234", -1, "/");
+  VALIDATE
+}
+
 TEST(UriParserTest, BracketedHostnameWithColon) {
   const UriTestCase testCase("connect://[192.168.100.132:]:1234", "connect",
  "192.168.100.132:", 1234, "/");
Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76736: [LLDB] Fix parsing of IPv6 host:port inside brackets

2020-03-24 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a reviewer: labath.
shafik added a comment.

We need tests it looks like `UriParserTest.cpp` is the right place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76736



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

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

2020-03-24 Thread Frédéric Riss via lldb-commits

Here’s the bot log: 
http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/15055/steps/test/logs/stdio

> On Mar 24, 2020, at 5:10 PM, Adrian McCarthy  wrote:
> 
> Oh, and in case I wasn't clear:  I re-enabled the test TestSettings.py 
> locally.
> 
> On Tue, Mar 24, 2020 at 5:04 PM Adrian McCarthy  > wrote:
> I took a stab at this, but I'm not seeing any new test failures.  Can you 
> point me to the specific test and provide a log showing the failures?
> 
> I've been using `ninja check-lldb`, which runs (almost everything) and none 
> of the failures I'm seeing are related to inherit-env.  I assume the problem 
> you're seeing is in TestSettings.py, but I've never figured out how to run 
> individual Python-based lldb tests since all the dotest.py stuff was 
> re-written.  I've started up lldb interactively and tried to emulate the 
> commands that test executes via SBAPI, but everything worked as expected.
> 
> Adrian.
> 
> On Tue, Mar 24, 2020 at 8:19 AM Adrian McCarthy  > wrote:
> I'll take a look this morning.
> 
> On Tue, Mar 24, 2020 at 7:00 AM Pavel Labath  > wrote:
> On 23/03/2020 17:17, Frédéric Riss via lldb-commits wrote:
> > The new testing for “inherit-env=false” is failing on Windows. I skipped 
> > the test for now.
> > 
> > Could it be that it never worked there? (In which case XFail would be a 
> > better resolution)
> > Does anyone have easy access to a Windows build to try it out?
> > 
> > Thanks,
> > Fred
> 
> My guess is that this "defensive check"
>   
> >
> prevents us from passing a completely blank environment. I am tempted to
> just delete it and see what happens, but maybe Adrian is able to do a
> quick test of this?
> 
> pl

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

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

2020-03-24 Thread Adrian McCarthy via lldb-commits

Oh, and in case I wasn't clear:  I re-enabled the test TestSettings.py
locally.

On Tue, Mar 24, 2020 at 5:04 PM Adrian McCarthy  wrote:

> I took a stab at this, but I'm not seeing any new test failures.  Can you
> point me to the specific test and provide a log showing the failures?
>
> I've been using `ninja check-lldb`, which runs (almost everything) and
> none of the failures I'm seeing are related to inherit-env.  I assume the
> problem you're seeing is in TestSettings.py, but I've never figured out how
> to run individual Python-based lldb tests since all the dotest.py stuff was
> re-written.  I've started up lldb interactively and tried to emulate the
> commands that test executes via SBAPI, but everything worked as expected.
>
> Adrian.
>
> On Tue, Mar 24, 2020 at 8:19 AM Adrian McCarthy 
> wrote:
>
>> I'll take a look this morning.
>>
>> On Tue, Mar 24, 2020 at 7:00 AM Pavel Labath  wrote:
>>
>>> On 23/03/2020 17:17, Frédéric Riss via lldb-commits wrote:
>>> > The new testing for “inherit-env=false” is failing on Windows. I
>>> skipped the test for now.
>>> >
>>> > Could it be that it never worked there? (In which case XFail would be
>>> a better resolution)
>>> > Does anyone have easy access to a Windows build to try it out?
>>> >
>>> > Thanks,
>>> > Fred
>>>
>>> My guess is that this "defensive check"
>>> <
>>> https://github.com/llvm/llvm-project/blob/master/lldb/source/Host/windows/ProcessLauncherWindows.cpp#L26
>>> >
>>> prevents us from passing a completely blank environment. I am tempted to
>>> just delete it and see what happens, but maybe Adrian is able to do a
>>> quick test of this?
>>>
>>> pl
>>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

2020-03-24 Thread Adrian McCarthy via lldb-commits

I took a stab at this, but I'm not seeing any new test failures.  Can you
point me to the specific test and provide a log showing the failures?

I've been using `ninja check-lldb`, which runs (almost everything) and none
of the failures I'm seeing are related to inherit-env.  I assume the
problem you're seeing is in TestSettings.py, but I've never figured out how
to run individual Python-based lldb tests since all the dotest.py stuff was
re-written.  I've started up lldb interactively and tried to emulate the
commands that test executes via SBAPI, but everything worked as expected.

Adrian.

On Tue, Mar 24, 2020 at 8:19 AM Adrian McCarthy  wrote:

> I'll take a look this morning.
>
> On Tue, Mar 24, 2020 at 7:00 AM Pavel Labath  wrote:
>
>> On 23/03/2020 17:17, Frédéric Riss via lldb-commits wrote:
>> > The new testing for “inherit-env=false” is failing on Windows. I
>> skipped the test for now.
>> >
>> > Could it be that it never worked there? (In which case XFail would be a
>> better resolution)
>> > Does anyone have easy access to a Windows build to try it out?
>> >
>> > Thanks,
>> > Fred
>>
>> My guess is that this "defensive check"
>> <
>> https://github.com/llvm/llvm-project/blob/master/lldb/source/Host/windows/ProcessLauncherWindows.cpp#L26
>> >
>> prevents us from passing a completely blank environment. I am tempted to
>> just delete it and see what happens, but maybe Adrian is able to do a
>> quick test of this?
>>
>> pl
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76736: [LLDB] Fix parsing of IPv6 host:port inside brackets

2020-03-24 Thread Emre Kultursay via Phabricator via lldb-commits

emrekultursay created this revision.
emrekultursay added a reviewer: LLDB.
Herald added a project: LLDB.

When using IPv6 host:port pairs, typically the host is put inside
brackets, such as [2601:1234:...:0213]:, and the UriParser
can handle this format.

However, the Android infrastructure in LLDB assumes an additional
brackets around the host:port pair, such that the entire host:port
string can be treated as the host (which is used as an Android Serial
Number), and UriParser cannot handle multiple brackets. Parsing
inputs with such extra backets requires searching the closing bracket
from the right.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76736

Files:
  lldb/source/Utility/UriParser.cpp


Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 


Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 
___
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

2020-03-24 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added inline comments.



Comment at: lldb/source/Core/SourceManager.cpp:415
 target->GetImages().ResolveSymbolContextForFilePath(
 file_spec.GetFilename().AsCString(), 0, check_inlines,
 SymbolContextItem(eSymbolContextModule |

This code could be more efficient than my previously proposed 
`GetImages.ForEach()` as it should be able to find the only one `Module` having 
that source file.
But there should be passed the full pathname incl. directories to prevent 
wrongly chosen accidentally filename-matching source files:
```
FileSystem::Instance().Exists(m_file_spec) ? 
file_spec.GetFilename().AsCString() : file_spec.GetCString(false/*denormalize*/)
```
And the `Exists()` check should be cached in this whole function as it is 
expensive.




Comment at: lldb/source/Core/SourceManager.cpp:460
+  if (!FileSystem::Instance().Exists(m_file_spec) &&
+  debuginfod::isAvailable() && sc.module_sp) {
+llvm::Expected cache_path = debuginfod::findSource(

jankratochvil wrote:
> Make the `debuginfod::isAvailable()` check first as it is zero-cost, 
> `FileSystem::Instance().Exists` is expensive filesystem operation.
> The problem with that `sc.module_sp` is it is initialized above with some 
> side effects. I think you should be fine without needing any `sc`. The 
> following code does not pass the testcase for me but I guess you may fix it 
> better:
> ```
>   // Try finding the file using elfutils' debuginfod
>   if (!FileSystem::Instance().Exists(m_file_spec) &&
>   debuginfod::isAvailable())
> target->GetImages().ForEach(
> [&](const ModuleSP _sp) -> bool {
>   llvm::Expected cache_path = debuginfod::findSource(
>   module_sp->GetUUID(), file_spec.GetCString());
>   if (!cache_path) {
> module_sp->ReportWarning(
> "An error occurred while finding the "
> "source file %s using debuginfod for build ID %s: %s",
> file_spec.GetCString(),
> sc.module_sp->GetUUID().GetAsString("").c_str(),
> llvm::toString(cache_path.takeError()).c_str());
>   } else if (!cache_path->empty()) {
> m_file_spec = FileSpec(*cache_path);
> m_mod_time = 
> FileSystem::Instance().GetModificationTime(*cache_path);
> return false;
>   }
>   return true;
> });
> ```
> 
Please ignore this comment + code fragment, I think it should not be needed.
(Just the `isAvailable()` check should be moved.)


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] D75750: [lldb] integrate debuginfod

2020-03-24 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil requested changes to this revision.
jankratochvil added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/cmake/modules/FindDebuginfod.cmake:59
+endif()
\ No newline at end of file


"No newline at end of file", this is what saving this diff, git apply --index 
and git diff says to me.



Comment at: lldb/include/lldb/Host/DebugInfoD.h:27
+// cached version of the file. If there  was an error, we return that instead.
+llvm::Expected findSource(const UUID ,
+   const std::string );

Describe what does mean a returned `std::string("")` - that no error happened 
but server does not know this UUID/path.



Comment at: lldb/source/Core/SourceManager.cpp:408
+  if ((!file_spec.GetDirectory() && file_spec.GetFilename()) ||
+  !FileSystem::Instance().Exists(m_file_spec)) {
 // If this is just a file name, lets see if we can find it in the

I do not like this extra line as it changes behavior of LLDB unrelated to 
`debuginfod`. IIUC if the source file with fully specified directory+filename 
in DWARF does not exist but the same filename exists in a different directory 
of the sourcetree LLDB will now quietly use the different file. That's a bug.
I think it is there as you needed to initialize `sc.module_sp`.



Comment at: lldb/source/Core/SourceManager.cpp:460
+  if (!FileSystem::Instance().Exists(m_file_spec) &&
+  debuginfod::isAvailable() && sc.module_sp) {
+llvm::Expected cache_path = debuginfod::findSource(

Make the `debuginfod::isAvailable()` check first as it is zero-cost, 
`FileSystem::Instance().Exists` is expensive filesystem operation.
The problem with that `sc.module_sp` is it is initialized above with some side 
effects. I think you should be fine without needing any `sc`. The following 
code does not pass the testcase for me but I guess you may fix it better:
```
  // Try finding the file using elfutils' debuginfod
  if (!FileSystem::Instance().Exists(m_file_spec) &&
  debuginfod::isAvailable())
target->GetImages().ForEach(
[&](const ModuleSP _sp) -> bool {
  llvm::Expected cache_path = debuginfod::findSource(
  module_sp->GetUUID(), file_spec.GetCString());
  if (!cache_path) {
module_sp->ReportWarning(
"An error occurred while finding the "
"source file %s using debuginfod for build ID %s: %s",
file_spec.GetCString(),
sc.module_sp->GetUUID().GetAsString("").c_str(),
llvm::toString(cache_path.takeError()).c_str());
  } else if (!cache_path->empty()) {
m_file_spec = FileSpec(*cache_path);
m_mod_time = 
FileSystem::Instance().GetModificationTime(*cache_path);
return false;
  }
  return true;
});
```




Comment at: lldb/source/Host/common/DebugInfoD.cpp:50
+   "invalid build ID: %s",
+   buildID.GetAsString("").c_str());
+

It should not be an error:
```
echo 'int main(void) { return 0; }' >/tmp/main2.c;gcc -o /tmp/main2 
/tmp/main2.c -Wall -g -Wl,--build-id=none;rm 
/tmp/main2.c;DEBUGINFOD_URLS=http://localhost:8002/ ./bin/lldb /tmp/main2 -o 'l 
main' -o q
(lldb) target create "/tmp/main2"
Current executable set to '/tmp/main2' (x86_64).
(lldb) l main
warning: (x86_64) /tmp/main2 An error occurred while finding the source file 
/tmp/main2.c using debuginfod for build ID A9C3D738: invalid build ID: A9C3D738
File: /tmp/main2.c
(lldb) q
```




Comment at: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp:103
+// TEST-3: File: /my/new/path/test.cpp
+// TEST-3: 123
+// TEST-3-NEXT:{{[0-9]+}}   // Some context lines before

`s/123/{{[0-9]+}}/?`



Comment at: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp:136
+// the function.
\ No newline at end of file


"No newline at end of file", this is what saving this diff, git apply --index 
and git diff says to me.


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] D76730: [RNBRemote/arm64] Remove a couple of unused functions

2020-03-24 Thread Davide Italiano via Phabricator via lldb-commits

davide created this revision.
davide added reviewers: jasonmolenda, LLDB.
Herald added a subscriber: kristof.beyls.

Jason, before submitting this I wanted to run it past you.


https://reviews.llvm.org/D76730

Files:
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -6068,148 +6068,6 @@
   return SendPacket("OK");
 }
 
-static bool MachHeaderIsMainExecutable(nub_process_t pid, uint32_t addr_size,
-   nub_addr_t mach_header_addr,
-   mach_header ) {
-  DNBLogThreadedIf(LOG_RNB_PROC, "GetMachHeaderForMainExecutable(pid = %u, "
- "addr_size = %u, mach_header_addr = "
- "0x%16.16llx)",
-   pid, addr_size, mach_header_addr);
-  const nub_size_t bytes_read =
-  DNBProcessMemoryRead(pid, mach_header_addr, sizeof(mh), );
-  if (bytes_read == sizeof(mh)) {
-DNBLogThreadedIf(
-LOG_RNB_PROC, "GetMachHeaderForMainExecutable(pid = %u, addr_size = "
-  "%u, mach_header_addr = 0x%16.16llx): mh = {\n  magic = "
-  "0x%8.8x\n  cpu = 0x%8.8x\n  sub = 0x%8.8x\n  filetype = "
-  "%u\n  ncmds = %u\n  sizeofcmds = 0x%8.8x\n  flags = "
-  "0x%8.8x }",
-pid, addr_size, mach_header_addr, mh.magic, mh.cputype, mh.cpusubtype,
-mh.filetype, mh.ncmds, mh.sizeofcmds, mh.flags);
-if ((addr_size == 4 && mh.magic == MH_MAGIC) ||
-(addr_size == 8 && mh.magic == MH_MAGIC_64)) {
-  if (mh.filetype == MH_EXECUTE) {
-DNBLogThreadedIf(LOG_RNB_PROC, "GetMachHeaderForMainExecutable(pid = "
-   "%u, addr_size = %u, mach_header_addr = "
-   "0x%16.16llx) -> this is the "
-   "executable!!!",
- pid, addr_size, mach_header_addr);
-return true;
-  }
-}
-  }
-  return false;
-}
-
-static nub_addr_t GetMachHeaderForMainExecutable(const nub_process_t pid,
- const uint32_t addr_size,
- mach_header ) {
-  struct AllImageInfos {
-uint32_t version;
-uint32_t dylib_info_count;
-uint64_t dylib_info_addr;
-  };
-
-  uint64_t mach_header_addr = 0;
-
-  const nub_addr_t shlib_addr = DNBProcessGetSharedLibraryInfoAddress(pid);
-  uint8_t bytes[256];
-  nub_size_t bytes_read = 0;
-  DNBDataRef data(bytes, sizeof(bytes), false);
-  DNBDataRef::offset_t offset = 0;
-  data.SetPointerSize(addr_size);
-
-  // When we are sitting at __dyld_start, the kernel has placed the
-  // address of the mach header of the main executable on the stack. If we
-  // read the SP and dereference a pointer, we might find the mach header
-  // for the executable. We also just make sure there is only 1 thread
-  // since if we are at __dyld_start we shouldn't have multiple threads.
-  if (DNBProcessGetNumThreads(pid) == 1) {
-nub_thread_t tid = DNBProcessGetThreadAtIndex(pid, 0);
-if (tid != INVALID_NUB_THREAD) {
-  DNBRegisterValue sp_value;
-  if (DNBThreadGetRegisterValueByID(pid, tid, REGISTER_SET_GENERIC,
-GENERIC_REGNUM_SP, _value)) {
-uint64_t sp =
-addr_size == 8 ? sp_value.value.uint64 : sp_value.value.uint32;
-bytes_read = DNBProcessMemoryRead(pid, sp, addr_size, bytes);
-if (bytes_read == addr_size) {
-  offset = 0;
-  mach_header_addr = data.GetPointer();
-  if (MachHeaderIsMainExecutable(pid, addr_size, mach_header_addr, mh))
-return mach_header_addr;
-}
-  }
-}
-  }
-
-  // Check the dyld_all_image_info structure for a list of mach header
-  // since it is a very easy thing to check
-  if (shlib_addr != INVALID_NUB_ADDRESS) {
-bytes_read =
-DNBProcessMemoryRead(pid, shlib_addr, sizeof(AllImageInfos), bytes);
-if (bytes_read > 0) {
-  AllImageInfos aii;
-  offset = 0;
-  aii.version = data.Get32();
-  aii.dylib_info_count = data.Get32();
-  if (aii.dylib_info_count > 0) {
-aii.dylib_info_addr = data.GetPointer();
-if (aii.dylib_info_addr != 0) {
-  const size_t image_info_byte_size = 3 * addr_size;
-  for (uint32_t i = 0; i < aii.dylib_info_count; ++i) {
-bytes_read = DNBProcessMemoryRead(pid, aii.dylib_info_addr +
-   i * image_info_byte_size,
-  image_info_byte_size, bytes);
-if (bytes_read != image_info_byte_size)
-  break;
-offset = 0;
-mach_header_addr =

[Lldb-commits] [PATCH] D76729: Convert for loops to entry-based iteration

2020-03-24 Thread Shivam Mittal via Phabricator via lldb-commits

shivammittal99 updated this revision to Diff 252426.
shivammittal99 added a comment.

Run clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76729

Files:
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectTarget.cpp


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -876,21 +876,18 @@
 Stream  = result.GetOutputStream();
 
 if (argc > 0) {
-
-  // TODO: Convert to entry-based iteration.  Requires converting
-  // DumpValueObject.
-  for (size_t idx = 0; idx < argc; ++idx) {
+  for (const Args::ArgEntry  : args) {
 VariableList variable_list;
 ValueObjectList valobj_list;
 
-const char *arg = args.GetArgumentAtIndex(idx);
 size_t matches = 0;
 bool use_var_name = false;
 if (m_option_variable.use_regex) {
-  RegularExpression regex(llvm::StringRef::withNullAsEmpty(arg));
+  RegularExpression regex(
+  llvm::StringRef::withNullAsEmpty(arg.c_str()));
   if (!regex.IsValid()) {
 result.GetErrorStream().Printf(
-"error: invalid regular expression: '%s'\n", arg);
+"error: invalid regular expression: '%s'\n", arg.c_str());
 result.SetStatus(eReturnStatusFailed);
 return false;
   }
@@ -900,14 +897,14 @@
   matches = variable_list.GetSize();
 } else {
   Status error(Variable::GetValuesForVariableExpressionPath(
-  arg, m_exe_ctx.GetBestExecutionContextScope(),
+  arg.c_str(), m_exe_ctx.GetBestExecutionContextScope(),
   GetVariableCallback, target, variable_list, valobj_list));
   matches = variable_list.GetSize();
 }
 
 if (matches == 0) {
   result.GetErrorStream().Printf(
-  "error: can't find global variable '%s'\n", arg);
+  "error: can't find global variable '%s'\n", arg.c_str());
   result.SetStatus(eReturnStatusFailed);
   return false;
 } else {
@@ -923,7 +920,7 @@
   if (valobj_sp)
 DumpValueObject(s, var_sp, valobj_sp,
 use_var_name ? var_sp->GetName().GetCString()
- : arg);
+ : arg.c_str());
 }
   }
 }
@@ -3058,17 +3055,14 @@
   module_list_ptr = >GetImages();
 }
   } else {
-// TODO: Convert to entry based iteration.  Requires converting
-// FindModulesByName.
-for (size_t i = 0; i < argc; ++i) {
+for (const Args::ArgEntry  : command) {
   // Dump specified images (by basename or fullpath)
-  const char *arg_cstr = command.GetArgumentAtIndex(i);
   const size_t num_matches = FindModulesByName(
-  target, arg_cstr, module_list, use_global_module_list);
+  target, arg.c_str(), module_list, use_global_module_list);
   if (num_matches == 0) {
 if (argc == 1) {
   result.AppendErrorWithFormat("no modules found that match '%s'",
-   arg_cstr);
+   arg.c_str());
   result.SetStatus(eReturnStatusFailed);
   return false;
 }
Index: lldb/source/Commands/CommandObjectSettings.cpp
===
--- lldb/source/Commands/CommandObjectSettings.cpp
+++ lldb/source/Commands/CommandObjectSettings.cpp
@@ -531,10 +531,8 @@
 if (argc > 0) {
   const bool dump_qualified_name = true;
 
-  // TODO: Convert to StringRef based enumeration.  Requires converting
-  // GetPropertyAtPath first.
-  for (size_t i = 0; i < argc; ++i) {
-const char *property_path = args.GetArgumentAtIndex(i);
+  for (const Args::ArgEntry  : args) {
+const char *property_path = arg.c_str();
 
 const Property *property =
 GetDebugger().GetValueProperties()->GetPropertyAtPath(


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -876,21 +876,18 @@
 Stream  = result.GetOutputStream();
 
 if (argc > 0) {
-
-  // TODO: Convert to entry-based iteration.  Requires converting
-  // DumpValueObject.
-  for (size_t idx = 0; idx < argc; ++idx) {
+  for (const Args::ArgEntry  : args) {
 VariableList variable_list;
 ValueObjectList valobj_list;
 
-const char *arg = args.GetArgumentAtIndex(idx);

[Lldb-commits] [PATCH] D76729: Convert for loops to entry-based iteration

2020-03-24 Thread Shivam Mittal via Phabricator via lldb-commits

shivammittal99 created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Convert index-based loops marked TODO in CommandObjectSettings and 
CommandObjectTarget to entry-based.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76729

Files:
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectTarget.cpp


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -876,21 +876,17 @@
 Stream  = result.GetOutputStream();
 
 if (argc > 0) {
-
-  // TODO: Convert to entry-based iteration.  Requires converting
-  // DumpValueObject.
-  for (size_t idx = 0; idx < argc; ++idx) {
+  for (const Args::ArgEntry  : args) {
 VariableList variable_list;
 ValueObjectList valobj_list;
 
-const char *arg = args.GetArgumentAtIndex(idx);
 size_t matches = 0;
 bool use_var_name = false;
 if (m_option_variable.use_regex) {
-  RegularExpression regex(llvm::StringRef::withNullAsEmpty(arg));
+  RegularExpression 
regex(llvm::StringRef::withNullAsEmpty(arg.c_str()));
   if (!regex.IsValid()) {
 result.GetErrorStream().Printf(
-"error: invalid regular expression: '%s'\n", arg);
+"error: invalid regular expression: '%s'\n", arg.c_str());
 result.SetStatus(eReturnStatusFailed);
 return false;
   }
@@ -900,14 +896,14 @@
   matches = variable_list.GetSize();
 } else {
   Status error(Variable::GetValuesForVariableExpressionPath(
-  arg, m_exe_ctx.GetBestExecutionContextScope(),
+  arg.c_str(), m_exe_ctx.GetBestExecutionContextScope(),
   GetVariableCallback, target, variable_list, valobj_list));
   matches = variable_list.GetSize();
 }
 
 if (matches == 0) {
   result.GetErrorStream().Printf(
-  "error: can't find global variable '%s'\n", arg);
+  "error: can't find global variable '%s'\n", arg.c_str());
   result.SetStatus(eReturnStatusFailed);
   return false;
 } else {
@@ -923,7 +919,7 @@
   if (valobj_sp)
 DumpValueObject(s, var_sp, valobj_sp,
 use_var_name ? var_sp->GetName().GetCString()
- : arg);
+ : arg.c_str());
 }
   }
 }
@@ -3058,17 +3054,14 @@
   module_list_ptr = >GetImages();
 }
   } else {
-// TODO: Convert to entry based iteration.  Requires converting
-// FindModulesByName.
-for (size_t i = 0; i < argc; ++i) {
+for (const Args::ArgEntry  : command) {
   // Dump specified images (by basename or fullpath)
-  const char *arg_cstr = command.GetArgumentAtIndex(i);
   const size_t num_matches = FindModulesByName(
-  target, arg_cstr, module_list, use_global_module_list);
+  target, arg.c_str(), module_list, use_global_module_list);
   if (num_matches == 0) {
 if (argc == 1) {
   result.AppendErrorWithFormat("no modules found that match '%s'",
-   arg_cstr);
+   arg.c_str());
   result.SetStatus(eReturnStatusFailed);
   return false;
 }
Index: lldb/source/Commands/CommandObjectSettings.cpp
===
--- lldb/source/Commands/CommandObjectSettings.cpp
+++ lldb/source/Commands/CommandObjectSettings.cpp
@@ -531,10 +531,8 @@
 if (argc > 0) {
   const bool dump_qualified_name = true;
 
-  // TODO: Convert to StringRef based enumeration.  Requires converting
-  // GetPropertyAtPath first.
-  for (size_t i = 0; i < argc; ++i) {
-const char *property_path = args.GetArgumentAtIndex(i);
+  for (const Args::ArgEntry  : args) {
+const char *property_path = arg.c_str();
 
 const Property *property =
 GetDebugger().GetValueProperties()->GetPropertyAtPath(


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -876,21 +876,17 @@
 Stream  = result.GetOutputStream();
 
 if (argc > 0) {
-
-  // TODO: Convert to entry-based iteration.  Requires converting
-  // DumpValueObject.
-  for (size_t idx = 0; idx < argc; ++idx) {
+  for (const Args::ArgEntry  : args) {
 VariableList variable_list;
 ValueObjectList valobj_list;
 
-const char *arg =

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-24 Thread walter erquinigo via Phabricator via lldb-commits

wallace abandoned this revision.
wallace added a comment.

With https://reviews.llvm.org/D76470, targets created by lldb-vscode by default 
are inheriting the debugger's environment. I don't need this change anymore.
We can work on providing a flag that disables that default behavior, but that 
can also be set by an initCommand if someone really needs it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636



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

[Lldb-commits] [lldb] 3e11d84 - [Darwin] Add another hint to find the kernel. NFC.

2020-03-24 Thread Davide Italiano via lldb-commits


Author: Davide Italiano
Date: 2020-03-24T13:04:36-07:00
New Revision: 3e11d84d9f77736af22f52753593c8214d76875a

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

LOG: [Darwin] Add another hint to find the kernel. NFC.

Added: 


Modified: 

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp 
b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
index 193b3bd829c5..68a0335682d3 100644
--- 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ 
b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -245,6 +245,7 @@ 
DynamicLoaderDarwinKernel::SearchForKernelWithDebugHints(Process *process) {
 
   Status read_err;
   addr_t kernel_addresses_64[] = {
+  0xfff02010ULL,
   0xfff04010ULL, // newest arm64 devices
   0xff804010ULL, // 2014-2015-ish arm64 devices
   0xff802010ULL, // oldest arm64 devices



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

[Lldb-commits] [PATCH] D76698: [lldb] Always log if acquiring packet sequence mutex fails

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ccc4de42eae: [lldb] Always log if acquiring packet sequence 
mutex fails (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76698

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2797,12 +2797,10 @@
   thread_ids.push_back(1);
 }
   } else {
-#if !defined(LLDB_CONFIGURATION_DEBUG)
 Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS |
GDBR_LOG_PACKETS));
-LLDB_LOGF(log, "error: failed to get packet sequence mutex, not sending "
-   "packet 'qfThreadInfo'");
-#endif
+LLDB_LOG(log, "error: failed to get packet sequence mutex, not sending "
+  "packet 'qfThreadInfo'");
 sequence_mutex_unavailable = true;
   }
   return thread_ids.size();


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2797,12 +2797,10 @@
   thread_ids.push_back(1);
 }
   } else {
-#if !defined(LLDB_CONFIGURATION_DEBUG)
 Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS |
GDBR_LOG_PACKETS));
-LLDB_LOGF(log, "error: failed to get packet sequence mutex, not sending "
-   "packet 'qfThreadInfo'");
-#endif
+LLDB_LOG(log, "error: failed to get packet sequence mutex, not sending "
+  "packet 'qfThreadInfo'");
 sequence_mutex_unavailable = true;
   }
   return thread_ids.size();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 03e29e2 - [lldb/DWARF] Reland: Use DW_AT_call_pc to determine artificial frame address

2020-03-24 Thread Vedant Kumar via lldb-commits


Author: Vedant Kumar
Date: 2020-03-24T12:54:40-07:00
New Revision: 03e29e2c19a8e1f6a225b1878df3eed4e54891e5

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

LOG: [lldb/DWARF] Reland: Use DW_AT_call_pc to determine artificial frame 
address

Reland with changes: the test modified in this change originally failed
on a Debian/x86_64 builder, and I suspect the cause was that lldb looked
up the line location for an artificial frame by subtracting 1 from the
frame's address. For artificial frames, the subtraction must not happen
because the address is already exact.

---

lldb currently guesses the address to use when creating an artificial
frame (i.e., a frame constructed by determining the sequence of (tail)
calls which must have happened).

Guessing the address creates problems -- use the actual address provided
by the DW_AT_call_pc attribute instead.

Depends on D76336.

rdar://60307600

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

Added: 


Modified: 
lldb/include/lldb/Symbol/Function.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrameList.cpp
lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/Function.h 
b/lldb/include/lldb/Symbol/Function.h
index 0db9a5116d25..40d316fa78eb 100644
--- a/lldb/include/lldb/Symbol/Function.h
+++ b/lldb/include/lldb/Symbol/Function.h
@@ -284,19 +284,33 @@ class CallEdge {
   /// Like \ref GetReturnPCAddress, but returns an unresolved file address.
   lldb::addr_t GetUnresolvedReturnPCAddress() const { return return_pc; }
 
+  /// Get the load PC address of the call instruction (or 
LLDB_INVALID_ADDRESS).
+  lldb::addr_t GetCallInstPC(Function , Target ) const;
+
   /// Get the call site parameters available at this call edge.
   llvm::ArrayRef GetCallSiteParameters() const {
 return parameters;
   }
 
 protected:
-  CallEdge(lldb::addr_t return_pc, CallSiteParameterArray &)
-  : return_pc(return_pc), parameters(std::move(parameters)) {}
+  CallEdge(lldb::addr_t return_pc, lldb::addr_t call_inst_pc,
+   CallSiteParameterArray &)
+  : return_pc(return_pc), call_inst_pc(call_inst_pc),
+parameters(std::move(parameters)) {}
+
+  /// Helper that finds the load address of \p unresolved_pc, a file address
+  /// which refers to an instruction within \p caller.
+  static lldb::addr_t GetLoadAddress(lldb::addr_t unresolved_pc,
+ Function , Target );
 
   /// An invalid address if this is a tail call. Otherwise, the return PC for
   /// the call. Note that this is a file address which must be resolved.
   lldb::addr_t return_pc;
 
+  /// The address of the call instruction. Usually an invalid address, unless
+  /// this is a tail call.
+  lldb::addr_t call_inst_pc;
+
   CallSiteParameterArray parameters;
 };
 
@@ -308,8 +322,8 @@ class DirectCallEdge : public CallEdge {
   /// Construct a call edge using a symbol name to identify the callee, and a
   /// return PC within the calling function to identify a specific call site.
   DirectCallEdge(const char *symbol_name, lldb::addr_t return_pc,
- CallSiteParameterArray &)
-  : CallEdge(return_pc, std::move(parameters)) {
+ lldb::addr_t call_inst_pc, CallSiteParameterArray 
&)
+  : CallEdge(return_pc, call_inst_pc, std::move(parameters)) {
 lazy_callee.symbol_name = symbol_name;
   }
 
@@ -339,8 +353,9 @@ class IndirectCallEdge : public CallEdge {
   /// Construct a call edge using a DWARFExpression to identify the callee, and
   /// a return PC within the calling function to identify a specific call site.
   IndirectCallEdge(DWARFExpression call_target, lldb::addr_t return_pc,
+   lldb::addr_t call_inst_pc,
CallSiteParameterArray &)
-  : CallEdge(return_pc, std::move(parameters)),
+  : CallEdge(return_pc, call_inst_pc, std::move(parameters)),
 call_target(std::move(call_target)) {}
 
   Function *GetCallee(ModuleList , ExecutionContext _ctx) override;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index a703b1e1217d..c98694fca6b5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3737,6 +3737,7 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, 
DWARFDIE function_die) {
 llvm::Optional call_origin;
 llvm::Optional call_target;
 addr_t return_pc = LLDB_INVALID_ADDRESS;
+addr_t call_inst_pc = LLDB_INVALID_ADDRESS;
 
 DWARFAttributes attributes;
 const size_t num_attributes =

[Lldb-commits] [PATCH] D76687: [lldb][NFC] Always update m_cache_{hits/misses} in FormatCache

2020-03-24 Thread Davide Italiano via Phabricator via lldb-commits

davide added a comment.

Testcase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76687



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

[Lldb-commits] [lldb] 0ccc4de - [lldb] Always log if acquiring packet sequence mutex fails

2020-03-24 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2020-03-24T20:24:50+01:00
New Revision: 0ccc4de42eae92d5a7a1b67b29d7921c7f144b8d

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

LOG: [lldb] Always log if acquiring packet sequence mutex fails

Summary:
Currently we only log in debug builds but I don't see why we would do this as 
this is neither
expensive and seems useful.

I looked into the git history of this code and it seems originally there was 
also an assert here
and the logging here was the #else branch branch for non-Debug builds.

Reviewers: #lldb, labath

Reviewed By: labath

Subscribers: JDevlieghere

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index edbd408622f1..fdf1397d7a69 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2797,12 +2797,10 @@ size_t 
GDBRemoteCommunicationClient::GetCurrentThreadIDs(
   thread_ids.push_back(1);
 }
   } else {
-#if !defined(LLDB_CONFIGURATION_DEBUG)
 Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS |
GDBR_LOG_PACKETS));
-LLDB_LOGF(log, "error: failed to get packet sequence mutex, not sending "
-   "packet 'qfThreadInfo'");
-#endif
+LLDB_LOG(log, "error: failed to get packet sequence mutex, not sending "
+  "packet 'qfThreadInfo'");
 sequence_mutex_unavailable = true;
   }
   return thread_ids.size();



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

[Lldb-commits] [PATCH] D76685: [lldb] Don't dump the frame in SBTarget::EvaluateExpression in LLDB_CONFIGURATION_DEBUG

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaef982e35acd: [lldb] Dont dump the frame in 
SBTarget::EvaluateExpression in… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76685

Files:
  lldb/source/API/SBTarget.cpp


Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -2340,16 +2340,6 @@
 Target *target = exe_ctx.GetTargetPtr();
 
 if (target) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-  StreamString frame_description;
-  if (frame)
-frame->DumpUsingSettingsFormat(_description);
-  llvm::PrettyStackTraceFormat stack_trace(
-  "SBTarget::EvaluateExpression (expr = \"%s\", fetch_dynamic_value = "
-  "%u) %s",
-  expr, options.GetFetchDynamicValue(),
-  frame_description.GetString().str().c_str());
-#endif
   target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
 
   expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());


Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -2340,16 +2340,6 @@
 Target *target = exe_ctx.GetTargetPtr();
 
 if (target) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-  StreamString frame_description;
-  if (frame)
-frame->DumpUsingSettingsFormat(_description);
-  llvm::PrettyStackTraceFormat stack_trace(
-  "SBTarget::EvaluateExpression (expr = \"%s\", fetch_dynamic_value = "
-  "%u) %s",
-  expr, options.GetFetchDynamicValue(),
-  frame_description.GetString().str().c_str());
-#endif
   target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
 
   expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76687: [lldb][NFC] Always update m_cache_{hits/misses} in FormatCache

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6b6a779ca8ce: [lldb][NFC] Always update 
m_cache_{hits/misses} in FormatCache (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76687

Files:
  lldb/include/lldb/DataFormatters/FormatCache.h
  lldb/source/DataFormatters/FormatCache.cpp


Index: lldb/source/DataFormatters/FormatCache.cpp
===
--- lldb/source/DataFormatters/FormatCache.cpp
+++ lldb/source/DataFormatters/FormatCache.cpp
@@ -51,15 +51,6 @@
   m_synthetic_sp = synthetic_sp;
 }
 
-FormatCache::FormatCache()
-: m_map(), m_mutex()
-#ifdef LLDB_CONFIGURATION_DEBUG
-  ,
-  m_cache_hits(0), m_cache_misses(0)
-#endif
-{
-}
-
 FormatCache::Entry ::GetEntry(ConstString type) {
   auto i = m_map.find(type), e = m_map.end();
   if (i != e)
@@ -87,15 +78,11 @@
   std::lock_guard guard(m_mutex);
   auto entry = GetEntry(type);
   if (entry.IsCached()) {
-#ifdef LLDB_CONFIGURATION_DEBUG
 m_cache_hits++;
-#endif
 entry.Get(format_impl_sp);
 return true;
   }
-#ifdef LLDB_CONFIGURATION_DEBUG
   m_cache_misses++;
-#endif
   format_impl_sp.reset();
   return false;
 }
Index: lldb/include/lldb/DataFormatters/FormatCache.h
===
--- lldb/include/lldb/DataFormatters/FormatCache.h
+++ lldb/include/lldb/DataFormatters/FormatCache.h
@@ -49,13 +49,13 @@
   CacheMap m_map;
   std::recursive_mutex m_mutex;
 
-  uint64_t m_cache_hits;
-  uint64_t m_cache_misses;
+  uint64_t m_cache_hits = 0;
+  uint64_t m_cache_misses = 0;
 
   Entry (ConstString type);
 
 public:
-  FormatCache();
+  FormatCache() = default;
 
   template  bool Get(ConstString type, ImplSP 
_impl_sp);
   void Set(ConstString type, lldb::TypeFormatImplSP _sp);


Index: lldb/source/DataFormatters/FormatCache.cpp
===
--- lldb/source/DataFormatters/FormatCache.cpp
+++ lldb/source/DataFormatters/FormatCache.cpp
@@ -51,15 +51,6 @@
   m_synthetic_sp = synthetic_sp;
 }
 
-FormatCache::FormatCache()
-: m_map(), m_mutex()
-#ifdef LLDB_CONFIGURATION_DEBUG
-  ,
-  m_cache_hits(0), m_cache_misses(0)
-#endif
-{
-}
-
 FormatCache::Entry ::GetEntry(ConstString type) {
   auto i = m_map.find(type), e = m_map.end();
   if (i != e)
@@ -87,15 +78,11 @@
   std::lock_guard guard(m_mutex);
   auto entry = GetEntry(type);
   if (entry.IsCached()) {
-#ifdef LLDB_CONFIGURATION_DEBUG
 m_cache_hits++;
-#endif
 entry.Get(format_impl_sp);
 return true;
   }
-#ifdef LLDB_CONFIGURATION_DEBUG
   m_cache_misses++;
-#endif
   format_impl_sp.reset();
   return false;
 }
Index: lldb/include/lldb/DataFormatters/FormatCache.h
===
--- lldb/include/lldb/DataFormatters/FormatCache.h
+++ lldb/include/lldb/DataFormatters/FormatCache.h
@@ -49,13 +49,13 @@
   CacheMap m_map;
   std::recursive_mutex m_mutex;
 
-  uint64_t m_cache_hits;
-  uint64_t m_cache_misses;
+  uint64_t m_cache_hits = 0;
+  uint64_t m_cache_misses = 0;
 
   Entry (ConstString type);
 
 public:
-  FormatCache();
+  FormatCache() = default;
 
   template  bool Get(ConstString type, ImplSP _impl_sp);
   void Set(ConstString type, lldb::TypeFormatImplSP _sp);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76337: [lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address

2020-03-24 Thread Vedant Kumar via Phabricator via lldb-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6905394d1539: [lldb/DWARF] Use DW_AT_call_pc to determine 
artificial frame address (authored by vsk).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76337

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp

Index: lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
===
--- lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
+++ lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
@@ -3,19 +3,28 @@
 void __attribute__((noinline)) sink() {
   x++; //% self.filecheck("bt", "main.cpp", "-implicit-check-not=artificial")
   // CHECK: frame #0: 0x{{[0-9a-f]+}} a.out`sink() at main.cpp:[[@LINE-1]]:4 [opt]
-  // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3{{.*}} [opt] [artificial]
-  // CHECK-NEXT: frame #2: 0x{{[0-9a-f]+}} a.out`func2{{.*}} [opt]
-  // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1{{.*}} [opt] [artificial]
+  // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:14:3 [opt] [artificial]
+  // CHECK-NEXT: frame #2: 0x{{[0-9a-f]+}} a.out`func2() {{.*}} [opt]
+  // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1() at main.cpp:23:3 [opt] [artificial]
   // CHECK-NEXT: frame #4: 0x{{[0-9a-f]+}} a.out`main{{.*}} [opt]
 }
 
-void __attribute__((noinline)) func3() { sink(); /* tail */ }
+void __attribute__((noinline)) func3() {
+  x++;
+  sink(); /* tail */
+}
 
-void __attribute__((disable_tail_calls, noinline)) func2() { func3(); /* regular */ }
+void __attribute__((disable_tail_calls, noinline)) func2() {
+  func3(); /* regular */
+}
 
-void __attribute__((noinline)) func1() { func2(); /* tail */ }
+void __attribute__((noinline)) func1() {
+  x++;
+  func2(); /* tail */
+}
 
 int __attribute__((disable_tail_calls)) main() {
+  // DEBUG: self.runCmd("log enable lldb step -f /tmp/lldbstep.log")
   func1(); /* regular */
   return 0;
 }
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -236,13 +236,17 @@
   m_frames.resize(num_frames);
 }
 
+/// A sequence of calls that comprise some portion of a backtrace. Each frame
+/// is represented as a pair of a callee (Function *) and an address within the
+/// callee.
+using CallSequence = std::vector>;
+
 /// Find the unique path through the call graph from \p begin (with return PC
 /// \p return_pc) to \p end. On success this path is stored into \p path, and 
 /// on failure \p path is unchanged.
 static void FindInterveningFrames(Function , Function ,
   ExecutionContext _ctx, Target ,
-  addr_t return_pc,
-  std::vector ,
+  addr_t return_pc, CallSequence ,
   ModuleList , Log *log) {
   LLDB_LOG(log, "Finding frames between {0} and {1}, retn-pc={2:x}",
begin.GetDisplayName(), end.GetDisplayName(), return_pc);
@@ -275,24 +279,27 @@
   // Fully explore the set of functions reachable from the first edge via tail
   // calls in order to detect ambiguous executions.
   struct DFS {
-std::vector active_path = {};
-std::vector solution_path = {};
+CallSequence active_path = {};
+CallSequence solution_path = {};
 llvm::SmallPtrSet visited_nodes = {};
 bool ambiguous = false;
 Function *end;
 ModuleList 
+Target 
 ExecutionContext 
 
-DFS(Function *end, ModuleList , ExecutionContext )
-: end(end), images(images), context(context) {}
+DFS(Function *end, ModuleList , Target ,
+ExecutionContext )
+: end(end), images(images), target(target), context(context) {}
 
-void search(Function _callee, std::vector ) {
-  dfs(first_callee);
+void search(CallEdge _edge, Function _callee,
+CallSequence ) {
+  dfs(first_edge, first_callee);
   if (!ambiguous)
 path = std::move(solution_path);
 }
 
-void dfs(Function ) {
+void dfs(CallEdge _edge, Function ) {
   // Found a path to the target function.
   if ( == end) {
 if (solution_path.empty())
@@ -312,13 +319,16 @@
   }
 
   // Search the calls made from this callee.
-  active_path.push_back();
+  active_path.emplace_back(, LLDB_INVALID_ADDRESS);
   for (const auto  : callee.GetTailCallingEdges()) {

[Lldb-commits] [lldb] 0a9b91c - Revert "[lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address"

2020-03-24 Thread Vedant Kumar via lldb-commits


Author: Vedant Kumar
Date: 2020-03-24T12:22:12-07:00
New Revision: 0a9b91c390b281e90e51d5839557c5a189dd5401

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

LOG: Revert "[lldb/DWARF] Use DW_AT_call_pc to determine artificial frame 
address"

This reverts commit 6905394d153960ded3a7b884a9747ed2d4a6e8d8. The
changed test is failing on Debian/x86_64, possibly because lldb is
subtracting an offset from the DW_AT_call_pc address used for the
artificial frame:

http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/7171/steps/test/logs/stdio

/home/worker/lldb-x86_64-debian/lldb-x86_64-debian/llvm-project/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp:6:17:
 error: CHECK-NEXT: expected string not found in input
 // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:14:3 [opt] 
[artificial]
^
:3:2: note: scanning from here
 frame #1: 0x00401127 a.out`func3() at main.cpp:13:4 [opt] [artificial]

Added: 


Modified: 
lldb/include/lldb/Symbol/Function.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrameList.cpp
lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/Function.h 
b/lldb/include/lldb/Symbol/Function.h
index 40d316fa78eb..0db9a5116d25 100644
--- a/lldb/include/lldb/Symbol/Function.h
+++ b/lldb/include/lldb/Symbol/Function.h
@@ -284,33 +284,19 @@ class CallEdge {
   /// Like \ref GetReturnPCAddress, but returns an unresolved file address.
   lldb::addr_t GetUnresolvedReturnPCAddress() const { return return_pc; }
 
-  /// Get the load PC address of the call instruction (or 
LLDB_INVALID_ADDRESS).
-  lldb::addr_t GetCallInstPC(Function , Target ) const;
-
   /// Get the call site parameters available at this call edge.
   llvm::ArrayRef GetCallSiteParameters() const {
 return parameters;
   }
 
 protected:
-  CallEdge(lldb::addr_t return_pc, lldb::addr_t call_inst_pc,
-   CallSiteParameterArray &)
-  : return_pc(return_pc), call_inst_pc(call_inst_pc),
-parameters(std::move(parameters)) {}
-
-  /// Helper that finds the load address of \p unresolved_pc, a file address
-  /// which refers to an instruction within \p caller.
-  static lldb::addr_t GetLoadAddress(lldb::addr_t unresolved_pc,
- Function , Target );
+  CallEdge(lldb::addr_t return_pc, CallSiteParameterArray &)
+  : return_pc(return_pc), parameters(std::move(parameters)) {}
 
   /// An invalid address if this is a tail call. Otherwise, the return PC for
   /// the call. Note that this is a file address which must be resolved.
   lldb::addr_t return_pc;
 
-  /// The address of the call instruction. Usually an invalid address, unless
-  /// this is a tail call.
-  lldb::addr_t call_inst_pc;
-
   CallSiteParameterArray parameters;
 };
 
@@ -322,8 +308,8 @@ class DirectCallEdge : public CallEdge {
   /// Construct a call edge using a symbol name to identify the callee, and a
   /// return PC within the calling function to identify a specific call site.
   DirectCallEdge(const char *symbol_name, lldb::addr_t return_pc,
- lldb::addr_t call_inst_pc, CallSiteParameterArray 
&)
-  : CallEdge(return_pc, call_inst_pc, std::move(parameters)) {
+ CallSiteParameterArray &)
+  : CallEdge(return_pc, std::move(parameters)) {
 lazy_callee.symbol_name = symbol_name;
   }
 
@@ -353,9 +339,8 @@ class IndirectCallEdge : public CallEdge {
   /// Construct a call edge using a DWARFExpression to identify the callee, and
   /// a return PC within the calling function to identify a specific call site.
   IndirectCallEdge(DWARFExpression call_target, lldb::addr_t return_pc,
-   lldb::addr_t call_inst_pc,
CallSiteParameterArray &)
-  : CallEdge(return_pc, call_inst_pc, std::move(parameters)),
+  : CallEdge(return_pc, std::move(parameters)),
 call_target(std::move(call_target)) {}
 
   Function *GetCallee(ModuleList , ExecutionContext _ctx) override;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index c98694fca6b5..a703b1e1217d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3737,7 +3737,6 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, 
DWARFDIE function_die) {
 llvm::Optional call_origin;
 llvm::Optional call_target;
 addr_t return_pc = LLDB_INVALID_ADDRESS;
-addr_t call_inst_pc = LLDB_INVALID_ADDRESS;
 
 DWARFAttributes attributes;
 const size_t

[Lldb-commits] [PATCH] D76672: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

2020-03-24 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

Also, the dSYM bundle for a binary contains both the debug information and 
scripts that may be useful in debugging that binary - data formatters and the 
like.  So even if the original debugging session crashed before they got around 
to using any of the facilities in their dSYM, they are still likely to come in 
handy in analyzing the state of the app in the reproducer.  For this reason, I 
think we should treat dSYM's as special and capture them in full.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76672



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

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

2020-03-24 Thread Davide Italiano via Phabricator via lldb-commits

davide added a comment.

In D76697#1939731 , @JDevlieghere 
wrote:

> According to the comment there is at least one valid reason for this code 
> path to trigger. An `lldb_assert` will encourage users to file bugs and I 
> think we have enough real issues that we can do without the additional noise.


I do believe users should file bugs if this triggers, yes. In the recent past, 
I looked at ObjC more than most, so these bugs can land on me.


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

https://reviews.llvm.org/D76697



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

[Lldb-commits] [PATCH] D76672: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

2020-03-24 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

In D76672#1939583 , @labath wrote:

> In D76672#1939377 , @JDevlieghere 
> wrote:
>
> > In D76672#1938629 , @labath wrote:
> >
> > > It's not clear to me why this is needed.
> > >
> > > I mean, if lldb touches any of the files inside the dsym bundle, then 
> > > they should be automatically recorded already, right? And if it doesn't 
> > > then it does not need them...
> >
> >
> > The dSYM can contain other resources than just the Mach-O companion file, 
> > such as script for the OS plugins or opt remarks, which might not be used 
> > by the reproducers or even LLDB at all. Once you have the reproducer on 
> > your system, tools will find and use it because spotlight indexed it. 
> > Having only a partial dSYM is really undesirable as it can break LLDB and 
> > other tools in really unexpected ways.
>
>
> Interesting. Could that be fixed by adding the funny `.noindex` suffix to the 
> reproducer name (or some subdirectory of it)?


Yes, .noindex would prevent Spotlight from finding it. lldb might still find it 
though if it happens to be next to the executable.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76672



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

[Lldb-commits] [lldb] 6b6a779 - [lldb][NFC] Always update m_cache_{hits/misses} in FormatCache

2020-03-24 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2020-03-24T20:16:43+01:00
New Revision: 6b6a779ca8ce4025ed0a38fbcfcb6c07334ace57

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

LOG: [lldb][NFC] Always update m_cache_{hits/misses} in FormatCache

Summary:
These two variables are only incremented under LLDB_CONFIGURATION_DEBUG but 
their
value is always logged when verbose lldb formatter logging is enabled, which 
causes that our
cache hit/miss log looks like this in non-Debug builds:

```
Cache hits: 0 - Cache Misses: 0
...
Cache hits: 0 - Cache Misses: 0
...
Cache hits: 0 - Cache Misses: 0
```

This just always increments those two counters independent of build mode.

Reviewers: JDevlieghere

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/FormatCache.h
lldb/source/DataFormatters/FormatCache.cpp

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/FormatCache.h 
b/lldb/include/lldb/DataFormatters/FormatCache.h
index 581744c04f79..e75aaee1a7bb 100644
--- a/lldb/include/lldb/DataFormatters/FormatCache.h
+++ b/lldb/include/lldb/DataFormatters/FormatCache.h
@@ -49,13 +49,13 @@ class FormatCache {
   CacheMap m_map;
   std::recursive_mutex m_mutex;
 
-  uint64_t m_cache_hits;
-  uint64_t m_cache_misses;
+  uint64_t m_cache_hits = 0;
+  uint64_t m_cache_misses = 0;
 
   Entry (ConstString type);
 
 public:
-  FormatCache();
+  FormatCache() = default;
 
   template  bool Get(ConstString type, ImplSP 
_impl_sp);
   void Set(ConstString type, lldb::TypeFormatImplSP _sp);

diff  --git a/lldb/source/DataFormatters/FormatCache.cpp 
b/lldb/source/DataFormatters/FormatCache.cpp
index f7e5c72f7781..5e0965fcdae4 100644
--- a/lldb/source/DataFormatters/FormatCache.cpp
+++ b/lldb/source/DataFormatters/FormatCache.cpp
@@ -51,15 +51,6 @@ void FormatCache::Entry::Set(lldb::SyntheticChildrenSP 
synthetic_sp) {
   m_synthetic_sp = synthetic_sp;
 }
 
-FormatCache::FormatCache()
-: m_map(), m_mutex()
-#ifdef LLDB_CONFIGURATION_DEBUG
-  ,
-  m_cache_hits(0), m_cache_misses(0)
-#endif
-{
-}
-
 FormatCache::Entry ::GetEntry(ConstString type) {
   auto i = m_map.find(type), e = m_map.end();
   if (i != e)
@@ -87,15 +78,11 @@ bool FormatCache::Get(ConstString type, ImplSP 
_impl_sp) {
   std::lock_guard guard(m_mutex);
   auto entry = GetEntry(type);
   if (entry.IsCached()) {
-#ifdef LLDB_CONFIGURATION_DEBUG
 m_cache_hits++;
-#endif
 entry.Get(format_impl_sp);
 return true;
   }
-#ifdef LLDB_CONFIGURATION_DEBUG
   m_cache_misses++;
-#endif
   format_impl_sp.reset();
   return false;
 }



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

[Lldb-commits] [lldb] aef982e - [lldb] Don't dump the frame in SBTarget::EvaluateExpression in LLDB_CONFIGURATION_DEBUG

2020-03-24 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2020-03-24T20:16:09+01:00
New Revision: aef982e35acd2a0c4f6064308601658745e78cfc

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

LOG: [lldb] Don't dump the frame in SBTarget::EvaluateExpression in 
LLDB_CONFIGURATION_DEBUG

Summary:
Dumping the frame using the user-set format could cause that a debug LLDB 
doesn't behave as a release LLDB,
which could potentially break replaying a reproducer.

Also it's kinda strange that the frame format set by the user is used in the 
internal log output.

Reviewers: JDevlieghere

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/API/SBTarget.cpp

Removed: 




diff  --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index a75104a12583..ca75e91bd906 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -2340,16 +2340,6 @@ lldb::SBValue SBTarget::EvaluateExpression(const char 
*expr,
 Target *target = exe_ctx.GetTargetPtr();
 
 if (target) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-  StreamString frame_description;
-  if (frame)
-frame->DumpUsingSettingsFormat(_description);
-  llvm::PrettyStackTraceFormat stack_trace(
-  "SBTarget::EvaluateExpression (expr = \"%s\", fetch_dynamic_value = "
-  "%u) %s",
-  expr, options.GetFetchDynamicValue(),
-  frame_description.GetString().str().c_str());
-#endif
   target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
 
   expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());



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

[Lldb-commits] [lldb] 6905394 - [lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address

2020-03-24 Thread Vedant Kumar via lldb-commits


Author: Vedant Kumar
Date: 2020-03-24T12:02:03-07:00
New Revision: 6905394d153960ded3a7b884a9747ed2d4a6e8d8

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

LOG: [lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address

lldb currently guesses the address to use when creating an artificial
frame (i.e., a frame constructed by determining the sequence of (tail)
calls which must have happened).

Guessing the address creates problems -- use the actual address provided
by the DW_AT_call_pc attribute instead.

Depends on D76336.

rdar://60307600

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

Added: 


Modified: 
lldb/include/lldb/Symbol/Function.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrameList.cpp
lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/Function.h 
b/lldb/include/lldb/Symbol/Function.h
index 0db9a5116d25..40d316fa78eb 100644
--- a/lldb/include/lldb/Symbol/Function.h
+++ b/lldb/include/lldb/Symbol/Function.h
@@ -284,19 +284,33 @@ class CallEdge {
   /// Like \ref GetReturnPCAddress, but returns an unresolved file address.
   lldb::addr_t GetUnresolvedReturnPCAddress() const { return return_pc; }
 
+  /// Get the load PC address of the call instruction (or 
LLDB_INVALID_ADDRESS).
+  lldb::addr_t GetCallInstPC(Function , Target ) const;
+
   /// Get the call site parameters available at this call edge.
   llvm::ArrayRef GetCallSiteParameters() const {
 return parameters;
   }
 
 protected:
-  CallEdge(lldb::addr_t return_pc, CallSiteParameterArray &)
-  : return_pc(return_pc), parameters(std::move(parameters)) {}
+  CallEdge(lldb::addr_t return_pc, lldb::addr_t call_inst_pc,
+   CallSiteParameterArray &)
+  : return_pc(return_pc), call_inst_pc(call_inst_pc),
+parameters(std::move(parameters)) {}
+
+  /// Helper that finds the load address of \p unresolved_pc, a file address
+  /// which refers to an instruction within \p caller.
+  static lldb::addr_t GetLoadAddress(lldb::addr_t unresolved_pc,
+ Function , Target );
 
   /// An invalid address if this is a tail call. Otherwise, the return PC for
   /// the call. Note that this is a file address which must be resolved.
   lldb::addr_t return_pc;
 
+  /// The address of the call instruction. Usually an invalid address, unless
+  /// this is a tail call.
+  lldb::addr_t call_inst_pc;
+
   CallSiteParameterArray parameters;
 };
 
@@ -308,8 +322,8 @@ class DirectCallEdge : public CallEdge {
   /// Construct a call edge using a symbol name to identify the callee, and a
   /// return PC within the calling function to identify a specific call site.
   DirectCallEdge(const char *symbol_name, lldb::addr_t return_pc,
- CallSiteParameterArray &)
-  : CallEdge(return_pc, std::move(parameters)) {
+ lldb::addr_t call_inst_pc, CallSiteParameterArray 
&)
+  : CallEdge(return_pc, call_inst_pc, std::move(parameters)) {
 lazy_callee.symbol_name = symbol_name;
   }
 
@@ -339,8 +353,9 @@ class IndirectCallEdge : public CallEdge {
   /// Construct a call edge using a DWARFExpression to identify the callee, and
   /// a return PC within the calling function to identify a specific call site.
   IndirectCallEdge(DWARFExpression call_target, lldb::addr_t return_pc,
+   lldb::addr_t call_inst_pc,
CallSiteParameterArray &)
-  : CallEdge(return_pc, std::move(parameters)),
+  : CallEdge(return_pc, call_inst_pc, std::move(parameters)),
 call_target(std::move(call_target)) {}
 
   Function *GetCallee(ModuleList , ExecutionContext _ctx) override;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index a703b1e1217d..c98694fca6b5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3737,6 +3737,7 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, 
DWARFDIE function_die) {
 llvm::Optional call_origin;
 llvm::Optional call_target;
 addr_t return_pc = LLDB_INVALID_ADDRESS;
+addr_t call_inst_pc = LLDB_INVALID_ADDRESS;
 
 DWARFAttributes attributes;
 const size_t num_attributes = child.GetAttributes(attributes);
@@ -3765,6 +3766,12 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, 
DWARFDIE function_die) {
   if (attr == DW_AT_call_return_pc)
 return_pc = form_value.Address();
 
+  // Extract DW_AT_call_pc (the PC at the call/branch instruction). It
+  // should only ever be unavailable for non-tail

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

2020-03-24 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

According to the comment there is at least one valid reason for this code path 
to trigger. An `lldb_assert` will encourage users to file bugs and I think we 
have enough real issues that we can do without the additional noise.


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

https://reviews.llvm.org/D76697



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

[Lldb-commits] [PATCH] D76337: [lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address

2020-03-24 Thread Vedant Kumar via Phabricator via lldb-commits

vsk added a comment.

I'll land this shortly if there aren't any objections.


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

https://reviews.llvm.org/D76337



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

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

2020-03-24 Thread Davide Italiano via Phabricator via lldb-commits

davide added a comment.

I don't recall the details.

  Soft assertions. LLDB provides lldb_assert() as a soft alternative to cover 
the middle ground of situations that indicate a recoverable bug in LLDB. In a 
Debug configuration lldb_assert() behaves like assert(). In a Release 
configuration it will print a warning and encourage the user to file a bug 
report, similar to LLVM’s crash handler, and then return execution. Use these 
sparingly and only if error handling is not otherwise feasible. Specifically, 
new code should not be using lldb_assert() and existing uses should be replaced 
by other means of error handling.

This is a recoverable bug, in my books.


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

https://reviews.llvm.org/D76697



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

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

2020-03-24 Thread Davide Italiano via Phabricator via lldb-commits

davide added a comment.

Also, this was asserting already -- so it's not introducing any new `assert()`.


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

https://reviews.llvm.org/D76697



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

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

2020-03-24 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D76697#1939634 , @davide wrote:

> Please don't do this. I've seen that assertion triggering. If anything, you 
> might want to make a `lldbassert`.

Did you investigate why the assertion triggered? Was it because of the forward 
declaration or was it one of the "other weird cases". To me this change seems 
perfectly in line with https://lldb.llvm.org/resources/contributing.html

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

https://reviews.llvm.org/D76697

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

[Lldb-commits] [PATCH] D76672: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

2020-03-24 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D76672#1939583 , @labath wrote:

> In D76672#1939377 , @JDevlieghere 
> wrote:
>
> > In D76672#1938629 , @labath wrote:
> >
> > > It's not clear to me why this is needed.
> > >
> > > I mean, if lldb touches any of the files inside the dsym bundle, then 
> > > they should be automatically recorded already, right? And if it doesn't 
> > > then it does not need them...
> >
> >
> > The dSYM can contain other resources than just the Mach-O companion file, 
> > such as script for the OS plugins or opt remarks, which might not be used 
> > by the reproducers or even LLDB at all. Once you have the reproducer on 
> > your system, tools will find and use it because spotlight indexed it. 
> > Having only a partial dSYM is really undesirable as it can break LLDB and 
> > other tools in really unexpected ways.
>
>
> Interesting. Could that be fixed by adding the funny `.noindex` suffix to the 
> reproducer name (or some subdirectory of it)?


Yes, it's an alternative I've considered. I prefer this approach because (1) 
conceptually a dSYM is a single unit, the fact that the bundle is actually a 
directory on disk is just an implementation detail and (2) it allows you to use 
the dSYM even when reproducer replay fails. As we're discovering bugs in the 
reproducers, we can usually still use the files inside it to the debug the 
issue, even if replay fails.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76672



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

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

2020-03-24 Thread Davide Italiano via Phabricator via lldb-commits

davide added a comment.

Please don't do this. I've seen that assertion triggering. If anything, you 
might want to make a `lldbassert`.


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

https://reviews.llvm.org/D76697



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

[Lldb-commits] [PATCH] D76672: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

2020-03-24 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D76672#1939377 , @JDevlieghere 
wrote:

> In D76672#1938629 , @labath wrote:
>
> > It's not clear to me why this is needed.
> >
> > I mean, if lldb touches any of the files inside the dsym bundle, then they 
> > should be automatically recorded already, right? And if it doesn't then it 
> > does not need them...
>
>
> The dSYM can contain other resources than just the Mach-O companion file, 
> such as script for the OS plugins or opt remarks, which might not be used by 
> the reproducers or even LLDB at all. Once you have the reproducer on your 
> system, tools will find and use it because spotlight indexed it. Having only 
> a partial dSYM is really undesirable as it can break LLDB and other tools in 
> really unexpected ways.


Interesting. Could that be fixed by adding the funny `.noindex` suffix to the 
reproducer name (or some subdirectory of it)?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76672



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

[Lldb-commits] [PATCH] Fix parsing of IPv6 host:port inside brackets

2020-03-24 Thread Emre Kultursay via lldb-commits




0001-Fix-parsing-of-IPv6-host-port-inside-brackets.patch
Description: Binary data
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76699: [lldb] Remove some debugging printfs from ITSession code

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb8dab9b3d5b9: [lldb] Remove some debugging printfs from 
ITSession code (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76699

Files:
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp


Index: lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -605,9 +605,6 @@
   // First count the trailing zeros of the IT mask.
   uint32_t TZ = llvm::countTrailingZeros(ITMask);
   if (TZ > 3) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT Mask ''\n");
-#endif
 return 0;
   }
   return (4 - TZ);
@@ -622,15 +619,9 @@
   // A8.6.50 IT
   unsigned short FirstCond = Bits32(bits7_0, 7, 4);
   if (FirstCond == 0xF) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond ''\n");
-#endif
 return false;
   }
   if (FirstCond == 0xE && ITCounter != 1) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond '1110' && Mask != '1000'\n");
-#endif
 return false;
   }
 


Index: lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -605,9 +605,6 @@
   // First count the trailing zeros of the IT mask.
   uint32_t TZ = llvm::countTrailingZeros(ITMask);
   if (TZ > 3) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT Mask ''\n");
-#endif
 return 0;
   }
   return (4 - TZ);
@@ -622,15 +619,9 @@
   // A8.6.50 IT
   unsigned short FirstCond = Bits32(bits7_0, 7, 4);
   if (FirstCond == 0xF) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond ''\n");
-#endif
 return false;
   }
   if (FirstCond == 0xE && ITCounter != 1) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond '1110' && Mask != '1000'\n");
-#endif
 return false;
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] b8dab9b - [lldb] Remove some debugging printfs from ITSession code

2020-03-24 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2020-03-24T18:09:27+01:00
New Revision: b8dab9b3d5b9b75f5ec9b8fed3e1c0586a82e3bf

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

LOG: [lldb] Remove some debugging printfs from ITSession code

Summary:
This seems only useful for debugging and it's just plainly printf'ing to the 
console instead
of some log, so let's remove this.

Reviewers: #lldb, JDevlieghere

Reviewed By: JDevlieghere

Subscribers: JDevlieghere

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

Added: 


Modified: 
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp 
b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
index aa99db418283..34f88d2b4443 100644
--- a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -605,9 +605,6 @@ static uint32_t CountITSize(uint32_t ITMask) {
   // First count the trailing zeros of the IT mask.
   uint32_t TZ = llvm::countTrailingZeros(ITMask);
   if (TZ > 3) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT Mask ''\n");
-#endif
 return 0;
   }
   return (4 - TZ);
@@ -622,15 +619,9 @@ bool ITSession::InitIT(uint32_t bits7_0) {
   // A8.6.50 IT
   unsigned short FirstCond = Bits32(bits7_0, 7, 4);
   if (FirstCond == 0xF) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond ''\n");
-#endif
 return false;
   }
   if (FirstCond == 0xE && ITCounter != 1) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond '1110' && Mask != '1000'\n");
-#endif
 return false;
   }
 



___
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

2020-03-24 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk marked 10 inline comments as done.
kwk added a comment.

@labath @jankratochvil @fche2 I've addressed all your comments and hope the 
patch is good to go now.




Comment at: lldb/source/Host/common/DebugInfoD.cpp:43
+  buildID.GetBytes().size() ==
+  sizeof(llvm::support::ulittle32_t)) // .gnu_debuglink crc32
+return llvm::createStringError(llvm::inconvertibleErrorCode(),

labath wrote:
> jankratochvil wrote:
> > If it is done this way (and not in `libdebuginfod.so`) I think there should 
> > be `<=8` because LLDB contains:
> > ```
> > if (gnu_debuglink_crc) {
> >   // Use 4 bytes of crc from the .gnu_debuglink section.
> >   u32le data(gnu_debuglink_crc);
> >   uuid = UUID::fromData(, sizeof(data));
> > } else if (core_notes_crc) {
> >   // Use 8 bytes - first 4 bytes for *magic* prefix, mainly to 
> > make
> >   // it look different form .gnu_debuglink crc followed by 4 
> > bytes
> >   // of note segments crc.
> >   u32le data[] = {u32le(g_core_uuid_magic), 
> > u32le(core_notes_crc)};
> >   uuid = UUID::fromData(data, sizeof(data));
> > }
> > ```
> > 
> 4 would have probably been fine too, as I don't think a core file "uuid" can 
> make its way into here. In either case, we should document what is this 
> working around, as 4 or 8 byte uuids are technically valid.
@labath. I've added a documentation for the workaround.


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] D76672: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

2020-03-24 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lldb/include/lldb/Utility/FileCollector.h:26
+private:
+  bool addDSYM(const llvm::Twine );
+};

This is something I've struggled with before. If I'm inheriting from an LLVM 
class do I use LLVM or LLDB naming schemes?



Comment at: lldb/source/Utility/FileCollector.cpp:1
+//===-- FileCollector.cpp ---*- C++ 
-*-===//
+//

The C++ is redundant. This is a .cpp file, so it's not ambiguous which language 
this is.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76672



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

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

2020-03-24 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk updated this revision to Diff 252353.
kwk added a comment.

- Add documentation for workaround on rejecting special build UUIDs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,135 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+//We use the prefix map in order to overwrite all DW_AT_decl_file paths
+//in the DWARF. We cd into the directory before compiling to get
+//DW_AT_comp_dir pickup %t.buildroot as well so it will be replaced by
+///my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   %t.buildroot/test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mv%t.buildroot/test.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rf %t.debuginfod_cache_path
+// RUN: mkdir -p %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -f "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place --
+
+
+// RUN: DEBUGINFOD_URLS="http://example.com/debuginfod; \
+// RUN:

[Lldb-commits] [lldb] 1f80e51 - [lldb/Reproducers] Collect files imported by command script import

2020-03-24 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2020-03-24T08:54:26-07:00
New Revision: 1f80e51546bf2bf77982fd013519631f4c86898b

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

LOG: [lldb/Reproducers] Collect files imported by command script import

Files imported by the script interpreter aren't opened by LLDB so they
don't end up in the reproducer. The solution is to explicitly add them
to the FileCollector.

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

Added: 
lldb/test/Shell/Reproducer/Inputs/foo.lua
lldb/test/Shell/Reproducer/Inputs/foo.py
lldb/test/Shell/Reproducer/TestLuaImport.test
lldb/test/Shell/Reproducer/TestPythonImport.test

Modified: 
lldb/include/lldb/Host/FileSystem.h
lldb/source/Host/common/FileSystem.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/FileSystem.h 
b/lldb/include/lldb/Host/FileSystem.h
index 565d1f24e456..8dcff3402592 100644
--- a/lldb/include/lldb/Host/FileSystem.h
+++ b/lldb/include/lldb/Host/FileSystem.h
@@ -186,8 +186,10 @@ class FileSystem {
 return m_fs;
   }
 
+  void Collect(const FileSpec _spec);
+  void Collect(const llvm::Twine );
+
 private:
-  void AddFile(const llvm::Twine );
   static llvm::Optional ();
   llvm::IntrusiveRefCntPtr m_fs;
   std::shared_ptr m_collector;

diff  --git a/lldb/source/Host/common/FileSystem.cpp 
b/lldb/source/Host/common/FileSystem.cpp
index 220d7672cfb5..dcfa594597a1 100644
--- a/lldb/source/Host/common/FileSystem.cpp
+++ b/lldb/source/Host/common/FileSystem.cpp
@@ -279,7 +279,7 @@ void FileSystem::Resolve(FileSpec _spec) {
 std::shared_ptr
 FileSystem::CreateDataBuffer(const llvm::Twine , uint64_t size,
  uint64_t offset) {
-  AddFile(path);
+  Collect(path);
 
   const bool is_volatile = !IsLocal(path);
   const ErrorOr external_path = GetExternalPath(path);
@@ -417,7 +417,7 @@ static mode_t GetOpenMode(uint32_t permissions) {
 Expected FileSystem::Open(const FileSpec _spec,
   File::OpenOptions options,
   uint32_t permissions, bool should_close_fd) {
-  AddFile(file_spec.GetPath());
+  Collect(file_spec.GetPath());
 
   const int open_flags = GetOpenFlags(options);
   const mode_t open_mode =
@@ -465,7 +465,11 @@ ErrorOr FileSystem::GetExternalPath(const 
FileSpec _spec) {
   return GetExternalPath(file_spec.GetPath());
 }
 
-void FileSystem::AddFile(const llvm::Twine ) {
+void FileSystem::Collect(const FileSpec _spec) {
+  Collect(file_spec.GetPath());
+}
+
+void FileSystem::Collect(const llvm::Twine ) {
   if (m_collector && !llvm::sys::fs::is_directory(file)) {
 m_collector->addFile(file);
   }

diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
index ecbd30c10ae0..f9b24ad83de5 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -89,6 +89,7 @@ bool ScriptInterpreterLua::LoadScriptingModule(
 const char *filename, bool init_session, lldb_private::Status ,
 StructuredData::ObjectSP *module_sp) {
 
+  FileSystem::Instance().Collect(filename);
   if (llvm::Error e = m_lua->LoadModule(filename)) {
 error.SetErrorStringWithFormatv("lua failed to import '{0}': {1}\n",
 filename, llvm::toString(std::move(e)));

diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 3e93ddbf18c8..f59b70ac31d2 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2772,6 +2772,7 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule(
   {
 FileSpec target_file(pathname);
 FileSystem::Instance().Resolve(target_file);
+FileSystem::Instance().Collect(target_file);
 std::string basename(target_file.GetFilename().GetCString());
 
 StreamString command_stream;

diff  --git a/lldb/test/Shell/Reproducer/Inputs/foo.lua 
b/lldb/test/Shell/Reproducer/Inputs/foo.lua
new file mode 100644
index ..8ed0c94cbba9
--- /dev/null
+++ b/lldb/test/Shell/Reproducer/Inputs/foo.lua
@@ -0,0 +1 @@
+print('95126')

diff  --git a/lldb/test/Shell/Reproducer/Inputs/foo.py 
b/lldb/test/Shell/Reproducer/Inputs/foo.py
new file mode 100644
index ..8ed0c94cbba9
--- /dev/null
+++ b/lldb/test/Shell/Reproducer/Inputs/foo.py
@@ -0,0 +1 @@
+print('95126')

diff  --git

[Lldb-commits] [PATCH] D76626: [lldb/Reproducers] Collect files imported by command script import

2020-03-24 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f80e51546bf: [lldb/Reproducers] Collect files imported by 
command script import (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76626

Files:
  lldb/include/lldb/Host/FileSystem.h
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/test/Shell/Reproducer/Inputs/foo.lua
  lldb/test/Shell/Reproducer/Inputs/foo.py
  lldb/test/Shell/Reproducer/TestLuaImport.test
  lldb/test/Shell/Reproducer/TestPythonImport.test

Index: lldb/test/Shell/Reproducer/TestPythonImport.test
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/TestPythonImport.test
@@ -0,0 +1,11 @@
+# REQUIRES: python
+# UNSUPPORTED: system-windows
+# Ensure that the reproducers know about imported Python modules.
+
+# RUN: rm -rf %t.repro
+# RUN: %lldb -x -b --capture --capture-path %t.repro -o 'command script import  %S/Inputs/foo.py' -o 'reproducer generate' | FileCheck %s --check-prefix CAPTURE
+
+# CAPTURE: 95126
+
+# RUN: %lldb -b -o 'reproducer dump -p files -f %t.repro' | FileCheck %s --check-prefix FILES
+# FILES: foo.py
Index: lldb/test/Shell/Reproducer/TestLuaImport.test
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/TestLuaImport.test
@@ -0,0 +1,11 @@
+# REQUIRES: lua
+# UNSUPPORTED: system-windows
+# Ensure that the reproducers know about imported Lua modules.
+
+# RUN: rm -rf %t.repro
+# RUN: %lldb -x -b --script-language lua --capture --capture-path %t.repro -o 'command script import  %S/Inputs/foo.lua' -o 'reproducer generate' | FileCheck %s --check-prefix CAPTURE
+
+# CAPTURE: 95126
+
+# RUN: %lldb -b -o 'reproducer dump -p files -f %t.repro' | FileCheck %s --check-prefix FILES
+# FILES: foo.lua
Index: lldb/test/Shell/Reproducer/Inputs/foo.py
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/Inputs/foo.py
@@ -0,0 +1 @@
+print('95126')
Index: lldb/test/Shell/Reproducer/Inputs/foo.lua
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/Inputs/foo.lua
@@ -0,0 +1 @@
+print('95126')
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2772,6 +2772,7 @@
   {
 FileSpec target_file(pathname);
 FileSystem::Instance().Resolve(target_file);
+FileSystem::Instance().Collect(target_file);
 std::string basename(target_file.GetFilename().GetCString());
 
 StreamString command_stream;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -89,6 +89,7 @@
 const char *filename, bool init_session, lldb_private::Status ,
 StructuredData::ObjectSP *module_sp) {
 
+  FileSystem::Instance().Collect(filename);
   if (llvm::Error e = m_lua->LoadModule(filename)) {
 error.SetErrorStringWithFormatv("lua failed to import '{0}': {1}\n",
 filename, llvm::toString(std::move(e)));
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -279,7 +279,7 @@
 std::shared_ptr
 FileSystem::CreateDataBuffer(const llvm::Twine , uint64_t size,
  uint64_t offset) {
-  AddFile(path);
+  Collect(path);
 
   const bool is_volatile = !IsLocal(path);
   const ErrorOr external_path = GetExternalPath(path);
@@ -417,7 +417,7 @@
 Expected FileSystem::Open(const FileSpec _spec,
   File::OpenOptions options,
   uint32_t permissions, bool should_close_fd) {
-  AddFile(file_spec.GetPath());
+  Collect(file_spec.GetPath());
 
   const int open_flags = GetOpenFlags(options);
   const mode_t open_mode =
@@ -465,7 +465,11 @@
   return GetExternalPath(file_spec.GetPath());
 }
 
-void FileSystem::AddFile(const llvm::Twine ) {
+void FileSystem::Collect(const FileSpec _spec) {
+  Collect(file_spec.GetPath());
+}
+
+void FileSystem::Collect(const llvm::Twine ) {
   if (m_collector && !llvm::sys::fs::is_directory(file)) {
 m_collector->addFile(file);
   }
Index: lldb/include/lldb/Host/FileSystem.h

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

2020-03-24 Thread Hsiangkai Wang via Phabricator via lldb-commits

HsiangKai added a comment.

In D74023#1938583 , @jhenderson wrote:

> In D74023#1937220 , @HsiangKai wrote:
>
> > In D74023#1933427 , @jhenderson 
> > wrote:
> >
> > > @HsiangKai, have you noticed that there are some test failures according 
> > > to the harbourmaster bot? They look like they could be related to this 
> > > somehow.
> >
> >
> > @jhenderson, yes, I found test failures in harbormaster. The failures are 
> > occurred after I rebased my patch on master branch. After digging into 
> > error messages, I found the failures are triggered by find_if(). Maybe I 
> > misuse find_if() in this patch? Do you have any idea about this?
> >  By the way, I also found some patch, D75015 
> > , landed even harbormaster is failed. I am 
> > curious about is it a necessary condition to pass harbormaster before 
> > landing?
>
>
> I don't have much understanding of how Harbormaster works, and it may be that 
> the failures are unrelated to anything you did, since I believe it just 
> applies your patch on top of the current HEAD of master, which might not work 
> for various reasons. Still, it's worth reviewing and locally checking the 
> same tests to make sure they aren't failing locally. If you review the logs 
> produced, you might spot an issue. If Harbormaster is failing for a reason 
> related to your patch, your patch will almost certainly cause build bot 
> failures, so in that case, it is necessary to fix the issues (but in other 
> cases, if the issues are unrelated, it isn't).
>
> As for why find_if isn't working, I don't know, and I'd suggest you debug it.


Thanks for your reply. I will try to figure out why Harbormaster is failed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

2020-03-24 Thread Hsiangkai Wang via Phabricator via lldb-commits

HsiangKai updated this revision to Diff 252300.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023

Files:
  lld/ELF/InputFiles.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/include/llvm/Support/ARMAttributeParser.h
  llvm/include/llvm/Support/ARMBuildAttributes.h
  llvm/include/llvm/Support/ELFAttributeParser.h
  llvm/include/llvm/Support/ELFAttributes.h
  llvm/include/llvm/Support/RISCVAttributeParser.h
  llvm/include/llvm/Support/RISCVAttributes.h
  llvm/lib/Object/ELF.cpp
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/ARMAttributeParser.cpp
  llvm/lib/Support/ARMBuildAttrs.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/ELFAttributeParser.cpp
  llvm/lib/Support/ELFAttributes.cpp
  llvm/lib/Support/RISCVAttributeParser.cpp
  llvm/lib/Support/RISCVAttributes.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/attribute.s
  llvm/test/MC/RISCV/invalid-attribute.s
  llvm/test/tools/llvm-objdump/RISCV/lit.local.cfg
  llvm/test/tools/llvm-objdump/RISCV/unknown-arch-attr.test
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/unittests/Support/ARMAttributeParser.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/ELFAttributeParserTest.cpp
  llvm/unittests/Support/RISCVAttributeParserTest.cpp

Index: llvm/unittests/Support/RISCVAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/RISCVAttributeParserTest.cpp
@@ -0,0 +1,69 @@
+//===- unittests/RISCVAttributeParserTest.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 "llvm/Support/RISCVAttributeParser.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ELFAttributes.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+
+struct RISCVAttributeSection {
+  unsigned Tag;
+  unsigned Value;
+
+  RISCVAttributeSection(unsigned tag, unsigned value) : Tag(tag), Value(value) {}
+
+  void write(raw_ostream ) {
+OS.flush();
+// length = length + "riscv\0" + TagFile + ByteSize + Tag + Value;
+// length = 17 bytes
+
+OS << 'A' << (uint8_t)17 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << "riscv" << '\0';
+OS << (uint8_t)1 << (uint8_t)7 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << (uint8_t)Tag << (uint8_t)Value;
+  }
+};
+
+static bool testAttribute(unsigned Tag, unsigned Value, unsigned ExpectedTag,
+   unsigned ExpectedValue) {
+  std::string buffer;
+  raw_string_ostream OS(buffer);
+  RISCVAttributeSection Section(Tag, Value);
+  Section.write(OS);
+  ArrayRef Bytes(reinterpret_cast(OS.str().c_str()),
+  OS.str().size());
+
+  RISCVAttributeParser Parser;
+  cantFail(Parser.parse(Bytes, support::little));
+
+  Optional Attr = Parser.getAttributeValue(ExpectedTag);
+  return Attr.hasValue() && Attr.getValue() == ExpectedValue;
+}
+
+static bool testTagString(unsigned Tag, const char *name) {
+  return ELFAttrs::attrTypeAsString(Tag, RISCVAttrs::RISCVAttributeTags)
+ .str() == name;
+}
+
+TEST(StackAlign, testAttribute) {
+  EXPECT_TRUE(testTagString(4, "Tag_stack_align"));
+  EXPECT_TRUE(
+  testAttribute(4, 4, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_4));
+  EXPECT_TRUE(
+  testAttribute(4, 16, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_16));
+}
+
+TEST(UnalignedAccess, testAttribute) {
+  EXPECT_TRUE(testTagString(6, "Tag_unaligned_access"));
+  EXPECT_TRUE(testAttribute(6, 0, RISCVAttrs::UNALIGNED_ACCESS,
+RISCVAttrs::NOT_ALLOWED));
+  EXPECT_TRUE(
+  testAttribute(6, 1, RISCVAttrs::UNALIGNED_ACCESS, RISCVAttrs::ALLOWED));
+}
Index: llvm/unittests/Support/ELFAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/ELFAttributeParserTest.cpp
@@ -0,0 +1,63 @@
+//===- unittests/ELFAttributeParserTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

2020-03-24 Thread Hsiangkai Wang via Phabricator via lldb-commits

HsiangKai marked 3 inline comments as done.
HsiangKai added inline comments.



Comment at: llvm/include/llvm/Support/ARMBuildAttributes.h:37
   File  = 1,
   CPU_raw_name  = 4,
   CPU_name  = 5,

jhenderson wrote:
> Same comment as elsewhere.
I prefer to keep the formatting in this file. We could create another NFC patch 
for it.



Comment at: llvm/lib/Support/ARMBuildAttrs.cpp:15
   { ARMBuildAttrs::File, "Tag_File" },
   { ARMBuildAttrs::Section, "Tag_Section" },
   { ARMBuildAttrs::Symbol, "Tag_Symbol" },

jhenderson wrote:
> By the way: this clang-format failure might be related to your changes, so 
> it's probably worth checking to see if it is incorrect without your changes, 
> and if not, reformat it as part of this patch.
I didn't change the formatting in this file. I think it is not related to this 
patch. We could create another NFC patch to correct the formatting in 
ARMBuildAttrs.cpp.



Comment at: llvm/unittests/Support/ELFAttributeParserTest.cpp:27
+public:
+  AttributeHeaderParser(ScopedPrinter *sw)
+  : ELFAttributeParser(sw, emptyTagNameMap, "test") {}

jhenderson wrote:
> `sw` seems a bit of a random name. What does it mean?
I do not know either. I borrowed the name from ARMAttributeParser. I will 
change the variable name to 'printer'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

2020-03-24 Thread James Henderson via Phabricator via lldb-commits

jhenderson added a comment.

In D74023#1937220 , @HsiangKai wrote:

> In D74023#1933427 , @jhenderson 
> wrote:
>
> > @HsiangKai, have you noticed that there are some test failures according to 
> > the harbourmaster bot? They look like they could be related to this somehow.
>
>
> @jhenderson, yes, I found test failures in harbormaster. The failures are 
> occurred after I rebased my patch on master branch. After digging into error 
> messages, I found the failures are triggered by find_if(). Maybe I misuse 
> find_if() in this patch? Do you have any idea about this?
>  By the way, I also found some patch, D75015 
> , landed even harbormaster is failed. I am 
> curious about is it a necessary condition to pass harbormaster before landing?

I don't have much understanding of how Harbormaster works, and it may be that 
the failures are unrelated to anything you did, since I believe it just applies 
your patch on top of the current HEAD of master, which might not work for 
various reasons. Still, it's worth reviewing and locally checking the same 
tests to make sure they aren't failing locally. If you review the logs 
produced, you might spot an issue. If Harbormaster is failing for a reason 
related to your patch, your patch will almost certainly cause build bot 
failures, so in that case, it is necessary to fix the issues (but in other 
cases, if the issues are unrelated, it isn't).

As for why find_if isn't working, I don't know, and I'd suggest you debug it.

Comment at: llvm/include/llvm/Support/ARMBuildAttributes.h:37
   File  = 1,
   CPU_raw_name  = 4,
   CPU_name  = 5,

Same comment as elsewhere.

Comment at: llvm/lib/Support/ARMBuildAttrs.cpp:15
   { ARMBuildAttrs::File, "Tag_File" },
   { ARMBuildAttrs::Section, "Tag_Section" },
   { ARMBuildAttrs::Symbol, "Tag_Symbol" },

By the way: this clang-format failure might be related to your changes, so it's 
probably worth checking to see if it is incorrect without your changes, and if 
not, reformat it as part of this patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023

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

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

2020-03-24 Thread Adrian McCarthy via lldb-commits

I'll take a look this morning.

On Tue, Mar 24, 2020 at 7:00 AM Pavel Labath  wrote:

> On 23/03/2020 17:17, Frédéric Riss via lldb-commits wrote:
> > The new testing for “inherit-env=false” is failing on Windows. I skipped
> the test for now.
> >
> > Could it be that it never worked there? (In which case XFail would be a
> better resolution)
> > Does anyone have easy access to a Windows build to try it out?
> >
> > Thanks,
> > Fred
>
> My guess is that this "defensive check"
> <
> https://github.com/llvm/llvm-project/blob/master/lldb/source/Host/windows/ProcessLauncherWindows.cpp#L26
> >
> prevents us from passing a completely blank environment. I am tempted to
> just delete it and see what happens, but maybe Adrian is able to do a
> quick test of this?
>
> pl
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

2020-03-24 Thread Pavel Labath via lldb-commits

On 23/03/2020 17:17, Frédéric Riss via lldb-commits wrote:
> The new testing for “inherit-env=false” is failing on Windows. I skipped the 
> test for now.
> 
> Could it be that it never worked there? (In which case XFail would be a 
> better resolution)
> Does anyone have easy access to a Windows build to try it out?
> 
> Thanks,
> Fred

My guess is that this "defensive check"

prevents us from passing a completely blank environment. I am tempted to
just delete it and see what happens, but maybe Adrian is able to do a
quick test of this?

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

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

2020-03-24 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

The code mostly fine for me, but this should be reviewed by properly by more 
people, once you're ready to take down the WIP tag.

I am still not happy with the test case.




Comment at: lldb/source/Host/common/DebugInfoD.cpp:43
+  buildID.GetBytes().size() ==
+  sizeof(llvm::support::ulittle32_t)) // .gnu_debuglink crc32
+return llvm::createStringError(llvm::inconvertibleErrorCode(),

jankratochvil wrote:
> If it is done this way (and not in `libdebuginfod.so`) I think there should 
> be `<=8` because LLDB contains:
> ```
> if (gnu_debuglink_crc) {
>   // Use 4 bytes of crc from the .gnu_debuglink section.
>   u32le data(gnu_debuglink_crc);
>   uuid = UUID::fromData(, sizeof(data));
> } else if (core_notes_crc) {
>   // Use 8 bytes - first 4 bytes for *magic* prefix, mainly to 
> make
>   // it look different form .gnu_debuglink crc followed by 4 bytes
>   // of note segments crc.
>   u32le data[] = {u32le(g_core_uuid_magic), 
> u32le(core_notes_crc)};
>   uuid = UUID::fromData(data, sizeof(data));
> }
> ```
> 
4 would have probably been fine too, as I don't think a core file "uuid" can 
make its way into here. In either case, we should document what is this working 
around, as 4 or 8 byte uuids are technically valid.


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] D76650: Data formatters: fix detection of C strings

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG177dd63c8d74: Data formatters: fix detection of C strings 
(authored by jarin, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76650

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
  
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp


Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = u8"";
+  return 0; // break here
+}
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect_expr("s", result_summary='""')
+self.expect_expr("(const char*)s", result_summary='""')
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 


Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = u8"";
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect_expr("s", result_summary='""')
+self.expect_expr("(const char*)s", result_summary='""')
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 177dd63 - Data formatters: fix detection of C strings

2020-03-24 Thread Raphael Isemann via lldb-commits


Author: Jaroslav Sevcik
Date: 2020-03-24T14:25:59+01:00
New Revision: 177dd63c8d742250dac6ea365e7c30f0fbab3257

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

LOG: Data formatters: fix detection of C strings

Summary:
Detection of C strings does not work well for pointers. If the value object 
holding a (char*) pointer does not have an address (e.g., if it is a temp), the 
value is not considered a C string and its formatting is left to 
DumpDataExtractor rather than the special handling in  
ValueObject::DumpPrintableRepresentation. This leads to inconsistent outputs, 
e.g., in escaping non-ASCII characters. See the test for an example; the second 
test expectation is not met (without this patch). With this patch, the C string 
detection only insists that the pointer value is valid. The patch makes the 
code consistent with how the pointer is obtained in 
ValueObject::ReadPointedString.

Reviewers: teemperor

Reviewed By: teemperor

Subscribers: lldb-commits

Tags: #lldb

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

Added: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile

lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp

Modified: 
lldb/source/Core/ValueObject.cpp

Removed: 




diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 4c9c44ea15f4..9e20ba76dccb 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@ bool ValueObject::IsCStringContainer(bool check_pointer) {
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 

diff  --git 
a/lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile 
b/lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
new file mode 100644
index ..8b20bcb0
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
 
b/lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
new file mode 100644
index ..c05e9e9b06ba
--- /dev/null
+++ 
b/lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect_expr("s", result_summary='""')
+self.expect_expr("(const char*)s", result_summary='""')

diff  --git 
a/lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp 
b/lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
new file mode 100644
index ..c1e8bcf242f4
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = u8"";
+  return 0; // break here
+}



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

[Lldb-commits] [PATCH] D76699: [lldb] Remove some debugging printfs from ITSession code

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added a subscriber: JDevlieghere.

This seems only useful for debugging and it's just plainly printf'ing to the 
console instead
of some log, so let's remove this.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D76699

Files:
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp


Index: lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -605,9 +605,6 @@
   // First count the trailing zeros of the IT mask.
   uint32_t TZ = llvm::countTrailingZeros(ITMask);
   if (TZ > 3) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT Mask ''\n");
-#endif
 return 0;
   }
   return (4 - TZ);
@@ -622,15 +619,9 @@
   // A8.6.50 IT
   unsigned short FirstCond = Bits32(bits7_0, 7, 4);
   if (FirstCond == 0xF) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond ''\n");
-#endif
 return false;
   }
   if (FirstCond == 0xE && ITCounter != 1) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond '1110' && Mask != '1000'\n");
-#endif
 return false;
   }
 


Index: lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
===
--- lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -605,9 +605,6 @@
   // First count the trailing zeros of the IT mask.
   uint32_t TZ = llvm::countTrailingZeros(ITMask);
   if (TZ > 3) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT Mask ''\n");
-#endif
 return 0;
   }
   return (4 - TZ);
@@ -622,15 +619,9 @@
   // A8.6.50 IT
   unsigned short FirstCond = Bits32(bits7_0, 7, 4);
   if (FirstCond == 0xF) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond ''\n");
-#endif
 return false;
   }
   if (FirstCond == 0xE && ITCounter != 1) {
-#ifdef LLDB_CONFIGURATION_DEBUG
-printf("Encoding error: IT FirstCond '1110' && Mask != '1000'\n");
-#endif
 return false;
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76698: [lldb] Always log if acquiring packet sequence mutex fails

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added a subscriber: JDevlieghere.

Currently we only log in debug builds but I don't see why we would do this as 
this is neither
expensive and seems useful.

I looked into the git history of this code and it seems originally there was 
also an assert here
and the logging here was the #else branch branch for non-Debug builds.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D76698

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2797,12 +2797,10 @@
   thread_ids.push_back(1);
 }
   } else {
-#if !defined(LLDB_CONFIGURATION_DEBUG)
 Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS |
GDBR_LOG_PACKETS));
-LLDB_LOGF(log, "error: failed to get packet sequence mutex, not sending "
-   "packet 'qfThreadInfo'");
-#endif
+LLDB_LOG(log, "error: failed to get packet sequence mutex, not sending "
+  "packet 'qfThreadInfo'");
 sequence_mutex_unavailable = true;
   }
   return thread_ids.size();


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2797,12 +2797,10 @@
   thread_ids.push_back(1);
 }
   } else {
-#if !defined(LLDB_CONFIGURATION_DEBUG)
 Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS |
GDBR_LOG_PACKETS));
-LLDB_LOGF(log, "error: failed to get packet sequence mutex, not sending "
-   "packet 'qfThreadInfo'");
-#endif
+LLDB_LOG(log, "error: failed to get packet sequence mutex, not sending "
+  "packet 'qfThreadInfo'");
 sequence_mutex_unavailable = true;
   }
   return thread_ids.size();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added a subscriber: JDevlieghere.
teemperor updated this revision to Diff 252289.
teemperor added a comment.

- Upload correct diff.


This assert only triggers in Debug builds (and not in Rel+Assert builds). Let's 
always create a log if this
situation happens where we can't find the full type in the runtime and remove 
the Debug-only assert.


https://reviews.llvm.org/D76697

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
@@ -230,15 +230,13 @@
 
 auto types = decl_vendor->FindTypes(ConstString(name), /*max_matches*/ 1);
 
-// The user can forward-declare something that has no definition.  The runtime
-// doesn't prohibit this at all. This is a rare and very weird case.  We keep
-// this assert in debug builds so we catch other weird cases.
-#ifdef LLDB_CONFIGURATION_DEBUG
-assert(!types.empty());
-#else
-if (types.empty())
+// The user can forward-declare something that has no definition.  The 
runtime
+// doesn't prohibit this at all. This is a rare and very weird case.
+if (types.empty()) {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES);
+  LLDB_LOG(log, "Could not find type with name '{0}' in runtime", name);
   return ast_ctx.getObjCIdType();
-#endif
+}
 
 return ClangUtil::GetQualType(types.front().GetPointerType());
   } else {


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
@@ -230,15 +230,13 @@
 
 auto types = decl_vendor->FindTypes(ConstString(name), /*max_matches*/ 1);
 
-// The user can forward-declare something that has no definition.  The runtime
-// doesn't prohibit this at all. This is a rare and very weird case.  We keep
-// this assert in debug builds so we catch other weird cases.
-#ifdef LLDB_CONFIGURATION_DEBUG
-assert(!types.empty());
-#else
-if (types.empty())
+// The user can forward-declare something that has no definition.  The runtime
+// doesn't prohibit this at all. This is a rare and very weird case.
+if (types.empty()) {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES);
+  LLDB_LOG(log, "Could not find type with name '{0}' in runtime", name);
   return ast_ctx.getObjCIdType();
-#endif
+}
 
 return ClangUtil::GetQualType(types.front().GetPointerType());
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor updated this revision to Diff 252289.
teemperor added a comment.

- Upload correct diff.


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

https://reviews.llvm.org/D76697

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
@@ -230,15 +230,13 @@
 
 auto types = decl_vendor->FindTypes(ConstString(name), /*max_matches*/ 1);
 
-// The user can forward-declare something that has no definition.  The runtime
-// doesn't prohibit this at all. This is a rare and very weird case.  We keep
-// this assert in debug builds so we catch other weird cases.
-#ifdef LLDB_CONFIGURATION_DEBUG
-assert(!types.empty());
-#else
-if (types.empty())
+// The user can forward-declare something that has no definition.  The 
runtime
+// doesn't prohibit this at all. This is a rare and very weird case.
+if (types.empty()) {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES);
+  LLDB_LOG(log, "Could not find type with name '{0}' in runtime", name);
   return ast_ctx.getObjCIdType();
-#endif
+}
 
 return ClangUtil::GetQualType(types.front().GetPointerType());
   } else {


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
@@ -230,15 +230,13 @@
 
 auto types = decl_vendor->FindTypes(ConstString(name), /*max_matches*/ 1);
 
-// The user can forward-declare something that has no definition.  The runtime
-// doesn't prohibit this at all. This is a rare and very weird case.  We keep
-// this assert in debug builds so we catch other weird cases.
-#ifdef LLDB_CONFIGURATION_DEBUG
-assert(!types.empty());
-#else
-if (types.empty())
+// The user can forward-declare something that has no definition.  The runtime
+// doesn't prohibit this at all. This is a rare and very weird case.
+if (types.empty()) {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES);
+  LLDB_LOG(log, "Could not find type with name '{0}' in runtime", name);
   return ast_ctx.getObjCIdType();
-#endif
+}
 
 return ClangUtil::GetQualType(types.front().GetPointerType());
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

2020-03-24 Thread Jaroslav Sevcik via Phabricator via lldb-commits

jarin updated this revision to Diff 252276.
jarin marked 2 inline comments as done.
jarin added a comment.

Addressed reviewer comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76650

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
  
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp


Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = u8"";
+  return 0; // break here
+}
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect_expr("s", result_summary='""')
+self.expect_expr("(const char*)s", result_summary='""')
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 


Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = u8"";
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect_expr("s", result_summary='""')
+self.expect_expr("(const char*)s", result_summary='""')
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

2020-03-24 Thread Jaroslav Sevcik via Phabricator via lldb-commits

jarin added a comment.

Thanks for the review! Could you possibly land this for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76650



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

[Lldb-commits] [lldb] 68687e7 - [lldb][NFC] Mark GetNextPersistentVariableName as overriden to silence warning

2020-03-24 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2020-03-24T12:30:03+01:00
New Revision: 68687e75e7cb494796439f127a93ea03f2710551

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

LOG: [lldb][NFC] Mark GetNextPersistentVariableName as overriden to silence 
warning

This was triggering -Winconsistent-missing-override warnings.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
index f6298911dd6c..f888b2d56e68 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
@@ -51,7 +51,7 @@ class ClangPersistentVariables : public 
PersistentExpressionState {
 
   void RemovePersistentVariable(lldb::ExpressionVariableSP variable) override;
 
-  virtual ConstString GetNextPersistentVariableName(bool is_error = false);
+  ConstString GetNextPersistentVariableName(bool is_error = false) override;
 
   /// Returns the next file name that should be used for user expressions.
   std::string GetNextExprFileName() {



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

[Lldb-commits] [PATCH] D76569: Convert CommandObjectCommands functions to return StringRefs

2020-03-24 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

In D76569#1938577 , @labath wrote:

> I wouldn't be too worried about asan as a StringRef provides the same 
> lifetime guarantees (== none) as a const char *.


That's true but ASAN failed for me for the first patch. But that looks as some 
GCC bug which I cannot reproduce on a minimal testcase.

  =
  ==722160==ERROR: AddressSanitizer: stack-use-after-scope on address 
0x7fffa4c0 at pc 0x7fffd9c605fc bp 0x7fff9cc0 sp 0x7fff9cb0
  READ of size 1 at 0x7fffa4c0 thread T0
  #0 0x7fffd9c605fb in void std::__cxx11::basic_string, std::allocator >::_M_construct(char 
const*, char const*, std::forward_iterator_tag) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x28d45fb)
  #1 0x7fffdb165322 in 
lldb_private::CommandObject::CommandObject(lldb_private::CommandInterpreter&, 
llvm::StringRef, llvm::StringRef, llvm::StringRef, unsigned int) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x3dd9322)
  #2 0x7fffdb16aed0 in 
lldb_private::CommandObjectRegexCommand::CommandObjectRegexCommand(lldb_private::CommandInterpreter&,
 llvm::StringRef, llvm::StringRef, llvm::StringRef, unsigned int, unsigned int, 
bool) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x3ddeed0)
  #3 0x7fffe2c933c7 in 
CommandObjectCommandsAddRegex::DoExecute(lldb_private::Args&, 
lldb_private::CommandReturnObject&) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0xb9073c7)
  #4 0x7fffdb160ae1 in lldb_private::CommandObjectParsed::Execute(char 
const*, lldb_private::CommandReturnObject&) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x3dd4ae1)
  #5 0x7fffdb149b62 in lldb_private::CommandInterpreter::HandleCommand(char 
const*, lldb_private::LazyBool, lldb_private::CommandReturnObject&, 
lldb_private::ExecutionContext*, bool, bool) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x3dbdb62)
  #6 0x7fffdb14f1dc in 
lldb_private::CommandInterpreter::IOHandlerInputComplete(lldb_private::IOHandler&,
 std::__cxx11::basic_string, std::allocator 
>&) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x3dc31dc)
  #7 0x7fffdad58e63 in lldb_private::IOHandlerEditline::Run() 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x39cce63)
  #8 0x7fffdacc8321 in lldb_private::Debugger::RunIOHandlers() 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x393c321)
  #9 0x7fffdb1032fc in 
lldb_private::CommandInterpreter::RunCommandInterpreter(bool, bool, 
lldb_private::CommandInterpreterRunOptions&) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x3d772fc)
  #10 0x7fffd9e53e6b in lldb::SBDebugger::RunCommandInterpreter(bool, bool) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0x2ac7e6b)
  #11 0x4134c6 in Driver::MainLoop() 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/lldb+0x4134c6)
  #12 0x42339d in main 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/lldb+0x42339d)
  #13 0x7fffd544f1a2 in __libc_start_main ../csu/libc-start.c:308
  #14 0x4078ad in _start 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/lldb+0x4078ad)
  
  Address 0x7fffa4c0 is located in stack of thread T0 at offset 944 in frame
  #0 0x7fffe2c92aa3 in 
CommandObjectCommandsAddRegex::DoExecute(lldb_private::Args&, 
lldb_private::CommandReturnObject&) 
(/quad/home/jkratoch/redhat/llvm-monorepo2-gccassertdebugasan/bin/../lib/liblldb.so.11git+0xb906aa3)
  
This frame has 39 object(s):
  [32, 33) ''
  [48, 49) ''
  [64, 65) ''
  [80, 81) ''
  [96, 97) ''
  [112, 113) ''
  [128, 129) ''
  [144, 145) ''
  [160, 161) ''
  [176, 177) ''
  [192, 193) ''
  [208, 209) ''
  [224, 225) ''
  [240, 241) ''
  [256, 264) ''
  [288, 304) ''
  [320, 336) 'name' (line 990)
  [352, 368) ''
  [384, 400) ''
  [416, 432) 'io_handler_sp' (line 999)
  [448, 464) ''
  [480, 496) ''
  [512, 528) ''
  [544, 560) ''
  [576, 592) ''
  [608, 624) ''
  [640, 656) ''
  [672, 688) ''
  [704, 720) ''
  [736, 752) ''
  [768, 784) ''
  [800, 816) ''
  [832, 848) ''
  [864, 880) ''
  [896, 912) 'cmd_sp' (line 1130)
  [928, 960) '' <== Memory access at offset 944 is inside this 
variable
  [992, 1024) ''
  [1056, 1096) 'error' (line 989)
  [1136, 1176) ''
  HINT: this may be a false positive if your program uses some custom stack 
unwind

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

2020-03-24 Thread Raphael Isemann via Phabricator via lldb-commits

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

LGTM, thanks for tracking this down! I only have a minor comment about the test 
character.




Comment at: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py:18
+self.expect("expr s", substrs=['"é"'])
+self.expect("expr (const char*)s", substrs=['"é"'])

`expect_expr` is the modern way to check this:
```
lang=python
self.expect_expr("s", result_summary='""')
self.expect_expr("(const char*)s", result_summary='""')
```



Comment at: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp:2
+int main() {
+  const char *s = "é";
+  return 0; // break here

Could you make this a string like `u8""`? "é" is also a valid character in a 
lot of extended-ASCII character sets (e.g., Latin-1) and we probably should 
take some unicode-exclusive character and make sure it's utf-8.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76650



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

[Lldb-commits] [PATCH] D76672: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

2020-03-24 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

It's not clear to me why this is needed.

I mean, if lldb touches any of the files inside the dsym bundle, then they 
should be automatically recorded already, right? And if it doesn't then it does 
not need them...

It sounds to me like we are failing to capture some of the file accesses. Can 
this be fixed by catching those instead?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76672



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

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-24 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D74636#1938283 , @clayborg wrote:

> I would either add the option to SBLaunchInfo so it can be specified, or 
> execute the command. If the target is created, it is setting a target 
> specific setting. If had to pick I would add the API to SBLaunchInfo. 
> Whenever I see something that can't be done through the API, I like to add 
> that API if it is warranted. In our case the value in the SBLaunchInfo should 
> probably be stored as a lldb_private::LazyBool which can have the following 
> values:
>
>   enum LazyBool { eLazyBoolCalculate = -1, eLazyBoolNo = 0, eLazyBoolYes = 1 
> };
>
>
> It would eLazyBoolCalculate to in the launch info, and if it gets set to true 
> or false, then we use that, if it is set to eLazyBoolCalculate we use the 
> target setting.

As of https://reviews.llvm.org/D76045, one can pass nullptr/None  as the 
environment to Launch/LaunchSimple in order to get the "default" environment 
handling. This is does not work with the SBLaunchInfo, but the same thing can 
be achieved there (after https://reviews.llvm.org/D76111) by doing 
launch_info.SetEnvironment(target.GetEnvironment()). So I don't think that 
another way of inheriting the default environment is needed (or even desired).

The question here isn't really about what can or cannot be done via 
SBLaunchInfo api (all of this can be done). It's more of the opposite. I.e., 
what to do if we **cannot** use SBLaunchInfo because the user wants to do the 
launch itself (via `launchCommands` => CLI). I can see how applying the other 
settings would be useful, but given that `launchCommands` is a fairly advanced 
feature, I think it would also be acceptable to leave this up to the user. But 
in either case, I think we should be consistent, and not apply a random subset 
of settings.

So, I think it would be best to leave the `target.inherit-env` alone here, and 
create a separate patch for the handling of settings in the `launchCommands` 
scenario.

Comment at: lldb/tools/lldb-vscode/package.json:90
+   "description": 
"Inherit the debugger environment when launching a process. Only works for 
binaries launched directly by LLDB.",
+   "default": false
+   },

Why is this false? It seems true is a much more reasonable default here, and it 
would be consistent with the normal lldb behavior.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636

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

[Lldb-commits] [PATCH] D76626: [lldb/Reproducers] Collect files imported by command script import

2020-03-24 Thread Pavel Labath via Phabricator via lldb-commits

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

This seems fine with a (fairly big) caveat that this approach will not work 
transitively loaded modules (== if the "command script import"ed module imports 
another module on its own). Supporting this would be possible, but it would 
require hooking fairly deep into the module loading mechanism of respective 
language. In lua, I believe that could be achieved by overriding the builtin 
`loadfile` function (to save the file when recording, and substitute it during 
replay). In python, that might be achieved by overriding the `__import__` 
function to record the module file name and by adjusting the module import 
mechanism (perhaps via import hooks 
) on replay.

However, all of that is going to be fairly tricky code, so I don't think that's 
needed if you just want to make simple imports work.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76626



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

[Lldb-commits] [PATCH] D76569: Convert CommandObjectCommands functions to return StringRefs

2020-03-24 Thread 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. I wouldn't be too worried about asan as a StringRef provides the 
same lifetime guarantees (== none) as a const char *.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76569



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

[Lldb-commits] [PATCH] D76672: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

2020-03-24 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, arphaman, aprantl.
JDevlieghere added a project: LLDB.
Herald added subscribers: teemperor, abidh, dexonsmith, hiraditya, mgorny.
JDevlieghere added a parent revision: D76671: [FileCollector] Add a method to 
add a whole directory and it contents..

The FileCollector in LLDB collects every files that's used during a debug 
session when capture is enabled. This ensures that the reproducer only contains 
the files necessary to reproduce. This approach is not a good fit for the dSYM 
bundle, which is a directory on disk, but should be treated as a single unit. 
On macOS LLDB have automatically find the matching dSYM for a binary by its 
UUID. Having a incomplete dSYM in a reproducer can break debugging even when 
reproducers are disabled.

This patch adds a custom FileCollector to LLDB that is aware of dSYM. When it 
encounters a dSYM bundle, it iterates over everything inside it and adds it to 
the reproducer. While this might seem like overkill right now, having custom 
FileCollector is going to be inevitable as soon as we want to be smarter in 
what we include in the reproducer. For example, to keep its size small, we 
might want to skip system libraries in the future.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D76672

Files:
  lldb/include/lldb/Host/FileSystem.h
  lldb/include/lldb/Utility/FileCollector.h
  lldb/include/lldb/Utility/Reproducer.h
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/FileCollector.cpp
  lldb/test/Shell/Reproducer/TestDSYM.test
  llvm/include/llvm/Support/FileCollector.h
  llvm/lib/Support/FileCollector.cpp

Index: llvm/lib/Support/FileCollector.cpp
===
--- llvm/lib/Support/FileCollector.cpp
+++ llvm/lib/Support/FileCollector.cpp
@@ -61,26 +61,25 @@
   return true;
 }
 
-void FileCollector::addFile(const Twine ) {
-  std::lock_guard lock(Mutex);
-  std::string FileStr = file.str();
-  if (markAsSeen(FileStr))
-addFileImpl(FileStr);
-}
+void FileCollector::addFile(const Twine ) { addFileImpl(file.str()); }
 
 void FileCollector::addDirectory(const Twine ) {
   assert(sys::fs::is_directory(dir));
   std::error_code EC;
   sys::fs::recursive_directory_iterator Iter(dir, EC);
   sys::fs::recursive_directory_iterator End;
-  addFile(dir); // Add the directory in case it's empty.
+  addFileImpl(dir.str()); // Add the directory in case it's empty.
   for (; Iter != End && !EC; Iter.increment(EC)) {
 if (sys::fs::is_regular_file(Iter->path()))
-  addFile(Iter->path());
+  addFileImpl(Iter->path());
   }
 }
 
 void FileCollector::addFileImpl(StringRef SrcPath) {
+  std::lock_guard lock(Mutex);
+  if (!markAsSeen(SrcPath))
+return;
+
   // We need an absolute src path to append to the root.
   SmallString<256> AbsoluteSrc = SrcPath;
   sys::fs::make_absolute(AbsoluteSrc);
Index: llvm/include/llvm/Support/FileCollector.h
===
--- llvm/include/llvm/Support/FileCollector.h
+++ llvm/include/llvm/Support/FileCollector.h
@@ -45,7 +45,7 @@
   createCollectorVFS(IntrusiveRefCntPtr BaseFS,
  std::shared_ptr Collector);
 
-private:
+protected:
   void addFileImpl(StringRef SrcPath);
 
   bool markAsSeen(StringRef Path) {
@@ -60,7 +60,6 @@
 VFSWriter.addFileMapping(VirtualPath, RealPath);
   }
 
-protected:
   /// Synchronizes adding files.
   std::mutex Mutex;
 
Index: lldb/test/Shell/Reproducer/TestDSYM.test
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/TestDSYM.test
@@ -0,0 +1,11 @@
+# REQUIRES: system-darwin
+# Ensure that the reproducers captures the whole dSYM bundle.
+
+# RUN: rm -rf %t.repro
+# RUN: %clang_host %S/Inputs/simple.c -g -o %t.out
+# RUN: touch %t.out.dSYM/foo.bar
+
+# RUN: %lldb -x -b --capture --capture-path %t.repro %t.out -o 'b main' -o 'run' -o 'reproducer generate'
+
+# RUN: %lldb -b -o 'reproducer dump -p files -f %t.repro' | FileCheck %s --check-prefix FILES
+# FILES: foo.bar
Index: lldb/source/Utility/FileCollector.cpp
===
--- /dev/null
+++ lldb/source/Utility/FileCollector.cpp
@@ -0,0 +1,43 @@
+//===-- FileCollector.cpp ---*- C++ -*-===//
+//
+// 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/Utility/FileCollector.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
+
+using namespace lldb_private;
+
+void FileCollector::addFile(const llvm::Twine ) {
+  // Add dSYM bundle if applicable.
+  if

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

2020-03-24 Thread walter erquinigo via Phabricator via lldb-commits

wallace added a comment.

Thanks for the notice. I apologize for not doing everything correctly. I'm 
still trying to understand how all the different piece of the system works and 
I end up learning from the things that fail...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [lldb] e0279d7 - [lldb-vscode] Add missing launchCommands entry in the package.json

2020-03-24 Thread Walter Erquinigo via lldb-commits


Author: Walter Erquinigo
Date: 2020-03-23T23:21:30-07:00
New Revision: e0279d720a6eebd8508d4f102c684aee9fe9d100

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

LOG: [lldb-vscode] Add missing launchCommands entry in the package.json

Added: 


Modified: 
lldb/tools/lldb-vscode/package.json

Removed: 




diff  --git a/lldb/tools/lldb-vscode/package.json 
b/lldb/tools/lldb-vscode/package.json
index dc27ab54900b..2058edf68550 100644
--- a/lldb/tools/lldb-vscode/package.json
+++ b/lldb/tools/lldb-vscode/package.json
@@ -140,6 +140,11 @@

"description": "Commands executed just before the program is launched.",

"default": []
},
+   "launchCommands": {
+   "type": 
"array",
+   
"description": "Custom commands that are executed instead of launching a 
process. A target will be created with the launch arguments prior to executing 
these commands. The commands may optionally create a new target and must 
perform a launch. A valid process must exist after these commands complete or 
the \"launch\" will fail.",
+   
"default": []
+   },
"stopCommands": {
"type": 
"array",

"description": "Commands executed each time the program stops.",



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

68 matches

Mail list logo