[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

2022-04-08 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

The reason for the funny `/*override*/` thingy in the mock classes is that it 
is impossible (in the gmock version that we use) to add the override keyword to 
the methods overridden by the MOCK macros. Then, having override keywords on 
hand-written methods triggered `-Winconsistent-missing-override`.

Or at least it used to -- have you checked that these changes don't introduce 
any new compiler warnings?


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

https://reviews.llvm.org/D123340

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

[Lldb-commits] [PATCH] D123375: [lldb][intelpt] Reduce trace memory usage by grouping instructions

2022-04-08 Thread Alisamar Husain via Phabricator via lldb-commits 
zrthxn created this revision.
zrthxn added reviewers: wallace, jj10306.
Herald added a project: All.
zrthxn requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Grouping instructions with the same high 48 bits and storing prefixes. Simple 
lookup improved to get xponentially faster instruction address lookups.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123375

Files:
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h

Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -46,6 +46,8 @@
   DecodedThreadSP m_decoded_thread_sp;
   /// Internal instruction index currently pointing at.
   size_t m_pos;
+  /// Internal instruction ID currently pointing at.
+  lldb::user_id_t m_id;
   /// Tsc range covering the current instruction.
   llvm::Optional m_tsc_range;
 };
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,8 +23,9 @@
   assert(m_decoded_thread_sp->GetInstructionsCount() > 0 &&
  "a trace should have at least one instruction or error");
   m_pos = m_decoded_thread_sp->GetInstructionsCount() - 1;
+  m_id = m_decoded_thread_sp->ToID(m_pos);
   m_tsc_range =
-  m_decoded_thread_sp->CalculateTscRange(m_pos, /*hint_range*/ None);
+  m_decoded_thread_sp->CalculateTscRangeByIndex(m_pos, /*hint_range*/ None);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -42,6 +43,7 @@
 
   while (canMoveOne()) {
 m_pos += IsForwards() ? 1 : -1;
+m_id = m_decoded_thread_sp->ToID(m_pos);
 
 if (m_tsc_range && !m_tsc_range->InRange(m_pos))
   m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev();
@@ -81,12 +83,14 @@
   };
 
   int64_t dist = FindDistanceAndSetPos();
-  m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos, m_tsc_range);
+  m_id = m_decoded_thread_sp->ToID(m_pos);
+  m_tsc_range =
+  m_decoded_thread_sp->CalculateTscRangeByIndex(m_pos, m_tsc_range);
   return dist;
 }
 
 bool TraceCursorIntelPT::IsError() {
-  return m_decoded_thread_sp->IsInstructionAnError(m_pos);
+  return m_decoded_thread_sp->IsInstructionAnError(m_id);
 }
 
 const char *TraceCursorIntelPT::GetError() {
@@ -94,7 +98,7 @@
 }
 
 lldb::addr_t TraceCursorIntelPT::GetLoadAddress() {
-  return m_decoded_thread_sp->GetInstructionLoadAddress(m_pos);
+  return m_decoded_thread_sp->GetInstructionLoadAddress(m_id);
 }
 
 Optional
@@ -110,16 +114,19 @@
 
 TraceInstructionControlFlowType
 TraceCursorIntelPT::GetInstructionControlFlowType() {
-  return m_decoded_thread_sp->GetInstructionControlFlowType(m_pos);
+  return m_decoded_thread_sp->GetInstructionControlFlowType(m_id);
 }
 
 bool TraceCursorIntelPT::GoToId(user_id_t id) {
-  if (m_decoded_thread_sp->GetInstructionsCount() <= id)
+  size_t idx = m_decoded_thread_sp->ToIndex(id);
+  if (m_decoded_thread_sp->GetInstructionsCount() <= idx)
 return false;
-  m_pos = id;
-  m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos, m_tsc_range);
+  m_pos = idx;
+  m_id = id;
+  m_tsc_range =
+  m_decoded_thread_sp->CalculateTscRangeByIndex(m_pos, m_tsc_range);
 
   return true;
 }
 
-user_id_t TraceCursorIntelPT::GetId() const { return m_pos; }
+user_id_t TraceCursorIntelPT::GetId() const { return m_id; }
Index: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
@@ -197,6 +197,8 @@
   AppendInstruction(decoded_thread, insn, tsc_info);
 }
   }
+
+  decoded_thread.CalcIPBlockSizes();
 }
 
 /// Callback used by libipt for reading the process memory.
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -109,6 +109,31 @@
 void RecordError(int libipt_error_code);
   };
 
+  /// \struct InstructionPointer
+  /// Structure that stores a 6 byte prefix a list of 2 byte suffixes for each
+  /// instruction that is appended to the trace, in order. Concatenating the
+  /// prefix and suffix for any instruction ID (which is the concatenation of
+  /// two indices) will give the instruction load address.
+  ///
+  /// Generally most consecutive inst

[Lldb-commits] [PATCH] D123375: [lldb][intelpt] Reduce trace memory usage by grouping instructions

2022-04-08 Thread Alisamar Husain via Phabricator via lldb-commits 
zrthxn added a comment.

Example of improvement in memory usage

  thread #1: tid = 42805
Raw trace size: 4096 KiB
Total number of instructions: 94
Total approximate memory usage: 4394.58 KiB
Average memory usage per instruction: 5.00 bytes

where previously the same trace took ~12 KiB and 13 bytes per instruction


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123375

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

[Lldb-commits] [PATCH] D123358: [trace][intelpt] Remove code smell when printing the raw trace size

2022-04-08 Thread Jakob Johnson via Phabricator via lldb-commits 
jj10306 added a comment.

Looks good. Any reason not to have `GetRawTraceSize()` at the `Trace` interface 
level instead of being specific to `TraceIntelPT`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123358

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

[Lldb-commits] [PATCH] D118794: [lldb] Remove non-address bits from read/write addresses in lldb

2022-04-08 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid added a comment.

Thanks for good detailed explanation. I think from the code readability point 
of view, we may use FixAddress function which i believe already exists in ABI 
and if not then introduce FixAnyAddress may be. We can put all the comments 
about PAC/TBI code vs data address bits there in AArch64 ABI code instead of 
putting a comment about code/data address everytime we use FixDataAddress in 
generic code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

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

[Lldb-commits] [PATCH] D118794: [lldb] Remove non-address bits from read/write addresses in lldb

2022-04-08 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett planned changes to this revision.
DavidSpickett added a comment.

Cool. I will apply this to existing code first, in another change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

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

[Lldb-commits] [PATCH] D123401: [lldb] Fix debug_info decorators for NO_DEBUG_INFO_TESTCASE

2022-04-08 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

Currently a NO_DEBUG_INFO_TESTCASE will return None as it's debug info
type, causing the decorator to consider it a match. If such a test case
is marked as an expected failure, the decorator will incorrectly cause
an XPASS.

I discovered this issue when I was trying to speed up our remote test
suite by running every test as a single variant.


https://reviews.llvm.org/D123401

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/test/API/test_utils/TestDecorators.py


