[Lldb-commits] [lldb] r283477 - xfail TestDarwinLogBasic.py for i386 macOS

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Thu Oct  6 13:25:54 2016
New Revision: 283477

URL: http://llvm.org/viewvc/llvm-project?rev=283477&view=rev
Log:
xfail TestDarwinLogBasic.py for i386 macOS

Tracked by:
rdar://28655626

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/basic/TestDarwinLogBasic.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/basic/TestDarwinLogBasic.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/basic/TestDarwinLogBasic.py?rev=283477&r1=283476&r2=283477&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/basic/TestDarwinLogBasic.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/darwin_log/basic/TestDarwinLogBasic.py
 Thu Oct  6 13:25:54 2016
@@ -21,6 +21,7 @@ class TestDarwinLogBasic(darwin_log.Darw
 
 @decorators.add_test_categories(['pyapi'])
 @decorators.skipUnlessDarwin
+@decorators.expectedFailureAll(archs=["i386"], bugnumber="rdar://28655626")
 def test_SBStructuredData_gets_broadcasted(self):
 """Exercise SBStructuredData API."""
 


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

[Lldb-commits] [lldb] r283481 - xfail TestDiagnoseDereferenceFunctionReturn.py on macOS i386

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Thu Oct  6 14:04:58 2016
New Revision: 283481

URL: http://llvm.org/viewvc/llvm-project?rev=283481&view=rev
Log:
xfail TestDiagnoseDereferenceFunctionReturn.py on macOS i386

Tracked by:
rdar://28656408

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/frame-diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/frame-diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/frame-diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py?rev=283481&r1=283480&r2=283481&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/frame-diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/frame-diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
 Thu Oct  6 14:04:58 2016
@@ -15,6 +15,7 @@ class TestDiagnoseDereferenceFunctionRet
 mydir = TestBase.compute_mydir(__file__)
 
 @skipUnlessDarwin
+@expectedFailureAll(oslist=['macosx'], archs=['i386'], 
bugnumber="rdar://28656408")
 def test_diagnose_dereference_function_return(self):
 TestBase.setUp(self)
 self.build()


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

[Lldb-commits] [lldb] r283482 - xfail TestExec.py on macOS i386

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Thu Oct  6 14:12:05 2016
New Revision: 283482

URL: http://llvm.org/viewvc/llvm-project?rev=283482&view=rev
Log:
xfail TestExec.py on macOS i386

Tracked by:
rdar://28656532

Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py?rev=283482&r1=283481&r2=283482&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py 
Thu Oct  6 14:12:05 2016
@@ -27,6 +27,7 @@ class ExecTestCase(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @skipUnlessDarwin
+@expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
 def test(self):
 if self.getArchitecture() == 'x86_64':
 source = os.path.join(os.getcwd(), "main.cpp")


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

[Lldb-commits] [lldb] r283483 - xfail TestDataFormatterNSIndexPath.py on macOS i386

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Thu Oct  6 14:18:48 2016
New Revision: 283483

URL: http://llvm.org/viewvc/llvm-project?rev=283483&view=rev
Log:
xfail TestDataFormatterNSIndexPath.py on macOS i386

Tracked by:
rdar://28656605

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsindexpath/TestDataFormatterNSIndexPath.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsindexpath/TestDataFormatterNSIndexPath.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsindexpath/TestDataFormatterNSIndexPath.py?rev=283483&r1=283482&r2=283483&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsindexpath/TestDataFormatterNSIndexPath.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsindexpath/TestDataFormatterNSIndexPath.py
 Thu Oct  6 14:18:48 2016
@@ -45,6 +45,7 @@ class NSIndexPathDataFormatterTestCase(T
 commands()
 
 @skipUnlessDarwin
+@expectedFailureAll(archs=['i386'], bugnumber="rdar://28656605")
 def test_nsindexpath_with_run_command(self):
 """Test formatters for NSIndexPath."""
 self.appkit_tester_impl(self.nsindexpath_data_formatter_commands)


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

[Lldb-commits] [lldb] r283484 - xfail TestSBTypeTypeClass.py on macOS i386

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Thu Oct  6 14:23:29 2016
New Revision: 283484

URL: http://llvm.org/viewvc/llvm-project?rev=283484&view=rev
Log:
xfail TestSBTypeTypeClass.py on macOS i386

Tracked by:
rdar://28656677

Modified:

lldb/trunk/packages/Python/lldbsuite/test/python_api/sbtype_typeclass/TestSBTypeTypeClass.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/sbtype_typeclass/TestSBTypeTypeClass.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/sbtype_typeclass/TestSBTypeTypeClass.py?rev=283484&r1=283483&r2=283484&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/sbtype_typeclass/TestSBTypeTypeClass.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/sbtype_typeclass/TestSBTypeTypeClass.py
 Thu Oct  6 14:23:29 2016
@@ -3,4 +3,8 @@ from lldbsuite.test import lldbinline
 
 lldbinline.MakeInlineTest(
 __file__, globals(), [
-decorators.skipIfFreeBSD, decorators.skipIfLinux, 
decorators.skipIfWindows])
+decorators.skipIfFreeBSD, decorators.skipIfLinux,
+decorators.skipIfWindows,
+decorators.expectedFailureAll(
+oslist=['macosx'], archs=['i386'],
+bugnumber='rdar://28656677')])


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

[Lldb-commits] [lldb] r283492 - xfail TestQueues on macOS

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Thu Oct  6 16:07:45 2016
New Revision: 283492

URL: http://llvm.org/viewvc/llvm-project?rev=283492&view=rev
Log:
xfail TestQueues on macOS

This test is failing on CI.  I cannot get it to fail on my
local setup.

Tracked by:
rdar://28658529

Modified:
lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py?rev=283492&r1=283491&r2=283492&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py Thu 
Oct  6 16:07:45 2016
@@ -18,6 +18,7 @@ class TestQueues(TestBase):
 
 @skipUnlessDarwin
 @add_test_categories(['pyapi'])
+@expectedFailureAll(bugnumber="rdar://28658529")
 def test_with_python_api(self):
 """Test queues inspection SB APIs."""
 self.build()


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

[Lldb-commits] [lldb] r283493 - xfail TestReportData.py on i386

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Thu Oct  6 16:16:37 2016
New Revision: 283493

URL: http://llvm.org/viewvc/llvm-project?rev=283493&view=rev
Log:
xfail TestReportData.py on i386

Tracked by:
rdar://28658860

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/asan/TestReportData.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/asan/TestReportData.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/asan/TestReportData.py?rev=283493&r1=283492&r2=283493&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/asan/TestReportData.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/asan/TestReportData.py
 Thu Oct  6 16:16:37 2016
@@ -24,6 +24,7 @@ class AsanTestReportDataCase(TestBase):
 @skipIfFreeBSD  # llvm.org/pr21136 runtimes not yet available by default
 @skipIfRemote
 @skipUnlessCompilerRt
+@expectedFailureAll(archs=['i386'], bugnumber="rdar://28658860")
 def test(self):
 self.build()
 self.asan_tests()


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

[Lldb-commits] [lldb] r283497 - disable TSAN tests on macOS i386

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Thu Oct  6 16:30:33 2016
New Revision: 283497

URL: http://llvm.org/viewvc/llvm-project?rev=283497&view=rev
Log:
disable TSAN tests on macOS i386

These are erroring out on macOS i386.

Tracked by:
rdar://28659145

Modified:
lldb/trunk/packages/Python/lldbsuite/test/decorators.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/decorators.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/decorators.py?rev=283497&r1=283496&r2=283497&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/decorators.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/decorators.py Thu Oct  6 16:30:33 
2016
@@ -5,6 +5,7 @@ from __future__ import print_function
 from distutils.version import LooseVersion, StrictVersion
 from functools import wraps
 import os
+import platform
 import re
 import sys
 import tempfile
@@ -667,6 +668,9 @@ def skipUnlessThreadSanitizer(func):
 compiler = os.path.basename(compiler_path)
 if not compiler.startswith("clang"):
 return "Test requires clang as compiler"
+# rdar://28659145 - TSAN tests don't look like they're supported on 
i386
+if self.getArchitecture() == 'i386' and platform.system() == 'Darwin':
+return "TSAN tests not compatible with i386 targets"
 f = tempfile.NamedTemporaryFile()
 cmd = "echo 'int main() {}' | %s -x c -o %s -" % (compiler_path, 
f.name)
 if os.popen(cmd).close() is not None:


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

[Lldb-commits] [PATCH] D23977: Support of lldb on Kfreebsd

[Todd Fiala via lldb-commits]

tfiala added inline comments.



Comment at: cmake/modules/LLDBConfig.cmake:414
+
+find_package(Backtrace REQUIRED)

Hi Sylvestre!

It's hard to tell without more context, but it looks like this location has 
most/all configurations going through it.  For OSes that don't actually have a 
backtrace package, I think this emits a cmake error, doesn't it?

This might need to be protected by the systems that need the backtrace package. 
 (Probably Unix-like systems only?)


https://reviews.llvm.org/D23977



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

[Lldb-commits] [PATCH] D25486: Fix lookup path for lldb-mi

[Todd Fiala via lldb-commits]

tfiala accepted this revision.
tfiala added a comment.

Looks good!


https://reviews.llvm.org/D25486



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

[Lldb-commits] [PATCH] D25487: Fix building tests without system headers on Darwin

[Todd Fiala via lldb-commits]

tfiala added a comment.

(Retro) yep, this looked fine.


Repository:
  rL LLVM

https://reviews.llvm.org/D25487



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

[Lldb-commits] [PATCH] D25488: Fix test suite lookup path for LLDB.h

[Todd Fiala via lldb-commits]

tfiala added a comment.

(Retro) LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D25488



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

[Lldb-commits] [PATCH] D25489: Use LLDB_SRC for relative paths

[Todd Fiala via lldb-commits]

tfiala added a comment.

(Retro) LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D25489



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

[Lldb-commits] [PATCH] D25490: [CMake] Cleanup check-lldb targets

[Todd Fiala via lldb-commits]

tfiala added a comment.

(Retro) Ah nice, I like the generator expression you put in there.


Repository:
  rL LLVM

https://reviews.llvm.org/D25490



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

[Lldb-commits] [PATCH] D25570: [CMake] Populate LLDB.framework's headers directory

[Todd Fiala via lldb-commits]

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

LGTM.


https://reviews.llvm.org/D25570



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

[Lldb-commits] [PATCH] D23977: Support of lldb on Kfreebsd

[Todd Fiala via lldb-commits]

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

Okay.  I think we need a minor tweak, to drop the REQUIRED on the Backtrace 
usage.


https://reviews.llvm.org/D23977



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

[Lldb-commits] [lldb] r284484 - xfail TestMiSyntax.py's test_lldbmi_output_grammar on macOS

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Tue Oct 18 10:15:24 2016
New Revision: 284484

URL: http://llvm.org/viewvc/llvm-project?rev=284484&view=rev
Log:
xfail TestMiSyntax.py's test_lldbmi_output_grammar on macOS

Needs to be investigated.  This is failing locally and on the
Xcode CI.

rdar://28805064

Modified:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/syntax/TestMiSyntax.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/syntax/TestMiSyntax.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/syntax/TestMiSyntax.py?rev=284484&r1=284483&r2=284484&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/syntax/TestMiSyntax.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/syntax/TestMiSyntax.py 
Tue Oct 18 10:15:24 2016
@@ -88,6 +88,7 @@ class MiSyntaxTestCase(lldbmi_testcase.M
 
 @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
 @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread 
races
+@expectedFailureAll(oslist=["macosx"], bugnumber="rdar://28805064")
 def test_lldbmi_output_grammar(self):
 """Test that 'lldb-mi --interpreter' uses standard output syntax."""
 


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

[Lldb-commits] [PATCH] D25750: When invoking Terminal, don't assume the default shell

[Todd Fiala via lldb-commits]

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

Yep, looks right.  We shouldn't be assuming the shell.


https://reviews.llvm.org/D25750



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

[Lldb-commits] [PATCH] D25753: Use clang --driver-mode instead of guessing c++ compiler path

[Todd Fiala via lldb-commits]

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

LGTM.


https://reviews.llvm.org/D25753



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

[Lldb-commits] [PATCH] D25745: [CMake] Rename lldb-launcher to darwin-debug

[Todd Fiala via lldb-commits]

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

This seems fine, but @labath might want to weigh in if Android tools have a 
hard-baked assumption anywhere on lldb-launcher.


https://reviews.llvm.org/D25745



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

[Lldb-commits] [PATCH] D25714: [Test Suite] Allow overriding codesign identity

[Todd Fiala via lldb-commits]

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

LGTM.


https://reviews.llvm.org/D25714



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

Re: [Lldb-commits] [PATCH] D25750: When invoking Terminal, don't assume the default shell

[Todd Fiala via lldb-commits]

Actually that's a good point. We can default to bash but add a setting to 
override. The issue we have to solve is that the current approach fails with 
non-POSIX shells.

-Todd

> On Oct 18, 2016, at 4:09 PM, Joerg Sonnenberger  wrote:
> 
> joerg added a comment.
> 
> OK, this is OSX specific, but I still think that hardcoding bash is a bad 
> idea. Does it need anything not in `/bin/sh`?
> 
> 
> https://reviews.llvm.org/D25750
> 
> 
> 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D25830: Search for llvm-config in LLDB_PATH_TO_LLVM_BUILD first

[Todd Fiala via lldb-commits]

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

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D25830



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

[Lldb-commits] [PATCH] D25830: Search for llvm-config in LLDB_PATH_TO_LLVM_BUILD first

[Todd Fiala via lldb-commits]

tfiala added a comment.

(It would be good to wait for feedback from the others, though).


Repository:
  rL LLVM

https://reviews.llvm.org/D25830



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

[Lldb-commits] [PATCH] D25830: Search for llvm-config in LLDB_PATH_TO_LLVM_BUILD first

[Todd Fiala via lldb-commits]

tfiala added inline comments.



Comment at: cmake/modules/LLDBStandalone.cmake:20
+  find_program(LLVM_CONFIG "llvm-config"
+HINTS ${FIND_PATHS})
   if(LLVM_CONFIG)

One question here would be what happens if FIND_PATHS stays "".  Does 
find_program deal with an empty HINTS?  If not, the HINTS clause itself might 
need to be conditionalized.


Repository:
  rL LLVM

https://reviews.llvm.org/D25830



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

[Lldb-commits] [PATCH] D25830: Search for llvm-config in LLDB_PATH_TO_LLVM_BUILD first

[Todd Fiala via lldb-commits]

tfiala commandeered this revision.
tfiala edited reviewers, added: vivkong; removed: tfiala.
tfiala added a comment.

@beanz and I discussed.  This isn't needed here at all.  The issue is entirely 
in the current manifestation of the GitHub side of swift-lldb.

Closing this out, we'll resolve in GitHub.


Repository:
  rL LLVM

https://reviews.llvm.org/D25830



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

[Lldb-commits] [PATCH] D25887: [Test Suite] Pull generateSource into lldbtest

[Todd Fiala via lldb-commits]

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

LGTM.


https://reviews.llvm.org/D25887



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

[Lldb-commits] [PATCH] D25886: [Test Suite] Properly respect --framework option

[Todd Fiala via lldb-commits]

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

Looks good.

I do wonder if we should have a general helper for lines like this:

  sys.platform.rstrip('0123456789') in ('freebsd', 'linux', 'netbsd', 'darwin')

but that doesn't have to be looked at here.


https://reviews.llvm.org/D25886



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

[Lldb-commits] [PATCH] D25922: Test infra: expose CFLAGS_NO_ARCH for use by test custom build rules

[Todd Fiala via lldb-commits]

tfiala created this revision.
tfiala added reviewers: jingham, labath.
tfiala added a subscriber: lldb-commits.

The TestUniversal.py test was attempting to build its own CFLAGS unreliably.  
Essentially it just wanted the prevailing CFLAGS without the arch spec.

This change does the following:

- introduces a new makefile variable CFLAGS_NO_ARCH, which custom build rules 
can use to grab the prevailing CFLAGS for the build without the arch-specific 
flags, and
- uses this new flag in TestUniversal.py's Makefile, eliminating the divergence 
it had from the CFLAGS used for standard test inferiors.


https://reviews.llvm.org/D25922

Files:
  Python/lldbsuite/test/macosx/universal/Makefile
  Python/lldbsuite/test/make/Makefile.rules


Index: Python/lldbsuite/test/make/Makefile.rules
===
--- Python/lldbsuite/test/make/Makefile.rules
+++ Python/lldbsuite/test/make/Makefile.rules
@@ -193,13 +193,10 @@
 DEBUG_INFO_FLAG ?= -g
 
 CFLAGS ?= $(DEBUG_INFO_FLAG) -O0 -fno-builtin
-ifeq "$(OS)" "Darwin"
-   CFLAGS += $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) 
-I$(LLDB_BASE_DIR)include
-else
-   CFLAGS += $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) 
-I$(LLDB_BASE_DIR)include
-endif
+CFLAGS += $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) -I$(LLDB_BASE_DIR)include 
-include $(THIS_FILE_DIR)test_common.h $(TRIPLE_CFLAGS)
+CFLAGS_NO_ARCH := $(CFLAGS)
 
-CFLAGS += -include $(THIS_FILE_DIR)test_common.h $(TRIPLE_CFLAGS)
+CFLAGS += $(ARCHFLAG)$(ARCH)
 
 # Use this one if you want to build one part of the result without debug 
information:
 ifeq "$(OS)" "Darwin"
Index: Python/lldbsuite/test/macosx/universal/Makefile
===
--- Python/lldbsuite/test/macosx/universal/Makefile
+++ Python/lldbsuite/test/macosx/universal/Makefile
@@ -1,5 +1,14 @@
 CC ?= clang
 
+all: testit
+
+LEVEL = ../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules
+
+
 testit: testit.i386 testit.x86_64
lipo -create -o testit testit.i386 testit.x86_64
 
@@ -10,10 +19,10 @@
$(CC) -arch x86_64 -o testit.x86_64 testit.x86_64.o
 
 testit.i386.o: main.c
-   $(CC) -g -O0 -arch i386 -c -o testit.i386.o main.c
+   $(CC) $(CFLAGS_NO_ARCH) -arch i386 -c -o testit.i386.o main.c
 
 testit.x86_64.o: main.c
-   $(CC) -g -O0 -arch x86_64 -c -o testit.x86_64.o main.c
+   $(CC) $(CFLAGS_NO_ARCH) -arch x86_64 -c -o testit.x86_64.o main.c
 
-clean:
+clean::
rm -rf $(wildcard testit* *~)


Index: Python/lldbsuite/test/make/Makefile.rules
===
--- Python/lldbsuite/test/make/Makefile.rules
+++ Python/lldbsuite/test/make/Makefile.rules
@@ -193,13 +193,10 @@
 DEBUG_INFO_FLAG ?= -g
 
 CFLAGS ?= $(DEBUG_INFO_FLAG) -O0 -fno-builtin
-ifeq "$(OS)" "Darwin"
-	CFLAGS += $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) -I$(LLDB_BASE_DIR)include
-else
-	CFLAGS += $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) -I$(LLDB_BASE_DIR)include
-endif
+CFLAGS += $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) -I$(LLDB_BASE_DIR)include -include $(THIS_FILE_DIR)test_common.h $(TRIPLE_CFLAGS)
+CFLAGS_NO_ARCH := $(CFLAGS)
 
-CFLAGS += -include $(THIS_FILE_DIR)test_common.h $(TRIPLE_CFLAGS)
+CFLAGS += $(ARCHFLAG)$(ARCH)
 
 # Use this one if you want to build one part of the result without debug information:
 ifeq "$(OS)" "Darwin"
Index: Python/lldbsuite/test/macosx/universal/Makefile
===
--- Python/lldbsuite/test/macosx/universal/Makefile
+++ Python/lldbsuite/test/macosx/universal/Makefile
@@ -1,5 +1,14 @@
 CC ?= clang
 
+all: testit
+
+LEVEL = ../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules
+
+
 testit: testit.i386 testit.x86_64
 	lipo -create -o testit testit.i386 testit.x86_64
 
@@ -10,10 +19,10 @@
 	$(CC) -arch x86_64 -o testit.x86_64 testit.x86_64.o
 
 testit.i386.o: main.c
-	$(CC) -g -O0 -arch i386 -c -o testit.i386.o main.c
+	$(CC) $(CFLAGS_NO_ARCH) -arch i386 -c -o testit.i386.o main.c
 
 testit.x86_64.o: main.c
-	$(CC) -g -O0 -arch x86_64 -c -o testit.x86_64.o main.c
+	$(CC) $(CFLAGS_NO_ARCH) -arch x86_64 -c -o testit.x86_64.o main.c
 
