Re: [Lldb-commits] [PATCH] D19991: [LLDB] Fix standalone build on RHEL6

2016-05-06 Thread Pavel Labath via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

looks fine. Out of curiosity, where were the undefined symbols coming from? I'm 
curious to know why they weren't showing up in none of our builds?


Repository:
  rL LLVM

http://reviews.llvm.org/D19991



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


Re: [Lldb-commits] [PATCH] D19998: Add a "-gmodules" category to the test suite.

2016-05-06 Thread Pavel Labath via lldb-commits
labath added a subscriber: labath.
labath added a comment.

I am glad to see more testing of the modules debugging. I have a couple of 
small comments though:

- `-fmodules`: Why is it not being added to CXXFLAGS? Is this how clang is 
supposed to be invoked? (I am not very familiar clang modules)
- there is a `@skipUnlessClangModules` decorator in decorators.py. As far as I 
can see, this patch should now make it obsolete. It seems that it can be 
removed and all invocations replaced with `add_test_categories(["gmodules"])`

And one meta-comment not directly related to this patch:
We already run most of the tests two times. Now we will be doing it once more, 
which will increase the test times even more. I think it's important for each 
debug info format to have good coverage, but I also feel that there are tests 
which have nothing to do with debug info (or their connection to debug info is 
only very peripheral), and it does not make to sense to slow down the tests 
runs by running those tests so many times. We already have a (not very elegant, 
but working) mechanism to avoid this (`NO_DEBUG_INFO_TESTCASE` member). I 
propose that we be more aggressive in using it for new tests which do not 
specifically test debug info. Also when looking at existing tests, we should 
re-evaluate whether the test really needs to be run that many times (right now, 
the largest candidate that comes to mind is TestConcurrentEvents, but I am sure 
there are others I can't think of by name now).



Comment at: packages/Python/lldbsuite/test/lldbtest.py:1488
@@ +1487,3 @@
+@wraps(attrvalue)
+def dwarf_test_method(self, attrvalue=attrvalue):
+self.debug_info = "gmodules"

Shouldn't this be `gmodules_test_method`?


http://reviews.llvm.org/D19998



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


Re: [Lldb-commits] [PATCH] D19998: Add a "-gmodules" category to the test suite.

2016-05-06 Thread Adrian Prantl via lldb-commits
aprantl added a comment.

In http://reviews.llvm.org/D19998#423277, @labath wrote:

> I am glad to see more testing of the modules debugging. I have a couple of 
> small comments though:
>
> - `-fmodules`: Why is it not being added to CXXFLAGS? Is this how clang is 
> supposed to be invoked? (I am not very familiar clang modules)


C++ modules are still a work in progress and not supported on all platforms 
(particularly on Darwin due to the way the system module maps interact with 
libc++, see https://llvm.org/bugs/show_bug.cgi?id=26928 for examples). On the 
platforms where C++ modules work well (such as Linux) on the other hand, module 
debugging hasn't been productized so far. Due to the way module debugging 
reuses DWO mechanisms I don't expect it to work without some fine-tuning.

> - there is a `@skipUnlessClangModules` decorator in decorators.py. As far as 
> I can see, this patch should now make it obsolete. It seems that it can be 
> removed and all invocations replaced with `add_test_categories(["gmodules"])`


Good catch. I didn't notice that!

> And one meta-comment not directly related to this patch:

>  We already run most of the tests two times. Now we will be doing it once 
> more, which will increase the test times even more. I think it's important 
> for each debug info format to have good coverage, but I also feel that there 
> are tests which have nothing to do with debug info (or their connection to 
> debug info is only very peripheral), and it does not make to sense to slow 
> down the tests runs by running those tests so many times. We already have a 
> (not very elegant, but working) mechanism to avoid this 
> (`NO_DEBUG_INFO_TESTCASE` member). I propose that we be more aggressive in 
> using it for new tests which do not specifically test debug info. Also when 
> looking at existing tests, we should re-evaluate whether the test really 
> needs to be run that many times (right now, the largest candidate that comes 
> to mind is TestConcurrentEvents, but I am sure there are others I can't think 
> of by name now).


That sounds like an all-around good idea, but probably out of scope for this 
patch.


http://reviews.llvm.org/D19998



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


Re: [Lldb-commits] [PATCH] D19998: Add a "-gmodules" category to the test suite.

2016-05-06 Thread Todd Fiala via lldb-commits
tfiala added a comment.

>   I propose that we be more aggressive in using it for new tests which do not 
> specifically test debug info. 


I totally agree, but I also caution that, as a debugger, the heart of much of 
what we do does use debug info.  So we need to be careful to make sure that we 
don't disable the fan-out across all debug styles if we are in fact using debug 
info.


http://reviews.llvm.org/D19998



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


Re: [Lldb-commits] [PATCH] D19991: [LLDB] Fix standalone build on RHEL6

2016-05-06 Thread Eugene Zelenko via lldb-commits
Eugene.Zelenko added a comment.

Undefined symbols were used in clang::CodeGen::CoverageMappingGen.


Repository:
  rL LLVM

http://reviews.llvm.org/D19991



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


[Lldb-commits] [lldb] r268750 - Fix standalone build on RHEL6.

2016-05-06 Thread Eugene Zelenko via lldb-commits
Author: eugenezelenko
Date: Fri May  6 12:03:09 2016
New Revision: 268750

URL: http://llvm.org/viewvc/llvm-project?rev=268750&view=rev
Log:
Fix standalone build on RHEL6.

Differential revision: http://reviews.llvm.org/D19991

Modified:
lldb/trunk/cmake/LLDBDependencies.cmake

Modified: lldb/trunk/cmake/LLDBDependencies.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/LLDBDependencies.cmake?rev=268750&r1=268749&r2=268750&view=diff
==
--- lldb/trunk/cmake/LLDBDependencies.cmake (original)
+++ lldb/trunk/cmake/LLDBDependencies.cmake Fri May  6 12:03:09 2016
@@ -195,6 +195,7 @@ set(LLVM_LINK_COMPONENTS
   runtimedyld
   option
   support
+  coverage
   )
 
 if ( NOT LLDB_DISABLE_PYTHON )


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


Re: [Lldb-commits] [PATCH] D19991: [LLDB] Fix standalone build on RHEL6

2016-05-06 Thread Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL268750: Fix standalone build on RHEL6. (authored by 
eugenezelenko).

Changed prior to commit:
  http://reviews.llvm.org/D19991?vs=56330&id=56430#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D19991

Files:
  lldb/trunk/cmake/LLDBDependencies.cmake

Index: lldb/trunk/cmake/LLDBDependencies.cmake
===
--- lldb/trunk/cmake/LLDBDependencies.cmake
+++ lldb/trunk/cmake/LLDBDependencies.cmake
@@ -195,6 +195,7 @@
   runtimedyld
   option
   support
+  coverage
   )
 
 if ( NOT LLDB_DISABLE_PYTHON )


Index: lldb/trunk/cmake/LLDBDependencies.cmake
===
--- lldb/trunk/cmake/LLDBDependencies.cmake
+++ lldb/trunk/cmake/LLDBDependencies.cmake
@@ -195,6 +195,7 @@
   runtimedyld
   option
   support
+  coverage
   )
 
 if ( NOT LLDB_DISABLE_PYTHON )
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r268755 - debugserver: fix some -Wformat-pedantic warnings