Index: lldb/test/API/test_utils/TestDecorators.py
===
--- /dev/null
+++ lldb/test/API/test_utils/TestDecorators.py
@@ -0,0 +1,13 @@
+from lldbsuite.test.lldbtest import Base
+from lldbsuite.test.decorators import *
+
+
+class TestDecorators(Base):
+
+mydir = Base.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+@expectedFailureAll(debug_info="dwarf")
+def test_decorator_skip_no_debug_info(self):
+"""Test that specifying a debug info category works for a 
NO_DEBUG_INFO_TESTCASE"""
+pass
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -183,7 +183,7 @@
 skip_for_arch = _match_decorator_property(
 archs, self.getArchitecture())
 skip_for_debug_info = _match_decorator_property(
-debug_info, self.getDebugInfo())
+debug_info, self.getDebugInfo() if self.getDebugInfo() else "")
 skip_for_triple = _match_decorator_property(
 triple, lldb.selected_platform.GetTriple())
 skip_for_remote = _match_decorator_property(


Index: lldb/test/API/test_utils/TestDecorators.py
===
--- /dev/null
+++ lldb/test/API/test_utils/TestDecorators.py
@@ -0,0 +1,13 @@
+from lldbsuite.test.lldbtest import Base
+from lldbsuite.test.decorators import *
+
+
+class TestDecorators(Base):
+
+mydir = Base.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+@expectedFailureAll(debug_info="dwarf")
+def test_decorator_skip_no_debug_info(self):
+"""Test that specifying a debug info category works for a NO_DEBUG_INFO_TESTCASE"""
+pass
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -183,7 +183,7 @@
 skip_for_arch = _match_decorator_property(
 archs, self.getArchitecture())
 skip_for_debug_info = _match_decorator_property(
-debug_info, self.getDebugInfo())
+debug_info, self.getDebugInfo() if self.getDebugInfo() else "")
 skip_for_triple = _match_decorator_property(
 triple, lldb.selected_platform.GetTriple())
 skip_for_remote = _match_decorator_property(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123401: [lldb] Fix debug_info decorators for NO_DEBUG_INFO_TESTCASE

2022-04-08 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

Good catch! LGTM!


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

https://reviews.llvm.org/D123401

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

[Lldb-commits] [PATCH] D123401: [lldb] Fix debug_info decorators for NO_DEBUG_INFO_TESTCASE

2022-04-08 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

I think this is actually a wider problem. I also ran into this recently (with 
triples), but didn't have time to create a patch yet.  I don't think that 
`None` as the "actual" value should ever be considered a match (except when the 
pattern is `None`, I guess), regardless of whether it is a debug info variant, 
triple, or anything else. (`None` as a "pattern" should of course continue to 
match anything.)


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

https://reviews.llvm.org/D123401

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

[Lldb-commits] [PATCH] D123401: [lldb] Fix debug_info decorators for NO_DEBUG_INFO_TESTCASE

2022-04-08 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D123401#3439560 , @labath wrote:

> I think this is actually a wider problem. I also ran into this recently (with 
> triples), but didn't have time to create a patch yet.  I don't think that 
> `None` as the "actual" value should ever be considered a match (except when 
> the pattern is `None`, I guess), regardless of whether it is a debug info 
> variant, triple, or anything else. (`None` as a "pattern" should of course 
> continue to match anything.)

Agreed. I tried that but that caused a bunch of test failures. It's probably 
worth investigating, but my current focus is addressing some issues with our 
on-device test suite.


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

https://reviews.llvm.org/D123401

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

[Lldb-commits] [lldb] af2ea18 - [lldb] Import Foundation in TestConflictingDefinition.py

2022-04-08 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-08T10:46:34-07:00
New Revision: af2ea183f5e3301406724bba116a7e200f108b79

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

LOG: [lldb] Import Foundation in TestConflictingDefinition.py

We need to import foundation to get a 'NSLog' declaration when building
against the iOS SDK. This doesn't appear necessary when building against
the macOS SDK, presumable because it gets transitively imported by
objc/NSObject.h

Added: 


Modified: 
lldb/test/API/lang/objc/conflicting-definition/Test/Test.m
lldb/test/API/lang/objc/conflicting-definition/TestExt/TestExt.m

Removed: 




diff  --git a/lldb/test/API/lang/objc/conflicting-definition/Test/Test.m 
b/lldb/test/API/lang/objc/conflicting-definition/Test/Test.m
index 6b2cb3af80865..34de8b9715966 100644
--- a/lldb/test/API/lang/objc/conflicting-definition/Test/Test.m
+++ b/lldb/test/API/lang/objc/conflicting-definition/Test/Test.m
@@ -1,3 +1,5 @@
+#import 
+
 #import "Test.h"
 
 @implementation Test

diff  --git a/lldb/test/API/lang/objc/conflicting-definition/TestExt/TestExt.m 
b/lldb/test/API/lang/objc/conflicting-definition/TestExt/TestExt.m
index a14c702787dba..163fde4312f2b 100644
--- a/lldb/test/API/lang/objc/conflicting-definition/TestExt/TestExt.m
+++ b/lldb/test/API/lang/objc/conflicting-definition/TestExt/TestExt.m
@@ -1,3 +1,5 @@
+#import 
+
 #import "TestExt.h"
 #import "Foo.h"
 



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

[Lldb-commits] [lldb] 257f984 - [lldb] Fix TestQuoting when run remotely

2022-04-08 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-08T10:46:34-07:00
New Revision: 257f98466222c1fba3d3f6c0afa5e93793d14109

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

LOG: [lldb] Fix TestQuoting when run remotely

Added: 


Modified: 
lldb/test/API/commands/settings/quoting/TestQuoting.py

Removed: 




diff  --git a/lldb/test/API/commands/settings/quoting/TestQuoting.py 
b/lldb/test/API/commands/settings/quoting/TestQuoting.py
index 04b57c8fbd767..b070d26664cf5 100644
--- a/lldb/test/API/commands/settings/quoting/TestQuoting.py
+++ b/lldb/test/API/commands/settings/quoting/TestQuoting.py
@@ -50,16 +50,17 @@ def expect_args(self, args_in, args_out):
 to stdout. Compare the stdout with args_out."""
 
 filename = SettingsCommandTestCase.output_file_name
+outfile = self.getBuildArtifact(filename)
 
 if lldb.remote_platform:
-outfile = lldb.remote_platform.GetWorkingDirectory() + filename
+outfile_arg = 
os.path.join(lldb.remote_platform.GetWorkingDirectory(), filename)
 else:
-outfile = self.getBuildArtifact(filename)
+outfile_arg = outfile
 
-self.runCmd("process launch -- %s %s" % (outfile, args_in))
+self.runCmd("process launch -- %s %s" % (outfile_arg, args_in))
 
 if lldb.remote_platform:
-src_file_spec = lldb.SBFileSpec(outfile, False)
+src_file_spec = lldb.SBFileSpec(outfile_arg, False)
 dst_file_spec = lldb.SBFileSpec(outfile, True)
 lldb.remote_platform.Get(src_file_spec, dst_file_spec)
 
@@ -67,5 +68,4 @@ def expect_args(self, args_in, args_out):
 output = f.read()
 
 self.RemoveTempFile(outfile)
-
 self.assertEqual(output, args_out)



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

[Lldb-commits] [lldb] 66b829a - [lldb] Skip a bunch of tests that shouldn't run remotely

2022-04-08 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-08T10:46:34-07:00
New Revision: 66b829ac7b6864f4546771668739d80fa34a7e17

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

LOG: [lldb] Skip a bunch of tests that shouldn't run remotely

Skip a bunch of tests that don't really make sense to run remotely.

Added: 


Modified: 

lldb/test/API/commands/expression/import-std-module/missing-module-sources/TestStdModuleSourcesMissing.py
lldb/test/API/commands/platform/basic/TestPlatformCommand.py
lldb/test/API/commands/platform/basic/TestPlatformPython.py
lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py
lldb/test/API/python_api/process/TestProcessAPI.py
lldb/test/API/python_api/sbmodule/TestSBModule.py
lldb/test/API/python_api/target/TestTargetAPI.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/import-std-module/missing-module-sources/TestStdModuleSourcesMissing.py
 
b/lldb/test/API/commands/expression/import-std-module/missing-module-sources/TestStdModuleSourcesMissing.py
index a74e0e7a09c7a..9e11ec62152a2 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/missing-module-sources/TestStdModuleSourcesMissing.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/missing-module-sources/TestStdModuleSourcesMissing.py
@@ -18,6 +18,7 @@ class TestCase(TestBase):
 # test configurations where libc++ is actually supposed to be tested.
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
+@skipIfRemote
 def test(self):
 # The path to our temporary target root that contains the temporary
 # module sources.

diff  --git a/lldb/test/API/commands/platform/basic/TestPlatformCommand.py 
b/lldb/test/API/commands/platform/basic/TestPlatformCommand.py
index 1c981e4bddd92..bc4825ae428cc 100644
--- a/lldb/test/API/commands/platform/basic/TestPlatformCommand.py
+++ b/lldb/test/API/commands/platform/basic/TestPlatformCommand.py
@@ -97,6 +97,7 @@ def test_shell_timeout(self):
 "error: timed out waiting for shell command to complete"])
 
 @no_debug_info_test
+@skipIfRemote
 def test_host_shell_interpreter(self):
 """ Test the host platform shell with a 
diff erent interpreter """
 self.build()

diff  --git a/lldb/test/API/commands/platform/basic/TestPlatformPython.py 
b/lldb/test/API/commands/platform/basic/TestPlatformPython.py
index 1a6f6f33e1df9..46f0d0c9503bb 100644
--- a/lldb/test/API/commands/platform/basic/TestPlatformPython.py
+++ b/lldb/test/API/commands/platform/basic/TestPlatformPython.py
@@ -82,6 +82,7 @@ def test_available_platform_list(self):
 
 @add_test_categories(['pyapi'])
 @no_debug_info_test
+@skipIfRemote
 def test_shell_interpreter(self):
 """ Test a shell with a custom interpreter """
 platform = self.dbg.GetSelectedPlatform()

diff  --git a/lldb/test/API/commands/platform/sdk/TestPlatformSDK.py 
b/lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
index 8bf950a1bd3c3..91ad5a446123e 100644
--- a/lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
+++ b/lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
@@ -43,6 +43,7 @@ def port_not_available(self):
 @skipUnlessDarwin
 @expectedFailureIfFn(no_debugserver)
 @expectedFailureIfFn(port_not_available)
+@skipIfRemote
 def test_macos_sdk(self):
 self.build()
 

diff  --git a/lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py 
b/lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py
index c89fbccb1e39f..86a4d808a167f 100644
--- a/lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py
+++ b/lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py
@@ -23,6 +23,7 @@ class TestDetachVrsProfile(TestBase):
 
 @skipUnlessDarwin
 @skipIfOutOfTreeDebugserver
+@skipIfRemote
 def test_profile_and_detach(self):
 """There can be many tests in a test case - describe this test here."""
 self.build()

diff  --git a/lldb/test/API/python_api/process/TestProcessAPI.py 
b/lldb/test/API/python_api/process/TestProcessAPI.py
index bfa1a22b9184d..6d1b3e4958837 100644
--- a/lldb/test/API/python_api/process/TestProcessAPI.py
+++ b/lldb/test/API/python_api/process/TestProcessAPI.py
@@ -320,6 +320,7 @@ def test_get_num_supported_hardware_watchpoints(self):
 print("Number of supported hardware watchpoints: %d" % num)
 
 @no_debug_info_test
+@skipIfRemote
 def test_get_process_info(self):
 """Test SBProcess::GetProcessInfo() API with a locally launched 
process."""
 self.build()

diff  --git a/lldb/test/API/python_api/sbmodule/TestSBModule.py 
b/lldb/test/API/python_api

[Lldb-commits] [PATCH] D123415: [lldb] Fix crash when a type has no definition

2022-04-08 Thread Sigurður Ásgeirsson via Phabricator via lldb-commits 
siggi-alpheus created this revision.
siggi-alpheus added reviewers: labath, jasonmolenda.
siggi-alpheus added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
siggi-alpheus requested review of this revision.
Herald added a subscriber: lldb-commits.

See issue 54761 .
It looks like Google Chrome's symbols contain some member functions whose this 
pointer type is missing, which causes a nullptr access and a crash. This patch 
makes lldb tolerant of this problem by avoiding the nullptr access.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123415

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7571,7 +7571,8 @@
   clang::CXXRecordDecl *cxx_record_decl =
   record_qual_type->getAsCXXRecordDecl();
 
-  if (cxx_record_decl == nullptr)
+  // Bail if no declaration or if the declaration has no definition.
+  if (cxx_record_decl == nullptr || !cxx_record_decl->hasDefinition())
 return nullptr;
 
   clang::QualType method_qual_type(ClangUtil::GetQualType(method_clang_type));


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7571,7 +7571,8 @@
   clang::CXXRecordDecl *cxx_record_decl =
   record_qual_type->getAsCXXRecordDecl();
 
-  if (cxx_record_decl == nullptr)
+  // Bail if no declaration or if the declaration has no definition.
+  if (cxx_record_decl == nullptr || !cxx_record_decl->hasDefinition())
 return nullptr;
 
   clang::QualType method_qual_type(ClangUtil::GetQualType(method_clang_type));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123421: Handle the new "selector-specific stub" ObjC Method dispatch method

2022-04-08 Thread Jim Ingham via Phabricator via lldb-commits 
jingham created this revision.
jingham added a reviewer: JDevlieghere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Clang is adding a feature to ObjC code generation, where instead of calling
objc_msgSend directly with an object & selector, it generates a stub that
gets passed only the object and the stub figures out the selector.

  

This patch adds support for following that dispatch method into the 
implementation
function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123421

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.h
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
@@ -56,7 +56,7 @@
 ' 21 key/value pairs'
 ])
 
-lldbutil.run_break_set_by_regexp(self, 'setAtoms')
+lldbutil.run_break_set_by_symbol(self, '-[Molecule setAtoms:]')
 
 self.runCmd("continue")
 self.expect("frame variable _cmd", substrs=['setAtoms:'])
Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -22,6 +22,7 @@
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Symbol/Type.h"
 #include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-private.h"
 
 class CommandObjectObjC_ClassTable_Dump;
@@ -242,11 +243,19 @@
 
   virtual bool HasReadObjCLibrary() = 0;
 
+  // These two methods actually use different caches.  The only time we'll
+  // cache a sel_str is if we found a "selector specific stub" for the selector
+  // and conversely we only add to the SEL cache if we saw a regular dispatch.
   lldb::addr_t LookupInMethodCache(lldb::addr_t class_addr, lldb::addr_t sel);
+  lldb::addr_t LookupInMethodCache(lldb::addr_t class_addr,
+   llvm::StringRef sel_str);
 
   void AddToMethodCache(lldb::addr_t class_addr, lldb::addr_t sel,
 lldb::addr_t impl_addr);
 
+  void AddToMethodCache(lldb::addr_t class_addr, llvm::StringRef sel_str,
+lldb::addr_t impl_addr);
+
   TypeAndOrName LookupInClassNameCache(lldb::addr_t class_addr);
 
   void AddToClassNameCache(lldb::addr_t class_addr, const char *name,
@@ -343,20 +352,22 @@
   }
 
 private:
-  // We keep a map of ->Implementation so we don't have to call
-  // the resolver function over and over.
+  // We keep two maps of ->Implementation so we don't have
+  // to call the resolver function over and over.
+  // The first comes from regular obj_msgSend type dispatch, and maps the
+  // class + uniqued SEL value to an implementation.
+  // The second comes from the "selector-specific stubs", which are always
+  // of the form _objc_msgSend$SelectorName, so we don't know the uniqued
+  // selector, only the string name.
 
   // FIXME: We need to watch for the loading of Protocols, and flush the cache
   // for any
   // class that we see so changed.
 
   struct ClassAndSel {
-ClassAndSel() {
-  sel_addr = LLDB_INVALID_ADDRESS;
-  class_addr = LLDB_INVALID_ADDRESS;
-}
+ClassAndSel() = default;
 
-ClassAndSel(lldb::addr_t in_sel_addr, lldb::addr_t in_class_addr)
+ClassAndSel(lldb::addr_t in_class_addr, lldb::addr_t in_sel_addr)
 : class_addr(in_class_addr), sel_addr(in_sel_addr) {}
 
 bool operator==(const ClassAndSel &rhs) {
@@ -379,11 +390,39 @@
   }
 }
 
-lldb::addr_t class_addr;
-lldb::addr_t sel_addr;
+lldb::addr_t class_addr = LLDB_INVALID_ADDRESS;
+lldb::addr_t sel_addr = LLDB_INVALID_ADDRESS;
+  };
+
+  struct ClassAndSelStr {
+ClassAndSelStr() = default;
+
+ClassAndSelStr(lldb::addr_t in_class_addr, llvm::StringRef in_sel_name)
+: class_addr(in_class_addr), sel_name(in_sel_name) {}
+
+bool operator==(const ClassAndSelStr &rhs)

[Lldb-commits] [PATCH] D123421: Handle the new "selector-specific stub" ObjC Method dispatch method

2022-04-08 Thread Jim Ingham via Phabricator via lldb-commits 
jingham updated this revision to Diff 421638.
jingham added a comment.

-U on the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123421

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.h
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
@@ -56,7 +56,7 @@
 ' 21 key/value pairs'
 ])
 
-lldbutil.run_break_set_by_regexp(self, 'setAtoms')
+lldbutil.run_break_set_by_symbol(self, '-[Molecule setAtoms:]')
 
 self.runCmd("continue")
 self.expect("frame variable _cmd", substrs=['setAtoms:'])
Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -22,6 +22,7 @@
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Symbol/Type.h"
 #include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-private.h"
 
 class CommandObjectObjC_ClassTable_Dump;
@@ -242,11 +243,19 @@
 
   virtual bool HasReadObjCLibrary() = 0;
 
+  // These two methods actually use different caches.  The only time we'll
+  // cache a sel_str is if we found a "selector specific stub" for the selector
+  // and conversely we only add to the SEL cache if we saw a regular dispatch.
   lldb::addr_t LookupInMethodCache(lldb::addr_t class_addr, lldb::addr_t sel);
+  lldb::addr_t LookupInMethodCache(lldb::addr_t class_addr,
+   llvm::StringRef sel_str);
 
   void AddToMethodCache(lldb::addr_t class_addr, lldb::addr_t sel,
 lldb::addr_t impl_addr);
 
+  void AddToMethodCache(lldb::addr_t class_addr, llvm::StringRef sel_str,
+lldb::addr_t impl_addr);
+
   TypeAndOrName LookupInClassNameCache(lldb::addr_t class_addr);
 
   void AddToClassNameCache(lldb::addr_t class_addr, const char *name,
@@ -343,20 +352,22 @@
   }
 
 private:
-  // We keep a map of ->Implementation so we don't have to call
-  // the resolver function over and over.
+  // We keep two maps of ->Implementation so we don't have
+  // to call the resolver function over and over.
+  // The first comes from regular obj_msgSend type dispatch, and maps the
+  // class + uniqued SEL value to an implementation.
+  // The second comes from the "selector-specific stubs", which are always
+  // of the form _objc_msgSend$SelectorName, so we don't know the uniqued
+  // selector, only the string name.
 
   // FIXME: We need to watch for the loading of Protocols, and flush the cache
   // for any
   // class that we see so changed.
 
   struct ClassAndSel {
-ClassAndSel() {
-  sel_addr = LLDB_INVALID_ADDRESS;
-  class_addr = LLDB_INVALID_ADDRESS;
-}
+ClassAndSel() = default;
 
-ClassAndSel(lldb::addr_t in_sel_addr, lldb::addr_t in_class_addr)
+ClassAndSel(lldb::addr_t in_class_addr, lldb::addr_t in_sel_addr)
 : class_addr(in_class_addr), sel_addr(in_sel_addr) {}
 
 bool operator==(const ClassAndSel &rhs) {
@@ -379,11 +390,39 @@
   }
 }
 
-lldb::addr_t class_addr;
-lldb::addr_t sel_addr;
+lldb::addr_t class_addr = LLDB_INVALID_ADDRESS;
+lldb::addr_t sel_addr = LLDB_INVALID_ADDRESS;
+  };
+
+  struct ClassAndSelStr {
+ClassAndSelStr() = default;
+
+ClassAndSelStr(lldb::addr_t in_class_addr, llvm::StringRef in_sel_name)
+: class_addr(in_class_addr), sel_name(in_sel_name) {}
+
+bool operator==(const ClassAndSelStr &rhs) {
+  if (class_addr == rhs.class_addr && sel_name == rhs.sel_name)
+return true;
+  else
+return false;
+}
+
+bool operator<(const ClassAndSelStr &rhs) const {
+  if (class_addr < rhs.class_addr)
+return true;
+  else if (class_addr > rhs.class_addr)
+return false;
+  else {
+return ConstString::Compare(se

[Lldb-commits] [PATCH] D123421: Handle the new "selector-specific stub" ObjC Method dispatch method

2022-04-08 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:52-53
+return_struct.impl_addr =
+class_getMethodImplementation_stret (return_struct.class_addr,
+ return_struct.sel_addr);
+  } else {

Nit: Seems like the formatting is a bit off. I would copy/paste this code in a 
separate file, run clang-format over it, and paste it back. 



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:637-643
 // Also we will use the version of the lookup code that doesn't rely on the
 // stret version of the function.
-m_lookup_implementation_function_code =
-g_lookup_implementation_no_stret_function_code;
+m_lookup_implementation_function_code.append(
+g_lookup_implementation_no_stret_function_code);
   } else {
-m_lookup_implementation_function_code =
-g_lookup_implementation_with_stret_function_code;
+m_lookup_implementation_function_code.append(
+g_lookup_implementation_with_stret_function_code);

I don't understand why we need to have two versions of the code. IIUC, if 
there's a "stret return lookup function", then if we pass `is_stret == true` 
we'll use it. Otherwise we'll unconditionally use the straight lookup. Why do 
we need two variants of the code at all? Can't we pass false to `is_stret` and 
achieve the same result with the 
`g_lookup_implementation_with_stret_function_code` variant? 



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:816-818
+if (curr_sym) {
+  sym_name = curr_sym->GetName().GetStringRef();
+}

Nit: for consistency with the line below



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:1012-1015
+if (log) {
+  LLDB_LOG(log, "Resolving call for class - {0} and selector - {1}",
+   isa_addr, sel_addr);
+}

The `if (log)` check is redundant. It's the purpose of the macro to expand to 
that that. Same below.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:1074
+if (sel_str_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
+  if (log)
+LLDB_LOG(log,

redundant



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.h:55-65
+  // These hold the code for the function that finds the implementation of
+  // an ObjC message send given the class & selector and the kind of dispatch.
+  // There are two variants depending on whether the platform uses a separate
+  // _stret passing convention (e.g. Intel) or not (e.g. ARM).  The difference
+  // is only at the very end of the function, so the code is broken into the
+  // common prefix and the suffix, which get composed appropriately before
+  // the function gets compiled.

Nit: I think these should be doxygen comments, so ///. You can group them like 
this:



Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp:82
+  Log *log = GetLog(LLDBLog::Step);
+  if (log) {
+LLDB_LOG(log, "Caching: class {0} selector {1} implementation {2}.",

redundant



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h:404-407
+  if (class_addr == rhs.class_addr && sel_name == rhs.sel_name)
+return true;
+  else
+return false;

```
return class_addr == rhs.class_addr && sel_name == rhs.sel_name;
```



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h:411-417
+  if (class_addr < rhs.class_addr)
+return true;
+  else if (class_addr > rhs.class_addr)
+return false;
+  else {
+return ConstString::Compare(sel_name, rhs.sel_name);
+  }

```
  if (class_addr < rhs.class_addr)
return true;
  if (class_addr > rhs.class_addr)
return false;
  return ConstString::Compare(sel_name, rhs.sel_name);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123421

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

[Lldb-commits] [lldb] 065e3c9 - [lldb] Skip more tests that don't make sense to run remotely

2022-04-08 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-08T15:02:22-07:00
New Revision: 065e3c9a8e55aacf06eb069d8916597ce13b36b5

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

LOG: [lldb] Skip more tests that don't make sense to run remotely

Skip another batch of tests that don't really make sense to run
remotely.

Added: 


Modified: 
lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py

lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py

lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
lldb/test/API/macosx/stack-corefile/TestStackCorefile.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py 
b/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
index 32b78391aa22d..c745cc4ac7e0a 100644
--- a/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
+++ b/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
@@ -40,20 +40,24 @@ def port_not_available(self):
 return None
 
 @skipUnlessDarwin
+@skipIfRemote
 def test_stop_default_platform_sync(self):
 self.do_test_stop_at_entry(True, False)
 
 @skipUnlessDarwin
+@skipIfRemote
 def test_stop_default_platform_async(self):
 self.do_test_stop_at_entry(False, False)
 
 @skipUnlessDarwin
+@skipIfRemote
 @expectedFailureIfFn(no_debugserver)
 @expectedFailureIfFn(port_not_available)
 def test_stop_remote_platform_sync(self):
 self.do_test_stop_at_entry(True, True)
 
 @skipUnlessDarwin
+@skipIfRemote
 @expectedFailureIfFn(no_debugserver)
 @expectedFailureIfFn(port_not_available)
 def test_stop_remote_platform_async(self):

diff  --git 
a/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
 
b/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
index 9f1c055c7ee96..5556c9a36b696 100644
--- 
a/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
+++ 
b/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
@@ -40,6 +40,7 @@ def get_module_with_name(self, target, name):
 
 @skipUnlessDarwin
 @skipIfOutOfTreeDebugserver
+@skipIfRemote
 def test_launch_scripted_process_stack_frames(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""

diff  --git 
a/lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py 
b/lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
index 0468685110be5..33f4839fddc4e 100644
--- 
a/lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
+++ 
b/lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
@@ -17,6 +17,7 @@ class TestCorefileExceptionReason(TestBase):
 @no_debug_info_test
 @skipUnlessDarwin
 @skipIf(archs=no_match(['arm64','arm64e']))
+@skipIfRemote
 def test(self):
 
 corefile = self.getBuildArtifact("process.core")

diff  --git a/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py 
b/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
index 4257aa5ca26a5..1df04988d3a38 100644
--- a/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
+++ b/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
@@ -19,6 +19,7 @@ class TestSkinnyCorefile(TestBase):
 @skipIfOutOfTreeDebugserver  # newer debugserver required for these 
qMemoryRegionInfo types
 @skipIf(debug_info=no_match(["dsym"]), bugnumber="This test is looking 
explicitly for a dSYM")
 @skipUnlessDarwin
+@skipIfRemote
 def test_lc_note(self):
 self.build()
 self.aout_exe = self.getBuildArtifact("a.out")

diff  --git a/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py 
b/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
index b1c0fa98712e3..04a87c3cb9e0b 100644
--- a/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
+++ b/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
@@ -16,6 +16,7 @@ class TestStackCorefile(TestBase):
 @skipIfOutOfTreeDebugserver  # newer debugserver required for these 
qMemoryRegionInfo types
 @no_debug_info_test
 @skipUnlessDarwin
+@skipIfRemote
 def test(self):
 
 corefile = self.getBuildArtifact("process.core")



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

[Lldb-commits] [lldb] de2ddc8 - [lldb] XFAIL tests that aren't passing remotely

2022-04-08 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-08T15:02:26-07:00
New Revision: de2ddc8f3146bd87152ea86b533541039541efe1

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

LOG: [lldb] XFAIL tests that aren't passing remotely

XFAIL a series of tests that are failing remotely.

Added: 


Modified: 
lldb/test/API/assert_messages_test/TestAssertMessages.py
lldb/test/API/functionalities/archives/TestBSDArchives.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py

lldb/test/API/lang/objc/direct-dispatch-step/TestObjCDirectDispatchStepping.py
lldb/test/API/lang/objc/objc-optimized/TestObjcOptimized.py
lldb/test/API/lang/objc/objc-stepping/TestObjCStepping.py

Removed: 




diff  --git a/lldb/test/API/assert_messages_test/TestAssertMessages.py 
b/lldb/test/API/assert_messages_test/TestAssertMessages.py
index 93b4087aab2a4..7127311ac5b5e 100644
--- a/lldb/test/API/assert_messages_test/TestAssertMessages.py
+++ b/lldb/test/API/assert_messages_test/TestAssertMessages.py
@@ -6,6 +6,7 @@
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
 from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
 from textwrap import dedent
 
 
@@ -24,6 +25,7 @@ def assert_expect_fails_with(self, cmd, expect_args, 
expected_msg):
 else:
 self.fail("Initial expect should have raised AssertionError!")
 
+@expectedFailureAll(remote=True)
 def test_createTestTarget(self):
 try:
self.createTestTarget("doesnt_exist")

diff  --git a/lldb/test/API/functionalities/archives/TestBSDArchives.py 
b/lldb/test/API/functionalities/archives/TestBSDArchives.py
index 500c1763b6908..bb4b469cf436a 100644
--- a/lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ b/lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -24,6 +24,7 @@ def setUp(self):
 @expectedFailureAll(
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows")
+@expectedFailureAll(remote=True)
 def test(self):
 """Break inside a() and b() defined within libfoo.a."""
 self.build()

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
index 7b3572ab78aba..fe4e3ed0e1eca 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
@@ -14,6 +14,7 @@
 
 class ObjCDataFormatterKVO(ObjCDataFormatterTestCase):
 
+@expectedFailureAll(remote=True)
 def test_kvo_with_run_command(self):
 """Test the behavior of formatters when KVO is in use."""
 self.build()

diff  --git 
a/lldb/test/API/lang/objc/direct-dispatch-step/TestObjCDirectDispatchStepping.py
 
b/lldb/test/API/lang/objc/direct-dispatch-step/TestObjCDirectDispatchStepping.py
index 25aef29a94e67..4c2c9fae0d16f 100644
--- 
a/lldb/test/API/lang/objc/direct-dispatch-step/TestObjCDirectDispatchStepping.py
+++ 
b/lldb/test/API/lang/objc/direct-dispatch-step/TestObjCDirectDispatchStepping.py
@@ -21,6 +21,7 @@ def setUp(self):
 self.main_source = lldb.SBFileSpec("stepping-tests.m")
 
 @add_test_categories(['pyapi', 'basic_process'])
+@expectedFailureAll(remote=True)
 def test_with_python_api(self):
 """Test stepping through the 'direct dispatch' optimized method 
calls."""
 self.build()

diff  --git a/lldb/test/API/lang/objc/objc-optimized/TestObjcOptimized.py 
b/lldb/test/API/lang/objc/objc-optimized/TestObjcOptimized.py
index 702a30bf43af6..3410cc6bedd1b 100644
--- a/lldb/test/API/lang/objc/objc-optimized/TestObjcOptimized.py
+++ b/lldb/test/API/lang/objc/objc-optimized/TestObjcOptimized.py
@@ -26,6 +26,7 @@ class ObjcOptimizedTestCase(TestBase):
 mymethod = "description"
 method_spec = "-[%s %s]" % (myclass, mymethod)
 
+@expectedFailureAll(remote=True)
 def test_break(self):
 """Test 'expr member' continues to work for optimized build."""
 self.build()

diff  --git a/lldb/test/API/lang/objc/objc-stepping/TestObjCStepping.py 
b/lldb/test/API/lang/objc/objc-stepping/TestObjCStepping.py
index b77a13ff38218..c974e4d7b4c66 100644
--- a/lldb/test/API/lang/objc/objc-stepping/TestObjCStepping.py
+++ b/lldb/test/API/lang/objc/objc-stepping/TestObjCStepping.py
@@ -30,6 +30,7 @@ def setUp(self):
 self.main_source, '// Step over nil should stop here.')
 
 @add_test_categories(['pyapi', 'basic_process'])
+@expectedFailureAll(remote=True)
 def test_with_python_api(self):

[Lldb-commits] [PATCH] D123426: [lldb] Don't report progress in the REPL

2022-04-08 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added a reviewer: aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

Don't report progress events in the REPL. Most of the progress events
are very debugger specific which are very useful when you're debugging,
but not so much when you're waiting for the next line to be executed.

This patch disables reporting of progress events when in REPL mode.

rdar://91502950


https://reviews.llvm.org/D123426

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Expression/REPL.cpp


Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -25,6 +25,7 @@
 REPL::REPL(LLVMCastKind kind, Target &target) : m_target(target), m_kind(kind) 
{
   // Make sure all option values have sane defaults
   Debugger &debugger = m_target.GetDebugger();
+  debugger.SetShowProgress(false);
   auto exe_ctx = debugger.GetCommandInterpreter().GetExecutionContext();
   m_format_options.OptionParsingStarting(&exe_ctx);
   m_varobj_options.OptionParsingStarting(&exe_ctx);
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -385,6 +385,12 @@
   nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
 }
 
+bool Debugger::SetShowProgress(bool show_progress) {
+  const uint32_t idx = ePropertyShowProgress;
+  return m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx,
+  show_progress);
+}
+
 llvm::StringRef Debugger::GetShowProgressAnsiPrefix() const {
   const uint32_t idx = ePropertyShowProgressAnsiPrefix;
   return m_collection_sp->GetPropertyAtIndexAsString(nullptr, idx, "");
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -301,6 +301,8 @@
 
   bool GetShowProgress() const;
 
+  bool SetShowProgress(bool show_progress);
+
   llvm::StringRef GetShowProgressAnsiPrefix() const;
 
   llvm::StringRef GetShowProgressAnsiSuffix() const;


Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -25,6 +25,7 @@
 REPL::REPL(LLVMCastKind kind, Target &target) : m_target(target), m_kind(kind) {
   // Make sure all option values have sane defaults
   Debugger &debugger = m_target.GetDebugger();
+  debugger.SetShowProgress(false);
   auto exe_ctx = debugger.GetCommandInterpreter().GetExecutionContext();
   m_format_options.OptionParsingStarting(&exe_ctx);
   m_varobj_options.OptionParsingStarting(&exe_ctx);
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -385,6 +385,12 @@
   nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
 }
 
+bool Debugger::SetShowProgress(bool show_progress) {
+  const uint32_t idx = ePropertyShowProgress;
+  return m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx,
+  show_progress);
+}
+
 llvm::StringRef Debugger::GetShowProgressAnsiPrefix() const {
   const uint32_t idx = ePropertyShowProgressAnsiPrefix;
   return m_collection_sp->GetPropertyAtIndexAsString(nullptr, idx, "");
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -301,6 +301,8 @@
 
   bool GetShowProgress() const;
 
+  bool SetShowProgress(bool show_progress);
+
   llvm::StringRef GetShowProgressAnsiPrefix() const;
 
   llvm::StringRef GetShowProgressAnsiSuffix() const;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 0bc9372 - Skip test on earlier clang versions

2022-04-08 Thread Adrian Prantl via lldb-commits 

Author: Adrian Prantl
Date: 2022-04-08T15:40:57-07:00
New Revision: 0bc9372fa74ad0cd4789b4485bade86570909718

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

LOG: Skip test on earlier clang versions

Added: 


Modified: 
lldb/test/API/iohandler/sigint/TestProcessIOHandlerInterrupt.py

Removed: 




diff  --git a/lldb/test/API/iohandler/sigint/TestProcessIOHandlerInterrupt.py 
b/lldb/test/API/iohandler/sigint/TestProcessIOHandlerInterrupt.py
index b1d93b6a30b1c..e4fe77de8f076 100644
--- a/lldb/test/API/iohandler/sigint/TestProcessIOHandlerInterrupt.py
+++ b/lldb/test/API/iohandler/sigint/TestProcessIOHandlerInterrupt.py
@@ -13,6 +13,7 @@ class TestCase(PExpectTest):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIf(compiler="clang", compiler_version=['<', '11.0'])
 @skipIf(oslist=["linux"], archs=["arm", "aarch64"])
 def test(self):
 self.build(dictionary={"CXX_SOURCES":"cat.cpp"})



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

[Lldb-commits] [PATCH] D123421: Handle the new "selector-specific stub" ObjC Method dispatch method

2022-04-08 Thread Jim Ingham via Phabricator via lldb-commits 
jingham updated this revision to Diff 421651.
jingham added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123421

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.h
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py
@@ -56,7 +56,7 @@
 ' 21 key/value pairs'
 ])
 
-lldbutil.run_break_set_by_regexp(self, 'setAtoms')
+lldbutil.run_break_set_by_symbol(self, '-[Molecule setAtoms:]')
 
 self.runCmd("continue")
 self.expect("frame variable _cmd", substrs=['setAtoms:'])
Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -22,6 +22,7 @@
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Symbol/Type.h"
 #include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-private.h"
 
 class CommandObjectObjC_ClassTable_Dump;
@@ -242,11 +243,19 @@
 
   virtual bool HasReadObjCLibrary() = 0;
 
+  // These two methods actually use different caches.  The only time we'll
+  // cache a sel_str is if we found a "selector specific stub" for the selector
+  // and conversely we only add to the SEL cache if we saw a regular dispatch.
   lldb::addr_t LookupInMethodCache(lldb::addr_t class_addr, lldb::addr_t sel);
+  lldb::addr_t LookupInMethodCache(lldb::addr_t class_addr,
+   llvm::StringRef sel_str);
 
   void AddToMethodCache(lldb::addr_t class_addr, lldb::addr_t sel,
 lldb::addr_t impl_addr);
 
+  void AddToMethodCache(lldb::addr_t class_addr, llvm::StringRef sel_str,
+lldb::addr_t impl_addr);
+
   TypeAndOrName LookupInClassNameCache(lldb::addr_t class_addr);
 
   void AddToClassNameCache(lldb::addr_t class_addr, const char *name,
@@ -343,20 +352,22 @@
   }
 
 private:
-  // We keep a map of ->Implementation so we don't have to call
-  // the resolver function over and over.
+  // We keep two maps of ->Implementation so we don't have
+  // to call the resolver function over and over.
+  // The first comes from regular obj_msgSend type dispatch, and maps the
+  // class + uniqued SEL value to an implementation.
+  // The second comes from the "selector-specific stubs", which are always
+  // of the form _objc_msgSend$SelectorName, so we don't know the uniqued
+  // selector, only the string name.
 
   // FIXME: We need to watch for the loading of Protocols, and flush the cache
   // for any
   // class that we see so changed.
 
   struct ClassAndSel {
-ClassAndSel() {
-  sel_addr = LLDB_INVALID_ADDRESS;
-  class_addr = LLDB_INVALID_ADDRESS;
-}
+ClassAndSel() = default;
 
-ClassAndSel(lldb::addr_t in_sel_addr, lldb::addr_t in_class_addr)
+ClassAndSel(lldb::addr_t in_class_addr, lldb::addr_t in_sel_addr)
 : class_addr(in_class_addr), sel_addr(in_sel_addr) {}
 
 bool operator==(const ClassAndSel &rhs) {
@@ -379,11 +390,35 @@
   }
 }
 
-lldb::addr_t class_addr;
-lldb::addr_t sel_addr;
+lldb::addr_t class_addr = LLDB_INVALID_ADDRESS;
+lldb::addr_t sel_addr = LLDB_INVALID_ADDRESS;
+  };
+
+  struct ClassAndSelStr {
+ClassAndSelStr() = default;
+
+ClassAndSelStr(lldb::addr_t in_class_addr, llvm::StringRef in_sel_name)
+: class_addr(in_class_addr), sel_name(in_sel_name) {}
+
+bool operator==(const ClassAndSelStr &rhs) {
+  return class_addr == rhs.class_addr && sel_name == rhs.sel_name;
+}
+
+bool operator<(const ClassAndSelStr &rhs) const {
+  if (class_addr < rhs.class_addr)
+return true;
+  else if (class_addr > rhs.class_addr)
+return false;
+  else
+return ConstString::Compare(sel_name, rhs.sel_name);
+}
+
+lldb::addr_t cla

[Lldb-commits] [PATCH] D123421: Handle the new "selector-specific stub" ObjC Method dispatch method

2022-04-08 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

I also made one functional change. Some code was checking 
`!m_lookup_implementation_function_code.empty()` to see if we had found an 
implementation function.  That won't work if I put the common prefix into this 
ivar in the constructor, so I change that to leave the member empty till we 
actually figure out that we want one or the other of the prefixes.  This is an 
error path that you have to intentionally mess things up to get to, but still 
it's worth preserving this signal.




Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:637-643
 // Also we will use the version of the lookup code that doesn't rely on the
 // stret version of the function.
-m_lookup_implementation_function_code =
-g_lookup_implementation_no_stret_function_code;
+m_lookup_implementation_function_code.append(
+g_lookup_implementation_no_stret_function_code);
   } else {
-m_lookup_implementation_function_code =
-g_lookup_implementation_with_stret_function_code;
+m_lookup_implementation_function_code.append(
+g_lookup_implementation_with_stret_function_code);

JDevlieghere wrote:
> I don't understand why we need to have two versions of the code. IIUC, if 
> there's a "stret return lookup function", then if we pass `is_stret == true` 
> we'll use it. Otherwise we'll unconditionally use the straight lookup. Why do 
> we need two variants of the code at all? Can't we pass false to `is_stret` 
> and achieve the same result with the 
> `g_lookup_implementation_with_stret_function_code` variant? 
On the systems that don't use stret returns, 
class_getMethodImplementation_stret doesn't exist.  So we would either have to 
have two versions of that part of the code, or passing function pointers to the 
two functions (making them the same in the no _stret case) or do some dlsym 
type ugliness.

This solution is straightforward, and reducing complexity in these Utility 
functions is a virtue.  Having two copies of all the code was a bit bogus, but 
this solution is straightforward and easy to debug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123421

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

[Lldb-commits] [PATCH] D123421: Handle the new "selector-specific stub" ObjC Method dispatch method

2022-04-08 Thread Jim Ingham via Phabricator via lldb-commits 
jingham marked 6 inline comments as done.
jingham added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:1012-1015
+if (log) {
+  LLDB_LOG(log, "Resolving call for class - {0} and selector - {1}",
+   isa_addr, sel_addr);
+}

JDevlieghere wrote:
> The `if (log)` check is redundant. It's the purpose of the macro to expand to 
> that that. Same below.
Old habits die hard...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123421

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

[Lldb-commits] [PATCH] D123421: Handle the new "selector-specific stub" ObjC Method dispatch method

2022-04-08 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Thanks Jim. This LGTM.




Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:637-643
 // Also we will use the version of the lookup code that doesn't rely on the
 // stret version of the function.
-m_lookup_implementation_function_code =
-g_lookup_implementation_no_stret_function_code;
+m_lookup_implementation_function_code.append(
+g_lookup_implementation_no_stret_function_code);
   } else {
-m_lookup_implementation_function_code =
-g_lookup_implementation_with_stret_function_code;
+m_lookup_implementation_function_code.append(
+g_lookup_implementation_with_stret_function_code);

jingham wrote:
> JDevlieghere wrote:
> > I don't understand why we need to have two versions of the code. IIUC, if 
> > there's a "stret return lookup function", then if we pass `is_stret == 
> > true` we'll use it. Otherwise we'll unconditionally use the straight 
> > lookup. Why do we need two variants of the code at all? Can't we pass false 
> > to `is_stret` and achieve the same result with the 
> > `g_lookup_implementation_with_stret_function_code` variant? 
> On the systems that don't use stret returns, 
> class_getMethodImplementation_stret doesn't exist.  So we would either have 
> to have two versions of that part of the code, or passing function pointers 
> to the two functions (making them the same in the no _stret case) or do some 
> dlsym type ugliness.
> 
> This solution is straightforward, and reducing complexity in these Utility 
> functions is a virtue.  Having two copies of all the code was a bit bogus, 
> but this solution is straightforward and easy to debug.
Thanks. I saw the `m_impl_stret_fn_addr` and figured we were passing in the 
function pointer ourselves. Makes sense. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123421

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

[Lldb-commits] [lldb] 833882b - Adapt the ObjC stepping algorithm to deal with "selector-stubs" in clang.

2022-04-08 Thread Jim Ingham via lldb-commits 

Author: Jim Ingham
Date: 2022-04-08T17:45:16-07:00
New Revision: 833882b32701ce3713c9dd9afdedf1126db691f0

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

LOG: Adapt the ObjC stepping algorithm to deal with "selector-stubs" in clang.

Clang is adding a feature to ObjC code generation, where instead of calling
objc_msgSend directly with an object & selector, it generates a stub that
gets passed only the object and the stub figures out the selector.

This patch adds support for following that dispatch method into the 
implementation
function.

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.h

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.h
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h

lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCKVO.py

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index b8104af5c1fa7..ccc68b55f5cb5 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1554,7 +1554,7 @@ 
AppleObjCRuntimeV2::DynamicClassInfoExtractor::GetClassInfoUtilityFunctionImpl(
   if (!utility_fn_or_error) {
 LLDB_LOG_ERROR(
 log, utility_fn_or_error.takeError(),
-"Failed to get utility function for implementation lookup: {0}");
+"Failed to get utility function for dynamic info extractor: {0}");
 return {};
   }
 
@@ -1684,7 +1684,7 @@ AppleObjCRuntimeV2::SharedCacheClassInfoExtractor::
   if (!utility_fn_or_error) {
 LLDB_LOG_ERROR(
 log, utility_fn_or_error.takeError(),
-"Failed to get utility function for implementation lookup: {0}");
+"Failed to get utility function for shared class info extractor: {0}");
 return nullptr;
   }
 

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
index 7164a490078d8..f9ccaf0115a20 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
@@ -33,6 +33,7 @@
 #include "lldb/Utility/Log.h"
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
 
@@ -45,239 +46,138 @@ const char 
*AppleObjCTrampolineHandler::g_lookup_implementation_function_name =
 "__lldb_objc_find_implementation_for_selector";
 const char *AppleObjCTrampolineHandler::
 g_lookup_implementation_with_stret_function_code =
-"   \n\
-extern \"C\" 
\n\
-{
\n\
-extern void *class_getMethodImplementation(void *objc_class, void *sel); 
\n\
-extern void *class_getMethodImplementation_stret(void *objc_class,   
\n\
- void *sel); 
\n\
-extern void * object_getClass (id object);   
\n\
-extern void * sel_getUid(char *name);
\n\
-extern int printf(const char *format, ...);  
\n\
-}
\n\
-extern \"C\" void * __lldb_objc_find_implementation_for_selector (   
\n\
-void *object,
\n\
-void *sel,   
\n\
-int is_stret,
\n\
-int is_super,
\n\
-int is_super2,   
\n\
-int is_fixup,
\n\
-int is_fixed,
\n\
-

[Lldb-commits] [PATCH] D123421: Handle the new "selector-specific stub" ObjC Method dispatch method

2022-04-08 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere closed this revision.
JDevlieghere added a comment.

Closed by 833882b32701ce3713c9dd9afdedf1126db691f0 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123421

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

[Lldb-commits] [lldb] ca68038 - Reland "[Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON""

2022-04-08 Thread Fangrui Song via lldb-commits 

Author: Fangrui Song
Date: 2022-04-08T23:40:18-07:00
New Revision: ca68038d12a23fa7ebb2b1ccbda93c321635e6bf

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

LOG: Reland "[Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON""

(With C++ exceptions, `clang++ --target=mips64{,el}-linux-gnu -fpie -pie
-fuse-ld=lld` has link errors (lld does not implement some strange R_MIPS_64
.eh_frame handling in GNU ld). However, sanitizer-x86_64-linux-qemu used this to
build ScudoUnitTests. Pined ScudoUnitTests to -no-pie.)

Default the option introduced in D113372 to ON to match all(?) major Linux
distros. This matches GCC and improves consistency with Android and linux-musl
which always default to PIE.
Note: CLANG_DEFAULT_PIE_ON_LINUX may be removed in the future.

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

Added: 


Modified: 
clang/CMakeLists.txt
clang/docs/ReleaseNotes.rst
clang/test/Driver/hip-fpie-option.hip

lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/test/Shell/ObjectFile/ELF/minidebuginfo-set-and-hit-breakpoint.test
llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index c0b1a20e8e8a0..3a77e7b0c8d60 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -245,7 +245,7 @@ set(PPC_LINUX_DEFAULT_IEEELONGDOUBLE OFF CACHE BOOL
 set(CLANG_SPAWN_CC1 OFF CACHE BOOL
 "Whether clang should use a new process for the CC1 invocation")
 
-option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on Linux" OFF)
+option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on linux-gnu" ON)
 
 # Manually handle default so we can change the meaning of a cached default.
 set(CLANG_ENABLE_OPAQUE_POINTERS "DEFAULT" CACHE STRING

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e8537e890714..079258c743704 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -288,6 +288,12 @@ Internal API Changes
 Build System Changes
 
 
+* CMake ``-DCLANG_DEFAULT_PIE_ON_LINUX=ON`` is now the default. This is used by
+  linux-gnu systems to decide whether ``-fPIE -pie`` is the default (instead of
+  ``-fno-pic -no-pie``). This matches GCC installations on many Linux distros.
+  Note: linux-android and linux-musl always default to ``-fPIE -pie``, ignoring
+  this variable. ``-DCLANG_DEFAULT_PIE_ON_LINUX`` may be removed in the future.
+
 AST Matchers
 
 

diff  --git a/clang/test/Driver/hip-fpie-option.hip 
b/clang/test/Driver/hip-fpie-option.hip
index 2e296a099dea5..ffd639dd5a6de 100644
--- a/clang/test/Driver/hip-fpie-option.hip
+++ b/clang/test/Driver/hip-fpie-option.hip
@@ -1,15 +1,15 @@
-// REQUIRES: clang-driver, amdgpu-registered-target
+// REQUIRES: clang-driver, amdgpu-registered-target, default-pie-on-linux
 
 // -fPIC and -fPIE only affects host relocation model.
 // device compilation always uses PIC. 
 
 // RUN: %clang -### -target x86_64-unknown-linux-gnu \
 // RUN:   --offload-arch=gfx906 %s -nogpulib -nogpuinc \
-// RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-STATIC %s
+// RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-PIE %s
 
 // RUN: %clang -### -target x86_64-unknown-linux-gnu \
 // RUN:   -fgpu-rdc --offload-arch=gfx906 %s -nogpulib -nogpuinc \
-// RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-STATIC %s
+// RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-PIE %s
 
 // RUN: %clang -### -target x86_64-unknown-linux-gnu \
 // RUN:   --offload-arch=gfx906 %s -nogpulib -nogpuinc \
@@ -32,7 +32,6 @@
 // RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-PIE %s
 
 // DEV-DAG: {{".*clang.*".* "-triple" "amdgcn-amd-amdhsa".* 
"-mrelocation-model" "pic" "-pic-level" "[1|2]".* "-mframe-pointer=all"}}
-// HOST-STATIC-DAG: {{".*clang.*".* "-triple" "x86_64-unknown-linux-gnu".* 
"-mrelocation-model" "static"}}
 // HOST-PIC-DAG: {{".*clang.*".* "-triple" "x86_64-unknown-linux-gnu".* 
"-mrelocation-model" "pic" "-pic-level" "2"}}
 // HOST-PIC-NOT: "-pic-is-pie"
 // HOST-PIE-DAG: {{".*clang.*".* "-triple" "x86_64-unknown-linux-gnu".* 
"-mrelocation-model" "pic" "-pic-level" "2" "-pic-is-pie"}}

diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
 
b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
index 19aad2ab1ec32..cef500f0e7754 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
@@ -2,6 +2,7 @@
 from lldb