-clean:
+clean::
 	rm -rf $(wildcard testit* *~)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r285032 - remove xfail from TestObjCNewSyntax.py test_expr_gmodules()

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Mon Oct 24 16:46:46 2016
New Revision: 285032

URL: http://llvm.org/viewvc/llvm-project?rev=285032&view=rev
Log:
remove xfail from TestObjCNewSyntax.py test_expr_gmodules()

Fixes:
rdar://27792848

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py?rev=285032&r1=285031&r2=285032&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
 Mon Oct 24 16:46:46 2016
@@ -33,10 +33,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 compiler_version=[
 '<',
 '7.0.0'])
-@expectedFailureAll(
-oslist=['macosx'],
-debug_info=['gmodules'],
-bugnumber='rdar://27792848')
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_expr(self):


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

[Lldb-commits] [PATCH] D25922: Test infra: expose CFLAGS_NO_ARCH for use by test custom build rules

[Todd Fiala via lldb-commits]

tfiala added a comment.

> I am not that excited by this, but I don't see a much better way to achieve 
> the result. :)
>  Possibly I'd consider making the variable name positive (instead of 
> CFLAGS_NO_DEBUG, have a INCLUDES var, and then the test can use $(INCLUDES) 
> $(TRIPLE_CFLAGS)).

Jim and I tried a few different ways to skin this.  The actual problem shows up 
on some CI downstream from here, where we need (but fail to get) the isysroot 
settings.  Those are built with a few more variables that have not yet moved up 
into the Makefile rules in LLVM.org.

The other way to skin this, since it is a macOS only test, is to strip out the 
arch flags in the TestUniversal Makefile.  This approach would work both here 
and downstream since it doesn't require accessing Makefile variables that don't 
exist here.

I'll come back with something slightly different here.




Comment at: Python/lldbsuite/test/make/Makefile.rules:197
-ifeq "$(OS)" "Darwin"
-   CFLAGS += $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) 
-I$(LLDB_BASE_DIR)include
-else

labath wrote:
> Are you sure this won't actually be missed on darwin for other tests? 
> `-archi386` does not seem to be a valid argument to clang (you need the space 
> here).
Hmm I ran it through x86_64 and that worked.  I didn't try i386.  I'll give 
that a shot.  If it blows up, I'll have a look at why a space is needed in one 
case and not the other on the macOS clang.  Thanks for highlighting.  (It 
looked like a meaningless divergence that I thought perhaps came from code 
motion over time).


https://reviews.llvm.org/D25922



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

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala created this revision.
tfiala added reviewers: labath, tberghammer.
tfiala added a subscriber: lldb-commits.
Herald added a subscriber: mgorny.

My original implementation of this last year was to just export everything from 
liblldb.so.  As lldb-mi recently started including a separate copy of llvm, 
exporting everything from liblldb.so has the side effect of allowing the Linux 
dynamic linker to resolve liblldb.so and lldb-mi's two copies of llvm to the 
same underlying storage.  This means any global constructors in LLVM (e.g. the 
'debug' command line option that triggered this change when NDEBUG is not 
defined) have side-effects applied twice to the same underlying data.

This change adjusts the -DLLDB_EXPORT_ALL_SYMBOLS flag's implementation, 
limiting exports to the normal exports plus the lldb_private namespace symbols. 
 This filters out exporting any of the LLVM symbols for the copy inside 
liblldb.so, maintaining the firewall between the separate copies of llvm. 
preventing two sets of global constructors from both occurring on the same 
underlying data.


https://reviews.llvm.org/D26093

Files:
  API/CMakeLists.txt
  API/liblldb-private.exports


Index: API/liblldb-private.exports
===
--- /dev/null
+++ API/liblldb-private.exports
@@ -0,0 +1,6 @@
+_ZN4lldb*
+_ZNK4lldb*
+_ZN12lldb_private*
+_ZNK12lldb_private*
+init_lld*
+PyInit__lldb*
Index: API/CMakeLists.txt
===
--- API/CMakeLists.txt
+++ API/CMakeLists.txt
@@ -103,16 +103,13 @@
 # If we're not exporting all symbols, we'll want to explicitly set
 # the exported symbols here.  This prevents 'log enable --stack ...'
 # from working on some systems but limits the liblldb size.
-MESSAGE("-- Symbols (liblldb): only exporting liblldb.exports symbols")
+MESSAGE("-- Symbols (liblldb): only exporting symbols in lldb namespace")
 add_llvm_symbol_exports(liblldb 
${CMAKE_CURRENT_SOURCE_DIR}/liblldb.exports)
   else()
 # Don't use an explicit export.  Instead, tell the linker to
 # export all symbols.
-MESSAGE("-- Symbols (liblldb): exporting all symbols")
-# Darwin linker doesn't need this extra step.
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  lldb_append_link_flags(liblldb "-Wl,--export-dynamic")
-endif()
+MESSAGE("-- Symbols (liblldb): exporting symbols from lldb and 
lldb_private namespace")
+add_llvm_symbol_exports(liblldb 
${CMAKE_CURRENT_SOURCE_DIR}/liblldb-private.exports)
   endif()
 endif()
 


Index: API/liblldb-private.exports
===
--- /dev/null
+++ API/liblldb-private.exports
@@ -0,0 +1,6 @@
+_ZN4lldb*
+_ZNK4lldb*
+_ZN12lldb_private*
+_ZNK12lldb_private*
+init_lld*
+PyInit__lldb*
Index: API/CMakeLists.txt
===
--- API/CMakeLists.txt
+++ API/CMakeLists.txt
@@ -103,16 +103,13 @@
 # If we're not exporting all symbols, we'll want to explicitly set
 # the exported symbols here.  This prevents 'log enable --stack ...'
 # from working on some systems but limits the liblldb size.
-MESSAGE("-- Symbols (liblldb): only exporting liblldb.exports symbols")
+MESSAGE("-- Symbols (liblldb): only exporting symbols in lldb namespace")
 add_llvm_symbol_exports(liblldb ${CMAKE_CURRENT_SOURCE_DIR}/liblldb.exports)
   else()
 # Don't use an explicit export.  Instead, tell the linker to
 # export all symbols.
-MESSAGE("-- Symbols (liblldb): exporting all symbols")
-# Darwin linker doesn't need this extra step.
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  lldb_append_link_flags(liblldb "-Wl,--export-dynamic")
-endif()
+MESSAGE("-- Symbols (liblldb): exporting symbols from lldb and lldb_private namespace")
+add_llvm_symbol_exports(liblldb ${CMAKE_CURRENT_SOURCE_DIR}/liblldb-private.exports)
   endif()
 endif()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added inline comments.



Comment at: API/CMakeLists.txt:106
 # from working on some systems but limits the liblldb size.
-MESSAGE("-- Symbols (liblldb): only exporting liblldb.exports symbols")
+MESSAGE("-- Symbols (liblldb): only exporting symbols in lldb namespace")
 add_llvm_symbol_exports(liblldb 
${CMAKE_CURRENT_SOURCE_DIR}/liblldb.exports)

I'll change this to:
-- Symbols (liblldb): exporting symbols in the lldb namespace

in the commit to normalize with the message I modified below.


https://reviews.llvm.org/D26093



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

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

In https://reviews.llvm.org/D26093#582395, @labath wrote:

> I am a bit surprised that this fixes the problem. I would have expected that 
> the problem would occur in case when we *do* restrict exports,


This matches my expectations.  When I was using the flag the exported all 
symbols indiscriminately from liblldb.so, it meant that the llvm symbols for 
the copy inside liblldb.so were public and known.  That public element allows 
the dynamic linker to coalesce the data storage locations with the ones for the 
llvm in lldb-mi.  Hiding them (as per the normal case, when we only export the 
'lldb' public namespace) prevents the dynamic linker from doing the coalesce.

as in that case, it is much easier to pull in copies of stuff twice. I wonder 
if in this case we could actually avoid having two copies of llvm.

> What would happen if you just remove the call to llvm_config in lldb-mi's 
> CMakeLists.txt. Will you get some unresolved symbols?
> 
> Adding @beanz, in case he has some ideas.


https://reviews.llvm.org/D26093



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

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

In https://reviews.llvm.org/D26093#582412, @tfiala wrote:

> In https://reviews.llvm.org/D26093#582395, @labath wrote:
>
> > I am a bit surprised that this fixes the problem. I would have expected 
> > that the problem would occur in case when we *do* restrict exports,
>
>
> This matches my expectations.  When I was using the flag the exported all 
> symbols indiscriminately from liblldb.so, it meant that the llvm symbols for 
> the copy inside liblldb.so were public and known.  That public element allows 
> the dynamic linker to coalesce the data storage locations with the ones for 
> the llvm in lldb-mi.  Hiding them (as per the normal case, when we only 
> export the 'lldb' public namespace) prevents the dynamic linker from doing 
> the coalesce.


So, the global constructor initialization sequence for liblldb.so, which 
includes a copy of llvm with Debug.cpp and therefore runs the global 
constructor for adding 'debug' to the command line, does so, but against 
*shared* locations for the backing storage.  And lldb-mi, which also contains a 
separate copy of the llvm code, also adds the constructor to the init sequence, 
using the shared data location that was coalesced by the dynamic linker.  So 
two separate copies of the code, using the same coalesced storage, end up 
blowing up because both instances of the global constructor think they're the 
only one operating on that data.


https://reviews.llvm.org/D26093



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

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

The reason why it doesn't blow up in the hidden case is that liblldb.so has 
already internally resolved the storage, doesn't involve dynamic resolution, 
and has its own data location.  Ditto for lldb-mi's copy.  So they are separate 
islands.

There are two totally separate other issues here that I see, that I think are 
best handled by other discrete changes:

1. Fix lldb-mi to not need a separate copy of llvm.
2. Fix lldb on Linux to not require private symbols to be exposed to get access 
to them during debugging, when the debug info is available.

Item 1 was recently introduced.  Item 2 appears to be a flaw in Linux LLDB, at 
least in the configurations we use.  I plan to tackle #2 in the near future.  I 
seem to recall #1 happened when some refactoring was taking place that ended up 
requiring private details from llvm?


https://reviews.llvm.org/D26093



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

[Lldb-commits] [PATCH] D25830: Search for llvm-config in LLDB_PATH_TO_LLVM_BUILD first

[Todd Fiala via lldb-commits]

tfiala abandoned this revision.
tfiala added a comment.

I'm at a loss for the workflow to close this.  I originally commandeered it to 
do an abort on it, but that didn't work.  I'm going to abandon it, and see if I 
can kill it then.


Repository:
  rL LLVM

https://reviews.llvm.org/D25830



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

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

In https://reviews.llvm.org/D26093#582583, @beanz wrote:

> This patch ends up hiding the problem, not fixing it.


Yes.

> If it unblocks something it might be ok, but it doesn't actually fix the 
> problem.

It allows debugging of lldb with lldb on Ubuntu.  This broke recently, and the 
CL here re-enables the ability to do that.

> The underlying problem is that liblldb and lldb-mi have their own copies of 
> LLVM with their own copies of the GlobalParser static 
> (llvm/lib/Support/CommandLine.cpp), and the corresponding static for the 
> debug cl::opt (llvm/lib/Support/Debug.cpp).
> 
> When the library exports the LLVM symbols, the dynamic loader loads lldb-mi 
> and it makes the cl::opt constructor point at the same instance of the 
> GlobalParser (following link-once ODR rules).
> 
> When the library doesn't export the LLVM symbols, the cl::opt variables 
> inside liblldb resolve to the GlobalParser inside liblldb (because it is 
> local), and the cl::opt variables inside lldb-mi resolve inside lldb-mi 
> because the loader cannot see the GlobalParser from liblldb because the 
> symbol isn't exported. This makes two instances of the GlobalParser exist at 
> the same time.

Right.  I think I said all that in comments above, but perhaps that is a 
succinct summary :-)

> This does work around the problem Todd was seeing. Unfortunately, it doesn't 
> actually fix the problem, and if lldb-mi is using cl::opt across the library 
> boundary it will cause subtle and difficult to diagnose bugs.

As lldb-mi doesn't use CommandLine to the best of my understanding by searching 
for cl usages in the code, I don't see that as critical.  The current usage of 
CommandLine via global static constructors in Debug.cpp seems like a poor 
choice, as it forces all users of llvm with NDEBUG not defined to get some 
command line behavior that they may never use.  Maybe some aspect of that can 
be made lazy?

Note that this change neither introduces the issue of having two islands of 
CommandLine data --- that is already there in standard builds of lldb-mi --- it 
simply prevents the "please allow me to debug lldb with lldb" case from getting 
clobbered by the dynamic linker coalescing which the old implementation of 
LLDB_EXPORT_ALL_SYMBOLS did not avoid.

Unless there is strong opposition, I'd like to put this in.


https://reviews.llvm.org/D26093



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

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

In https://reviews.llvm.org/D26093#582659, @beanz wrote:

> In https://reviews.llvm.org/D26093#582618, @tfiala wrote:
>
> > The current usage of CommandLine via global static constructors in 
> > Debug.cpp seems like a poor choice, as it forces all users of llvm with 
> > NDEBUG not defined to get some command line behavior that they may never 
> > use.  Maybe some aspect of that can be made lazy?
>
>
> It can't be made lazy without completely re-designing how cl::opt works. 
> Which is something I've had lengthy discussions with Chandler about. 
> Currently the biggest issue with fixing it relates to how the new pass 
> manager works. Specifically in order to provide meaningful -help output we 
> need a way to register command line options in passes, which we have today, 
> but won't with the new pass manager.
>
> It is an open problem that we're trying to come up with a solution to.


Cool - glad to see some thought is going into it.

On other projects where there are firewalls between linkage domains, I've had 
one subsystem on one side to be initialized with a pointer to the storage 
location(s) on the other, by way of an init() routine that is called by a third 
party passing it over the barrier.

Something like this, in terms of our problem here (pseudo-code and maybe wrong 
naming convention!):

// in lldb-mi::main(), main binary
int main() {

  // Ask lldb-mi's local llvm's CommandLine to return a handle to its internal 
data structures,
  // and pass that to the agent that controls the llvm embedded in liblldb.
  liblldbInitCommandLine(cl::CommandLine::GetHandle());
  ...

}

// in liblldb so/dylib/dll, exported method:

void liblldbInitCommandLine(void *commandLineHandle) {

  // Pass the given opaque handle to the LLVM CommandLine embedded in liblldb.
  cl::CommandLine::SetHandle(commandLineHandle);

}

// in CommandLine

void *CommandLine::GetHandle() {

  return static_cast(my_internal_pimpl);

}

void CommandLine::SetHandle(void *external_pimpl_to_use_here) {

  // Make concurrency safe if desired...
  // Merge existing results with those present in the passed in version, if 
desired (see below)
  delete my_internal_pimpl;
  my_internal_pimpl = (some_type*)(external_pimpl_to_use_here);

}

The trick to this working, though, is to not have uncontrolled, global static 
initializers permuting these on both sides until after the dance of 
synchronizing their storage explicitly.  And that isn't the case here.  We can 
probably work around it with 'debug' since we know both sides will do it, and 
so there is no race to avoid.  But if other clients on either side do the same 
thing, we can lose them.  Unless, we have some kind of merge semantics on the 
SetHandle(void*) call, which I suppose could be done to collate any items that 
were initialized on either side.

Anyways, food for thought...  I'm not thrilled with the manual aspect of it, 
but I have used it in production.

>> Unless there is strong opposition, I'd like to put this in.
> 
> No objection from me.

Okay - Pavel seemed alright enough with it for the time being as well, so I'll 
put this in, with the one wording correction I called out above.


https://reviews.llvm.org/D26093



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

[Lldb-commits] [lldb] r285484 - Limit LLDB_EXPORT_ALL_SYMBOLS to lldb symbols

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Fri Oct 28 19:29:15 2016
New Revision: 285484

URL: http://llvm.org/viewvc/llvm-project?rev=285484&view=rev
Log:
Limit LLDB_EXPORT_ALL_SYMBOLS to lldb symbols

LLDB_EXPORT_ALL_SYMBOLS used to instruct the build to export all
the symbols in liblldb on CMake builds.  This change limits the
CMake define to only add in the lldb_private namespace to the
symbols that normally get exported, such that we export all the
symbols in the public lldb namespace and the lldb_private namespace.

This is a fix for:
https://llvm.org/bugs/show_bug.cgi?id=30822

Reviewers: labath, beanz

Subscribers: lldb-commits

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

Added:
lldb/trunk/source/API/liblldb-private.exports
Modified:
lldb/trunk/source/API/CMakeLists.txt

Modified: lldb/trunk/source/API/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/CMakeLists.txt?rev=285484&r1=285483&r2=285484&view=diff
==
--- lldb/trunk/source/API/CMakeLists.txt (original)
+++ lldb/trunk/source/API/CMakeLists.txt Fri Oct 28 19:29:15 2016
@@ -103,16 +103,13 @@ if (NOT CMAKE_SYSTEM_NAME MATCHES "Windo
 # If we're not exporting all symbols, we'll want to explicitly set
 # the exported symbols here.  This prevents 'log enable --stack ...'
 # from working on some systems but limits the liblldb size.
-MESSAGE("-- Symbols (liblldb): only exporting liblldb.exports symbols")
+MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb 
namespace")
 add_llvm_symbol_exports(liblldb 
${CMAKE_CURRENT_SOURCE_DIR}/liblldb.exports)
   else()
 # Don't use an explicit export.  Instead, tell the linker to
 # export all symbols.
-MESSAGE("-- Symbols (liblldb): exporting all symbols")
-# Darwin linker doesn't need this extra step.
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  lldb_append_link_flags(liblldb "-Wl,--export-dynamic")
-endif()
+MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb and 
lldb_private namespaces")
+add_llvm_symbol_exports(liblldb 
${CMAKE_CURRENT_SOURCE_DIR}/liblldb-private.exports)
   endif()
 endif()
 

Added: lldb/trunk/source/API/liblldb-private.exports
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/liblldb-private.exports?rev=285484&view=auto
==
--- lldb/trunk/source/API/liblldb-private.exports (added)
+++ lldb/trunk/source/API/liblldb-private.exports Fri Oct 28 19:29:15 2016
@@ -0,0 +1,6 @@
+_ZN4lldb*
+_ZNK4lldb*
+_ZN12lldb_private*
+_ZNK12lldb_private*
+init_lld*
+PyInit__lldb*


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

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL285484: Limit LLDB_EXPORT_ALL_SYMBOLS to lldb symbols 
(authored by tfiala).

Changed prior to commit:
  https://reviews.llvm.org/D26093?vs=76229&id=76282#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26093

Files:
  lldb/trunk/source/API/CMakeLists.txt
  lldb/trunk/source/API/liblldb-private.exports


Index: lldb/trunk/source/API/CMakeLists.txt
===
--- lldb/trunk/source/API/CMakeLists.txt
+++ lldb/trunk/source/API/CMakeLists.txt
@@ -103,16 +103,13 @@
 # If we're not exporting all symbols, we'll want to explicitly set
 # the exported symbols here.  This prevents 'log enable --stack ...'
 # from working on some systems but limits the liblldb size.
-MESSAGE("-- Symbols (liblldb): only exporting liblldb.exports symbols")
+MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb 
namespace")
 add_llvm_symbol_exports(liblldb 
${CMAKE_CURRENT_SOURCE_DIR}/liblldb.exports)
   else()
 # Don't use an explicit export.  Instead, tell the linker to
 # export all symbols.
-MESSAGE("-- Symbols (liblldb): exporting all symbols")
-# Darwin linker doesn't need this extra step.
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  lldb_append_link_flags(liblldb "-Wl,--export-dynamic")
-endif()
+MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb and 
lldb_private namespaces")
+add_llvm_symbol_exports(liblldb 
${CMAKE_CURRENT_SOURCE_DIR}/liblldb-private.exports)
   endif()
 endif()
 
Index: lldb/trunk/source/API/liblldb-private.exports
===
--- lldb/trunk/source/API/liblldb-private.exports
+++ lldb/trunk/source/API/liblldb-private.exports
@@ -0,0 +1,6 @@
+_ZN4lldb*
+_ZNK4lldb*
+_ZN12lldb_private*
+_ZNK12lldb_private*
+init_lld*
+PyInit__lldb*


Index: lldb/trunk/source/API/CMakeLists.txt
===
--- lldb/trunk/source/API/CMakeLists.txt
+++ lldb/trunk/source/API/CMakeLists.txt
@@ -103,16 +103,13 @@
 # If we're not exporting all symbols, we'll want to explicitly set
 # the exported symbols here.  This prevents 'log enable --stack ...'
 # from working on some systems but limits the liblldb size.
-MESSAGE("-- Symbols (liblldb): only exporting liblldb.exports symbols")
+MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb namespace")
 add_llvm_symbol_exports(liblldb ${CMAKE_CURRENT_SOURCE_DIR}/liblldb.exports)
   else()
 # Don't use an explicit export.  Instead, tell the linker to
 # export all symbols.
-MESSAGE("-- Symbols (liblldb): exporting all symbols")
-# Darwin linker doesn't need this extra step.
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  lldb_append_link_flags(liblldb "-Wl,--export-dynamic")
-endif()
+MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb and lldb_private namespaces")
+add_llvm_symbol_exports(liblldb ${CMAKE_CURRENT_SOURCE_DIR}/liblldb-private.exports)
   endif()
 endif()
 
Index: lldb/trunk/source/API/liblldb-private.exports
===
--- lldb/trunk/source/API/liblldb-private.exports
+++ lldb/trunk/source/API/liblldb-private.exports
@@ -0,0 +1,6 @@
+_ZN4lldb*
+_ZNK4lldb*
+_ZN12lldb_private*
+_ZNK12lldb_private*
+init_lld*
+PyInit__lldb*
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

In https://reviews.llvm.org/D26093#582825, @labath wrote:

> In https://reviews.llvm.org/D26093#582618, @tfiala wrote:
>
> > > If it unblocks something it might be ok, but it doesn't actually fix the 
> > > problem.
> >
> > It allows debugging of lldb with lldb on Ubuntu.  This broke recently, and 
> > the CL here re-enables the ability to do that.
>
>
> Could you elaborate on this?


Sure, I'll see if I can reproduce the lack of backtrace symbol lookup that I 
was experiencing without exporting those symbols back in roughly Oct/Nov 2015.  
We've been using this flag ever since to address it.

> I've been debugging lldb without this switch and everything seems to work fine

That definitely was not my experience when I added the flag.  I am seeing if 
the old scenario holds true on 16.04 x86_64 with the current LLDB codebase.  It 
is possible that the issue we saw manifest has been addressed.)  I will make a 
best effort attempt this weekend, but it may be more like Monday when I will 
have dedicated time to repro it.  If it is no longer an issue, I'd consider 
this flag deprecated as its initial purpose is no longer valid.  If it is still 
an issue, it could be a config difference between stock Ubuntu and your setup 
(as we know is at least the case with kernel settings).