2016-05-06 Thread Saleem Abdulrasool via lldb-commits
Author: compnerd
Date: Fri May  6 12:32:58 2016
New Revision: 268755

URL: http://llvm.org/viewvc/llvm-project?rev=268755&view=rev
Log:
debugserver: fix some -Wformat-pedantic warnings

Perform explicit casts for the log message to address some `-Wformat-pedantic`
warnings from clang.  NFC.

Modified:
lldb/trunk/tools/debugserver/source/DNB.cpp

Modified: lldb/trunk/tools/debugserver/source/DNB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNB.cpp?rev=268755&r1=268754&r2=268755&view=diff
==
--- lldb/trunk/tools/debugserver/source/DNB.cpp (original)
+++ lldb/trunk/tools/debugserver/source/DNB.cpp Fri May  6 12:32:58 2016
@@ -360,21 +360,13 @@ DNBProcessLaunch (const char *path,
   char *err_str,
   size_t err_len)
 {
-DNBLogThreadedIf(LOG_PROCESS, "%s ( path='%s', argv = %p, envp = %p, 
working_dir=%s, stdin=%s, stdout=%s, stderr=%s, no-stdio=%i, launch_flavor = 
%u, disable_aslr = %d, err = %p, err_len = %llu) called...",
- __FUNCTION__, 
- path, 
- argv, 
- envp, 
- working_directory,
- stdin_path,
- stdout_path,
- stderr_path,
- no_stdio,
- launch_flavor, 
- disable_aslr, 
- err_str, 
- (uint64_t)err_len);
-
+DNBLogThreadedIf(LOG_PROCESS, "%s ( path='%s', argv = %p, envp = %p, 
working_dir=%s, stdin=%s, stdout=%s, "
+  "stderr=%s, no-stdio=%i, launch_flavor = %u, 
disable_aslr = %d, err = %p, err_len = "
+  "%llu) called...",
+ __FUNCTION__, path, static_cast(argv), 
static_cast(envp), working_directory,
+ stdin_path, stdout_path, stderr_path, no_stdio, 
launch_flavor, disable_aslr,
+ static_cast(err_str), 
static_cast(err_len));
+
 if (err_str && err_len > 0)
 err_str[0] = '\0';
 struct stat path_stat;
