• Comment constant
• Use nullptr

• Get modification time for adb push from source file (Pass FileSpec directly through)
• Hard-code file mode flags (to not rely on platform definitions)
• Return failure if a failure occurs reading the source file.

Make static and use function indentation conventions.

LGTM

And done in one more location that slipped by before :-).

Use platformIsDarwin instead of manually checking if the platform is in darwin triples.

zturner, I added a comment about Windows like you suggested.

• Check for hexagon machine.
• Added comment to fix object arch on Windows and detect global / anonymous namespace symbols there.

Check for compatible architecture as well as platform.

Use + to concatenate the platform lists.

In D9672#170938, @tberghammer wrote:

The concept looks good, but when PlatformLinux is the host and it runs on a non Linux OS then I think it shouldn't return any supported architecture. It was always broken but it would be a good point to address it as you change the full GetSupportedArchitectureAtIndex code.

Removed class and restored to comparison to index, and ensured that the returned arch is linux, otherwise return no supported archs.

Repeating my comment in what seems to be the active patch. Do we need to enumerate all of the architectures? I put up http://reviews.llvm.org/D9672 earlier today to get non x86 working again where it queries the remote PlatformGdbServer if not the host. WDTY?

Hey, do we need to enumerate all of the architectures? I put up http://reviews.llvm.org/D9672 to get non x86 working again where it queries the remote PlatformGdbServer if not the host. WDTY?

Thanks for your help diagnosing this, I've put up http://reviews.llvm.org/D9672 to hopefully fix the supported architecture issues.

lgtm, thanks

In D9511#166552, @labath wrote:

In D9511#166440, @flackr wrote:

Looking up the mmap function address from the symbol table seems to have started working - we should still load debug symbols when possible but that can be done in a separate change.

This change is pretty much the minimal required to get this working. The process is a ProcessGDBRemote, and the platform is a PlatformRemoteGDBServer so I think we still need to check the target architecture if we try to move this to the process / platform.

If you do a "platform select remote-linux", the platform is (or should be) PlatformLinux, which contains a PlatformRemoteGDBServer as a member and then proxies all the operations to it, if it does not want special handling. E.g., PlatformLinux::GetProcessInfo is implemented like

if (IsHost())
{
    success = Platform::GetProcessInfo (pid, process_info);
}
else
{
    if (m_remote_platform_sp)
        success = m_remote_platform_sp->GetProcessInfo (pid, process_info); // This is a PlatformRemoteGDBServer
}

So I think you should be able to thread your handling through the platform.

• Delegate to Platform to convert Mmap flags.
• Fix platform when running remote-linux from mac by hard-coding linux archs, previously used host binary arch spec (e.g. macosx when running from mac)

Looking up the mmap function address from the symbol table seems to have started working - we should still load debug symbols when possible but that can be done in a separate change.

As a sidenote, I wonder how many of these skip if linux checks should also be skipping if freebsd.

In D9177#161956, @flackr wrote:

I'm a little confused about why you're setting EXECUTABLE_SUFFIX in Android.cmake but checking CMAKE_EXECUTABLE_SUFFIX in Makefile.rules, shouldn't those both be CMAKE_EXECUTABLE_SUFFIX?

I'm a little confused about why you're setting EXECUTABLE_SUFFIX in Android.cmake but checking CMAKE_EXECUTABLE_SUFFIX in Makefile.rules, shouldn't those both be CMAKE_EXECUTABLE_SUFFIX?

The lovely test suite caught a couple errors which I've addressed in this patch

• NULL is acceptable input for arch or uuid to Symbols::FindSymbolFileInBundle to ignore that attribute.
• Properly check for dsym symbols even if there's no symbol filename in the module spec.

Greg, please check if I'm checking the module spec correctly. I'm not sure if I should only be looking for matching UUID and arch whereas the ModuleSpecList::FindMatchingModuleSpec seems like it will check the FileSpec and PlatformFileSpec as well. It passes the stripped symbols test though which I presume is testing it.

Use ObjectFile::GetModuleSpecifications to check for matching module spec.

I think we need to do the same in test/make/Makefile.rules for AR and OBJCOPY.

I wanted to float this patch by you to get an early impression. Previously we only looked for dsym debugging info on mac and only .debug on linux. This patch should allow us to locate either. What I've tried to do is move as much of the symbol location into common/Symbols.cpp that I can. What do you think of this direction?

Use consistent immediate expansion of CROSS_COMPILE in AR and OBJCOPY.

LGTM, this gets at least 3 more tests passing when running maxosx -> linux, woohoo!
TestCPPExceptionBreakpoint.py
TestCPPExceptionBreakpoints.py
TestGlobalVariables.py

Remove platformIsLinux rather than start creating platformIsX for each platform string getPlatform can return.

Revert first attempt to escape environment variables - need more platform specific investigation.

I'll try to track down which commit(s) broke the tests by applying this patch to earlier revisions.

This patch effectively re-enabled all of the following tests on darwin:
TestAddDsymCommand.py TestAddDsymMidExecutionCommand.py TestConstStrings.py TestFoundationDisassembly.py TestObjCBreakpoints.py TestObjCDynamicSBType.py TestObjCMethodReturningBOOL.py TestObjCMethods2.py TestObjCMethods.py TestObjcOptimized.py TestPrintObj.py TestRdar10967107.py TestRdar11355592.py TestRdar12408181.py TestRuntimeTypes.py TestSetData.py TestSymbolTable.py

Escape quotes and backslashes in environment variable assignments.

This seems to have broken the xcode build for me - confirmed that reverting fixes the build failure:

Looks like the skipUnlessPlatform decorator works with individual test cases, but not entire classes. I believe I have a fix in http://reviews.llvm.org/D8903.