> (modulo problems evaluating fancy expressions, but I don't think that is 
> related to this. Or is it?).

At the time the issue was symbol lookup - we were somewhere short-circuiting so 
that we didn't consider debug symbols properly.  While I didn't try it at the 
time as I wasn't aware of it, it is possible the issue may be related to 
needing the -fno-limit-debug-info flag.  When I get to reproducing the original 
issue, if it does surface, I will try adding that flag as a second step to see 
if that addresses it.

For expressions that need to evaluate any symbols we were failing to resolve, I 
could imagine that being at least one factor.  Not sure though without a 
specific case to look at.

More later when I have some results.


Repository:
  rL LLVM

https://reviews.llvm.org/D26093



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

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

>> Could you elaborate on this?
> 
> Sure, I'll see if I can reproduce the lack of backtrace symbol lookup that I 
> was experiencing without exporting those symbols back in roughly Oct/Nov 
> 2015.  We've been using this flag ever since to address it.
> 
>> I've been debugging lldb without this switch and everything seems to work 
>> fine
> 
> That definitely was not my experience when I added the flag.  I am seeing if 
> the old scenario holds true on 16.04 x86_64 with the current LLDB codebase.  
> It is possible that the issue we saw manifest has been addressed.)  I will 
> make a best effort attempt this weekend, but it may be more like Monday when 
> I will have dedicated time to repro it.  If it is no longer an issue, I'd 
> consider this flag deprecated as its initial purpose is no longer valid.  If 
> it is still an issue, it could be a config difference between stock Ubuntu 
> and your setup (as we know is at least the case with kernel settings).
> 
>> (modulo problems evaluating fancy expressions, but I don't think that is 
>> related to this. Or is it?).
> 
> At the time the issue was symbol lookup - we were somewhere short-circuiting 
> so that we didn't consider debug symbols properly.  While I didn't try it at 
> the time as I wasn't aware of it, it is possible the issue may be related to 
> needing the -fno-limit-debug-info flag.  When I get to reproducing the 
> original issue, if it does surface, I will try adding that flag as a second 
> step to see if that addresses it.
> 
> For expressions that need to evaluate any symbols we were failing to resolve, 
> I could imagine that being at least one factor.  Not sure though without a 
> specific case to look at.
> 
> More later when I have some results.

Here is an example of where symbolic lookup of backtraces from within 
liblldb.so utterly fail without using this flag.  In the example below, I'm 
using:
(lldb) log enable -S lldb process
(lldb) target create /bin/ls
(lldb) run

And here is my output:

  Process::SetPublicState (exited) -- unlocking run lock
  0  liblldb.so.40   0x7fb6041f6cd0
  1  liblldb.so.40   0x7fb600834e22
  2  liblldb.so.40   0x7fb600834b3f
  3  liblldb.so.40   0x7fb600a5f601
  4  liblldb.so.40   0x7fb600a5f047
  5  liblldb.so.40   0x7fb6007b897e
  6  libpthread.so.0 0x7fb60841570a
  7  libc.so.6   0x7fb5ffa2882d clone + 109
  8  libc.so.6   00 clone + 6125632
  9  liblldb.so.40   0x7fb6007e7a68
  10 liblldb.so.40   0x7fb6007e7e96
  11 liblldb.so.40   0x7fb6007b897e
  12 libpthread.so.0 0x7fb60841570a
  13 libc.so.6   0x7fb5ffa2882d clone + 109
  Process 31919 exited with status = 0 (0x)
  Process::RunPrivateStateThread (arg = 0x1d05580, pid = 31919) thread 
exiting...
  0  liblldb.so.40   0x7fb6041f6cd0
  1  liblldb.so.40   0x7fb600834e22
  2  liblldb.so.40   0x7fb600834b3f
  3  liblldb.so.40   0x7fb600a5f601
  4  liblldb.so.40   0x7fb600a5f047
  5  liblldb.so.40   0x7fb6007b897e
  6  libpthread.so.0 0x7fb60841570a
  7  libc.so.6   0x7fb5ffa2882d clone + 109
  (lldb-superior) MonitorChildProcessThreadFunction (arg = 0x1d25390) thread 
exiting because pid received exit signal...
  0  liblldb.so.40   0x7fb6041f6cd0
  1  liblldb.so.40   0x7fb600834e22
  2  liblldb.so.40   0x7fb600834b3f
  3  liblldb.so.40   0x7fb60079fd23
  4  liblldb.so.40   0x7fb6007b897e
  5  libpthread.so.0 0x7fb60841570a
  6  libc.so.6   0x7fb5ffa2882d clone + 109
  ...

The exact cmake line was:

  cmake -GNinja -DCMAKE_BUILD_TYPE=Debug ../llvm

where the build tree is a sibling directory to the llvm source root directory.

I see very similar behavior when trying to set breakpoints and step into lldb's 
internals with this same lldb.  Those all change to expected behavior when I 
build and run with the -DLLDB_EXPORT_ALL_SYMBOLS:BOOL=YES setting.

Let me know if you get different behavior without with the same setup.  If so, 
we should try to find out what about your Ubuntu load out is different than a 
stock Ubuntu, since this clearly affects LLDB developers using Linux.


Repository:
  rL LLVM

https://reviews.llvm.org/D26093



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

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

Here's what you get with the same codebase, but using this cmake line instead:

  cmake -GNinja ../llvm -DCMAKE_BUILD_TYPE=Debug 
-DLLDB_EXPORT_ALL_SYMBOLS:BOOL=YES



  bin/lldb
  (lldb) log enable -S lldb process
  (lldb) target create /bin/ls
  Current executable set to '/bin/ls' (x86_64).
  (lldb) run
  ...
  Process::SetPublicState (state = launching, restarted = 0)
  0  liblldb.so.40 0x7f8f75b0c970
  1  liblldb.so.40 0x7f8f7214aac2 lldb_private::Log::VAPrintf(char const*, 
__va_list_tag*) + 712
  2  liblldb.so.40 0x7f8f7214a7df lldb_private::Log::Printf(char const*, 
...) + 195
  3  liblldb.so.40 0x7f8f7236c66e 
lldb_private::Process::SetPublicState(lldb::StateType, bool) + 100
  4  liblldb.so.40 0x7f8f72370ae5 
lldb_private::Process::Launch(lldb_private::ProcessLaunchInfo&) + 667
  5  liblldb.so.40 0x7f8f7262cbaf 
lldb_private::platform_linux::PlatformLinux::DebugProcess(lldb_private::ProcessLaunchInfo&,
 lldb_private::Debugger&, lldb_private::Target*, lldb_private::Error&) + 1839
  6  liblldb.so.40 0x7f8f723bd932 
lldb_private::Target::Launch(lldb_private::ProcessLaunchInfo&, 
lldb_private::Stream*) + 1288
  7  liblldb.so.40 0x7f8f7281ffd6
  8  liblldb.so.40 0x7f8f7221ffa0 
lldb_private::CommandObjectParsed::Execute(char const*, 
lldb_private::CommandReturnObject&) + 808
  9  liblldb.so.40 0x7f8f722102d2 
lldb_private::CommandInterpreter::HandleCommand(char const*, 
lldb_private::LazyBool, lldb_private::CommandReturnObject&, 
lldb_private::ExecutionContext*, bool, bool) + 2468
  10 liblldb.so.40 0x7f8f72214748 
lldb_private::CommandInterpreter::IOHandlerInputComplete(lldb_private::IOHandler&,
 std::__cxx11::basic_string, std::allocator 
>&) + 324
  11 liblldb.so.40 0x7f8f7212b71d lldb_private::IOHandlerEditline::Run() + 
545
  12 liblldb.so.40 0x7f8f720fb32e 
lldb_private::Debugger::ExecuteIOHandlers() + 110
  13 liblldb.so.40 0x7f8f72215488 
lldb_private::CommandInterpreter::RunCommandInterpreter(bool, bool, 
lldb_private::CommandInterpreterRunOptions&) + 176
  14 liblldb.so.40 0x7f8f71eb314b 
lldb::SBDebugger::RunCommandInterpreter(bool, bool) + 117
  15 lldb  0x00407045
  16 lldb  0x00407408
  17 libc.so.6 0x7f8f7111f830 __libc_start_main + 240
  18 lldb  0x004038e9
  MonitorChildProcessThreadFunction (arg = 0x12883e0) thread starting...
  0  liblldb.so.40   0x7f8f75b0c970
  1  liblldb.so.40   0x7f8f7214aac2 lldb_private::Log::VAPrintf(char 
const*, __va_list_tag*) + 712
  2  liblldb.so.40   0x7f8f7214a7df lldb_private::Log::Printf(char const*, 
...) + 195
  3  liblldb.so.40   0x7f8f720b553f
  4  liblldb.so.40   0x7f8f720ce61e 
lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) + 236
  5  libpthread.so.0 0x7f8f79d3770a
  6  libc.so.6   0x7f8f7120582d clone + 109
  MonitorChildProcessThreadFunction ::waitpid (pid = 46995, &status, options = 
1073741824)...
  0  liblldb.so.40   0x7f8f75b0c970
  1  liblldb.so.40   0x7f8f7214aac2 lldb_private::Log::VAPrintf(char 
const*, __va_list_tag*) + 712
  2  liblldb.so.40   0x7f8f7214a7df lldb_private::Log::Printf(char const*, 
...) + 195
  3  liblldb.so.40   0x7f8f720b56a4
  4  liblldb.so.40   0x7f8f720ce61e 
lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) + 236
  5  libpthread.so.0 0x7f8f79d3770a
  6  libc.so.6   0x7f8f7120582d clone + 109
  started monitoring child process.
  0  liblldb.so.40   0x7f8f75b0c970
  1  liblldb.so.40   0x7f8f7214aac2 lldb_private::Log::VAPrintf(char 
const*, __va_list_tag*) + 712
  2  liblldb.so.40   0x7f8f7214a7df lldb_private::Log::Printf(char const*, 
...) + 195
  3  liblldb.so.40   0x7f8f720b553f
  4  liblldb.so.40   0x7f8f720ce61e 
lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) + 236
  5  libpthread.so.0 0x7f8f79d3770a
  6  libc.so.6   0x7f8f7120582d clone + 109
  7  libc.so.6   00 clone + 2397022272
  8  liblldb.so.40   0x7f8f725afe6c 
lldb_private::process_gdb_remote::ProcessGDBRemote::EstablishConnectionIfNeeded(lldb_private::ProcessInfo
 const&) + 212
  9  liblldb.so.40   0x7f8f725a4364 
lldb_private::process_gdb_remote::ProcessGDBRemote::DoLaunch(lldb_private::Module*,
 lldb_private::ProcessLaunchInfo&) + 1072
  10 liblldb.so.40   0x7f8f72370b3d 
lldb_private::Process::Launch(lldb_private::ProcessLaunchInfo&) + 755
  11 liblldb.so.40   0x7f8f7262cbaf 
lldb_private::platform_linux::PlatformLinux::DebugProcess(lldb_private::ProcessLaunchInfo&,
 lldb_private::Debugger&, lldb_private::Target*, lldb_private::Error&) + 1839
  12 liblldb.so.40   0x7f8f723bd932 
lldb_private::Target::Launch(lldb_private::ProcessLaunchInfo&, 
lldb_private::Stream*) + 1288
  13 liblldb.so.40   0x7f8f7281ffd6
  14 liblldb.so.40   0x7f8f7221ffa0 
