It seems this CL broke lldb cmake build - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/17910/steps/ninja%20build%20local/logs/stdio, could you take a look?

Looks like this CL broke LLDB cmake buildbot - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/17873/steps/ninja%20build%20local/logs/stdio, could you take a look?

It seems this CL is breaking lldb cmake build - could you take a look?

Files:

/lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp
/lldb/trunk/source/Plugins/Platform/Android/AdbClient.h
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp

In D22081#477746, @labath wrote:

Thanks for adding the additional checks. Looks good apart from the clang-format changes. This looks like the llvm style. I am not sure how you've run this but it does not seem to have picked up LLDB's .clang-format file.

Are you using git? You need to set clangFormat.binary setting to a sufficient new version (ideally the one you've just built), so it knows how to handle our AlwaysBreakAfterReturnType: All format. Then git clang-format HEAD^ should just work (assuming you have git-clang-format in your path).

Reran with proper clang-format.

In D22081#476766, @labath wrote:

You raised good points here, Luke.

I've thought about the quoting issue, but as you have already noticed there is no way to pass arguments containing quotes to adb correctly. Thinking about it more, what we could do is detect this situation (look for ' in the file name) and abort the whole process even before issuing the "adb shell" command.

As for the return value, this should not be a big problem, as further down the line we will discover that this is not a real ELF file and ignore it, but perhaps we could help here by searching for the file name in the first 1K bytes of the result, and ignoring the file if we find it?

Oleksiy, what do you think?

Addressed review comments:

• Check whether shell command output starts with /system/bin/sh: in order to find out whether command failed
• Fail-fast download if filename contains single-quotes
• clang-format

LGTM

Files:

/lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp
/lldb/trunk/source/Plugins/Platform/Android/AdbClient.h
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.h

LGTM

Files:

/lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp
/lldb/trunk/source/Plugins/Platform/Android/AdbClient.h
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.h
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp

If you okay with that I'd like to submit this CL as-is and come up with unit test with a separate change list.

In D21770#469876, @labath wrote:

Is AdbClient getting big enough to deserve a couple of unit tests? All it would take is to add the ability to specify the address to connect to and make a small mock server that listens at that address?

LGTM

In D16861#441964, @tfiala wrote:

This change broke connection to debugserver on OS X - the constructed URL on debugserver does not take the tcp:// (URL-scheme) portion. This would manifest when 'lldb-server platform' was attempting to launch debugserver.

I'm going to be making a fix for the debugserver case. I'll make sure it doesn't break Linux.

Submitted as r270590

In D20548#437862, @clayborg wrote:

At some point we should probably make a host layer call like:

const char *const char *Host::GetIllegalFilenameCharacters();" so that each host can determine the correct thing for the current host. Then FileSpec can be updated to use this function handle this with a method like "void FileSpec::NormalizePathForHost()". This will probably need to be done elsewhere.

In D20548#437567, @labath wrote:

According to https://msdn.microsoft.com/en-gb/library/windows/desktop/aa365247%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396, characters with codes 1..31 (basically < ' '), are illegal as well. We might as well convert those, just in case.

Agree - done.

I've been sitting on a unit test for the module cache locally for a while now... I guess it's time to put that in, and then we can add a test for this as well.

Sounds good.

Added the check for symbol in [1;31] range.

Added asterisk symbol

Thank you, Sean.

I reverted this CL with r269575 because of test failures on CMake and Android buildbots - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/14699. Feel free to re-submit it once the test issue is resolved.

Looks like this CL broke CMake build bot - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/14634, could you take a look?

Looks like this CL broke CMake buildbot - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/14464, could you take a look?

LGTM

LGTM

LGTM

LGTM

LGTM

we have bunch of new failures on LLDB cmake build bot with ToT - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/13420
It's failing due clang issue - CalledProcessError: Command 'make ARCH=x86_64 CC="/lldb-buildbot/lldbSlave/buildWorkingDir/scripts/../build/bin/clang" ' returned non-zero exit status 2
Is it possible that it's caused by this your CL?

In D19092#401450, @jingham wrote:

Why is this necessary? stdout is a local variable defined in this scope. Why would the android g++ have problems with this?

Anyway, if you have to avoid using stdout as a name, maybe name it std_out as that better reflects its meaning.

Submitted as http://reviews.llvm.org/rL266274

It seems this CL is breaking LLDB Windows builder - http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/5228, could you take a look?

Hi Greg,

It seems this CL made TestDebugBreak to fail on x86_64 on darwin build bot - http://lab.llvm.org:8011/builders/lldb-x86_64-darwin-13.4/builds/9529. Do we need to revert it for now?

In D18886#397637, @amccarth wrote:

FYI: According to git bisect, this patch seems to have introduced a new test failure on Windows.

it seems cmake build bot started to fail after this CL - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/13203. Could you take a look?

Submitted as http://reviews.llvm.org/rL265843

In D18886#395216, @labath wrote:

Does this fix an existing test or is a new issue? If it's new (it sounds like it is, as I don't see any test failures), could you also add a test for this. It shouldn't be too difficult to write one... Would something like this:

dbg.SetAsync(true)
process.Continue()
lldbutil.expect_state_changes(self,dbg.GetListener(), [eStateRunning])
target.BreakpointCreateBySourceRegex("// some code which will not get executed by the inferior")
do_some_assertions_on_the_breakpoint()
while dbg.GetListener().WaitForEvent(2, event):
  if SBProcess.GetStateFromEvent(event) == eStateStopped and SBProcess.GetRestartedFromEvent(state):
    continue
  if SBProcess.GetStateFromEvent(event) == eStateRunning:
    continue
  self.fail("Setting a breakpoint generated an unexpected event: %s"%SBDebugger.StateAsCString(SBProcess.GetStateFromEvent(event)));

do the trick?

Added new TestBreakpointSetRestart test to cover the addressed issue.

lgtm

looks like this CL broke Windows build bot - http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/4882.
Could you take a look?

LGTM

LGTM

It seems this CL broke LLDB CMake build bot - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/11554
Could you take a look?

It seems this CL broke LLDB Windows buildbot - http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/4316
Could you take a look?

LGTM

LGTM

Submitted as http://reviews.llvm.org/rL259714

LGTM

CL is failing on lldb cmake buildbot - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/10660

It seems lldb xcode build started to fail after this CL - http://lab.llvm.org:8011/builders/lldb-x86_64-darwin-13.4/builds/8218

Submitted as http://reviews.llvm.org/rL258150

LGTM

If vdso bug pertains to ELF format then it looks reasonable to keep the fix within ObjectFileELF.
I experimented a while ago with making ObjectFileELF to read from arbitrary offsets - please see http://reviews.llvm.org/D16151 as reflection of this idea (patch is pretty old and cannot be applied as-is).
Please take a look into it and let us know whether such approach may solve vdso problem.

vdso issue is related to ELF format and I'm wondering whether we can keep the fix contained in ObjectFileELF - for example, if a section that we're trying to read is beyond the boundaries of memory buffer we can read its content from inferior's memory using ObjectFile::m_process_wp.

LGTM

LGTM

Submitted as http://reviews.llvm.org/rL254638