@@ -548,7 +540,6 @@ DNBProcessAttach (nub_process_t attach_p
 
 switch (pid_state)
 {
-default:
 case eStateInvalid:
 case eStateUnloaded:
 case eStateAttaching:


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


[Lldb-commits] [lldb] r268756 - debugserver: fix a few -Wcovered-swift-default warnings

2016-05-06 Thread Saleem Abdulrasool via lldb-commits
Author: compnerd
Date: Fri May  6 12:33:01 2016
New Revision: 268756

URL: http://llvm.org/viewvc/llvm-project?rev=268756&view=rev
Log:
debugserver: fix a few -Wcovered-swift-default warnings

Remove a couple of `default` cases from switches which are covered.  This is
beneficial since it would allow the compiler to indicate when a new enum value
is added and the switch is not updated.  Fixes some warnings from clang.  NFC.

Modified:
lldb/trunk/tools/debugserver/source/DNBDataRef.cpp
lldb/trunk/tools/debugserver/source/RNBRemote.cpp

Modified: lldb/trunk/tools/debugserver/source/DNBDataRef.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNBDataRef.cpp?rev=268756&r1=268755&r2=268756&view=diff
==
--- lldb/trunk/tools/debugserver/source/DNBDataRef.cpp (original)
+++ lldb/trunk/tools/debugserver/source/DNBDataRef.cpp Fri May  6 12:33:01 2016
@@ -362,7 +362,6 @@ DNBDataRef::Dump
 // the snprintf call each time through this loop
 switch (type)
 {
-default:
 case TypeUInt8:   str_offset += snprintf(str + str_offset, 
sizeof(str) - str_offset, format ? format : " %2.2x", Get8(&offset)); break;
 case TypeChar:
 {

Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=268756&r1=268755&r2=268756&view=diff
==
--- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Fri May  6 12:33:01 2016
@@ -1026,7 +1026,6 @@ RNBRemote::ThreadFunctionReadRemoteData(
 case rnb_success:
 break;
 
-default:
 case rnb_err:
 DNBLogThreadedIf (LOG_RNB_REMOTE, "RNBSocket::GetCommData 
returned error %u", err);
 done = true;


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


[Lldb-commits] [lldb] r268759 - debugserver; fix -Wunused-local-typedef, -Wunused-variable warnings

2016-05-06 Thread Saleem Abdulrasool via lldb-commits
Author: compnerd
Date: Fri May  6 12:33:13 2016
New Revision: 268759

URL: http://llvm.org/viewvc/llvm-project?rev=268759&view=rev
Log:
debugserver; fix -Wunused-local-typedef, -Wunused-variable warnings

Remove the typedef and local structure which was unused.  Fixes last of the new
clang warnings in the debugserver build.  NFC.

Modified:
lldb/trunk/tools/debugserver/source/RNBRemote.cpp

Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=268759&r1=268758&r2=268759&view=diff
==
--- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Fri May  6 12:33:13 2016
@@ -3555,13 +3555,6 @@ RNBRemote::HandlePacket_v (const char *p
 }
 else if (strstr (p, "vCont") == p)
 {
-typedef struct
-{
-nub_thread_t tid;
-char action;
-int signal;
-} vcont_action_t;
-
 DNBThreadResumeActions thread_actions;
 char *c = (char *)(p += strlen("vCont"));
 char *c_end = c + strlen(c);


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


[Lldb-commits] [lldb] r268757 - debugserver: fix some -Wpessimizing-move warnings

2016-05-06 Thread Saleem Abdulrasool via lldb-commits
Author: compnerd
Date: Fri May  6 12:33:04 2016
New Revision: 268757

URL: http://llvm.org/viewvc/llvm-project?rev=268757&view=rev
Log:
debugserver: fix some -Wpessimizing-move warnings

Remove the unnecessary use of std::move to permit the compiler to perform NVRO
instead.  Fixes more warnings from clang.  NFC.

Modified:
lldb/trunk/tools/debugserver/source/RNBRemote.cpp

Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=268757&r1=268756&r2=268757&view=diff
==
--- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Fri May  6 12:33:04 2016
@@ -2570,13 +2570,13 @@ RNBRemote::DispatchQueueOffsets::GetThre
 nub_addr_t pointer_to_label_address = dispatch_queue_t + 
dqo_label;
 nub_addr_t label_addr = DNBProcessMemoryReadPointer (pid, 
pointer_to_label_address);
 if (label_addr)
-queue_name = std::move(DNBProcessMemoryReadCString (pid, 
label_addr));
+queue_name = DNBProcessMemoryReadCString(pid, label_addr);
 }
 else
 {
 // libdispatch versions 1-3, dispatch name is a fixed width 
char array
 // in the queue structure.
-queue_name = std::move(DNBProcessMemoryReadCStringFixed(pid, 
dispatch_queue_t + dqo_label, dqo_label_size));
+queue_name = DNBProcessMemoryReadCStringFixed(pid, 
dispatch_queue_t + dqo_label, dqo_label_size);
 }
 }
 }
@@ -5687,7 +5687,7 @@ RNBRemote::HandlePacket_qSymbol (const c
 if (*p)
 {
 // We have a symbol name
-symbol_name = std::move(decode_hex_ascii_string(p));
+symbol_name = decode_hex_ascii_string(p);
 if (!symbol_value_str.empty())
 {
 nub_addr_t symbol_value = decode_uint64(symbol_value_str.c_str(), 
16);


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


[Lldb-commits] [lldb] r268758 - debugserver: fix a couple of -Wmissing-field-initializers warnings

2016-05-06 Thread Saleem Abdulrasool via lldb-commits
Author: compnerd
Date: Fri May  6 12:33:09 2016
New Revision: 268758

URL: http://llvm.org/viewvc/llvm-project?rev=268758&view=rev
Log:
debugserver: fix a couple of -Wmissing-field-initializers warnings

Explicitly provide an initializer for the std::vector in the constructed type.
Addresses -Wmissing-field-initializers warnings from clang.  NFC.

Modified:
lldb/trunk/tools/debugserver/source/RNBRemote.cpp

Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=268758&r1=268757&r2=268758&view=diff
==
--- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Fri May  6 12:33:09 2016
@@ -1227,7 +1227,9 @@ RNBRemote::InitializeRegisters (bool for
 register_map_entry_t reg_entry = {
 regnum++,   // register number 
starts at zero and goes up with no gaps
 reg_data_offset,// Offset into 
register context data, no gaps between registers
-reg_sets[set].registers[reg]// DNBRegisterInfo
+reg_sets[set].registers[reg],   // DNBRegisterInfo
+{},
+{},
 };
 
 name_to_regnum[reg_entry.nub_info.name] = 
reg_entry.debugserver_regnum;


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


Re: [Lldb-commits] [PATCH] D19998: Add a "-gmodules" category to the test suite.

2016-05-06 Thread Pavel Labath via lldb-commits
labath added a comment.

In http://reviews.llvm.org/D19998#423541, @aprantl wrote:

> In http://reviews.llvm.org/D19998#423277, @labath wrote:
>
> > I am glad to see more testing of the modules debugging. I have a couple of 
> > small comments though:
> >
> > - `-fmodules`: Why is it not being added to CXXFLAGS? Is this how clang is 
> > supposed to be invoked? (I am not very familiar clang modules)
>
>
> C++ modules are still a work in progress and not supported on all platforms 
> (particularly on Darwin due to the way the system module maps interact with 
> libc++, see https://llvm.org/bugs/show_bug.cgi?id=26928 for examples). On the 
> platforms where C++ modules work well (such as Linux) on the other hand, 
> module debugging hasn't been productized so far. Due to the way module 
> debugging reuses DWO mechanisms I don't expect it to work without some 
> fine-tuning.


Thank you for the comprehensive explanation. This information is interesting. 
Does that mean that in case of tests with c++ source code, there will be no 
difference in the test executables between modules and non-modules versions of 
the tests? I am wondering whether we should avoid running the modules tests in 
this case... I just did a quick count and we seem to have 129 C source files 
vs. 262 C++ files, so the difference is not actually as big as I expected, but 
it is still a substantial amount of test time going to waste. Maybe it's not 
that important though, we can possibly optimize that later, if it turns out to 
be necessary.

> > And one meta-comment not directly related to this patch:

> 

> >  We already run most of the tests two times. Now we will be doing it once 
> > more, which will increase the test times even more. I think it's important 
> > for each debug info format to have good coverage, but I also feel that 
> > there are tests which have nothing to do with debug info (or their 
> > connection to debug info is only very peripheral), and it does not make to 
> > sense to slow down the tests runs by running those tests so many times. We 
> > already have a (not very elegant, but working) mechanism to avoid this 
> > (`NO_DEBUG_INFO_TESTCASE` member). I propose that we be more aggressive in 
> > using it for new tests which do not specifically test debug info. Also when 
> > looking at existing tests, we should re-evaluate whether the test really 
> > needs to be run that many times (right now, the largest candidate that 
> > comes to mind is TestConcurrentEvents, but I am sure there are others I 
> > can't think of by name now).

> 

> 

> That sounds like an all-around good idea, but probably out of scope for this 
> patch.


Yes, I am definitely not requesting you make any changes like that here. I 
using just testing the waters about what people think about this idea. It's a 
bit of a hijack though, so sorry about that. We can move the discussion 
outside, if it starts to get more involved.

In http://reviews.llvm.org/D19998#423562, @tfiala wrote:

> >   I propose that we be more aggressive in using it for new tests which do 
> > not specifically test debug info. 
>
>
> I totally agree, but I also caution that, as a debugger, the heart of much of 
> what we do does use debug info.  So we need to be careful to make sure that 
> we don't disable the fan-out across all debug styles if we are in fact using 
> debug info.


I guess the question here is what do we consider as "using the debug info". To 
take TestConcurrentEvents as an example, it certainly *uses* debug info, in the 
sense that it sets a breakpoint at a certain line, and sets some variables, but 
I don't think this is what the test is about. It is about testing that the 
debugger handles the situation where there are multiple events happening in the 
target concurrently, which should not be related to the debug info at all. Any 
failure it this test which would be caused by a debug info problem should 
already be caught by some other test which actually targets these things.

At least, that's my opinion...



Comment at: packages/Python/lldbsuite/test/plugins/builder_base.py:144
@@ -143,1 +143,3 @@
 
+def buildDwarf(sender=None, architecture=None, compiler=None, dictionary=None, 
clean=True):
+"""Build the binaries with dwarf debug info."""

shouldn't this be `buildModules` or something?


http://reviews.llvm.org/D19998



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


[Lldb-commits] [lldb] r268802 - Fix LLDB after removal of PDB_ErrorCode

2016-05-06 Thread Zachary Turner via lldb-commits
Author: zturner
Date: Fri May  6 16:35:47 2016
New Revision: 268802

URL: http://llvm.org/viewvc/llvm-project?rev=268802&view=rev
Log:
Fix LLDB after removal of PDB_ErrorCode

Modified:
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp?rev=268802&r1=268801&r2=268802&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Fri May  6 
16:35:47 2016
@@ -118,7 +118,7 @@ SymbolFilePDB::CalculateAbilities()
 // Lazily load and match the PDB file, but only do this once.
 std::string exePath = m_obj_file->GetFileSpec().GetPath();
 auto error = loadDataForEXE(PDB_ReaderType::DIA, 
llvm::StringRef(exePath), m_session_up);
-if (error != PDB_ErrorCode::Success)
+if (error)
 return 0;
 }
 return CompileUnits | LineTables;


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


[Lldb-commits] [PATCH] D20035: Add source file mapping to inline frames' SymbolContext line_entry.file

2016-05-06 Thread Ted Woodward via lldb-commits
ted created this revision.
ted added a reviewer: jingham.
ted added a subscriber: lldb-commits.

new StackFrame objects created in StackFrameList::GetFramesUpTo are constructed 
with SymbolContext set to nullptr, except for inline frames. Calling the ctor 
with nullptr causes StackFrame::GetSymbolContext to apply the target.source-map 
to line_entry.file. Calling it with a valid SC will cause line_entry.file to 
have the original file from the DWARF info, so the file comparison in 
ThreadPlanStepRange::InRange will fail when it should succeed.

This patch adds the mapping from StackFrame::GetSymbolContext to the inline 
frame case in StackFrameList::GetFramesUpTo.

http://reviews.llvm.org/D20035

Files:
  source/Target/StackFrameList.cpp

Index: source/Target/StackFrameList.cpp
===
--- source/Target/StackFrameList.cpp
+++ source/Target/StackFrameList.cpp
@@ -350,6 +350,8 @@
 if (unwind_block)
 {
 Address curr_frame_address 
(unwind_frame_sp->GetFrameCodeAddress());
+TargetSP target_sp = m_thread.CalculateTarget();
+
 // Be sure to adjust the frame address to match the address
 // that was used to lookup the symbol context above. If we are
 // in the first concrete frame, then we lookup using the 
current
@@ -362,9 +364,8 @@
 // If curr_frame_address points to the first address 
in a section then after
 // adjustment it will point to an other section. In 
that case resolve the
 // address again to the correct section plus offset 
form.
-TargetSP target = m_thread.CalculateTarget();
-addr_t load_addr = 
curr_frame_address.GetOpcodeLoadAddress(target.get(), eAddressClassCode);
-curr_frame_address.SetOpcodeLoadAddress(load_addr - 1, 
target.get(), eAddressClassCode);
+addr_t load_addr = 
curr_frame_address.GetOpcodeLoadAddress(target_sp.get(), eAddressClassCode);
+curr_frame_address.SetOpcodeLoadAddress(load_addr - 1, 
target_sp.get(), eAddressClassCode);
 }
 else
 {
@@ -377,17 +378,25 @@
 
 while (unwind_sc.GetParentOfInlinedScope(curr_frame_address, 
next_frame_sc, next_frame_address))
 {
-StackFrameSP frame_sp(new StackFrame 
(m_thread.shared_from_this(),
-  m_frames.size(),
-  idx,
-  
unwind_frame_sp->GetRegisterContextSP (),
-  cfa,
-  
next_frame_address,
-  
&next_frame_sc));  
-
-m_frames.push_back (frame_sp);
-unwind_sc = next_frame_sc;
-curr_frame_address = next_frame_address;
+if (target_sp)
+{
+// Be sure to apply and file remappings to our file 
and line
+// entries when handing out a line entry
+FileSpec new_file_spec;
+if 
(target_sp->GetSourcePathMap().FindFile(next_frame_sc.line_entry.file, 
new_file_spec))
+next_frame_sc.line_entry.file = new_file_spec;
+}
+StackFrameSP frame_sp(new 
StackFrame(m_thread.shared_from_this(),
+ m_frames.size(),
+ idx,
+ 
unwind_frame_sp->GetRegisterContextSP (),
+ cfa,
+ next_frame_address,
+ &next_frame_sc));  
+
+m_frames.push_back (frame_sp);
+unwind_sc = next_frame_sc;
+curr_frame_address = next_frame_address;
 }
 }
 } while (m_frames.size() - 1 < end_idx);


Index: source/Target/StackFrameList.cpp
===
--- source/Target/StackFrameList.cpp
+++ source/Target/StackFrameList.cpp
@@ -350,6 +350,8 @@
 if (unwind_block)
 {
 Address curr_frame_address (unwind_frame_sp->GetFrameCodeAddress());
+TargetSP target_sp = m_thread.CalculateTarget();
+
 // Be 

[Lldb-commits] [lldb] r268823 - Fix the way the ShouldStopHere checker handles the general case of "stepping through line 0 code".

2016-05-06 Thread Jim Ingham via lldb-commits
Author: jingham
Date: Fri May  6 18:44:10 2016
New Revision: 268823

URL: http://llvm.org/viewvc/llvm-project?rev=268823&view=rev
Log:
Fix the way the ShouldStopHere checker handles the general case of "stepping 
through line 0 code".  
That's good 'cause it means all the different kinds of source line stepping 
won't leave user in the middle of
compiler implementation code or code inlined from odd places, etc.  But it 
turns out that the compiler
also marks functions it MIGHT inline as all being of line 0.  That would mean 
we single step through this code
instead of just stepping out.  That is both inefficient, and more error prone 
'cause these little nuggets tend
to be bits of hand-written assembly and the like and are hard to step through.

This change just checks and if the entire function is marked with line 0, we 
step out rather than step through.



Modified:
lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp

Modified: lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp?rev=268823&r1=268822&r2=268823&view=diff
==
--- lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp Fri May  6 18:44:10 
2016
@@ -11,10 +11,11 @@
 // C++ Includes
 // Other libraries and framework includes
 // Project includes
+#include "lldb/Core/Log.h"
+#include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlanShouldStopHere.h"
-#include "lldb/Core/Log.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -113,19 +114,45 @@ ThreadPlanShouldStopHere::DefaultStepFro
 ThreadPlanSP return_plan_sp;
 // If we are stepping through code at line number 0, then we need to step 
over this range.  Otherwise
 // we will step out.
+Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP));
+
 StackFrame *frame = 
current_plan->GetThread().GetStackFrameAtIndex(0).get();
 if (!frame)
 return return_plan_sp;
 SymbolContext sc;
 sc = frame->GetSymbolContext (eSymbolContextLineEntry);
+sc = frame->GetSymbolContext 
(eSymbolContextLineEntry|eSymbolContextSymbol);
 if (sc.line_entry.line == 0)
 {
 AddressRange range = sc.line_entry.range;
-return_plan_sp = 
current_plan->GetThread().QueueThreadPlanForStepOverRange(false,
-   
range,
-   
sc,
-   
eOnlyDuringStepping,
-   
eLazyBoolNo);
+
+   // If the whole function is marked line 0 just step out, that's 
easier & faster than continuing
+// to step through it.
+   bool just_step_out = false;
+   if (sc.symbol && sc.symbol->ValueIsAddress())
+   {
+   Address symbol_end = sc.symbol->GetAddress();
+   symbol_end.Slide(sc.symbol->GetByteSize() - 1);
+if (range.ContainsFileAddress(sc.symbol->GetAddress()) && 
range.ContainsFileAddress(symbol_end))
+   {
+   if (log)
+   log->Printf("Stopped in a function with only line 0 
lines, just stepping out.");
+just_step_out = true;
+   }
+   }
+if (!just_step_out)
+{
+if (log)
+log->Printf 
("ThreadPlanShouldStopHere::DefaultStepFromHereCallback Queueing StepInRange 
plan to step through line 0 code.");
+
+return_plan_sp = 
current_plan->GetThread().QueueThreadPlanForStepInRange(false,
+   
range,
+   
sc,
+   
NULL,
+   
eOnlyDuringStepping,
+   
eLazyBoolCalculate,
+   
eLazyBoolNo);
+}
 }
 
 if (!return_plan_sp)


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


[Lldb-commits] [lldb] r268827 - Remove some lldbassert's from the packet checking code.

2016-05-06 Thread Jim Ingham via lldb-commits
Author: jingham
Date: Fri May  6 19:52:18 2016
New Revision: 268827

URL: http://llvm.org/viewvc/llvm-project?rev=268827&view=rev
Log:
Remove some lldbassert's from the packet checking code.

Greg says he doesn't need these asserts anymore and since they cause occasional 
test suite
crashes, out they go.

Modified:
lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp

Modified: lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp?rev=268827&r1=268826&r2=268827&view=diff
==
--- lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp (original)
+++ lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp Fri May  6 19:52:18 
2016
@@ -14,7 +14,6 @@
 // Other libraries and framework includes
 // Project includes
 #include "Utility/StringExtractorGDBRemote.h"
-#include "lldb/Utility/LLDBAssert.h"
 
 StringExtractorGDBRemote::ResponseType
 StringExtractorGDBRemote::GetResponseType () const
@@ -405,7 +404,6 @@ OKErrorNotSupportedResponseValidator(voi
 case StringExtractorGDBRemote::eResponse:
 break;
 }
-lldbassert(!"Packet validatation failed, check why this is happening");
 return false;
 }
 
@@ -438,7 +436,6 @@ JSONResponseValidator(void *, const Stri
 }
 break;
 }
-lldbassert(!"Packet validatation failed, check why this is happening");
 return false;
 }
 
@@ -463,7 +460,6 @@ ASCIIHexBytesResponseValidator(void *, c
 {
 if (!isxdigit(ch))
 {
-lldbassert(!"Packet validatation failed, check why 
this is happening");
 return false;
 }
 if (++valid_count >= 16)
@@ -473,7 +469,6 @@ ASCIIHexBytesResponseValidator(void *, c
 }
 break;
 }
-lldbassert(!"Packet validatation failed, check why this is happening");
 return false;
 }
 


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


[Lldb-commits] [lldb] r268828 - Take the API lock in SBThread::IsValid & SBFrame::IsValid.

2016-05-06 Thread Jim Ingham via lldb-commits
Author: jingham
Date: Fri May  6 19:54:56 2016
New Revision: 268828

URL: http://llvm.org/viewvc/llvm-project?rev=268828&view=rev
Log:
Take the API lock in SBThread::IsValid & SBFrame::IsValid.

The IsValid calls can try to reconstruct the thread & frame, which can 
take various internal locks.  This can cause A/B locking issues with
the Target lock, so these calls need to that the Target lock.

Modified:
lldb/trunk/source/API/SBFrame.cpp
lldb/trunk/source/API/SBThread.cpp

Modified: lldb/trunk/source/API/SBFrame.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBFrame.cpp?rev=268828&r1=268827&r2=268828&view=diff
==
--- lldb/trunk/source/API/SBFrame.cpp (original)
+++ lldb/trunk/source/API/SBFrame.cpp Fri May  6 19:54:56 2016
@@ -105,7 +105,20 @@ SBFrame::SetFrameSP (const StackFrameSP
 bool
 SBFrame::IsValid() const
 {
-return GetFrameSP().get() != nullptr;
+Mutex::Locker api_locker;
+ExecutionContext exe_ctx (m_opaque_sp.get(), api_locker);
+
+Target *target = exe_ctx.GetTargetPtr();
+Process *process = exe_ctx.GetProcessPtr();
+if (target && process)
+{
+Process::StopLocker stop_locker;
+if (stop_locker.TryLock(&process->GetRunLock()))
+return GetFrameSP().get() != nullptr;
+}
+
+// Without a target & process we can't have a valid stack frame.
+return false;
 }
 
 SBSymbolContext

Modified: lldb/trunk/source/API/SBThread.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBThread.cpp?rev=268828&r1=268827&r2=268828&view=diff
==
--- lldb/trunk/source/API/SBThread.cpp (original)
+++ lldb/trunk/source/API/SBThread.cpp Fri May  6 19:54:56 2016
@@ -130,7 +130,19 @@ SBThread::GetQueue () const
 bool
 SBThread::IsValid() const
 {
-return m_opaque_sp->GetThreadSP().get() != NULL;
+Mutex::Locker api_locker;
+ExecutionContext exe_ctx (m_opaque_sp.get(), api_locker);
+
+Target *target = exe_ctx.GetTargetPtr();
+Process *process = exe_ctx.GetProcessPtr();
+if (target && process)
+{
+Process::StopLocker stop_locker;
+if (stop_locker.TryLock(&process->GetRunLock()))
+return m_opaque_sp->GetThreadSP().get() != NULL;
+}
+// Without a valid target & process, this thread can't be valid.
+return false;
 }
 
 void


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


[Lldb-commits] [PATCH] D20041: File path comparisons should be case-insensitive on OS X

2016-05-06 Thread Vyacheslav Karpukhin via lldb-commits
stigger created this revision.
stigger added a reviewer: zturner.
stigger added a subscriber: lldb-commits.

- File path comparisons should be case-insensitive on OS X 
- Fixed TestBreakpointCaseSensitivity

http://reviews.llvm.org/D20041

Files:
  include/lldb/Host/FileSpec.h
  
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/TestBreakpointCaseSensitivity.py
  source/Host/common/FileSpec.cpp
  source/Host/common/HostInfoBase.cpp

Index: source/Host/common/HostInfoBase.cpp
===
--- source/Host/common/HostInfoBase.cpp
+++ source/Host/common/HostInfoBase.cpp
@@ -59,15 +59,15 @@
 ArchSpec m_host_arch_32;
 ArchSpec m_host_arch_64;
 
-FileSpec m_lldb_so_dir;
-FileSpec m_lldb_support_exe_dir;
-FileSpec m_lldb_headers_dir;
-FileSpec m_lldb_python_dir;
-FileSpec m_lldb_clang_resource_dir;
-FileSpec m_lldb_system_plugin_dir;
-FileSpec m_lldb_user_plugin_dir;
-FileSpec m_lldb_process_tmp_dir;
-FileSpec m_lldb_global_tmp_dir;
+FileSpec m_lldb_so_dir = FileSpec(false);
+FileSpec m_lldb_support_exe_dir = FileSpec(false);
+FileSpec m_lldb_headers_dir = FileSpec(false);
+FileSpec m_lldb_python_dir = FileSpec(false);
+FileSpec m_lldb_clang_resource_dir = FileSpec(false);
+FileSpec m_lldb_system_plugin_dir = FileSpec(false);
+FileSpec m_lldb_user_plugin_dir = FileSpec(false);
+FileSpec m_lldb_process_tmp_dir = FileSpec(false);
+FileSpec m_lldb_global_tmp_dir = FileSpec(false);
 };
 
 HostInfoBaseFields *g_fields = nullptr;
Index: source/Host/common/FileSpec.cpp
===
--- source/Host/common/FileSpec.cpp
+++ source/Host/common/FileSpec.cpp
@@ -36,6 +36,7 @@
 #include "lldb/Host/FileSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/CleanUp.h"
 
 #include "llvm/ADT/StringRef.h"
@@ -304,9 +305,15 @@
 }
 
 FileSpec::FileSpec() : 
+FileSpec{IsCaseSensitive(ePathSyntaxHostNative)}
+{
+}
+
+FileSpec::FileSpec(bool case_sensitive) :
 m_directory(), 
 m_filename(), 
-m_syntax(FileSystem::GetNativePathSyntax())
+m_syntax(FileSystem::GetNativePathSyntax()),
+m_case_sensitive(case_sensitive)
 {
 }
 
@@ -318,15 +325,17 @@
 m_directory(),
 m_filename(),
 m_is_resolved(false),
-m_syntax(syntax)
+m_syntax(syntax),
+m_case_sensitive(IsCaseSensitive(syntax))
 {
 if (pathname && pathname[0])
-SetFile(pathname, resolve_path, syntax);
+SetFile(pathname, resolve_path, m_case_sensitive, syntax);
 }
 
 FileSpec::FileSpec(const char *pathname, bool resolve_path, ArchSpec arch) :
 FileSpec{pathname, resolve_path, arch.GetTriple().isOSWindows() ? ePathSyntaxWindows : ePathSyntaxPosix}
 {
+m_case_sensitive = IsCaseSensitive(m_syntax, &arch.GetTriple());
 }
 
 FileSpec::FileSpec(const std::string &path, bool resolve_path, PathSyntax syntax) :
@@ -346,7 +355,8 @@
 m_directory (rhs.m_directory),
 m_filename (rhs.m_filename),
 m_is_resolved (rhs.m_is_resolved),
-m_syntax (rhs.m_syntax)
+m_syntax (rhs.m_syntax),
+m_case_sensitive (rhs.m_case_sensitive)
 {
 }
 
@@ -380,6 +390,7 @@
 m_filename = rhs.m_filename;
 m_is_resolved = rhs.m_is_resolved;
 m_syntax = rhs.m_syntax;
+m_case_sensitive = rhs.m_case_sensitive;
 }
 return *this;
 }
@@ -390,11 +401,12 @@
 // string values for quick comparison and efficient memory usage.
 //--
 void
-FileSpec::SetFile (const char *pathname, bool resolve, PathSyntax syntax)
+FileSpec::SetFile(const char *pathname, bool resolve, bool case_sensitive, PathSyntax syntax)
 {
 m_filename.Clear();
 m_directory.Clear();
 m_is_resolved = false;
+m_case_sensitive = case_sensitive;
 m_syntax = (syntax == ePathSyntaxHostNative) ? FileSystem::GetNativePathSyntax() : syntax;
 
 if (pathname == NULL || pathname[0] == '\0')
@@ -431,12 +443,19 @@
 }
 
 void
+FileSpec::SetFile (const char *pathname, bool resolve, PathSyntax syntax, llvm::Triple *triple)
+{
+return SetFile(pathname, resolve, IsCaseSensitive(syntax, triple), syntax);
+}
+
+void
 FileSpec::SetFile(const char *pathname, bool resolve, ArchSpec arch)
 {
 return SetFile(pathname, resolve,
 arch.GetTriple().isOSWindows()
 ? ePathSyntaxWindows
-: ePathSyntaxPosix);
+: ePathSyntaxPosix,
+&arch.GetTriple());
 }
 
 void
@@ -850,7 +869,7 @@
 if (!GetPath (path_buf, PATH_MAX, false))
 return false;
 // SetFile(...) will set m_is_resolved correctly if it can resolve the path
-SetFile (path_buf, true);
+SetFile(path_buf, true, m_case_sensitive, m_syntax);
 return m_is_r

Re: [Lldb-commits] [PATCH] D20041: File path comparisons should be case-insensitive on OS X

2016-05-06 Thread Jason Molenda via lldb-commits
jasonmolenda added a subscriber: jasonmolenda.
jasonmolenda added a comment.

I don't know if this is a safe change.  Mac OS X filesystems like HFS Plus may 
be case-insensitive (but case preserving) or they may be case sensitive.  
Network filesystems mounted on a Mac may be case sensitive.  Without doing some 
attribute check on the filesystem (I don't know what API can tell you this 
off-hand) where the FileSpec is, I don't think this is a good idea.


http://reviews.llvm.org/D20041



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


Re: [Lldb-commits] [PATCH] D20041: File path comparisons should be case-insensitive on OS X

2016-05-06 Thread Vyacheslav Karpukhin via lldb-commits
stigger added a comment.

These are valid points, but they apply to Linux and Windows as well: nothing 
prevents you from mounting a case-insensitive FS on Linux or a case-sensitive 
network share on Windows. However, in most cases the file system is going to be 
case-sensitive on Linux and case-insensitive on Windows.

It is true, that HFS supports optional case-sensitivity, but by default it is 
not being used, and 99% of Mac users out there are on case-insensitive FS.

Of course, it would be nice to dynamically check case sensitivity for every 
path, but that should be done for all platforms and is kind of out of scope of 
the problem that I tried to solve here. Plus, it might affect the performance.


http://reviews.llvm.org/D20041



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


Re: [Lldb-commits] [PATCH] D20041: File path comparisons should be case-insensitive on OS X

2016-05-06 Thread Jason Molenda via lldb-commits
jasonmolenda added a comment.

Agreed, filesystems are almost always case sensitive on mac/ios.  I'd hate for 
anyone to make the assumption that it's always the case, though - I work with 
network filesystems that are case sensitive every day so I'm very aware of the 
possibility. :)


http://reviews.llvm.org/D20041



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