lldb_private::CommandObjectParsed::Execute(char const*, 
lldb_private::C

[Lldb-commits] [PATCH] D26093: Limit LLDB_EXPORT_ALL_SYMBOLS to additionally export only the lldb_private namespace symbols

[Todd Fiala via lldb-commits]

tfiala added a comment.

In https://reviews.llvm.org/D26093#583358, @labath wrote:

> The snippets you showed are pretty-much expected behavior. The backtrace that 
> gets printed as a part of the log messages is coming from the backtrace(3) 
> library, which has pretty limited backtracing capabilities -- it only looks 
> at the symbols in the `.dynsym` section (because that's the only thing that 
> is loaded into memory). I am pretty sure you would have problems backtracing 
> if you compiled with -fomit-frame-pointer as well.
>
> I think you fix of just making sure that all symbols show up in the .dynsym 
> section would be fine, were it not for that fact that we have an inconsistent 
> linking policy, which means you get wildly different links depending on 
> whether you export something or not. If we get that straight that this would 
> just be a size optimization (that was the reason we introduced it), with no 
> impact on the behavior, that we could even consider turning off by default (I 
> personally have never used the -S option of logging, but I can see how it 
> could be useful in some workflows). I'll get back to the link policy in a 
> separate email.
>
> That said, you mentioned you also had some problems with setting breakpoints 
> and stuff. Now, if this was true, that would be extremely worrying, but am 
> not able to reproduce that on my side -- breakpoints everything else works 
> fine. The reason for that is that lldb reads the .symtab section (if it is 
> present, which it will be unless you strip the executable), and these linker 
> options do not affect that section. If that is not the case, then we 
> definitely need to figure that out (I suggest stepping through 
> ObjectFileELF::GetSymtab() to get an initial idea of what is going on).
>
> FWIW, this is how a debug session looks like for me. The backtrace() output 
> has addresses only, but lldb is able to symbolicate it with no problem:
>
>   $ bin/lldb -- bin/lldb /bin/ls
>   (lldb) target create "bin/lldb"
>   Current executable set to 'bin/lldb' (x86_64).
>   (lldb) settings set -- target.run-args  "/bin/ls"
>   (lldb) br set -n Log::VAPrintf
>   Breakpoint 1: no locations (pending).
>   WARNING:  Unable to resolve breakpoint to any actual locations.
>   (lldb) pr la
>   Process 87438 launched: 
> '/usr/local/google/home/labath/ll/build/dbg/bin/lldb' (x86_64)
>   1 location added to breakpoint 1
>   Process 87438 stopped and restarted: thread 1 received signal: SIGCHLD
>   (lldb) target create "/bin/ls"
>   Current executable set to '/bin/ls' (x86_64).
>   (lldb) log enable -S lldb process
>   (lldb) pr la
>   Process 87438 stopped
>   * thread #1: tid = 87438, 0x70d1cfeb 
> liblldb.so.40`lldb_private::Log::VAPrintf(this=0x0059a560, 
> format="ProcessLaunchInfo::%s at least one of stdin/stdout/stderr was not 
> set, evaluating default handling", args=0x7fffc1a0) + 27 at 
> Log.cpp:73, name = 'lldb', stop reason = breakpoint 1.1
>   frame #0: 0x70d1cfeb 
> liblldb.so.40`lldb_private::Log::VAPrintf(this=0x0059a560, 
> format="ProcessLaunchInfo::%s at least one of stdin/stdout/stderr was not 
> set, evaluating default handling", args=0x7fffc1a0) + 27 at Log.cpp:73
>  70   void Log::VAPrintf(const char *format, va_list args) {
>  71 // Make a copy of our stream shared pointer in case someone 
> disables our
>  72 // log while we are logging and releases the stream
>   -> 73 StreamSP stream_sp(m_stream_sp);
>  74 if (stream_sp) {
>  75   static uint32_t g_sequence_id = 0;
>  76   StreamString header;
>   (lldb) c
>   Process 87438 resuming
>   ProcessLaunchInfo::FinalizeFileActions at least one of stdin/stdout/stderr 
> was not set, evaluating default handling
>   0  liblldb.so.40 0x74d5a36f
>   1  liblldb.so.40 0x70d1d49f
>   2  liblldb.so.40 0x70d1cfbb
>   3  liblldb.so.40 0x70fff4c4
>   4  liblldb.so.40 0x7103ad63
>   5  liblldb.so.40 0x715c4f9f
>   6  liblldb.so.40 0x70e33fe0
>   7  liblldb.so.40 0x70e20317
>   8  liblldb.so.40 0x70e25c0d
>   9  liblldb.so.40 0x70e265a7
>   10 liblldb.so.40 0x70cf5a60
>   11 liblldb.so.40 0x70cb5c3c
>   12 liblldb.so.40 0x70e26fdf
>   13 liblldb.so.40 0x709d8d21 
> lldb::SBDebugger::RunCommandInterpreter(bool, bool) + 129
>   14 lldb  0x00407c15
>   15 lldb  0x00408257
>   16 libc.so.6 0x7fffeeeb8f45 __libc_start_main + 245
>   17 lldb  0x004038d9
>   Process 87438 stopped
>   * thread #1: tid = 87438, 0x70d1cfeb 
> liblldb.so.40`lldb_private::Log::VAPrintf(this=0x0059a560, 
> format="ProcessLaunchInfo::%s target stdin='%s', target stdout='%s', 
> stderr='%s'", args=0x7fffc1a0) + 27 at Log.cpp:73, name = 'lldb', 
> stop reason = breakpoint 1.1
>   frame #0: 0x70d1cfeb 

[Lldb-commits] [PATCH] D26170: Find clang resource directory via *nix-style lookup

[Todd Fiala via lldb-commits]

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

Looks reasonable.




Comment at: source/Host/macosx/HostInfoMacOSX.mm:235-239
   size_t framework_pos = raw_path.find("LLDB.framework");
-  if (framework_pos != std::string::npos) {
-framework_pos += strlen("LLDB.framework");
-raw_path.resize(framework_pos);
-raw_path.append("/Resources/Clang");
-  }
+  if (framework_pos == std::string::npos)
+return HostInfoPosix::ComputeClangDirectory(file_spec);
+  
+  framework_pos += strlen("LLDB.framework");

We should have a const in place of the two uses of "LLDB.framework", but that 
can be a separate cleanup.


https://reviews.llvm.org/D26170



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

[Lldb-commits] [PATCH] D26188: [RFC] Solve linking inconsistency, proposal one

[Todd Fiala via lldb-commits]

tfiala added a comment.

I'll let Greg comment on the public ABI expansion (i.e. including llvm of a 
specific version, which may differ from LLDB.framework clients that contain 
different versions of LLVM).  My guess is this is not going to work for us, 
since we have long-lived frameworks shipped with Xcode that get used by clients 
where we cannot assume what they are or are not using.  However, we could 
address that by adding the reverse flag, which would be "don't export LLVM", 
and have that be exported by default.

The bit I care about is the "modulo backtrace(3)..." part.  This change is 
taking away my ability to use log-based stacktraces on Linux if I read it 
right.  How were you envisioning I do that with this change?


https://reviews.llvm.org/D26188



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

[Lldb-commits] [PATCH] D26188: [RFC] Solve linking inconsistency, proposal one

[Todd Fiala via lldb-commits]

tfiala added inline comments.



Comment at: source/API/CMakeLists.txt:109
   else()
-# Don't use an explicit export.  Instead, tell the linker to
-# export all symbols.

> This does not affect affect the state of backtracing. The option to export 
> all symbols could still stay there, and maybe could even be the default. 

But you eliminate it here, don't you?  (Maybe you just missed that I was also 
exporting the lldb_private symbols in the case of 
"LLDB_EXPORT_ALL_SYMBOLS:BOOL=YES"?)


https://reviews.llvm.org/D26188



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

[Lldb-commits] [PATCH] D26188: [RFC] Solve linking inconsistency, proposal one

[Todd Fiala via lldb-commits]

tfiala added a comment.

> @zturner wrote:
>  Can I have some background? What is the linking problem?

Hi Zachary,

This review has comments that pretty much cover it exhaustively:
https://reviews.llvm.org/D26093

That'd be a good place to start.


https://reviews.llvm.org/D26188



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

[Lldb-commits] [lldb] r285726 - change ProcessAttach test to no-debug-info

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Tue Nov  1 13:50:34 2016
New Revision: 285726

URL: http://llvm.org/viewvc/llvm-project?rev=285726&view=rev
Log:
change ProcessAttach test to no-debug-info

Fixes:
https://bugs.swift.org/browse/SR-3103

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py?rev=285726&r1=285725&r2=285726&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py
 Tue Nov  1 13:50:34 2016
@@ -19,6 +19,8 @@ class ProcessAttachTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+NO_DEBUG_INFO_TESTCASE = True
+
 @skipIfiOSSimulator
 def test_attach_to_process_by_id(self):
 """Test attach by process id"""


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

[Lldb-commits] [PATCH] D26470: Fix weak symbol linkage in SBStructuredData, update docs.

[Todd Fiala via lldb-commits]

tfiala created this revision.
tfiala added a reviewer: jingham.
tfiala added a subscriber: lldb-commits.

This change fixes an issue where I was leaking a weakly-linked symbol in the 
SBAPI.  It also updates the docs to call out what I did wrong.


https://reviews.llvm.org/D26470

Files:
  API/SBStructuredData.cpp
  SB-api-coding-rules.html
  lldb/API/SBStructuredData.h

Index: SB-api-coding-rules.html
===
--- SB-api-coding-rules.html
+++ SB-api-coding-rules.html
@@ -48,7 +48,10 @@
 make the SB object hold a shared or unique pointer to the Impl object.  The theory behind this is
 that if you need more state in the SB object, those needs are likely to change over time, 
 and this way the impl class can pick up members without changing the size of the object.
-An example of this is the SBValue class.
+An example of this is the SBValue class.  Please note it is necessary for the Impl class to
+not be a class embedded in the SB class, but rather should be a separate class that is not
+present in the public lldb namespace.  Failure to do so leads to leakage of weak-linked symbols
+in the SBAPI.
 
   In order to fit into the Python API's, we need to be able to default construct all the SB objects.
 Since the ivars of the classes are all pointers of one sort or other, this can easily be done, but
Index: API/SBStructuredData.cpp
===
--- API/SBStructuredData.cpp
+++ API/SBStructuredData.cpp
@@ -20,66 +20,62 @@
 using namespace lldb_private;
 
 #pragma mark--
-#pragma mark Impl
+#pragma mark StructuredDataImpl
 
-class SBStructuredData::Impl {
+class StructuredDataImpl {
 public:
-  Impl() : m_plugin_wp(), m_data_sp() {}
+  StructuredDataImpl() : m_plugin_wp(), m_data_sp() {}
 
-  Impl(const Impl &rhs) = default;
+  StructuredDataImpl(const StructuredDataImpl &rhs) = default;
 
-  Impl(const EventSP &event_sp)
+  StructuredDataImpl(const EventSP &event_sp)
   : m_plugin_wp(
 EventDataStructuredData::GetPluginFromEvent(event_sp.get())),
 m_data_sp(EventDataStructuredData::GetObjectFromEvent(event_sp.get())) {
   }
 
-  ~Impl() = default;
+  ~StructuredDataImpl() = default;
 
-  Impl &operator=(const Impl &rhs) = default;
+  StructuredDataImpl &operator=(const StructuredDataImpl &rhs) = default;
 
   bool IsValid() const { return m_data_sp.get() != nullptr; }
 
   void Clear() {
 m_plugin_wp.reset();
 m_data_sp.reset();
   }
 
-  SBError GetAsJSON(lldb::SBStream &stream) const {
+  SBError GetAsJSON(lldb_private::Stream &stream) const {
 SBError sb_error;
 
 if (!m_data_sp) {
   sb_error.SetErrorString("No structured data.");
   return sb_error;
 }
 
-m_data_sp->Dump(stream.ref());
+m_data_sp->Dump(stream);
 return sb_error;
   }
 
-  lldb::SBError GetDescription(lldb::SBStream &stream) const {
-SBError sb_error;
+  Error GetDescription(lldb_private::Stream &stream) const {
+Error error;
 
 if (!m_data_sp) {
-  sb_error.SetErrorString("Cannot pretty print structured data: "
-  "no data to print.");
-  return sb_error;
+  error.SetErrorString("Cannot pretty print structured data: "
+   "no data to print.");
+  return error;
 }
 
 // Grab the plugin.
 auto plugin_sp = StructuredDataPluginSP(m_plugin_wp);
 if (!plugin_sp) {
-  sb_error.SetErrorString("Cannot pretty print structured data: "
-  "plugin doesn't exist.");
-  return sb_error;
+  error.SetErrorString("Cannot pretty print structured data: "
+   "plugin doesn't exist.");
+  return error;
 }
 
 // Get the data's description.
-auto error = plugin_sp->GetDescription(m_data_sp, stream.ref());
-if (!error.Success())
-  sb_error.SetError(error);
-
-return sb_error;
+return plugin_sp->GetDescription(m_data_sp, stream);
   }
 
 private:
@@ -90,13 +86,13 @@
 #pragma mark--
 #pragma mark SBStructuredData
 
-SBStructuredData::SBStructuredData() : m_impl_up(new Impl()) {}
+SBStructuredData::SBStructuredData() : m_impl_up(new StructuredDataImpl()) {}
 
 SBStructuredData::SBStructuredData(const lldb::SBStructuredData &rhs)
-: m_impl_up(new Impl(*rhs.m_impl_up.get())) {}
+: m_impl_up(new StructuredDataImpl(*rhs.m_impl_up.get())) {}
 
 SBStructuredData::SBStructuredData(const lldb::EventSP &event_sp)
-: m_impl_up(new Impl(event_sp)) {}
+: m_impl_up(new Struc

[Lldb-commits] [PATCH] D26470: Fix weak symbol linkage in SBStructuredData, update docs.

[Todd Fiala via lldb-commits]

tfiala added a comment.

Making changes to the text...




Comment at: SB-api-coding-rules.html:51-54
+An example of this is the SBValue 
class.  Please note it is necessary for the Impl class to
+not be a class embedded in the SB 
class, but rather should be a separate class that is not
+present in the public lldb namespace.  
Failure to do so leads to leakage of weak-linked symbols
+in the SBAPI.

jingham wrote:
> I think it's more straightforward to say:
> 
> Please note that you should not put this Impl class in the lldb namespace.  
> Failure to do so...
> 
> After all, it would be equally bad to have put it inside the "namespace lldb" 
> but not in the class.
> 
Okay, I can use your verbiage.   The end of one sentence did explicitly call 
out having it not be in the public lldb namespace:

"...but rather should be a separate class that is not present in the public 
lldb namespace."

but I think your prose is clearer, though.  Thank you!


https://reviews.llvm.org/D26470



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

[Lldb-commits] [PATCH] D26470: Fix weak symbol linkage in SBStructuredData, update docs.

[Todd Fiala via lldb-commits]

tfiala updated this revision to Diff 77405.
tfiala added a comment.

Adjusted web docs.


https://reviews.llvm.org/D26470

Files:
  API/SBStructuredData.cpp
  SB-api-coding-rules.html
  lldb/API/SBStructuredData.h

Index: SB-api-coding-rules.html
===
--- SB-api-coding-rules.html
+++ SB-api-coding-rules.html
@@ -36,7 +36,7 @@
 
   You also need to choose the ivars for the class with care, since you can't add or remove ivars
 without breaking binary compatibility.  In some cases, the SB class is a thin wrapper around
-an interal lldb_private object.  In that case, the class can have a single ivar, which is 
+an internal lldb_private object.  In that case, the class can have a single ivar, which is
 either a pointer, shared_ptr or unique_ptr to the object in the lldb_private API.  All the
 lldb_private classes that get used this way are declared as opaque classes in lldb_forward.h,
 which is included in SBDefines.h.  So if you need an SB class to wrap an lldb_private class
@@ -47,8 +47,9 @@
 as a direct ivar in the SB Class.  Instead, make an Impl class in the SB's .cpp file, and then
 make the SB object hold a shared or unique pointer to the Impl object.  The theory behind this is
 that if you need more state in the SB object, those needs are likely to change over time, 
-and this way the impl class can pick up members without changing the size of the object.
-An example of this is the SBValue class.
+and this way the Impl class can pick up members without changing the size of the object.
+An example of this is the SBValue class.  Please note that you should not put this Impl class
+in the lldb namespace.  Failure to do so leads to leakage of weak-linked symbols in the SBAPI.
 
   In order to fit into the Python API's, we need to be able to default construct all the SB objects.
 Since the ivars of the classes are all pointers of one sort or other, this can easily be done, but
Index: API/SBStructuredData.cpp
===
--- API/SBStructuredData.cpp
+++ API/SBStructuredData.cpp
@@ -20,66 +20,62 @@
 using namespace lldb_private;
 
 #pragma mark--
-#pragma mark Impl
+#pragma mark StructuredDataImpl
 
-class SBStructuredData::Impl {
+class StructuredDataImpl {
 public:
-  Impl() : m_plugin_wp(), m_data_sp() {}
+  StructuredDataImpl() : m_plugin_wp(), m_data_sp() {}
 
-  Impl(const Impl &rhs) = default;
+  StructuredDataImpl(const StructuredDataImpl &rhs) = default;
 
-  Impl(const EventSP &event_sp)
+  StructuredDataImpl(const EventSP &event_sp)
   : m_plugin_wp(
 EventDataStructuredData::GetPluginFromEvent(event_sp.get())),
 m_data_sp(EventDataStructuredData::GetObjectFromEvent(event_sp.get())) {
   }
 
-  ~Impl() = default;
+  ~StructuredDataImpl() = default;
 
-  Impl &operator=(const Impl &rhs) = default;
+  StructuredDataImpl &operator=(const StructuredDataImpl &rhs) = default;
 
   bool IsValid() const { return m_data_sp.get() != nullptr; }
 
   void Clear() {
 m_plugin_wp.reset();
 m_data_sp.reset();
   }
 
-  SBError GetAsJSON(lldb::SBStream &stream) const {
+  SBError GetAsJSON(lldb_private::Stream &stream) const {
 SBError sb_error;
 
 if (!m_data_sp) {
   sb_error.SetErrorString("No structured data.");
   return sb_error;
 }
 
-m_data_sp->Dump(stream.ref());
+m_data_sp->Dump(stream);
 return sb_error;
   }
 
-  lldb::SBError GetDescription(lldb::SBStream &stream) const {
-SBError sb_error;
+  Error GetDescription(lldb_private::Stream &stream) const {
+Error error;
 
 if (!m_data_sp) {
-  sb_error.SetErrorString("Cannot pretty print structured data: "
-  "no data to print.");
-  return sb_error;
+  error.SetErrorString("Cannot pretty print structured data: "
+   "no data to print.");
+  return error;
 }
 
 // Grab the plugin.
 auto plugin_sp = StructuredDataPluginSP(m_plugin_wp);
 if (!plugin_sp) {
-  sb_error.SetErrorString("Cannot pretty print structured data: "
-  "plugin doesn't exist.");
-  return sb_error;
+  error.SetErrorString("Cannot pretty print structured data: "
+   

[Lldb-commits] [PATCH] D26478: Unify Darwin and Non-Darwin printing of version output

[Todd Fiala via lldb-commits]

tfiala accepted this revision.
tfiala added a reviewer: tfiala.
tfiala added a comment.
This revision is now accepted and ready to land.

Looks good, @beanz!


https://reviews.llvm.org/D26478



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

[Lldb-commits] [PATCH] D26470: Fix weak symbol linkage in SBStructuredData, update docs.

[Todd Fiala via lldb-commits]

tfiala added a comment.

Thanks, Jim!


https://reviews.llvm.org/D26470



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

[Lldb-commits] [lldb] r286413 - Fix weak symbol linkage in SBStructuredData, update docs.

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Wed Nov  9 17:21:04 2016
New Revision: 286413

URL: http://llvm.org/viewvc/llvm-project?rev=286413&view=rev
Log:
Fix weak symbol linkage in SBStructuredData, update docs.

Summary:
This change fixes an issue where I was leaking a weakly-linked symbol in
the SBAPI. It also updates the docs to call out what I did wrong.

Fixes:
rdar://28882483

Reviewers: jingham

Subscribers: lldb-commits

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

Modified:
lldb/trunk/include/lldb/API/SBStructuredData.h
lldb/trunk/source/API/SBStructuredData.cpp
lldb/trunk/www/SB-api-coding-rules.html

Modified: lldb/trunk/include/lldb/API/SBStructuredData.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBStructuredData.h?rev=286413&r1=286412&r2=286413&view=diff
==
--- lldb/trunk/include/lldb/API/SBStructuredData.h (original)
+++ lldb/trunk/include/lldb/API/SBStructuredData.h Wed Nov  9 17:21:04 2016
@@ -13,6 +13,8 @@
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBModule.h"
 
+class StructuredDataImpl;
+
 namespace lldb {
 
 class SBStructuredData {
@@ -36,8 +38,7 @@ public:
   lldb::SBError GetDescription(lldb::SBStream &stream) const;
 
 private:
-  class Impl;
-  std::unique_ptr m_impl_up;
+  std::unique_ptr m_impl_up;
 };
 }
 

Modified: lldb/trunk/source/API/SBStructuredData.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBStructuredData.cpp?rev=286413&r1=286412&r2=286413&view=diff
==
--- lldb/trunk/source/API/SBStructuredData.cpp (original)
+++ lldb/trunk/source/API/SBStructuredData.cpp Wed Nov  9 17:21:04 2016
@@ -20,23 +20,23 @@ using namespace lldb;
 using namespace lldb_private;
 
 #pragma mark--
-#pragma mark Impl
+#pragma mark StructuredDataImpl
 
-class SBStructuredData::Impl {
+class StructuredDataImpl {
 public:
-  Impl() : m_plugin_wp(), m_data_sp() {}
+  StructuredDataImpl() : m_plugin_wp(), m_data_sp() {}
 
-  Impl(const Impl &rhs) = default;
+  StructuredDataImpl(const StructuredDataImpl &rhs) = default;
 
-  Impl(const EventSP &event_sp)
+  StructuredDataImpl(const EventSP &event_sp)
   : m_plugin_wp(
 EventDataStructuredData::GetPluginFromEvent(event_sp.get())),
 m_data_sp(EventDataStructuredData::GetObjectFromEvent(event_sp.get())) 
{
   }
 
-  ~Impl() = default;
+  ~StructuredDataImpl() = default;
 
-  Impl &operator=(const Impl &rhs) = default;
+  StructuredDataImpl &operator=(const StructuredDataImpl &rhs) = default;
 
   bool IsValid() const { return m_data_sp.get() != nullptr; }
 
@@ -45,7 +45,7 @@ public:
 m_data_sp.reset();
   }
 
-  SBError GetAsJSON(lldb::SBStream &stream) const {
+  SBError GetAsJSON(lldb_private::Stream &stream) const {
 SBError sb_error;
 
 if (!m_data_sp) {
@@ -53,33 +53,29 @@ public:
   return sb_error;
 }
 
-m_data_sp->Dump(stream.ref());
+m_data_sp->Dump(stream);
 return sb_error;
   }
 
-  lldb::SBError GetDescription(lldb::SBStream &stream) const {
-SBError sb_error;
+  Error GetDescription(lldb_private::Stream &stream) const {
+Error error;
 
 if (!m_data_sp) {
-  sb_error.SetErrorString("Cannot pretty print structured data: "
-  "no data to print.");
-  return sb_error;
+  error.SetErrorString("Cannot pretty print structured data: "
+   "no data to print.");
+  return error;
 }
 
 // Grab the plugin.
 auto plugin_sp = StructuredDataPluginSP(m_plugin_wp);
 if (!plugin_sp) {
-  sb_error.SetErrorString("Cannot pretty print structured data: "
-  "plugin doesn't exist.");
-  return sb_error;
+  error.SetErrorString("Cannot pretty print structured data: "
+   "plugin doesn't exist.");
+  return error;
 }
 
 // Get the data's description.
-auto error = plugin_sp->GetDescription(m_data_sp, stream.ref());
-if (!error.Success())
-  sb_error.SetError(error);
-
-return sb_error;
+return plugin_sp->GetDescription(m_data_sp, stream);
   }
 
 private:
@@ -90,13 +86,13 @@ private:
 #pragma mark--
 #pragma mark SBStructuredData
 
-SBStructuredData::SBStructuredData() : m_impl_up(new Impl()) {}
+SBStructuredData::SBStructuredData() : m_impl_up(new StructuredDataImpl()) {}
 
 SBStructuredData::SBStructuredData(const lldb::SBStructuredData &rhs)
-: m_impl_up(new Impl(*rhs.m_impl_up.get())) {}
+: m_impl_up(new StructuredDataImpl(*rhs.m_impl_up.get())) {}
 
 SBStructuredData::SBStructuredData(const lldb::EventSP &event_sp)
-: m_impl_up(new Impl(event_sp)) {}
+: m_impl_up(new StructuredDataImpl(event_sp)) {}
 
 SBStructuredData::~SBStructuredData() {}
 
@@ -111,9 +107,12 @@ bool SBStructuredData::IsValid() const {
 void SBStructuredData::Clear() { m_impl_up->Clear(); }
 
 SBError SBStructuredData::G

[Lldb-commits] [PATCH] D26470: Fix weak symbol linkage in SBStructuredData, update docs.

[Todd Fiala via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL286413: Fix weak symbol linkage in SBStructuredData, update 
docs. (authored by tfiala).

Changed prior to commit:
  https://reviews.llvm.org/D26470?vs=77405&id=77411#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26470

Files:
  lldb/trunk/include/lldb/API/SBStructuredData.h
  lldb/trunk/source/API/SBStructuredData.cpp
  lldb/trunk/www/SB-api-coding-rules.html

Index: lldb/trunk/source/API/SBStructuredData.cpp
===
--- lldb/trunk/source/API/SBStructuredData.cpp
+++ lldb/trunk/source/API/SBStructuredData.cpp
@@ -20,66 +20,62 @@
 using namespace lldb_private;
 
 #pragma mark--
-#pragma mark Impl
+#pragma mark StructuredDataImpl
 
-class SBStructuredData::Impl {
+class StructuredDataImpl {
 public:
-  Impl() : m_plugin_wp(), m_data_sp() {}
+  StructuredDataImpl() : m_plugin_wp(), m_data_sp() {}
 
-  Impl(const Impl &rhs) = default;
+  StructuredDataImpl(const StructuredDataImpl &rhs) = default;
 
-  Impl(const EventSP &event_sp)
+  StructuredDataImpl(const EventSP &event_sp)
   : m_plugin_wp(
 EventDataStructuredData::GetPluginFromEvent(event_sp.get())),
 m_data_sp(EventDataStructuredData::GetObjectFromEvent(event_sp.get())) {
   }
 
-  ~Impl() = default;
+  ~StructuredDataImpl() = default;
 
-  Impl &operator=(const Impl &rhs) = default;
+  StructuredDataImpl &operator=(const StructuredDataImpl &rhs) = default;
 
   bool IsValid() const { return m_data_sp.get() != nullptr; }
 
   void Clear() {
 m_plugin_wp.reset();
 m_data_sp.reset();
   }
 
-  SBError GetAsJSON(lldb::SBStream &stream) const {
+  SBError GetAsJSON(lldb_private::Stream &stream) const {
 SBError sb_error;
 
 if (!m_data_sp) {
   sb_error.SetErrorString("No structured data.");
   return sb_error;
 }
 
-m_data_sp->Dump(stream.ref());
+m_data_sp->Dump(stream);
 return sb_error;
   }
 
-  lldb::SBError GetDescription(lldb::SBStream &stream) const {
-SBError sb_error;
+  Error GetDescription(lldb_private::Stream &stream) const {
+Error error;
 
 if (!m_data_sp) {
-  sb_error.SetErrorString("Cannot pretty print structured data: "
-  "no data to print.");
-  return sb_error;
+  error.SetErrorString("Cannot pretty print structured data: "
+   "no data to print.");
+  return error;
 }
 
 // Grab the plugin.
 auto plugin_sp = StructuredDataPluginSP(m_plugin_wp);
 if (!plugin_sp) {
-  sb_error.SetErrorString("Cannot pretty print structured data: "
-  "plugin doesn't exist.");
-  return sb_error;
+  error.SetErrorString("Cannot pretty print structured data: "
+   "plugin doesn't exist.");
+  return error;
 }
 
 // Get the data's description.
-auto error = plugin_sp->GetDescription(m_data_sp, stream.ref());
-if (!error.Success())
-  sb_error.SetError(error);
-
-return sb_error;
+return plugin_sp->GetDescription(m_data_sp, stream);
   }
 
 private:
@@ -90,13 +86,13 @@
 #pragma mark--
 #pragma mark SBStructuredData
 
-SBStructuredData::SBStructuredData() : m_impl_up(new Impl()) {}
+SBStructuredData::SBStructuredData() : m_impl_up(new StructuredDataImpl()) {}
 
 SBStructuredData::SBStructuredData(const lldb::SBStructuredData &rhs)
-: m_impl_up(new Impl(*rhs.m_impl_up.get())) {}
+: m_impl_up(new StructuredDataImpl(*rhs.m_impl_up.get())) {}
 
 SBStructuredData::SBStructuredData(const lldb::EventSP &event_sp)
-: m_impl_up(new Impl(event_sp)) {}
+: m_impl_up(new StructuredDataImpl(event_sp)) {}
 
 SBStructuredData::~SBStructuredData() {}
 
@@ -111,9 +107,12 @@
 void SBStructuredData::Clear() { m_impl_up->Clear(); }
 
 SBError SBStructuredData::GetAsJSON(lldb::SBStream &stream) const {
-  return m_impl_up->GetAsJSON(stream);
+  return m_impl_up->GetAsJSON(stream.ref());
 }
 
 lldb::SBError SBStructuredData::GetDescription(lldb::SBStream &stream) const {
-  return m_impl_up->GetDescription(stream);
+  Error error = m_impl_up->GetDescription(stream.ref());
+  SBError sb_error;
+  sb_error.SetError(error);
+  return sb_error;
 }
Index: lldb/trunk/www/SB-api-coding-rules.html
===
--- lldb/trunk/www/SB-api-coding-rules.html
+++ lldb/trunk/www/SB-api-coding-rules.html
@@ -36,7 +36,7 @@
 
   You also need to choose the ivars for the class with care, since you can't add or remove ivars
 without breaking binary compatibility.  In some cases, the SB class is a thin wrapper around
-an interal lldb_private object.  In that case, the class can have a single ivar, which is 
+an internal lldb_private object.  In that case, the class c

[Lldb-commits] [PATCH] D26510: Xcode build: specify NDEBUG for all binaries built using the BuildAndIntegration configuration

[Todd Fiala via lldb-commits]

tfiala created this revision.
tfiala added a reviewer: clayborg.
tfiala added a subscriber: lldb-commits.

BuildAndIntegration is the configuration used to build Apple-built LLDB.  This 
configuration currently requires NDEBUG to be specified.  This change adds a 
new Xcode variable, LLDB_NDEBUG_CFLAGS, which specifies "-DNDEBUG" when using 
the BuildAndIntegration configuration, and specifies nothing for all other 
configurations.  All binaries have been modified to include 
$(LLDB_NDEBUG_CFLAGS) in the full set of other cflags and other cxxflags.


https://reviews.llvm.org/D26510

Files:
  project.pbxproj


Index: project.pbxproj
===
--- project.pbxproj
+++ project.pbxproj
@@ -7957,6 +7957,7 @@
"LLDB_DISABLE_PYTHON[sdk=iphoneos*]" = 1;
LLDB_ENABLE_COVERAGE = 0;
LLDB_FRAMEWORK_INSTALL_DIR = 
/Applications/Xcode.app/Contents/SharedFrameworks;
+   LLDB_NDEBUG_CFLAGS = "";
LLDB_TOOLS_INSTALL_DIR = /usr/bin;
LLDB_ZLIB_CFLAGS = "-DHAVE_LIBZ=1";
LLDB_ZLIB_LDFLAGS = "-lz";
@@ -7973,6 +7974,7 @@
"$(LLDB_COMPRESSION_CFLAGS)",
"$(LLDB_COVERAGE_CFLAGS)",
"-Wimplicit-fallthrough",
+   "$(LLDB_NDEBUG_CFLAGS)",
);
OTHER_LDFLAGS = (
"$(LLDB_COMPRESSION_LDFLAGS)",
@@ -8050,6 +8052,7 @@
"LLDB_DISABLE_PYTHON[sdk=iphoneos*]" = 1;
LLDB_ENABLE_COVERAGE = 0;
LLDB_FRAMEWORK_INSTALL_DIR = 
/Applications/Xcode.app/Contents/SharedFrameworks;
+   LLDB_NDEBUG_CFLAGS = "";
LLDB_TOOLS_INSTALL_DIR = /usr/bin;
LLDB_ZLIB_CFLAGS = "-DHAVE_LIBZ=1";
LLDB_ZLIB_LDFLAGS = "-lz";
@@ -8066,6 +8069,7 @@
"$(LLDB_COMPRESSION_CFLAGS)",
"$(LLDB_COVERAGE_CFLAGS)",
"-Wimplicit-fallthrough",
+   "$(LLDB_NDEBUG_CFLAGS)",
);
OTHER_LDFLAGS = (
"$(LLDB_COMPRESSION_LDFLAGS)",
@@ -8375,6 +8379,7 @@
"$(LLDB_COMPRESSION_CFLAGS)",
"$(LLDB_GTESTS_CFLAGS)",
"-DGTEST_HAS_RTTI=0",
+   "$(LLDB_NDEBUG_CFLAGS)",
);
OTHER_LDFLAGS = (
"$(inherited)",
@@ -8417,6 +8422,7 @@
"$(LLDB_COMPRESSION_CFLAGS)",
"$(LLDB_GTESTS_CFLAGS)",
"-DGTEST_HAS_RTTI=0",
+   "$(LLDB_NDEBUG_CFLAGS)",
);
OTHER_LDFLAGS = (
"$(inherited)",
@@ -8459,6 +8465,7 @@
"$(LLDB_COMPRESSION_CFLAGS)",
"$(LLDB_GTESTS_CFLAGS)",
"-DGTEST_HAS_RTTI=0",
+   "$(LLDB_NDEBUG_CFLAGS)",
);
OTHER_LDFLAGS = (
"$(inherited)",
@@ -8501,6 +8508,7 @@
"$(LLDB_COMPRESSION_CFLAGS)",
"$(LLDB_GTESTS_CFLAGS)",
"-DGTEST_HAS_RTTI=0",
+   "$(LLDB_NDEBUG_CFLAGS)",
);
OTHER_LDFLAGS = (
"$(inherited)",
@@ -8963,6 +8971,7 @@
LLDB_ENABLE_COVERAGE = 0;
LLDB_FRAMEWORK_INSTALL_DIR = 
/Applications/Xcode.app/Contents/SharedFrameworks;
"LLDB_FRAMEWORK_INSTALL_DIR[sdk=iphoneos*]" = 
/System/Library/PrivateFrameworks;
+   LLDB_NDEBUG_CFLAGS = "-DNDEBUG";
LLDB_TOOLS_INSTALL_DIR = 
/Applications/Xcode.app/Contents/Developer/usr/bin;
"LLDB_TOOLS_INSTALL_DIR[sdk=iphoneos*]" = 
/usr/local/bin;
LLDB_ZLIB_CFLAGS =

[Lldb-commits] [PATCH] D26510: Xcode build: specify NDEBUG for all binaries built using the BuildAndIntegration configuration

[Todd Fiala via lldb-commits]

tfiala added a comment.

Adding Jim, as I just want one of him or Greg to review.


https://reviews.llvm.org/D26510



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

[Lldb-commits] [PATCH] D26513: [Test-Suite] Fix all the sanitizer tests to be based on compiler capabilities

[Todd Fiala via lldb-commits]

tfiala added a comment.

Looks fine to me pending that one question on the TSAN check (i.e. if the 
existing TSAN check is as good as your new ASAN check, LGTM).




Comment at: 
packages/Python/lldbsuite/test/functionalities/tsan/basic/TestTsanBasic.py:23
 @skipIfRemote
-@skipUnlessCompilerRt
 @skipUnlessThreadSanitizer
 def test(self):

Seems fine as long as @skipUnlessThreadSanitizer is doing the right type of 
check like you added for @skipUnlessAddressSanitizer.


https://reviews.llvm.org/D26513



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

Re: [Lldb-commits] [lldb] r286479 - Unify Darwin and Non-Darwin printing of version output

[Todd Fiala via lldb-commits]

I'm at a point where I can look at it.

On Thu, Nov 10, 2016 at 12:17 PM, Tim Hammerquist  wrote:

> Looks like the quotes around the lldb version string aren't properly
> preserved in the xcodeproj file as they are in CMake.
>
> Can any of the LLDB devs more comfortable with plumbing the depths of
> Xcode project configuration provide some guidance here?
>
> http://lab.llvm.org:8080/green/job/lldb_build_test/21828/
>
> /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/source/lldb.cpp:63:22:
> error: unexpected namespace name 'lldb': expected expression
> g_version_str += LLDB_VERSION_STRING;
>  ^
> In file included from :356:
> :4:29: note: expanded from here
> #define LLDB_VERSION_STRING lldb-360.99.0
> ^
> /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/source/lldb.cpp:63:22:
> error: invalid suffix '.0' on floating constant
> In file included from :356:
> :4:40: note: expanded from here
> #define LLDB_VERSION_STRING lldb-360.99.0
>^
> 2 errors generated.
>
>
> On Thu, Nov 10, 2016 at 9:33 AM, Chris Bieneman via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> Author: cbieneman
>> Date: Thu Nov 10 11:33:19 2016
>> New Revision: 286479
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=286479&view=rev
>> Log:
>> Unify Darwin and Non-Darwin printing of version output
>>
>> Summary:
>> This change unifies and simplifies the code paths between the Darwin and
>> non-Darwin code to print the LLDB version information.
>>
>> It also introduces a new variable in CMake LLDB_VERSION_STRING which can
>> be used to specify custom version information. On Darwin this value is
>> implicitly set based on the resource/LLDB-Info.plist file.
>>
>> With the LLDB_VERSION_STRING variable set to lldb-360.99.0, the -version
>> output is:
>>
>> > ./bin/lldb -version
>> lldb version 4.0.0 (lldb-360.99.0)
>>   clang revision 286264
>>   llvm revision 286265
>>
>> This behavior is unified across all target platforms.
>>
>> Reviewers: lldb-commits
>>
>> Subscribers: mgorny, tfiala
>>
>> Differential Revision: https://reviews.llvm.org/D26478
>>
>> Added:
>> lldb/trunk/cmake/modules/EmbedAppleVersion.cmake
>> Modified:
>> lldb/trunk/lldb.xcodeproj/project.pbxproj
>> lldb/trunk/source/CMakeLists.txt
>> lldb/trunk/source/lldb.cpp
>>
>> Added: lldb/trunk/cmake/modules/EmbedAppleVersion.cmake
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules
>> /EmbedAppleVersion.cmake?rev=286479&view=auto
>> 
>> ==
>> --- lldb/trunk/cmake/modules/EmbedAppleVersion.cmake (added)
>> +++ lldb/trunk/cmake/modules/EmbedAppleVersion.cmake Thu Nov 10 11:33:19
>> 2016
>> @@ -0,0 +1,11 @@
>> +execute_process(COMMAND /usr/libexec/PlistBuddy -c
>> "Print:CFBundleVersion" ${LLDB_INFO_PLIST}
>> +OUTPUT_VARIABLE BundleVersion
>> +OUTPUT_STRIP_TRAILING_WHITESPACE)
>> +
>> +file(APPEND "${HEADER_FILE}.tmp"
>> +"#define LLDB_VERSION_STRING \"lldb-${BundleVersion}\"\n")
>> +
>> +execute_process(COMMAND ${CMAKE_COMMAND} -E copy_if_different
>> +  "${HEADER_FILE}.tmp" "${HEADER_FILE}")
>> +
>> +file(REMOVE "${HEADER_FILE}.tmp")
>>
>> Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodepro
>> j/project.pbxproj?rev=286479&r1=286478&r2=286479&view=diff
>> 
>> ==
>> --- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
>> +++ lldb/trunk/lldb.xcodeproj/project.pbxproj Thu Nov 10 11:33:19 2016
>> @@ -8775,6 +8775,20 @@
>> "\"$(SYSTEM_LIBRARY_DIR)/Priva
>> teFrameworks\"",
>> );
>> GCC_INLINES_ARE_PRIVATE_EXTERN = NO;
>> +   GCC_PREPROCESSOR_DEFINITIONS = (
>> +   __STDC_CONSTANT_MACROS,
>> +   __STDC_LIMIT_MACROS,
>> +   LLDB_CONFIGURATION_DEBUG,
>> +   "LLDB_VERSION_STRING=\"lldb-$
>> {CURRENT_PROJECT_VERSION}\"",
>> +   );
>> +   "GCC_PREPROCESSOR_
>> DEFINITIONS[sdk=iphoneos*][arch=*]" = (
>> +   __STDC_CONSTANT_MACROS,
>> +   __STDC_LIMIT_MACROS,
>> +   LLDB_CONFIGURATION_DEBUG,
>> +   LLDB_DISABLE_PYTHON,
>> +   NO_XPC_SERVICES,
>> +   "LLDB_VERSION_STRING=\"lldb-$
>> {CURRENT_PROJECT_VERSION}\"",
>> +   );
>> HEADER_SEARCH_PATHS =
>> /usr/include/libxml2;
>> L

Re: [Lldb-commits] [lldb] r286479 - Unify Darwin and Non-Darwin printing of version output

[Todd Fiala via lldb-commits]

Okay cool, thanks Chris!

-Todd

> On Nov 10, 2016, at 1:40 PM, Chris Bieneman  wrote:
> 
> I just pushed r286504, which expects the string to not be quoted. This should 
> solve the bot failures.
> 
> -Chris
> 
>> On Nov 10, 2016, at 1:30 PM, Todd Fiala > > wrote:
>> 
>> I'm at a point where I can look at it.
>> 
>> On Thu, Nov 10, 2016 at 12:17 PM, Tim Hammerquist > > wrote:
>> Looks like the quotes around the lldb version string aren't properly 
>> preserved in the xcodeproj file as they are in CMake.
>> 
>> Can any of the LLDB devs more comfortable with plumbing the depths of Xcode 
>> project configuration provide some guidance here?
>> 
>> http://lab.llvm.org:8080/green/job/lldb_build_test/21828/ 
>> 
>> 
>> /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/source/lldb.cpp:63:22: 
>> error: unexpected namespace name 'lldb': expected expression
>> g_version_str += LLDB_VERSION_STRING;
>>  ^
>> In file included from :356:
>> :4:29: note: expanded from here
>> #define LLDB_VERSION_STRING lldb-360.99.0
>> ^
>> /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/source/lldb.cpp:63:22: 
>> error: invalid suffix '.0' on floating constant
>> In file included from :356:
>> :4:40: note: expanded from here
>> #define LLDB_VERSION_STRING lldb-360.99.0
>>^
>> 2 errors generated.
>> 
>> 
>> On Thu, Nov 10, 2016 at 9:33 AM, Chris Bieneman via lldb-commits 
>> mailto:lldb-commits@lists.llvm.org>> wrote:
>> Author: cbieneman
>> Date: Thu Nov 10 11:33:19 2016
>> New Revision: 286479
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=286479&view=rev 
>> 
>> Log:
>> Unify Darwin and Non-Darwin printing of version output
>> 
>> Summary:
>> This change unifies and simplifies the code paths between the Darwin and 
>> non-Darwin code to print the LLDB version information.
>> 
>> It also introduces a new variable in CMake LLDB_VERSION_STRING which can be 
>> used to specify custom version information. On Darwin this value is 
>> implicitly set based on the resource/LLDB-Info.plist file.
>> 
>> With the LLDB_VERSION_STRING variable set to lldb-360.99.0, the -version 
>> output is:
>> 
>> > ./bin/lldb -version
>> lldb version 4.0.0 (lldb-360.99.0)
>>   clang revision 286264
>>   llvm revision 286265
>> 
>> This behavior is unified across all target platforms.
>> 
>> Reviewers: lldb-commits
>> 
>> Subscribers: mgorny, tfiala
>> 
>> Differential Revision: https://reviews.llvm.org/D26478 
>> 
>> 
>> Added:
>> lldb/trunk/cmake/modules/EmbedAppleVersion.cmake
>> Modified:
>> lldb/trunk/lldb.xcodeproj/project.pbxproj
>> lldb/trunk/source/CMakeLists.txt
>> lldb/trunk/source/lldb.cpp
>> 
>> Added: lldb/trunk/cmake/modules/EmbedAppleVersion.cmake
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/EmbedAppleVersion.cmake?rev=286479&view=auto
>>  
>> 
>> ==
>> --- lldb/trunk/cmake/modules/EmbedAppleVersion.cmake (added)
>> +++ lldb/trunk/cmake/modules/EmbedAppleVersion.cmake Thu Nov 10 11:33:19 2016
>> @@ -0,0 +1,11 @@
>> +execute_process(COMMAND /usr/libexec/PlistBuddy -c "Print:CFBundleVersion" 
>> ${LLDB_INFO_PLIST}
>> +OUTPUT_VARIABLE BundleVersion
>> +OUTPUT_STRIP_TRAILING_WHITESPACE)
>> +
>> +file(APPEND "${HEADER_FILE}.tmp"
>> +"#define LLDB_VERSION_STRING \"lldb-${BundleVersion}\"\n")
>> +
>> +execute_process(COMMAND ${CMAKE_COMMAND} -E copy_if_different
>> +  "${HEADER_FILE}.tmp" "${HEADER_FILE}")
>> +
>> +file(REMOVE "${HEADER_FILE}.tmp")
>> 
>> Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=286479&r1=286478&r2=286479&view=diff
>>  
>> 
>> ==
>> --- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
>> +++ lldb/trunk/lldb.xcodeproj/project.pbxproj Thu Nov 10 11:33:19 2016
>> @@ -8775,6 +8775,20 @@
>> 
>> "\"$(SYSTEM_LIBRARY_DIR)/PrivateFrameworks\"",
>> );
>> GCC_INLINES_ARE_PRIVATE_EXTERN = NO;
>> +   GCC_PREPROCESSOR_DEFINITIONS = (
>> +   __STDC_CONSTANT_MACROS,
>> +   __STDC_LIMIT_MACROS,
>> +   LLDB_CONFIGURATION_DEBUG,
>> +

[Lldb-commits] [PATCH] D26553: Remove weak-linked symbols for SBBreakpointListImpl

[Todd Fiala via lldb-commits]

tfiala created this revision.
tfiala added a reviewer: jingham.
tfiala added a subscriber: lldb-commits.

Similar to SBStructuredData's Impl class, SBBreakpointListImpl was getting 
weak-link exported in the lldb namespace.  This change list fixes that by 
moving out of the lldb public namespace, which removes it from public export 
visibility.


https://reviews.llvm.org/D26553

Files:
  API/SBBreakpoint.cpp
  lldb/API/SBBreakpoint.h
  lldb/API/SBTarget.h


Index: API/SBBreakpoint.cpp
===
--- API/SBBreakpoint.cpp
+++ API/SBBreakpoint.cpp
@@ -712,11 +712,11 @@
 }
 
 // This is simple collection of breakpoint id's and their target.
-class lldb::SBBreakpointListImpl {
+class SBBreakpointListImpl {
 public:
-  SBBreakpointListImpl(SBTarget &target) : m_target_wp() {
-if (target.IsValid())
-  m_target_wp = target.GetSP();
+  SBBreakpointListImpl(lldb::TargetSP target_sp) : m_target_wp() {
+if (target_sp && target_sp->IsValid())
+  m_target_wp = target_sp;
   }
 
   ~SBBreakpointListImpl() = default;
@@ -796,7 +796,7 @@
 };
 
 SBBreakpointList::SBBreakpointList(SBTarget &target)
-: m_opaque_sp(new lldb::SBBreakpointListImpl(target)) {}
+: m_opaque_sp(new SBBreakpointListImpl(target.GetSP())) {}
 
 SBBreakpointList::~SBBreakpointList() {}
 
Index: lldb/API/SBTarget.h
===
--- lldb/API/SBTarget.h
+++ lldb/API/SBTarget.h
@@ -819,7 +819,7 @@
 protected:
   friend class SBAddress;
   friend class SBBlock;
-  friend class SBBreakpointListImpl;
+  friend class SBBreakpointList;
   friend class SBDebugger;
   friend class SBExecutionContext;
   friend class SBFunction;
Index: lldb/API/SBBreakpoint.h
===
--- lldb/API/SBBreakpoint.h
+++ lldb/API/SBBreakpoint.h
@@ -12,6 +12,8 @@
 
 #include "lldb/API/SBDefines.h"
 
+class SBBreakpointListImpl;
+
 namespace lldb {
 
 class LLDB_API SBBreakpoint {
@@ -146,8 +148,6 @@
   lldb::BreakpointSP m_opaque_sp;
 };
 
-class SBBreakpointListImpl;
-
 class LLDB_API SBBreakpointList {
 public:
   SBBreakpointList(SBTarget &target);


Index: API/SBBreakpoint.cpp
===
--- API/SBBreakpoint.cpp
+++ API/SBBreakpoint.cpp
@@ -712,11 +712,11 @@
 }
 
 // This is simple collection of breakpoint id's and their target.
-class lldb::SBBreakpointListImpl {
+class SBBreakpointListImpl {
 public:
-  SBBreakpointListImpl(SBTarget &target) : m_target_wp() {
-if (target.IsValid())
-  m_target_wp = target.GetSP();
+  SBBreakpointListImpl(lldb::TargetSP target_sp) : m_target_wp() {
+if (target_sp && target_sp->IsValid())
+  m_target_wp = target_sp;
   }
 
   ~SBBreakpointListImpl() = default;
@@ -796,7 +796,7 @@
 };
 
 SBBreakpointList::SBBreakpointList(SBTarget &target)
-: m_opaque_sp(new lldb::SBBreakpointListImpl(target)) {}
+: m_opaque_sp(new SBBreakpointListImpl(target.GetSP())) {}
 
 SBBreakpointList::~SBBreakpointList() {}
 
Index: lldb/API/SBTarget.h
===
--- lldb/API/SBTarget.h
+++ lldb/API/SBTarget.h
@@ -819,7 +819,7 @@
 protected:
   friend class SBAddress;
   friend class SBBlock;
-  friend class SBBreakpointListImpl;
+  friend class SBBreakpointList;
   friend class SBDebugger;
   friend class SBExecutionContext;
   friend class SBFunction;
Index: lldb/API/SBBreakpoint.h
===
--- lldb/API/SBBreakpoint.h
+++ lldb/API/SBBreakpoint.h
@@ -12,6 +12,8 @@
 
 #include "lldb/API/SBDefines.h"
 
+class SBBreakpointListImpl;
+
 namespace lldb {
 
 class LLDB_API SBBreakpoint {
@@ -146,8 +148,6 @@
   lldb::BreakpointSP m_opaque_sp;
 };
 
-class SBBreakpointListImpl;
-
 class LLDB_API SBBreakpointList {
 public:
   SBBreakpointList(SBTarget &target);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r286631 - Remove weak-linked symbols for SBBreakpointListImpl

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Fri Nov 11 15:06:40 2016
New Revision: 286631

URL: http://llvm.org/viewvc/llvm-project?rev=286631&view=rev
Log:
Remove weak-linked symbols for SBBreakpointListImpl

Summary:
Similar to SBStructuredData's Impl class, SBBreakpointListImpl was
getting weak-link exported in the lldb namespace. This change list fixes
that by moving out of the lldb public namespace, which removes it from
public export visibility.

Fixes:
rdar://28960344

Reviewers: jingham

Subscribers: lldb-commits

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

Modified:
lldb/trunk/include/lldb/API/SBBreakpoint.h
lldb/trunk/include/lldb/API/SBTarget.h
lldb/trunk/source/API/SBBreakpoint.cpp

Modified: lldb/trunk/include/lldb/API/SBBreakpoint.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBBreakpoint.h?rev=286631&r1=286630&r2=286631&view=diff
==
--- lldb/trunk/include/lldb/API/SBBreakpoint.h (original)
+++ lldb/trunk/include/lldb/API/SBBreakpoint.h Fri Nov 11 15:06:40 2016
@@ -12,6 +12,8 @@
 
 #include "lldb/API/SBDefines.h"
 
+class SBBreakpointListImpl;
+
 namespace lldb {
 
 class LLDB_API SBBreakpoint {
@@ -146,8 +148,6 @@ private:
   lldb::BreakpointSP m_opaque_sp;
 };
 
-class SBBreakpointListImpl;
-
 class LLDB_API SBBreakpointList {
 public:
   SBBreakpointList(SBTarget &target);

Modified: lldb/trunk/include/lldb/API/SBTarget.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBTarget.h?rev=286631&r1=286630&r2=286631&view=diff
==
--- lldb/trunk/include/lldb/API/SBTarget.h (original)
+++ lldb/trunk/include/lldb/API/SBTarget.h Fri Nov 11 15:06:40 2016
@@ -819,7 +819,7 @@ public:
 protected:
   friend class SBAddress;
   friend class SBBlock;
-  friend class SBBreakpointListImpl;
+  friend class SBBreakpointList;
   friend class SBDebugger;
   friend class SBExecutionContext;
   friend class SBFunction;

Modified: lldb/trunk/source/API/SBBreakpoint.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBBreakpoint.cpp?rev=286631&r1=286630&r2=286631&view=diff
==
--- lldb/trunk/source/API/SBBreakpoint.cpp (original)
+++ lldb/trunk/source/API/SBBreakpoint.cpp Fri Nov 11 15:06:40 2016
@@ -712,11 +712,11 @@ SBBreakpoint::GetNumBreakpointLocationsF
 }
 
 // This is simple collection of breakpoint id's and their target.
-class lldb::SBBreakpointListImpl {
+class SBBreakpointListImpl {
 public:
-  SBBreakpointListImpl(SBTarget &target) : m_target_wp() {
-if (target.IsValid())
-  m_target_wp = target.GetSP();
+  SBBreakpointListImpl(lldb::TargetSP target_sp) : m_target_wp() {
+if (target_sp && target_sp->IsValid())
+  m_target_wp = target_sp;
   }
 
   ~SBBreakpointListImpl() = default;
@@ -796,7 +796,7 @@ private:
 };
 
 SBBreakpointList::SBBreakpointList(SBTarget &target)
-: m_opaque_sp(new lldb::SBBreakpointListImpl(target)) {}
+: m_opaque_sp(new SBBreakpointListImpl(target.GetSP())) {}
 
 SBBreakpointList::~SBBreakpointList() {}
 


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

[Lldb-commits] [PATCH] D26553: Remove weak-linked symbols for SBBreakpointListImpl

[Todd Fiala via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL286631: Remove weak-linked symbols for SBBreakpointListImpl 
(authored by tfiala).

Changed prior to commit:
  https://reviews.llvm.org/D26553?vs=77639&id=77662#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26553

Files:
  lldb/trunk/include/lldb/API/SBBreakpoint.h
  lldb/trunk/include/lldb/API/SBTarget.h
  lldb/trunk/source/API/SBBreakpoint.cpp


Index: lldb/trunk/source/API/SBBreakpoint.cpp
===
--- lldb/trunk/source/API/SBBreakpoint.cpp
+++ lldb/trunk/source/API/SBBreakpoint.cpp
@@ -712,11 +712,11 @@
 }
 
 // This is simple collection of breakpoint id's and their target.
-class lldb::SBBreakpointListImpl {
+class SBBreakpointListImpl {
 public:
-  SBBreakpointListImpl(SBTarget &target) : m_target_wp() {
-if (target.IsValid())
-  m_target_wp = target.GetSP();
+  SBBreakpointListImpl(lldb::TargetSP target_sp) : m_target_wp() {
+if (target_sp && target_sp->IsValid())
+  m_target_wp = target_sp;
   }
 
   ~SBBreakpointListImpl() = default;
@@ -796,7 +796,7 @@
 };
 
 SBBreakpointList::SBBreakpointList(SBTarget &target)
-: m_opaque_sp(new lldb::SBBreakpointListImpl(target)) {}
+: m_opaque_sp(new SBBreakpointListImpl(target.GetSP())) {}
 
 SBBreakpointList::~SBBreakpointList() {}
 
Index: lldb/trunk/include/lldb/API/SBTarget.h
===
--- lldb/trunk/include/lldb/API/SBTarget.h
+++ lldb/trunk/include/lldb/API/SBTarget.h
@@ -819,7 +819,7 @@
 protected:
   friend class SBAddress;
   friend class SBBlock;
-  friend class SBBreakpointListImpl;
+  friend class SBBreakpointList;
   friend class SBDebugger;
   friend class SBExecutionContext;
   friend class SBFunction;
Index: lldb/trunk/include/lldb/API/SBBreakpoint.h
===
--- lldb/trunk/include/lldb/API/SBBreakpoint.h
+++ lldb/trunk/include/lldb/API/SBBreakpoint.h
@@ -12,6 +12,8 @@
 
 #include "lldb/API/SBDefines.h"
 
+class SBBreakpointListImpl;
+
 namespace lldb {
 
 class LLDB_API SBBreakpoint {
@@ -146,8 +148,6 @@
   lldb::BreakpointSP m_opaque_sp;
 };
 
-class SBBreakpointListImpl;
-
 class LLDB_API SBBreakpointList {
 public:
   SBBreakpointList(SBTarget &target);


Index: lldb/trunk/source/API/SBBreakpoint.cpp
===
--- lldb/trunk/source/API/SBBreakpoint.cpp
+++ lldb/trunk/source/API/SBBreakpoint.cpp
@@ -712,11 +712,11 @@
 }
 
 // This is simple collection of breakpoint id's and their target.
-class lldb::SBBreakpointListImpl {
+class SBBreakpointListImpl {
 public:
-  SBBreakpointListImpl(SBTarget &target) : m_target_wp() {
-if (target.IsValid())
-  m_target_wp = target.GetSP();
+  SBBreakpointListImpl(lldb::TargetSP target_sp) : m_target_wp() {
+if (target_sp && target_sp->IsValid())
+  m_target_wp = target_sp;
   }
 
   ~SBBreakpointListImpl() = default;
@@ -796,7 +796,7 @@
 };
 
 SBBreakpointList::SBBreakpointList(SBTarget &target)
-: m_opaque_sp(new lldb::SBBreakpointListImpl(target)) {}
+: m_opaque_sp(new SBBreakpointListImpl(target.GetSP())) {}
 
 SBBreakpointList::~SBBreakpointList() {}
 
Index: lldb/trunk/include/lldb/API/SBTarget.h
===
--- lldb/trunk/include/lldb/API/SBTarget.h
+++ lldb/trunk/include/lldb/API/SBTarget.h
@@ -819,7 +819,7 @@
 protected:
   friend class SBAddress;
   friend class SBBlock;
-  friend class SBBreakpointListImpl;
+  friend class SBBreakpointList;
   friend class SBDebugger;
   friend class SBExecutionContext;
   friend class SBFunction;
Index: lldb/trunk/include/lldb/API/SBBreakpoint.h
===
--- lldb/trunk/include/lldb/API/SBBreakpoint.h
+++ lldb/trunk/include/lldb/API/SBBreakpoint.h
@@ -12,6 +12,8 @@
 
 #include "lldb/API/SBDefines.h"
 
+class SBBreakpointListImpl;
+
 namespace lldb {
 
 class LLDB_API SBBreakpoint {
@@ -146,8 +148,6 @@
   lldb::BreakpointSP m_opaque_sp;
 };
 
-class SBBreakpointListImpl;
-
 class LLDB_API SBBreakpointList {
 public:
   SBBreakpointList(SBTarget &target);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26618: Make some code not manipulate the underlying buffer of a StreamString

[Todd Fiala via lldb-commits]

tfiala added a comment.

I will give this a run a bit later this morning.  Should be no later than early 
afternoon.


https://reviews.llvm.org/D26618



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

[Lldb-commits] [PATCH] D26618: Make some code not manipulate the underlying buffer of a StreamString

[Todd Fiala via lldb-commits]

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

LGTM.  Built and passed all existing tests.


https://reviews.llvm.org/D26618



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

[Lldb-commits] [PATCH] D26698: Remove SBStream accessor that lets you manipulate the internal buffer.

[Todd Fiala via lldb-commits]

tfiala added a comment.

I can run this through in the morning on macOS.


https://reviews.llvm.org/D26698



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

[Lldb-commits] [PATCH] D26721: Make AutoComplete code use StringRef

[Todd Fiala via lldb-commits]

tfiala added a comment.

I can give this one a run though on macOS in the morning.


https://reviews.llvm.org/D26721



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

[Lldb-commits] [PATCH] D26698: Remove SBStream accessor that lets you manipulate the internal buffer.

[Todd Fiala via lldb-commits]

tfiala added a comment.

Hi Zachary,

I'm uploading a file of diffs I needed to get the macOS build to compile and 
link.  There were a small handful of macOS-only paths that needed an 
adjustment.  You'll want to look those over to see if I did them the right way.

I'm now in the process of running the tests.  Back on that in a bit.F2590157: 
D26698_macos_build_fixes.diff 


https://reviews.llvm.org/D26698



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

[Lldb-commits] [PATCH] D26698: Remove SBStream accessor that lets you manipulate the internal buffer.

[Todd Fiala via lldb-commits]

tfiala added a comment.

TestTerminal.py's test_launch_in_terminal() is timing out consistently with the 
current change + my diff attached above.

I'm looking into it now.  (It quite possibly is due to the build fixes I did 
above).

I'm attaching the 'sample' (callstack sampling) from the failure here.   
F2590212: TestTerminal.py-58148.sample.zip 


https://reviews.llvm.org/D26698



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

[Lldb-commits] [PATCH] D26698: Remove SBStream accessor that lets you manipulate the internal buffer.

[Todd Fiala via lldb-commits]

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

Here is the adjusted patch that fixes the issue I was seeing on 
TestTerminal.py.  My first set of changes had a lifetime issue where I needed a 
const char* that was synthesized on the fly and went away by the time I needed 
it.

LGTM on macOS with this patch applied:
F2590224: D26698_macos_build_fixes_v2.diff 


https://reviews.llvm.org/D26698



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

[Lldb-commits] [PATCH] D26721: Make AutoComplete code use StringRef

[Todd Fiala via lldb-commits]

tfiala added a comment.

I'm going to test this one now, stacked on top of the final macOS-working 
version of https://reviews.llvm.org/D26698.  Tell me now if you want it tested 
independently of https://reviews.llvm.org/D26698.


https://reviews.llvm.org/D26721



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

Re: [Lldb-commits] [PATCH] D26721: Make AutoComplete code use StringRef

[Todd Fiala via lldb-commits]

Yep - I followed that.  I'm just doing the "build + test" verification on
macOS.


On Wed, Nov 16, 2016 at 11:41 AM, Zachary Turner  wrote:

> BTW, I would still like to get Chris to take a look at my usage of
> llvm::Twine.  Even if it works, I'm not sure if I used it correctly.
>
> On Wed, Nov 16, 2016 at 11:36 AM Zachary Turner 
> wrote:
>
>> Either way is fine, I think you might have hit a merge conflict if you
>> stacked them, but if you've already worked through it, then no big deal.
>>
>> On Wed, Nov 16, 2016 at 11:30 AM Todd Fiala  wrote:
>>
>> tfiala added a comment.
>>
>> I'm going to test this one now, stacked on top of the final macOS-working
>> version of https://reviews.llvm.org/D26698.  Tell me now if you want it
>> tested independently of https://reviews.llvm.org/D26698.
>>
>>
>> https://reviews.llvm.org/D26721
>>
>>
>>
>>


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

Re: [Lldb-commits] [PATCH] D26721: Make AutoComplete code use StringRef

[Todd Fiala via lldb-commits]

Hey Zachary,

I had to run home unexpectedly. The build worked but I left before the test
results came out.

If you ran it on Linux and the Linux tests passed, we can address issues
that show up on the macOS side.

I will also try it at home on macOS, but I don't think you need to hold up
with the aforementioned caveat on running the Linux tests.

On Wednesday, November 16, 2016, Zachary Turner  wrote:

> Hey Todd, Did you run the tests earlier?  If so what was the result?  No
> worries if you didn't run them yet, but it sounded like you were already
> kicking it off at 11:30.  Just want to make sure you didn't finish and
> forget to update with the result :)
>
> On Wed, Nov 16, 2016 at 12:38 PM Todd Fiala  > wrote:
>
>> Yep - I followed that.  I'm just doing the "build + test" verification on
>> macOS.
>>
>>
>> On Wed, Nov 16, 2016 at 11:41 AM, Zachary Turner > > wrote:
>>
>>> BTW, I would still like to get Chris to take a look at my usage of
>>> llvm::Twine.  Even if it works, I'm not sure if I used it correctly.
>>>
>>> On Wed, Nov 16, 2016 at 11:36 AM Zachary Turner >> > wrote:
>>>
 Either way is fine, I think you might have hit a merge conflict if you
 stacked them, but if you've already worked through it, then no big deal.

 On Wed, Nov 16, 2016 at 11:30 AM Todd Fiala >>> > wrote:

> tfiala added a comment.
>
> I'm going to test this one now, stacked on top of the final
> macOS-working version of https://reviews.llvm.org/D26698.  Tell me
> now if you want it tested independently of https://reviews.llvm.org/
> D26698.
>
>
> https://reviews.llvm.org/D26721
>
>
>
>
>>
>>
>> --
>> -Todd
>>
>

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

Re: [Lldb-commits] [PATCH] D26721: Make AutoComplete code use StringRef

[Todd Fiala via lldb-commits]

Okay cool.  I'm building against this now.  Should I hold off testing?

On Wed, Nov 16, 2016 at 3:41 PM, Zachary Turner  wrote:

> It's no problem, I actually found a few functions I forgot to convert, so
> I'm making some additional changes.  Nothing that requires additional
> testing, but at the very least I won't be able to get this in until
> tomorrow at the earliest, so it's no biggie.
>
> On Wed, Nov 16, 2016 at 3:17 PM Todd Fiala  wrote:
>
>> Hey Zachary,
>>
>> I had to run home unexpectedly. The build worked but I left before the
>> test results came out.
>>
>> If you ran it on Linux and the Linux tests passed, we can address issues
>> that show up on the macOS side.
>>
>> I will also try it at home on macOS, but I don't think you need to hold
>> up with the aforementioned caveat on running the Linux tests.
>>
>>
>> On Wednesday, November 16, 2016, Zachary Turner 
>> wrote:
>>
>> Hey Todd, Did you run the tests earlier?  If so what was the result?  No
>> worries if you didn't run them yet, but it sounded like you were already
>> kicking it off at 11:30.  Just want to make sure you didn't finish and
>> forget to update with the result :)
>>
>> On Wed, Nov 16, 2016 at 12:38 PM Todd Fiala  wrote:
>>
>> Yep - I followed that.  I'm just doing the "build + test" verification on
>> macOS.
>>
>>
>> On Wed, Nov 16, 2016 at 11:41 AM, Zachary Turner 
>> wrote:
>>
>> BTW, I would still like to get Chris to take a look at my usage of
>> llvm::Twine.  Even if it works, I'm not sure if I used it correctly.
>>
>> On Wed, Nov 16, 2016 at 11:36 AM Zachary Turner 
>> wrote:
>>
>> Either way is fine, I think you might have hit a merge conflict if you
>> stacked them, but if you've already worked through it, then no big deal.
>>
>> On Wed, Nov 16, 2016 at 11:30 AM Todd Fiala  wrote:
>>
>> tfiala added a comment.
>>
>> I'm going to test this one now, stacked on top of the final macOS-working
>> version of https://reviews.llvm.org/D26698.  Tell me now if you want it
>> tested independently of https://reviews.llvm.org/D26698.
>>
>>
>> https://reviews.llvm.org/D26721
>>
>>
>>
>>
>>
>>
>> --
>> -Todd
>>
>>
>>
>> --
>> -Todd
>>
>>


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

Re: [Lldb-commits] [PATCH] D26721: Make AutoComplete code use StringRef

[Todd Fiala via lldb-commits]

Great.  Back soon...

On Wed, Nov 16, 2016 at 3:50 PM, Zachary Turner  wrote:

> No you can go ahead.  The new changes I found are not specific to Mac, I
> should find problems if there's any with the additional changes.
>
> On Wed, Nov 16, 2016 at 3:42 PM Todd Fiala  wrote:
>
>> Okay cool.  I'm building against this now.  Should I hold off testing?
>>
>> On Wed, Nov 16, 2016 at 3:41 PM, Zachary Turner 
>> wrote:
>>
>> It's no problem, I actually found a few functions I forgot to convert, so
>> I'm making some additional changes.  Nothing that requires additional
>> testing, but at the very least I won't be able to get this in until
>> tomorrow at the earliest, so it's no biggie.
>>
>> On Wed, Nov 16, 2016 at 3:17 PM Todd Fiala  wrote:
>>
>> Hey Zachary,
>>
>> I had to run home unexpectedly. The build worked but I left before the
>> test results came out.
>>
>> If you ran it on Linux and the Linux tests passed, we can address issues
>> that show up on the macOS side.
>>
>> I will also try it at home on macOS, but I don't think you need to hold
>> up with the aforementioned caveat on running the Linux tests.
>>
>>
>> On Wednesday, November 16, 2016, Zachary Turner 
>> wrote:
>>
>> Hey Todd, Did you run the tests earlier?  If so what was the result?  No
>> worries if you didn't run them yet, but it sounded like you were already
>> kicking it off at 11:30.  Just want to make sure you didn't finish and
>> forget to update with the result :)
>>
>> On Wed, Nov 16, 2016 at 12:38 PM Todd Fiala  wrote:
>>
>> Yep - I followed that.  I'm just doing the "build + test" verification on
>> macOS.
>>
>>
>> On Wed, Nov 16, 2016 at 11:41 AM, Zachary Turner 
>> wrote:
>>
>> BTW, I would still like to get Chris to take a look at my usage of
>> llvm::Twine.  Even if it works, I'm not sure if I used it correctly.
>>
>> On Wed, Nov 16, 2016 at 11:36 AM Zachary Turner 
>> wrote:
>>
>> Either way is fine, I think you might have hit a merge conflict if you
>> stacked them, but if you've already worked through it, then no big deal.
>>
>> On Wed, Nov 16, 2016 at 11:30 AM Todd Fiala  wrote:
>>
>> tfiala added a comment.
>>
>> I'm going to test this one now, stacked on top of the final macOS-working
>> version of https://reviews.llvm.org/D26698.  Tell me now if you want it
>> tested independently of https://reviews.llvm.org/D26698.
>>
>>
>> https://reviews.llvm.org/D26721
>>
>>
>>
>>
>>
>>
>> --
>> -Todd
>>
>>
>>
>> --
>> -Todd
>>
>>
>>
>>
>> --
>> -Todd
>>
>


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

[Lldb-commits] [PATCH] D26721: Make AutoComplete code use StringRef

[Todd Fiala via lldb-commits]

tfiala accepted this revision.
tfiala added a comment.

LGTM!  All tests passed on macOS 10.12.2 public beta with Xcode 8.1.


https://reviews.llvm.org/D26721



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

[Lldb-commits] [lldb] r288044 - fix up Xcode build for r287916

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Mon Nov 28 11:19:03 2016
New Revision: 288044

URL: http://llvm.org/viewvc/llvm-project?rev=288044&view=rev
Log:
fix up Xcode build for r287916

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=288044&r1=288043&r2=288044&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Mon Nov 28 11:19:03 2016
@@ -69,6 +69,7 @@
236124A51986B4E2004EFC37 /* Socket.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 236124A31986B4E2004EFC37 /* Socket.cpp */; };
2374D7531D4BB2FF005C9575 /* GDBRemoteClientBase.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 2374D74E1D4BB299005C9575 /* 
GDBRemoteClientBase.cpp */; };
2377C2F819E613C100737875 /* PipePosix.cpp in Sources */ = {isa 
= PBXBuildFile; fileRef = 2377C2F719E613C100737875 /* PipePosix.cpp */; };
+   237A8BAF1DEC9C7800CEBAFF /* RegisterInfoPOSIX_arm64.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 237A8BAB1DEC9BBC00CEBAFF /* 
RegisterInfoPOSIX_arm64.cpp */; };
238F2B9E1D2C82D0001FF92A /* StructuredDataPlugin.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 238F2B9D1D2C82D0001FF92A /* 
StructuredDataPlugin.cpp */; };
238F2BA11D2C835A001FF92A /* StructuredDataPlugin.h in Headers 
*/ = {isa = PBXBuildFile; fileRef = 238F2B9F1D2C835A001FF92A /* 
StructuredDataPlugin.h */; };
238F2BA21D2C835A001FF92A /* SystemRuntime.h in Headers */ = 
{isa = PBXBuildFile; fileRef = 238F2BA01D2C835A001FF92A /* SystemRuntime.h */; 
};
@@ -917,7 +918,6 @@
AF1FA88A1A60A69500272AFC /* RegisterNumber.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF1FA8891A60A69500272AFC /* RegisterNumber.cpp 
*/; };
AF20F7661AF18F8500751A6E /* ABISysV_arm.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF20F7641AF18F8500751A6E /* ABISysV_arm.cpp */; 
};
AF20F76A1AF18F9000751A6E /* ABISysV_arm64.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF20F7681AF18F9000751A6E /* ABISysV_arm64.cpp 
*/; };
-   AF20F7701AF1902900751A6E /* RegisterContextLinux_arm64.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = AF20F76E1AF1902900751A6E /* 
RegisterContextLinux_arm64.cpp */; };
AF23B4DB19009C66003E2A58 /* FreeBSDSignals.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF23B4D919009C66003E2A58 /* FreeBSDSignals.cpp 
*/; };
AF248A4D1DA71C77000B814D /* TestArm64InstEmulation.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = AF248A4C1DA71C77000B814D /* 
TestArm64InstEmulation.cpp */; };
AF254E31170CCC33007AE5C9 /* PlatformDarwinKernel.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = AF254E2F170CCC33007AE5C9 /* 
PlatformDarwinKernel.cpp */; };
@@ -980,7 +980,6 @@
E769331C1A94D15400C73337 /* lldb-gdbserver.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26D6F3F4183E7F9300194858 /* lldb-gdbserver.cpp 
*/; };
E769331E1A94D18100C73337 /* lldb-server.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = E769331D1A94D18100C73337 /* lldb-server.cpp */; 
};
E7723D441AC4A7FB002BA082 /* RegisterContextPOSIXCore_arm64.cpp 
in Sources */ = {isa = PBXBuildFile; fileRef = E7723D421AC4A7FB002BA082 /* 
RegisterContextPOSIXCore_arm64.cpp */; };
-   E7723D481AC4A8C8002BA082 /* RegisterContextFreeBSD_arm64.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = E7723D461AC4A8C8002BA082 /* 
RegisterContextFreeBSD_arm64.cpp */; };
E7723D4C1AC4A944002BA082 /* RegisterContextPOSIX_arm64.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = E7723D4A1AC4A944002BA082 /* 
RegisterContextPOSIX_arm64.cpp */; };
E778E9A21B062D1700247609 /* EmulateInstructionMIPS.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = E778E99F1B062D1700247609 /* 
EmulateInstructionMIPS.cpp */; };
E7E94ABC1B54961F00D0AE30 /* GDBRemoteSignals.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = E73A15A41B548EC500786197 /* 
GDBRemoteSignals.cpp */; };
@@ -1268,6 +1267,8 @@
2374D74E1D4BB299005C9575 /* GDBRemoteClientBase.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = GDBRemoteClientBase.cpp; sourceTree = ""; };
2374D74F1D4BB299005C9575 /* GDBRemoteClientBase.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
GDBRemoteClientBase.h; sourceTree = ""; };
2377C2F719E613C100737875 /* PipePosix.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = PipePosix.cpp; sourceTree = ""; };
+   237A8BAB1DEC9BBC00CEBAFF /* RegisterInfoPOSIX_arm64.

Re: [Lldb-commits] [lldb] r288044 - fix up Xcode build for r287916

[Todd Fiala via lldb-commits]

Sure thing :-)

On Mon, Nov 28, 2016 at 1:17 PM, Tim Hammerquist  wrote:

> Thanks, Todd!
>
> On Mon, Nov 28, 2016 at 9:19 AM, Todd Fiala via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> Author: tfiala
>> Date: Mon Nov 28 11:19:03 2016
>> New Revision: 288044
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=288044&view=rev
>> Log:
>> fix up Xcode build for r287916
>>
>> Modified:
>> lldb/trunk/lldb.xcodeproj/project.pbxproj
>>
>> Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodepro
>> j/project.pbxproj?rev=288044&r1=288043&r2=288044&view=diff
>> 
>> ==
>> --- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
>> +++ lldb/trunk/lldb.xcodeproj/project.pbxproj Mon Nov 28 11:19:03 2016
>> @@ -69,6 +69,7 @@
>> 236124A51986B4E2004EFC37 /* Socket.cpp in Sources */ =
>> {isa = PBXBuildFile; fileRef = 236124A31986B4E2004EFC37 /* Socket.cpp */; };
>> 2374D7531D4BB2FF005C9575 /* GDBRemoteClientBase.cpp in
>> Sources */ = {isa = PBXBuildFile; fileRef = 2374D74E1D4BB299005C9575 /*
>> GDBRemoteClientBase.cpp */; };
>> 2377C2F819E613C100737875 /* PipePosix.cpp in Sources */ =
>> {isa = PBXBuildFile; fileRef = 2377C2F719E613C100737875 /* PipePosix.cpp
>> */; };
>> +   237A8BAF1DEC9C7800CEBAFF /* RegisterInfoPOSIX_arm64.cpp
>> in Sources */ = {isa = PBXBuildFile; fileRef = 237A8BAB1DEC9BBC00CEBAFF /*
>> RegisterInfoPOSIX_arm64.cpp */; };
>> 238F2B9E1D2C82D0001FF92A /* StructuredDataPlugin.cpp in
>> Sources */ = {isa = PBXBuildFile; fileRef = 238F2B9D1D2C82D0001FF92A /*
>> StructuredDataPlugin.cpp */; };
>> 238F2BA11D2C835A001FF92A /* StructuredDataPlugin.h in
>> Headers */ = {isa = PBXBuildFile; fileRef = 238F2B9F1D2C835A001FF92A /*
>> StructuredDataPlugin.h */; };
>> 238F2BA21D2C835A001FF92A /* SystemRuntime.h in Headers */
>> = {isa = PBXBuildFile; fileRef = 238F2BA01D2C835A001FF92A /*
>> SystemRuntime.h */; };
>> @@ -917,7 +918,6 @@
>> AF1FA88A1A60A69500272AFC /* RegisterNumber.cpp in Sources
>> */ = {isa = PBXBuildFile; fileRef = AF1FA8891A60A69500272AFC /*
>> RegisterNumber.cpp */; };
>> AF20F7661AF18F8500751A6E /* ABISysV_arm.cpp in Sources */
>> = {isa = PBXBuildFile; fileRef = AF20F7641AF18F8500751A6E /*
>> ABISysV_arm.cpp */; };
>> AF20F76A1AF18F9000751A6E /* ABISysV_arm64.cpp in Sources
>> */ = {isa = PBXBuildFile; fileRef = AF20F7681AF18F9000751A6E /*
>> ABISysV_arm64.cpp */; };
>> -   AF20F7701AF1902900751A6E /*
>> RegisterContextLinux_arm64.cpp in Sources */ = {isa = PBXBuildFile; fileRef
>> = AF20F76E1AF1902900751A6E /* RegisterContextLinux_arm64.cpp */; };
>> AF23B4DB19009C66003E2A58 /* FreeBSDSignals.cpp in Sources
>> */ = {isa = PBXBuildFile; fileRef = AF23B4D919009C66003E2A58 /*
>> FreeBSDSignals.cpp */; };
>> AF248A4D1DA71C77000B814D /* TestArm64InstEmulation.cpp in
>> Sources */ = {isa = PBXBuildFile; fileRef = AF248A4C1DA71C77000B814D /*
>> TestArm64InstEmulation.cpp */; };
>> AF254E31170CCC33007AE5C9 /* PlatformDarwinKernel.cpp in
>> Sources */ = {isa = PBXBuildFile; fileRef = AF254E2F170CCC33007AE5C9 /*
>> PlatformDarwinKernel.cpp */; };
>> @@ -980,7 +980,6 @@
>> E769331C1A94D15400C73337 /* lldb-gdbserver.cpp in Sources
>> */ = {isa = PBXBuildFile; fileRef = 26D6F3F4183E7F9300194858 /*
>> lldb-gdbserver.cpp */; };
>> E769331E1A94D18100C73337 /* lldb-server.cpp in Sources */
>> = {isa = PBXBuildFile; fileRef = E769331D1A94D18100C73337 /*
>> lldb-server.cpp */; };
>> E7723D441AC4A7FB002BA082 /* 
>> RegisterContextPOSIXCore_arm64.cpp
>> in Sources */ = {isa = PBXBuildFile; fileRef = E7723D421AC4A7FB002BA082 /*
>> RegisterContextPOSIXCore_arm64.cpp */; };
>> -   E7723D481AC4A8C8002BA082 /* RegisterContextFreeBSD_arm64.cpp
>> in Sources */ = {isa = PBXBuildFile; fileRef = E7723D461AC4A8C8002BA082 /*
>> RegisterContextFreeBSD_arm64.cpp */; };
>> E7723D4C1AC4A944002BA082 /*
>> RegisterContextPOSIX_arm64.cpp in Sources */ = {isa = PBXBuildFile; fileRef
>> = E7723D4A1AC4A944002BA082 /* RegisterContextPOSIX_arm64.cpp */; };
>> E778E9A21B062D1700247609 /* EmulateInstructionMIPS.cpp in
>> S

[Lldb-commits] [lldb] r289479 - Removing myself from code ownership file

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Mon Dec 12 16:42:00 2016
New Revision: 289479

URL: http://llvm.org/viewvc/llvm-project?rev=289479&view=rev
Log:
Removing myself from code ownership file

I'm transitioning away from my current employer, and I do not foresee myself
spending much time on LLDB in the near future. Ideally somebody on the Google
Android team takes over the gdb-remote protocol tests, and somebody with decent
familiarity with the test suite infrastructure takes over the parallel test
runner and test event stream portions of the Python test suite.

Modified:
lldb/trunk/CODE_OWNERS.txt

Modified: lldb/trunk/CODE_OWNERS.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CODE_OWNERS.txt?rev=289479&r1=289478&r2=289479&view=diff
==
--- lldb/trunk/CODE_OWNERS.txt (original)
+++ lldb/trunk/CODE_OWNERS.txt Mon Dec 12 16:42:00 2016
@@ -49,8 +49,3 @@ D: Test suite
 N: Pavel Labath
 E: lab...@google.com
 D: Linux, Android
-
-N: Todd Fiala
-E: todd.fi...@gmail.com
-D: Test Suite, esp. subsystems (concurrent test runners, test events, 
TestResults system), gdb-remote protocol tests
-


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

Re: [Lldb-commits] [PATCH] D28666: Fix TestRegisterVariables for linux arm/arm64 gcc ver > 5

[Todd Fiala via lldb-commits]

LGTM.

Jim and I also thought this test was incredibly brittle. You really want
assembly or something that guarantees that variables will be in registers.

On Fri, Jan 13, 2017 at 1:43 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:

> labath accepted this revision.
>
> labath added a comme
>
> This revision is now accepted and ready to land.
>
>
>
> Seems reasonable. Maybe add a comment explaining why is that macro defined.
>
>
>
> This test is incredibly brittle. I think we should find a better way to
> test the feature, but I don't really have a good idea so far...
>
>
>
>
>
> https://reviews.llvm.org/D28666
>
>
>
>
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism

[Todd Fiala via lldb-commits]

tfiala accepted this revision.
tfiala added a reviewer: tfiala.
tfiala added a comment.
This revision is now accepted and ready to land.

Accepting with given changes.


http://reviews.llvm.org/D13124



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

[Lldb-commits] [lldb] r248846 - Skipping TestAttachDenied.py on Linux as it is hanging on a buildbot after r248834.

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Tue Sep 29 18:06:56 2015
New Revision: 248846

URL: http://llvm.org/viewvc/llvm-project?rev=248846&view=rev
Log:
Skipping TestAttachDenied.py on Linux as it is hanging on a buildbot after 
r248834.

I'll track down what is happening here.

Modified:

lldb/trunk/test/functionalities/process_attach/attach_denied/TestAttachDenied.py

Modified: 
lldb/trunk/test/functionalities/process_attach/attach_denied/TestAttachDenied.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/process_attach/attach_denied/TestAttachDenied.py?rev=248846&r1=248845&r2=248846&view=diff
==
--- 
lldb/trunk/test/functionalities/process_attach/attach_denied/TestAttachDenied.py
 (original)
+++ 
lldb/trunk/test/functionalities/process_attach/attach_denied/TestAttachDenied.py
 Tue Sep 29 18:06:56 2015
@@ -21,6 +21,7 @@ class AttachDeniedTestCase(TestBase):
 return (err, shell_command.GetStatus(), shell_command.GetOutput())
 
 @skipIfWindows
+@skipIfLinux # hanging after reviews D13124 change went in
 def test_attach_to_process_by_id_denied(self):
 """Test attach by process id denied"""
 


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

[Lldb-commits] [lldb] r248936 - Fixes a potential hang in test runner timeout logic.

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Wed Sep 30 15:13:58 2015
New Revision: 248936

URL: http://llvm.org/viewvc/llvm-project?rev=248936&view=rev
Log:
Fixes a potential hang in test runner timeout logic.

Part of https://llvm.org/bugs/show_bug.cgi?id=25002

In writing the new process_control test included here,
I discovered that the scenario would hang indefinitely,
bypassing the timeout logic.

This fixes the indefinite hang, converting an error
in the new test to a failure.  I'll fix the failure
next.  This one was heinous enough to fix on its own,
though.

Modified:
lldb/trunk/test/test_runner/lib/process_control.py
lldb/trunk/test/test_runner/test/inferior.py
lldb/trunk/test/test_runner/test/process_control_tests.py

Modified: lldb/trunk/test/test_runner/lib/process_control.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/test_runner/lib/process_control.py?rev=248936&r1=248935&r2=248936&view=diff
==
--- lldb/trunk/test/test_runner/lib/process_control.py (original)
+++ lldb/trunk/test/test_runner/lib/process_control.py Wed Sep 30 15:13:58 2015
@@ -297,17 +297,23 @@ class UnixProcessHelper(ProcessHelper):
 log_file.write("skipping soft_terminate(): no process id")
 return False
 
-# Don't kill if it's already dead.
-popen_process.poll()
-if popen_process.returncode is not None:
-# It has a returncode.  It has already stopped.
-if log_file:
-log_file.write(
-"requested to terminate pid {} but it has already "
-"terminated, returncode {}".format(
-popen_process.pid, popen_process.returncode))
-# Move along...
-return False
+# We only do the process liveness check if we're not using
+# process groups.  With process groups, checking if the main
+# inferior process is dead and short circuiting here is no
+# good - children of it in the process group could still be
+# alive, and they should be killed during a timeout.
+if not popen_process.using_process_groups:
+# Don't kill if it's already dead.
+popen_process.poll()
+if popen_process.returncode is not None:
+# It has a returncode.  It has already stopped.
+if log_file:
+log_file.write(
+"requested to terminate pid {} but it has already "
+"terminated, returncode {}".format(
+popen_process.pid, popen_process.returncode))
+# Move along...
+return False
 
 # Good to go.
 return True

Modified: lldb/trunk/test/test_runner/test/inferior.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/test_runner/test/inferior.py?rev=248936&r1=248935&r2=248936&view=diff
==
--- lldb/trunk/test/test_runner/test/inferior.py (original)
+++ lldb/trunk/test/test_runner/test/inferior.py Wed Sep 30 15:13:58 2015
@@ -3,6 +3,7 @@
 import argparse
 import datetime
 import signal
+import subprocess
 import sys
 import time
 
@@ -25,6 +26,15 @@ def parse_args(command_line):
 default=[],
 help="ignore the given signal number (if possible)")
 parser.add_argument(
+"--launch-child-share-handles",
+action="store_true",
+help=("launch a child inferior.py that shares stdout/stderr/stdio and "
+  "never returns"))
+parser.add_argument(
+"--never-return",
+action="store_true",
+help="run in an infinite loop, never return")
+parser.add_argument(
 "--return-code",
 "-r",
 type=int,
@@ -43,7 +53,7 @@ def parse_args(command_line):
 return parser.parse_args(command_line)
 
 
-def maybe_ignore_signals(options, signals):
+def handle_ignore_signals(options, signals):
 """Ignores any signals provided to it.
 
 @param options the command line options parsed by the program.
@@ -61,7 +71,7 @@ def maybe_ignore_signals(options, signal
 signal.signal(signum, signal.SIG_IGN)
 
 
-def maybe_sleep(options, sleep_seconds):
+def handle_sleep(options, sleep_seconds):
 """Sleeps the number of seconds specified, restarting as needed.
 
 @param options the command line options parsed by the program.
@@ -90,7 +100,27 @@ def maybe_sleep(options, sleep_seconds):
 sleep_seconds = sleep_interval.total_seconds()
 if sleep_seconds > 0:
 time.sleep(sleep_seconds)
-except:
+except:  # pylint: disable=bare-except
+pass
+
+
+def handle_launch_children(options):
+if options.launch_child_share_handles:
+# Launch the child, share our file handles.
+# We won't bother reaping it since it will likely outlive us.
+

[Lldb-commits] [PATCH] D13362: Enhance test runner timeout logic to detect successful completion of spawned process when stdout/stderr are shared to still-existing child processes

[Todd Fiala via lldb-commits]

tfiala created this revision.
tfiala added reviewers: labath, zturner.
tfiala added a subscriber: lldb-commits.

This makes one of the internal process_control tests go green.  It covers the 
case where spawned process P1 itself spawns a child process C1, shared 
stdout/stderr file handles with C1, and then P1 terminates.

Prior to this change, we would end up timing out rather than detecting the 
immediate termination of P1 because we would wait for the stdout/stderr file 
handles to wrap up.

Now we wait on a condition variable that is set via a thread parked on 
subprocess.Popen.wait() and another on subprocess.Popen.communicate().  This 
allows us to catch the scenario above.  There's an additional thread (for the 
thread parked on wait()).  Both the wait() and the communicate() threads wait 
efficiently, so this should be minimal cost. 

http://reviews.llvm.org/D13362

Files:
  test/test_runner/lib/process_control.py
  test/test_runner/test/inferior.py
  test/test_runner/test/process_control_tests.py

Index: test/test_runner/test/process_control_tests.py
===
--- test/test_runner/test/process_control_tests.py
+++ test/test_runner/test/process_control_tests.py
@@ -14,7 +14,6 @@
 
 # System imports.
 import os
-import platform
 import unittest
 import sys
 import threading
@@ -27,9 +26,11 @@
 
 
 class TestInferiorDriver(process_control.ProcessDriver):
-def __init__(self, soft_terminate_timeout=None):
+def __init__(self, soft_terminate_timeout=5.0):
+# override the default
 super(TestInferiorDriver, self).__init__(
 soft_terminate_timeout=soft_terminate_timeout)
+
 self.started_event = threading.Event()
 self.started_event.clear()
 
@@ -105,10 +106,13 @@
 def test_run_completes_with_code(self):
 """Test that running completes and gets expected stdout/stderr."""
 driver = TestInferiorDriver()
-driver.run_command(self.inferior_command(options="-r10"))
+expected_returncode = 10
+driver.run_command(self.inferior_command(options="-r{}".format(
+expected_returncode)))
 self.assertTrue(
 driver.completed_event.wait(5), "process failed to complete")
-self.assertEqual(driver.returncode, 10, "return code does not match")
+self.assertIsNotNone(driver.returncode)
+self.assertEqual(driver.returncode, expected_returncode)
 
 
 class ProcessControlTimeoutTests(ProcessControlTests):
@@ -204,27 +208,30 @@
 """
 driver = TestInferiorDriver()
 
+timeout_seconds = 5
+return_code = 3
 # Create the inferior (I1), and instruct it to create a child (C1)
 # that shares the stdout/stderr handles with the inferior.
 # C1 will then loop forever.
 driver.run_command_with_timeout(
 self.inferior_command(
-options="--launch-child-share-handles --return-code 3"),
-"5s",
+options="--launch-child-share-handles --return-code {}".format(
+return_code)),
+"{}s".format(timeout_seconds),
 False)
 
 # We should complete without a timetout.  I1 should end
 # immediately after launching C1.
 self.assertTrue(
-driver.completed_event.wait(5),
+driver.completed_event.wait(timeout_seconds),
 "process failed to complete")
 
 # Ensure we didn't receive a timeout.
-self.assertTrue(
+self.assertFalse(
 driver.was_timeout, "inferior should have completed normally")
 
 self.assertEqual(
-driver.returncode, 3,
+driver.returncode, return_code,
 "expected inferior process to end with expected returncode")
 
 
Index: test/test_runner/test/inferior.py
===
--- test/test_runner/test/inferior.py
+++ test/test_runner/test/inferior.py
@@ -140,4 +140,6 @@
 return options.return_code
 
 if __name__ == "__main__":
-sys.exit(main(sys.argv[1:]))
+RETURN_CODE = main(sys.argv[1:])
+print "returning {}".format(RETURN_CODE)
+sys.exit(RETURN_CODE)
Index: test/test_runner/lib/process_control.py
===
--- test/test_runner/lib/process_control.py
+++ test/test_runner/lib/process_control.py
@@ -13,6 +13,7 @@
 """
 
 # System imports
+import datetime
 import os
 import re
 import signal
@@ -21,31 +22,40 @@
 import threading
 
 
-class CommunicatorThread(threading.Thread):
-"""Provides a thread class that communicates with a subprocess."""
-def __init__(self, process, event, output_file):
-super(CommunicatorThread, self).__init__()
+class CallAndNotifyThread(threading.Thread):
+"""Provides a thread class that calls a method, then notifies.
+
+Implements a thread that sits on a synchronous call, then notif

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

[Todd Fiala via lldb-commits]

tfiala added a comment.

Ooo neat, I'll have a look!  Might take me 24 hours.


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

[Todd Fiala via lldb-commits]

tfiala added a comment.

It might need to be conditionalized for libedit support if the CMakeLists 
doesn't already cover that.  I think we had it set up to allow conditional lldb 
building without libedit support.


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13362: Enhance test runner timeout logic to detect successful completion of spawned process when stdout/stderr are shared to still-existing child processes

[Todd Fiala via lldb-commits]

tfiala added a comment.

Hmm, I'm getting a few more tests to time out now with this change:

TIMEOUT: LLDB (suite) :: TestCallStopAndContinue.py (Linux rad 
3.13.0-57-generic #95-Ubuntu SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)
TIMEOUT: LLDB (suite) :: TestCommandScript.py (Linux rad 3.13.0-57-generic 
#95-Ubuntu SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)
TIMEOUT: LLDB (suite) :: TestFrames.py (Linux rad 3.13.0-57-generic #95-Ubuntu 
SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)
TIMEOUT: LLDB (suite) :: TestInlineStepping.py (Linux rad 3.13.0-57-generic 
#95-Ubuntu SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)
TIMEOUT: LLDB (suite) :: TestReturnValue.py (Linux rad 3.13.0-57-generic 
#95-Ubuntu SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)
TIMEOUT: LLDB (suite) :: TestStepNoDebug.py (Linux rad 3.13.0-57-generic 
#95-Ubuntu SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)
TIMEOUT: LLDB (suite) :: TestThreadAPI.py (Linux rad 3.13.0-57-generic 
#95-Ubuntu SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)
TIMEOUT: LLDB (suite) :: TestThreadJump.py (Linux rad 3.13.0-57-generic 
#95-Ubuntu SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)
TIMEOUT: LLDB (suite) :: TestThreadStepping.py (Linux rad 3.13.0-57-generic 
#95-Ubuntu SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)
TIMEOUT: LLDB (suite) :: TestValueMD5Crash.py (Linux rad 3.13.0-57-generic 
#95-Ubuntu SMP Fri Jun 19 09:28:15 UTC 2015 x86_64 x86_64)

I'll need to drill into that.


http://reviews.llvm.org/D13362



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

Re: [Lldb-commits] [PATCH] D13362: Enhance test runner timeout logic to detect successful completion of spawned process when stdout/stderr are shared to still-existing child processes

[Todd Fiala via lldb-commits]

tfiala added a comment.

On OS X the only timeout I'm getting is:

TIMEOUT: LLDB (suite) :: TestValueObjectRecursion.py (Darwin 
tfiala-mbp-01.local 15.0.0 Darwin Kernel Version 15.0.0: Sat Sep 19 15:53:45 
PDT 2015; root:xnu-3247.10.11~1/DEVELOPMENT_X86_64 x86_64 i386)


http://reviews.llvm.org/D13362



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

Re: [Lldb-commits] [PATCH] D13362: Enhance test runner timeout logic to detect successful completion of spawned process when stdout/stderr are shared to still-existing child processes

[Todd Fiala via lldb-commits]

tfiala added a comment.

The OS X one seems to be related to load (not terribly surprising).  I'm seeing 
timeouts as I crank up the --threads.  But I don't think that's the whole 
story.   On the Linux side, I've been cranking down the threads and I'm still 
seeing it.

In light of the more fundamental issue I just filed here:
https://llvm.org/bugs/show_bug.cgi?id=25019

I'm going to address that piece first (upstream) before I take this part on 
here.  That part needs to be straightened out first.  The then I'll come back 
here.


http://reviews.llvm.org/D13362



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

Re: [Lldb-commits] [PATCH] D13362: Enhance test runner timeout logic to detect successful completion of spawned process when stdout/stderr are shared to still-existing child processes

[Todd Fiala via lldb-commits]

tfiala added a comment.



> I think that is a reasonable course of action. Having three threads servicing 
> the same process seems wasteful/hard to maintain and has a very big race 
> potential.


Yeah I'm not liking how complex this is getting.

I'll have more comments once I address the other issue depending on how the 
solution falls out.


http://reviews.llvm.org/D13362



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

[Lldb-commits] [lldb] r249182 - Fix race on subprocess.Popen return values.

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Fri Oct  2 15:51:11 2015
New Revision: 249182

URL: http://llvm.org/viewvc/llvm-project?rev=249182&view=rev
Log:
Fix race on subprocess.Popen return values.

This fixes:
https://llvm.org/bugs/show_bug.cgi?id=25019

Modified:
lldb/trunk/test/test_runner/lib/process_control.py
lldb/trunk/test/test_runner/test/process_control_tests.py

Modified: lldb/trunk/test/test_runner/lib/process_control.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/test_runner/lib/process_control.py?rev=249182&r1=249181&r2=249182&view=diff
==
--- lldb/trunk/test/test_runner/lib/process_control.py (original)
+++ lldb/trunk/test/test_runner/lib/process_control.py Fri Oct  2 15:51:11 2015
@@ -611,3 +611,48 @@ class ProcessDriver(object):
 self.io_thread.output,
 not completed_normally,
 self.returncode)
+
+
+def patched_init(self, *args, **kwargs):
+self.original_init(*args, **kwargs)
+# Initialize our condition variable that protects wait()/poll().
+self.wait_condition = threading.Condition()
+
+
+def patched_wait(self):
+self.wait_condition.acquire()
+try:
+result = self.original_wait()
+# The process finished.  Signal the condition.
+self.wait_condition.notify_all()
+return result
+finally:
+self.wait_condition.release()
+
+
+def patched_poll(self):
+self.wait_condition.acquire()
+try:
+result = self.original_poll()
+if self.returncode is not None:
+# We did complete, and we have the return value.
+# Signal the event to indicate we're done.
+self.wait_condition.notify_all()
+return result
+finally:
+self.wait_condition.release()
+
+
+def patch_up_subprocess_popen():
+subprocess.Popen.original_init = subprocess.Popen.__init__
+subprocess.Popen.__init__ = patched_init
+
+subprocess.Popen.original_wait = subprocess.Popen.wait
+subprocess.Popen.wait = patched_wait
+
+subprocess.Popen.original_poll = subprocess.Popen.poll
+subprocess.Popen.poll = patched_poll
+
+# Replace key subprocess.Popen() threading-unprotected methods with
+# threading-protected versions.
+patch_up_subprocess_popen()

Modified: lldb/trunk/test/test_runner/test/process_control_tests.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/test_runner/test/process_control_tests.py?rev=249182&r1=249181&r2=249182&view=diff
==
--- lldb/trunk/test/test_runner/test/process_control_tests.py (original)
+++ lldb/trunk/test/test_runner/test/process_control_tests.py Fri Oct  2 
15:51:11 2015
@@ -202,6 +202,9 @@ class ProcessControlTimeoutTests(Process
 """inferior exit detected when inferior children are live with shared
 stdout/stderr handles.
 """
+# Requires review D13362 or equivalent to be implemented.
+self.skipTest("http://reviews.llvm.org/D13362";)
+
 driver = TestInferiorDriver()
 
 # Create the inferior (I1), and instruct it to create a child (C1)
@@ -220,7 +223,7 @@ class ProcessControlTimeoutTests(Process
 "process failed to complete")
 
 # Ensure we didn't receive a timeout.
-self.assertTrue(
+self.assertFalse(
 driver.was_timeout, "inferior should have completed normally")
 
 self.assertEqual(


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

Re: [Lldb-commits] [PATCH] D13362: Enhance test runner timeout logic to detect successful completion of spawned process when stdout/stderr are shared to still-existing child processes

[Todd Fiala via lldb-commits]

tfiala abandoned this revision.
tfiala added a comment.

I've fixed:
https://llvm.org/bugs/show_bug.cgi?id=25019

I think for now I am not interested in trying to tackle the intent of this 
change as it unduly complicates the timeout detection logic.

I am okay with saying:
"If you run a process http://reviews.llvm.org/P1, and that process creates 
child processes C1..CN, and shares the stdout/stderr file handles from 
http://reviews.llvm.org/P1 to C1..CN, and if http://reviews.llvm.org/P1 exits, 
we don't detect the exit until all stdout/stderr handles shared with C1..CN are 
closed."  That's just a bad test if it is leaving children around.  It will 
time out.

Addressing that in Python should be possible, as I was working towards here, 
but I don't see that as being worthwhile.  If we didn't time out (as was the 
case prior to an earlier fix yesterday), that would be an issue.  But that's no 
longer the case.


http://reviews.llvm.org/D13362



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

[Todd Fiala via lldb-commits]

tfiala added a comment.

Starting to look at this now.

The code looks reasonable, trying now.


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

[Todd Fiala via lldb-commits]

tfiala added a comment.

You'll need to guard against exclusion of libedit:

  #ifndef LLDB_DISABLE_LIBEDIT
  ...
  #endif

We can't exclude this whole file if libedit is disabled, because if we don't 
have libedit, we still need this null stub in on Linux. (i.e. the existing 
behavior needs to be present if no libedit).


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

[Todd Fiala via lldb-commits]

tfiala added a comment.

I'm adjusting the patch for optional libedit.


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

[Todd Fiala via lldb-commits]

tfiala added a comment.

I'm still testing, but this seems to do the trick:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 55fdf77..dd2c440 100644

- a/CMakeLists.txt

+++ b/CMakeLists.txt
@@ -4,7 +4,14 @@ include(cmake/modules/LLDBStandalone.cmake)
 include(cmake/modules/LLDBConfig.cmake)
 include(cmake/modules/AddLLDB.cmake)

-#add_subdirectory(include)
+# We need libedit support to go down both the source and
+# the scripts directories.
+set(LLDB_DISABLE_LIBEDIT ${LLDB_DEFAULT_DISABLE_LIBEDIT} CACHE BOOL "Disables 
the use of editline.")
+if (LLDB_DISABLE_LIBEDIT)
+  add_definitions( -DLLDB_DISABLE_LIBEDIT )
+endif()
+
+# add_subdirectory(include)
 add_subdirectory(docs)
 if (NOT LLDB_DISABLE_PYTHON)

  add_subdirectory(scripts)

diff --git a/scripts/Python/modules/readline/CMakeLists.txt 
b/scripts/Python/modules/readline/CMakeLists.txt
index 11089c6..0a4376c 100644

- a/scripts/Python/modules/readline/CMakeLists.txt

+++ b/scripts/Python/modules/readline/CMakeLists.txt
@@ -7,7 +7,11 @@ SET(PYTHON_DIRECTORY 
python${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR}/site-
 include_directories(${PYTHON_INCLUDE_DIR})
 add_library(readline SHARED readline.cpp)

-target_link_libraries(readline ${PYTHON_LIBRARY})
+if (NOT LLDB_DISABLE_LIBEDIT)
+  target_link_libraries(readline ${PYTHON_LIBRARY} edit)
+else()
+  target_link_libraries(readline ${PYTHON_LIBRARY})
+endif()

1. FIXME: the LIBRARY_OUTPUT_PATH seems to be ignored - this is not a
2. functional issue for the build dir, though, since the shared lib dir

diff --git a/scripts/Python/modules/readline/readline.cpp 
b/scripts/Python/modules/readline/readline.cpp
index b0d6b74..38268f8 100644

- a/scripts/Python/modules/readline/readline.cpp

+++ b/scripts/Python/modules/readline/readline.cpp
@@ -1,23 +1,68 @@
 #include 
 #include "Python.h"

-// Python readline module intentionally built to not implement the
-// readline module interface. This is meant to work around llvm
-// pr18841 to avoid seg faults in the stock Python readline.so linked
-// against GNU readline.
+#ifndef LLDB_DISABLE_LIBEDIT
+#include 
+#endif
+
+// Simple implementation of the Python readline module using libedit.
+// In the event that libedit is excluded from the build, this turns
+// back into a null implementation that blocks the module from pulling
+// in the GNU readline shared lib, which causes linkage confusion when
+// both readline and libedit's readline compatibility symbols collide.
+//
+// Currently it only installs a PyOS_ReadlineFunctionPointer, without
+// implementing any of the readline module methods. This is meant to
+// work around LLVM pr18841 to avoid seg faults in the stock Python
+// readline.so linked against GNU readline.

static struct PyMethodDef moduleMethods[] =
 {

  {nullptr, nullptr, 0, nullptr}

};

+#ifndef LLDB_DISABLE_LIBEDIT
+PyDoc_STRVAR(
+moduleDocumentation,
+"Simple readline module implementation based on libedit.");
+#else
 PyDoc_STRVAR(

  moduleDocumentation,

- "Stub module meant to effectively disable readline support.");

+"Stub module meant to avoid linking GNU readline.");
+#endif
+
+#ifndef LLDB_DISABLE_LIBEDIT
+static char*
+simple_readline(FILE *stdin, FILE *stdout, char *prompt)
+{
+rl_instream = stdin;
+rl_outstream = stdout;
+char* line = readline(prompt);
+if (!line)
+{
+char* ret = (char*)PyMem_Malloc(1);
+if (ret != NULL)
+*ret = '\0';
+return ret;
+}
+if (*line)
+add_history(line);
+int n = strlen(line);
+char* ret = (char*)PyMem_Malloc(n + 2);
+strncpy(ret, line, n);
+free(line);
+ret[n] = '\n';
+ret[n+1] = '\0';
+return ret;
+}
+#endif

PyMODINIT_FUNC
 initreadline(void)
 {
+#ifndef LLDB_DISABLE_LIBEDIT
+PyOS_ReadlineFunctionPointer = simple_readline;
+#endif

  Py_InitModule4(
  "readline",
  moduleMethods,

diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
index 0470e51..8e85498 100644

- a/source/CMakeLists.txt

+++ b/source/CMakeLists.txt
@@ -6,11 +6,6 @@ else()

  set(LLDB_DEFAULT_DISABLE_LIBEDIT 0)

endif ()

-set(LLDB_DISABLE_LIBEDIT ${LLDB_DEFAULT_DISABLE_LIBEDIT} CACHE BOOL "Disables 
the use of editline.")
-if (LLDB_DISABLE_LIBEDIT)

- add_definitions( -DLLDB_DISABLE_LIBEDIT )

-endif()

- if ( CMAKE_SYSTEM_NAME MATCHES "Linux" ) include_directories( 
Plugins/Process/Linux


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

[Todd Fiala via lldb-commits]

tfiala added a comment.

Yep, that modified patch worked for both:

  cmake -DLLDB_DISABLE_LIBEDIT=1

where libedit is disabled from the build, and for the normal case where libedit 
is available.  In both cases it did the right thing - simple readline support 
with libedit and your change (minor tweaks by me solely on comments and 
conditional libedit), and for the no-libedit case (where it behaves as before, 
including the no-crash part :-) ).

If you can update the patch with that, it looks good to me.


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

[Todd Fiala via lldb-commits]

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

(requesting changes per the previous patch and commentary)


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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