[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting with reading memory in Scripted Process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 476019.
mib marked 2 inline comments as done.
mib retitled this revision from "[lldb/Plugins] Improve error reporting with 
reading/writing memory in a Scripted Process (WIP)" to "[lldb/Plugins] Improve 
error reporting with reading memory in Scripted Process".
mib edited the summary of this revision.
mib added a reviewer: kastiglione.
mib added a comment.

Redone the whole patch (taking into account @labath's previous comments)

It makes use of:

- Generic Variadic Lambdas
- Fold Expressions
- Index Sequences
- Template Partial Specializations
- Variadic template parameters and tuples

...


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

https://reviews.llvm.org/D134033

Files:
  lldb/bindings/python/python-swigsafecast.swig
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/API/SBError.h
  lldb/source/API/SBError.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
  lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
  lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py

Index: lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
@@ -65,9 +65,8 @@
 def get_registers_for_thread(self, tid: int):
 return {}
 
-def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
 data = lldb.SBData()
-error = lldb.SBError()
 bytes_read = self.corefile_process.ReadMemory(addr, size, error)
 
 if error.Fail():
Index: lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
@@ -20,7 +20,8 @@
 def get_registers_for_thread(self, tid: int):
 return {}
 
-def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+error.SetErrorString("This is an invalid scripted process!")
 return None
 
 def get_loaded_images(self):
Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -20,11 +20,12 @@
 def get_registers_for_thread(self, tid: int):
 return {}
 
-def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
 data = lldb.SBData().CreateDataFromCString(
 self.target.GetByteOrder(),
 self.target.GetCodeByteSize(),
 "Hello, world!")
+
 return data
 
 def get_loaded_images(self):
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -77,6 +77,12 @@
 self.assertEqual(process.GetProcessID(), 666)
 self.assertEqual(process.GetNumThreads(), 0)
 
+addr = 0x5
+buff = process.ReadMemory(addr, 4, error)
+self.assertEqual(buff, None)
+self.assertTrue(error.Fail())
+self.assertEqual(error.GetCString(), "This is an invalid scripted process!")
+
 with open(log_file, 'r') as f:
 log = f.read()
 
@@ -109,9 +115,14 @@
 process = target.Launch(launch_info, error)
 self.assertTrue(process and process.IsValid(), PROCESS_IS_VALID)
 self.assertEqual(process.GetProcessID(), 42)
-
 self.assertEqual(process.GetNumThreads(), 1)
 
+addr = 0x5
+message = "Hello, world!"
+buff = process.ReadCStringFromMemory(addr, 

[Lldb-commits] [PATCH] D132510: [RISCV][LLDB] Add initial SysV ABI support

[kasper via Phabricator via lldb-commits]

kasper81 added a comment.

@labath, if there is anything needed by me, please let me know. i'm new to 
phabricator patch system. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132510

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

[Lldb-commits] [PATCH] D138164: [LLDB][Android] Fix Android serial number handling

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack added a comment.

Either way works as long as we can pass the Android serial number to 
AdbClient.cpp.

This diff just followed the current code which is actually already working for 
USB-connected devices and emulator. So adding TCPIP-connected device support 
here makes sense.

  m_device_id = parsed_url->hostname.str();

If platform connect will accept platform-specific flags, potentially it may 
want these.

- --serial (Android serial number)
- --local-gdb-port (D136465 )
- --local-port (D136465 )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138164

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

[Lldb-commits] [PATCH] D138164: [LLDB][Android] Fix Android serial number handling

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Just wanted to let you know that the "platform connect" parameters are not 
fixed. Each platform can make their own arguments. The Android platform 
currently expects a URL that will get forwarded to an internal Connection class 
that has registered a connection URL prefix ("unix-abstract-connect" in this 
case). But it doesn't have to be this way.

If the "unix-abstract-connect://emulator-5554/data/local/tmp/debug.sock" is not 
actually just being passed to the ConnectionFileDescriptor::Connect(...) 
function in lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp then we 
might want just modify the "platform connect" in the android platform to accept 
extra options since each Platform plug-in could have any number of options 
needed to connect. So instead of adding the device ID to the URL we could use a 
--device option. Currently we have:

  (lldb) platform connect 
unix-abstract-connect://emulator-5554/data/local/tmp/debug.sock

But this could be:

  platform connect --device emulator-5554 
unix-abstract-connect://data/local/tmp/debug.sock

Same kind of thing for the TCP address. Not sure if we would specify this with 
"--device 127.0.0.1:5556" or if we would add a "--tcp 127.0.0.1:5556" option?

So I just wanted to let you know that we don't have to fit all of the 
connection information into a single URL. I am also not sure if the current 
android platform code was already doing this kind of thing? The 
"unix-abstract-connect" is a url that should forward the contents directly to a 
connection plug-in where we can have connection subclasses register connection 
URL prefixes that any plug-in can use. So I am not sure if adding the device 
name to this URL ("unix-abstract-connect://data/local/tmp/debug.sock") is the 
right way to go.

Let me know what you think and if I have misunderstood anything of how things 
currently work versus how this patch is modifying things to work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138164

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

[Lldb-commits] [PATCH] D138181: [test] Allow skipTestIfFn to apply to entire classes for skipIfNoSBHeaders

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
Herald added a project: All.
rupprecht requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Some test cases are already marked @skipIfNoSBHeaders, but they make use of 
SBAPI headers in test setup. The setup will fail if the headers are missing, so 
it is too late to wait until the test case to apply the skip annotation.

In addition to allowing this to apply to entire classes, I also changed all the 
existing annotations from test cases to test classes where 
necessary/appropriate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138181

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
  lldb/test/API/api/multithreaded/TestMultithreaded.py
  lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py

Index: lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py
===
--- lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py
+++ lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py
@@ -12,7 +12,6 @@
 
 def setUp(self):
 TestBase.setUp(self)
-self.generateSource('plugin.cpp')
 
 @skipIfNoSBHeaders
 # Requires a compatible arch and platform to link against the host's built
@@ -22,6 +21,7 @@
 @no_debug_info_test
 def test_load_plugin(self):
 """Test that plugins that load commands work correctly."""
+self.generateSource('plugin.cpp')
 
 plugin_name = "plugin"
 if sys.platform.startswith("darwin"):
Index: lldb/test/API/api/multithreaded/TestMultithreaded.py
===
--- lldb/test/API/api/multithreaded/TestMultithreaded.py
+++ lldb/test/API/api/multithreaded/TestMultithreaded.py
@@ -9,6 +9,7 @@
 from lldbsuite.test import lldbutil
 
 
+@skipIfNoSBHeaders
 class SBBreakpointCallbackCase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
@@ -25,7 +26,6 @@
 self.generateSource('test_stop-hook.cpp')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_python_stop_hook(self):
@@ -34,7 +34,6 @@
 'test_python_stop_hook')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_breakpoint_callback(self):
@@ -43,7 +42,6 @@
 'test_breakpoint_callback')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_breakpoint_location_callback(self):
@@ -52,7 +50,6 @@
 'test_breakpoint_location_callback')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
@@ -63,7 +60,6 @@
 'test_listener_event_description')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
@@ -76,7 +72,6 @@
 'test_listener_event_process_state')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
Index: lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
===
--- lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
+++ lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
@@ -8,6 +8,7 @@
 from lldbsuite.test import lldbutil
 
 
+@skipIfNoSBHeaders
 class SBDirCheckerCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
@@ -16,7 +17,6 @@
 self.source = 'main.cpp'
 self.generateSource(self.source)
 
-@skipIfNoSBHeaders
 def test_sb_api_directory(self):
 """Test the SB API directory and make sure there's no unwanted stuff."""
 
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -140,8 +140,10 @@
 def skipTestIfFn(expected_fn, bugnumber=None):
 def skipTestIfFn_impl(func):
 if isinstance(func, type) and issubclass(func, unittest2.TestCase):
-raise Exception(
-"@skipTestIfFn can only be used to decorate a test method")
+reason = expected_fn()
+# The return value is the reason (or None if we don't skip), so
+# reason is used for both args.
+return unittest2.skipIf(reason, reason)(func)
 
 @wraps(func)
 def 

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 475988.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

Address @friss' feedback.


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

https://reviews.llvm.org/D138176

Files:
  llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
  llvm/lib/DWARFLinker/DWARFLinker.cpp
  llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp

Index: llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
===
--- llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
+++ llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
@@ -78,13 +78,13 @@
 
 /// Keep track of a forward cross-cu reference from this unit
 /// to \p Die that lives in \p RefUnit.
-void CompileUnit::noteForwardReference(DIE *Die, const CompileUnit *RefUnit,
-   DeclContext *Ctxt, PatchLocation Attr) {
-  ForwardDIEReferences.emplace_back(Die, RefUnit, Ctxt, Attr);
+void CompileUnit::noteReference(DIE *Die, const CompileUnit *RefUnit,
+DeclContext *Ctxt, PatchLocation Attr) {
+  DIEReferences.emplace_back(Die, RefUnit, Ctxt, Attr);
 }
 
-void CompileUnit::fixupForwardReferences() {
-  for (const auto  : ForwardDIEReferences) {
+void CompileUnit::fixupReferences() {
+  for (const auto  : DIEReferences) {
 DIE *RefDie;
 const CompileUnit *RefUnit;
 PatchLocation Attr;
Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -934,9 +934,9 @@
   }
 
   if (!RefInfo.Clone) {
-assert(Ref > InputDIE.getOffset());
 // We haven't cloned this DIE yet. Just create an empty one and
 // store it. It'll get really cloned when we process it.
+RefInfo.Reference = true;
 RefInfo.Clone = DIE::get(DIEAlloc, dwarf::Tag(RefDie.getTag()));
   }
   NewRefDie = RefInfo.Clone;
@@ -949,7 +949,7 @@
 // FIXME: we should be able to design DIEEntry reliance on
 // DwarfDebug away.
 uint64_t Attr;
-if (Ref < InputDIE.getOffset()) {
+if (Ref < InputDIE.getOffset() && !RefInfo.Reference) {
   // We must have already cloned that DIE.
   uint32_t NewRefOffset =
   RefUnit->getStartOffset() + NewRefDie->getOffset();
@@ -957,12 +957,12 @@
   Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::DW_FORM_ref_addr, DIEInteger(Attr));
 } else {
-  // A forward reference. Note and fixup later.
+  // A forward or backward reference. Note and fixup later.
   Attr = 0xBADDEF;
-  Unit.noteForwardReference(
-  NewRefDie, RefUnit, RefInfo.Ctxt,
-  Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
-   dwarf::DW_FORM_ref_addr, DIEInteger(Attr)));
+  Unit.noteReference(NewRefDie, RefUnit, RefInfo.Ctxt,
+ Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
+  dwarf::DW_FORM_ref_addr,
+  DIEInteger(Attr)));
 }
 return U.getRefAddrByteSize();
   }
@@ -2237,7 +2237,7 @@
   if (LLVM_LIKELY(!Linker.Options.Update))
 Linker.generateUnitRanges(*CurrentUnit);
 
-  CurrentUnit->fixupForwardReferences();
+  CurrentUnit->fixupReferences();
 
   if (!CurrentUnit->getOutputUnitDIE())
 continue;
Index: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
===
--- llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -77,6 +77,9 @@
 
 /// Is ODR marking done?
 bool ODRMarkingDone : 1;
+
+/// Is this a reference to a DIE that hasn't been cloned yet?
+bool Reference : 1;
   };
 
   CompileUnit(DWARFUnit , unsigned ID, bool CanUseODR,
@@ -160,15 +163,15 @@
   /// current debug_info section size).
   uint64_t computeNextUnitOffset(uint16_t DwarfVersion);
 
-  /// Keep track of a forward reference to DIE \p Die in \p RefUnit by \p
-  /// Attr. The attribute should be fixed up later to point to the absolute
-  /// offset of \p Die in the debug_info section or to the canonical offset of
-  /// \p Ctxt if it is non-null.
-  void noteForwardReference(DIE *Die, const CompileUnit *RefUnit,
-DeclContext *Ctxt, PatchLocation Attr);
+  /// Keep track of a forward or backward reference to DIE \p Die in \p RefUnit
+  /// by \p Attr. The attribute should be fixed up later to point to the
+  /// absolute offset of \p Die in the debug_info section or to the canonical
+  /// offset of \p Ctxt if it is non-null.
+  void noteReference(DIE *Die, const CompileUnit *RefUnit, DeclContext *Ctxt,
+ PatchLocation Attr);
 
-  /// Apply all fixups recorded by noteForwardReference().
-  void fixupForwardReferences();
+  /// Apply all 

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

[Frederic Riss via Phabricator via lldb-commits]

friss added inline comments.



Comment at: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h:81-82
+
+/// Is DIE a forward or backward reference?
+bool Reference : 1;
   };

When I read the comment, I was confused as I thought the flag was used to 
differentiate between forward and backward references. But in fact it's just 
signaling a reference to a DIE that has not been emitted yet. Maybe tweak the 
comment.



Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:961
+  uint64_t Attr = 0xBADDEF;
   Unit.noteForwardReference(
   NewRefDie, RefUnit, RefInfo.Ctxt,

Should we change the name of this function now that it's not only about forward 
references.


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

https://reviews.llvm.org/D138176

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

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 475985.
JDevlieghere added a comment.

Remove unrelated changes


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

https://reviews.llvm.org/D138176

Files:
  llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
  llvm/lib/DWARFLinker/DWARFLinker.cpp


Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -934,9 +934,9 @@
   }
 
   if (!RefInfo.Clone) {
-assert(Ref > InputDIE.getOffset());
 // We haven't cloned this DIE yet. Just create an empty one and
 // store it. It'll get really cloned when we process it.
+RefInfo.Reference = true;
 RefInfo.Clone = DIE::get(DIEAlloc, dwarf::Tag(RefDie.getTag()));
   }
   NewRefDie = RefInfo.Clone;
@@ -949,7 +949,7 @@
 // FIXME: we should be able to design DIEEntry reliance on
 // DwarfDebug away.
 uint64_t Attr;
-if (Ref < InputDIE.getOffset()) {
+if (Ref < InputDIE.getOffset() && !RefInfo.Reference) {
   // We must have already cloned that DIE.
   uint32_t NewRefOffset =
   RefUnit->getStartOffset() + NewRefDie->getOffset();
Index: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
===
--- llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -77,6 +77,9 @@
 
 /// Is ODR marking done?
 bool ODRMarkingDone : 1;
+
+/// Is DIE a forward or backward reference?
+bool Reference : 1;
   };
 
   CompileUnit(DWARFUnit , unsigned ID, bool CanUseODR,


Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -934,9 +934,9 @@
   }
 
   if (!RefInfo.Clone) {
-assert(Ref > InputDIE.getOffset());
 // We haven't cloned this DIE yet. Just create an empty one and
 // store it. It'll get really cloned when we process it.
+RefInfo.Reference = true;
 RefInfo.Clone = DIE::get(DIEAlloc, dwarf::Tag(RefDie.getTag()));
   }
   NewRefDie = RefInfo.Clone;
@@ -949,7 +949,7 @@
 // FIXME: we should be able to design DIEEntry reliance on
 // DwarfDebug away.
 uint64_t Attr;
-if (Ref < InputDIE.getOffset()) {
+if (Ref < InputDIE.getOffset() && !RefInfo.Reference) {
   // We must have already cloned that DIE.
   uint32_t NewRefOffset =
   RefUnit->getStartOffset() + NewRefDie->getOffset();
Index: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
===
--- llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -77,6 +77,9 @@
 
 /// Is ODR marking done?
 bool ODRMarkingDone : 1;
+
+/// Is DIE a forward or backward reference?
+bool Reference : 1;
   };
 
   CompileUnit(DWARFUnit , unsigned ID, bool CanUseODR,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: avl, friss, aprantl.
Herald added a subscriber: hiraditya.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

D125469  changed the assumptions regarding 
forward references. Before the patch, a reference to an uncloned DIE was always 
a forward reference. After the change, that assumption no longer holds. It was 
enforced by the assertion in 
`DWARFLinker::DIECloner::cloneDieReferenceAttribute` (DWARFLinker.cpp:937):

  assert(Ref > InputDIE.getOffset());

This patch is relatively straightforward: it adds a field to the `DIEInfo` to 
remember the DIE is a forward or backward reference. We then treat backward 
references like a forward references by fixing them up after the fact, when all 
DIEs have been cloned.

We tripped this assertion for an internal project built with LTO. Unfortunately 
that means I'm not able to share a test case. I've spent a few hours trying to 
craft a test case that would hit this case but didn't succeed. Getting a 
backward reference is easy, but a backward reference to a DIE that hasn't been 
cloned yet is not something I managed to reproduce. I'd love to have a test 
case to avoid regressing this in the future, so please let me know if you have 
an idea on how to trigger this behavior from an artificial test case.


https://reviews.llvm.org/D138176

Files:
  llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
  llvm/lib/DWARFLinker/DWARFLinker.cpp


Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -934,9 +934,9 @@
   }
 
   if (!RefInfo.Clone) {
-assert(Ref > InputDIE.getOffset());
 // We haven't cloned this DIE yet. Just create an empty one and
 // store it. It'll get really cloned when we process it.
+RefInfo.Reference = true;
 RefInfo.Clone = DIE::get(DIEAlloc, dwarf::Tag(RefDie.getTag()));
   }
   NewRefDie = RefInfo.Clone;
@@ -948,17 +948,16 @@
 // to find the unit offset. (We don't have a DwarfDebug)
 // FIXME: we should be able to design DIEEntry reliance on
 // DwarfDebug away.
-uint64_t Attr;
-if (Ref < InputDIE.getOffset()) {
+if (Ref < InputDIE.getOffset() && !RefInfo.Reference) {
   // We must have already cloned that DIE.
   uint32_t NewRefOffset =
   RefUnit->getStartOffset() + NewRefDie->getOffset();
-  Attr = NewRefOffset;
+  uint64_t Attr = NewRefOffset;
   Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::DW_FORM_ref_addr, DIEInteger(Attr));
 } else {
   // A forward reference. Note and fixup later.
-  Attr = 0xBADDEF;
+  uint64_t Attr = 0xBADDEF;
   Unit.noteForwardReference(
   NewRefDie, RefUnit, RefInfo.Ctxt,
   Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
Index: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
===
--- llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -77,6 +77,9 @@
 
 /// Is ODR marking done?
 bool ODRMarkingDone : 1;
+
+/// Is DIE a forward or backward reference?
+bool Reference : 1;
   };
 
   CompileUnit(DWARFUnit , unsigned ID, bool CanUseODR,


Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -934,9 +934,9 @@
   }
 
   if (!RefInfo.Clone) {
-assert(Ref > InputDIE.getOffset());
 // We haven't cloned this DIE yet. Just create an empty one and
 // store it. It'll get really cloned when we process it.
+RefInfo.Reference = true;
 RefInfo.Clone = DIE::get(DIEAlloc, dwarf::Tag(RefDie.getTag()));
   }
   NewRefDie = RefInfo.Clone;
@@ -948,17 +948,16 @@
 // to find the unit offset. (We don't have a DwarfDebug)
 // FIXME: we should be able to design DIEEntry reliance on
 // DwarfDebug away.
-uint64_t Attr;
-if (Ref < InputDIE.getOffset()) {
+if (Ref < InputDIE.getOffset() && !RefInfo.Reference) {
   // We must have already cloned that DIE.
   uint32_t NewRefOffset =
   RefUnit->getStartOffset() + NewRefDie->getOffset();
-  Attr = NewRefOffset;
+  uint64_t Attr = NewRefOffset;
   Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::DW_FORM_ref_addr, DIEInteger(Attr));
 } else {
   // A forward reference. Note and fixup later.
-  Attr = 0xBADDEF;
+  uint64_t Attr = 0xBADDEF;
   Unit.noteForwardReference(
   NewRefDie, RefUnit, RefInfo.Ctxt,
   Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
Index: 

[Lldb-commits] [lldb] 3f36421 - Fix use of std::unique / erase. This fixes a bot error after D136650.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2022-11-16T17:35:51-08:00
New Revision: 3f36421aa74031fb554c780dba66c1c1a437d953

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

LOG: Fix use of std::unique / erase. This fixes a bot error after D136650.

Added: 


Modified: 
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 9e252c4dbb04..22071b76a805 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2361,7 +2361,8 @@ Target::GetScratchTypeSystems(bool create_on_demand) {
   return a.get() <= b.get();
 });
   scratch_type_systems.erase(
-  std::unique(scratch_type_systems.begin(), scratch_type_systems.end()));
+  std::unique(scratch_type_systems.begin(), scratch_type_systems.end()),
+  scratch_type_systems.end());
   return scratch_type_systems;
 }
 



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

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

Looks like this broke the windows lldb bot: 
https://lab.llvm.org/buildbot/#/builders/83/builds/26042


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136650

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

[Lldb-commits] [PATCH] D138164: [LLDB][Android] Fix Android serial number handling

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack created this revision.
Herald added a subscriber: danielkiss.
Herald added a project: All.
splhack added a reviewer: clayborg.
splhack updated this revision to Diff 475962.
splhack added a comment.
splhack published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

First, recap: what is the Android serial number.
In Android ADB protocal, the serial number following the `host-serial:` is 
URL-safe string and it could be a `host:port` string.

https://cs.android.com/android/platform/superproject/+/763ab7bb17e9c63b238222085e1e69552d978367:packages/modules/adb/sockets.cpp;l=825?q=host-serial:

  if (android::base::ConsumePrefix(, "host-serial:")) {
  // serial number should follow "host:" and could be a host:port string.

Second, LLDB is not able to connect it to Android device when multiple devices 
are connected to the host.

  (lldb) platform select remote-android
  (lldb) platform connect unix-abstract-connect:///data/local/tmp/debug.sock
  error: Expected a single connected device, got instead 2 - try setting 
'ANDROID_SERIAL'

However setting ANDROID_SERIAL environment variable is not always a viable 
option.
For instance, VS Code + lldb-server and debugging multiple Android devices at 
the same time.

The potential solution is to use platform connect URL.

  (lldb) platform select remote-android
  (lldb) platform connect 
unix-abstract-connect://emulator-5554/data/local/tmp/debug.sock

This works for USB-connected devices and emulators.
But it does not work for TCP/IP-connected devices.

  (lldb) platform select remote-android
  (lldb) platform connect 
unix-abstract-connect://127.0.0.1:5556/data/local/tmp/debug.sock

The reason is that the current code only uses the hostname part of the URL.

https://github.com/llvm/llvm-project/blob/ed9638c44bc0c95314694fb878b21006e6c87510/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp#L154-L155

So m_device_id will be `127.0.0.1`. But this should be `127.0.0.1:5556`
Also `localhost` should not be skipped when the port number is available. 
m_device_id should be `localhost:5556`

  (lldb) platform connect 
unix-abstract-connect://localhost:5556/data/local/tmp/debug.sock

Third, the host could be IPv6, for instance `[::1]`.

  (lldb) platform connect 
unix-abstract-connect://[::1]:5556/data/local/tmp/debug.sock

This is a valid URL and serial number `[::1]:5556`.

The parsed_url does not look it has the information of IPv6 brackets. Thus, 
`GetDeviceIDFromURL` checks the raw URL again for the bracket.

Fourth, when a port number is available in the connect URL, `MakeConnectURL` 
function create a new TCP/IP forwarding with the port number. When `adb 
devices` shows a device with `host:port` style serial number, the port in the 
serial number is already connected or forwarded to the adb server. 
`host-serial:host:port` should work.

https://github.com/llvm/llvm-project/blob/ed9638c44bc0c95314694fb878b21006e6c87510/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp#L191-L192

https://github.com/llvm/llvm-project/blob/ed9638c44bc0c95314694fb878b21006e6c87510/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp#L42-L46


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138164

Files:
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
  lldb/unittests/Platform/Android/CMakeLists.txt
  lldb/unittests/Platform/Android/PlatformAndroidRemoteGDBServerTest.cpp

Index: lldb/unittests/Platform/Android/PlatformAndroidRemoteGDBServerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/Android/PlatformAndroidRemoteGDBServerTest.cpp
@@ -0,0 +1,50 @@
+//===-- PlatformAndroidRemoteGDBServerTest.cpp
+//-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace lldb_private {
+namespace platform_android {
+
+class PlatformAndroidRemoteGDBServerTest : public ::testing::Test {};
+
+TEST(PlatformAndroidRemoteGDBServerTest, GetDeviceIDFromURL) {
+  const static std::vector> data{
+  {{"url", "unix-abstract-connect:///sock"}, {"serial", ""}},
+  {{"url", "unix-abstract-connect://localhost/sock"}, {"serial", ""}},
+  {{"url", "unix-abstract-connect://emulator-5554/sock"},
+   {"serial", "emulator-5554"}},
+  {{"url", "unix-abstract-connect://localhost:5556/sock"},
+   {"serial", 

[Lldb-commits] [lldb] 0ec24e1 - Only use major version in the clang resource directory for standalone builds.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-11-16T16:00:26-08:00
New Revision: 0ec24e1f956616c54f38e104b482dec9309a93fa

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

LOG: Only use major version in the clang resource directory for standalone 
builds.

Commit e1b88c8a09be changed the name of the clang resource directory so that
it was "lib/clang/" but missed the place in the LLDB standalone
build where we search for the resource directory.  That was still looking for
...  The standalone lldb bot has been failing since this
commit.

Added: 


Modified: 
lldb/cmake/modules/LLDBConfig.cmake

Removed: 




diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index 2dd14d05a92aa..1079e4b2def17 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -271,7 +271,7 @@ endif()
 # directory that LLDB is using for its embedded Clang instance needs to point
 # to the resource directory of the used Clang installation.
 if (NOT TARGET clang-resource-headers)
-  set(LLDB_CLANG_RESOURCE_DIR_NAME 
"${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")
+  set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}")
   # Iterate over the possible places where the external resource directory
   # could be and pick the first that exists.
   foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"



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

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/include/lldb/Symbol/Type.h:300
 
-  TypeSystem *GetTypeSystem(bool prefer_dynamic);
+  CompilerType::TypeSystemSPWrapper GetTypeSystem(bool prefer_dynamic);
 

labath wrote:
> Maybe the wrapper type should just be its own top-level entity? Or 
> `TypeSystem::SPWrapper`?
I tried it and then immediately undid it because TypeSystem.h already depends 
on CompilerType.h.


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

https://reviews.llvm.org/D136650

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

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:13
 #include "lldb/Host/macosx/HostInfoMacOSX.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Utility/Args.h"

labath wrote:
> We shouldn't have Host->Core dependencies. Could you report the error in the 
> caller?
How strong to you fell about this?
It would be an odd interface. `GetXcodeSDKPath` also caches negative results 
because negative lookups are really slow. This means we would only raise an 
error on the first call and not on the subsequent ones. I can do that, but it 
also feels weird. I guess we could cache the error string as well?


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

https://reviews.llvm.org/D138060

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

[Lldb-commits] [PATCH] D137873: [LLDB][Minidump] Set abi environment for windows.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added a comment.

In D137873#3931888 , @mstorsjo wrote:

> In D137873#3930683 , @labath wrote:
>
>> If that doesn't work then (besides knowing why), I'd like us try some kind 
>> of a single-setting solution, as I don't think it makes sense to have two 
>> different settings like this (happy to hear counterarguments to that)
>
> Overall, the ideal is if the user usually needs to twiddle one single 
> setting. For a majority of cases, picking the default format (like is done 
> for the objectfile setting, and afaik this one does too even though I didn't 
> quite clearly see the codepath for it) will be what you'd want: If you're 
> working with mingw tools, debugging with a mingw built lldb, then your app 
> that you're debugging also probably uses mingw ABIs. But it's clearly not 
> implausible to want to debug executables from the other ecosystem, so being 
> able to change it with a single setting is good too.
>
> And for the objectfile setting, it's possible to set it even on a per-DLL 
> basis. This may sound contrieved, but it's actually totally reasonable. While 
> the C++ ABIs are incompatible, such DLLs can interoperate over C APIs - this 
> is not an uncommon scenario. Plus, every mingw executable links against 
> msvcrt.dll or ucrtbase.dll (provided by the system) which use the MSVC C++ 
> ABIs internally (if you use the versions of them with debug info).
>
> So the ability to set it per DLL for the objectfile part is totally 
> warranted. How that fits in towards minidump reading, I don't really know 
> though (since the whole minidump contains stuff for the whole process). In 
> which cases does the C++ ABI form matter for the minidump btw - since the 
> actual interpretation of things still would be based on per-DLL debug info 
> (either as DWARF or PDB)?

lldb creates AST for user input expression which uses the abi from minidump 
process. That AST's class layout info is imported from another AST created from 
debug info that uses the abi specified in debug info. The current situation is 
that minidump process uses itanium ABI (because it's not specified in minidump, 
falls back to use itanium), but the debug info uses msvc ABI. The inconsistent 
causes crash when lowering class.

It looks like it's impossible to make it works with DLLs that have different 
ABIs, no matter which ABI we set for minidump. Unless we change how clang 
interacts with external layout: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/RecordLayoutBuilder.cpp#L1357.
 Maybe change it so that if using external layout failed due to mismatched ABI, 
try again with another ABI 
https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/RecordLayoutBuilder.cpp#L3302?




Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:452
 ArchSpec ProcessMinidump::GetArchitecture() {
+  // "settings set plugin.process.minidump.abi" overrides the env in minidump.
+  ArchSpec arch = m_minidump_parser->GetArchitecture();

labath wrote:
> Is "overrides" the correct word here? Are there any circumstances in which we 
> are able to determine the environment from the minidump file? Because if it 
> is, then I would expect this to be the other way around (that the environment 
> from a specific file overrides the generic catch-all setting)...
Yeah, the word is not correct. It should be "sets". 
I think minidump file just doesn't have that environment info: 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/BinaryFormat/Minidump.h#L161-L179.
 lldb sets the environment based on platform for android: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp#L195.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137873

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

[Lldb-commits] [PATCH] D137873: [LLDB][Minidump] Set abi environment for windows.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a subscriber: alvinhochun.
mstorsjo added a comment.

In D137873#3930683 , @labath wrote:

> If that doesn't work then (besides knowing why), I'd like us try some kind of 
> a single-setting solution, as I don't think it makes sense to have two 
> different settings like this (happy to hear counterarguments to that)

Overall, the ideal is if the user usually needs to twiddle one single setting. 
For a majority of cases, picking the default format (like is done for the 
objectfile setting, and afaik this one does too even though I didn't quite 
clearly see the codepath for it) will be what you'd want: If you're working 
with mingw tools, debugging with a mingw built lldb, then your app that you're 
debugging also probably uses mingw ABIs. But it's clearly not implausible to 
want to debug executables from the other ecosystem, so being able to change it 
with a single setting is good too.

And for the objectfile setting, it's possible to set it even on a per-DLL 
basis. This may sound contrieved, but it's actually totally reasonable. While 
the C++ ABIs are incompatible, such DLLs can interoperate over C APIs - this is 
not an uncommon scenario. Plus, every mingw executable links against msvcrt.dll 
or ucrtbase.dll (provided by the system) which use the MSVC C++ ABIs internally 
(if you use the versions of them with debug info).

So the ability to set it per DLL for the objectfile part is totally warranted. 
How that fits in towards minidump reading, I don't really know though (since 
the whole minidump contains stuff for the whole process). In which cases does 
the C++ ABI form matter for the minidump btw - since the actual interpretation 
of things still would be based on per-DLL debug info (either as DWARF or PDB)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137873

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

[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added inline comments.



Comment at: lld/COFF/LTO.cpp:238
+  sys::path::append(path, directory,
+outputFileBaseName + ".lto." + baseName);
+  sys::path::remove_dots(path, true);

tejohnson wrote:
> MaskRay wrote:
> > What if two input bitcode files have the same basename, e.g. `dir1/a.obj` 
> > and `dir2/a.obj`?
> I think that should be ok as the output file path created here includes the 
> directory. So you should get dir1/a.out.lto.a.obj and dir2/a.out.lto.a.obj.
Yes, you will get dir1/a.out.lto.a.obj and dir2/a.out.lto.a.obj.



Comment at: lld/COFF/LTO.cpp:229
+StringRef ltoObjName;
+if (bitcodeFilePath == "ld-temp.o") {
+  ltoObjName =

tejohnson wrote:
> MaskRay wrote:
> > MaskRay wrote:
> > > tejohnson wrote:
> > > > zequanwu wrote:
> > > > > tejohnson wrote:
> > > > > > This case should always be i==0 I think?
> > > > > IIUC, "ld-temp.o" is the name of combined module. Do you mean there 
> > > > > will be only 1 combined module and it will always be the first task?
> > > > Yes. So you don't need the handling for i==0 in the name in this case 
> > > > (you could probably assert that i==0).
> > > This looks like a hack. `assert(i == 0)` will fail with 
> > > `-opt:lldltopartitions=2`: `buf[1]` will be called `ld-temp.o` as well.
> > > 
> > > In addition, if an input bitcode file is called `ld-temp.o` (for some 
> > > reason they don't use `.obj`, just `ld-temp.o`), `assert(i == 0)` will 
> > > fail as well.
> > I guess `if (i < config->ltoPartitions)` may fix the issue.
> Ah ok, forgot about the lto partitions case. Sorry, @zequanwu, looks like you 
> will need to go back to your old handling that appends i if it is non-zero.
I reverted this part back. Since input bitcode file can be called `ld-temp.o`, 
i could be any number smaller than maxTasks, removed assertion. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

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

[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 475895.
zequanwu marked an inline comment as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  lld/COFF/LTO.cpp
  lld/COFF/LTO.h
  lld/ELF/LTO.cpp
  lld/MachO/LTO.cpp
  lld/test/COFF/lto-obj-path.ll
  lld/test/COFF/pdb-thinlto.ll
  lld/test/COFF/thinlto.ll
  lld/wasm/LTO.cpp
  lldb/source/Core/DataFileCache.cpp
  llvm/include/llvm/Support/Caching.h
  llvm/lib/Debuginfod/Debuginfod.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/Support/Caching.cpp
  llvm/tools/llvm-lto/llvm-lto.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -411,7 +411,8 @@
   if (HasErrors)
 return 1;
 
-  auto AddStream = [&](size_t Task) -> std::unique_ptr {
+  auto AddStream = [&](size_t Task,
+   Twine ModuleName) -> std::unique_ptr {
 std::string Path = OutputFilename + "." + utostr(Task);
 
 std::error_code EC;
@@ -420,8 +421,9 @@
 return std::make_unique(std::move(S), Path);
   };
 
-  auto AddBuffer = [&](size_t Task, std::unique_ptr MB) {
-*AddStream(Task)->OS << MB->getBuffer();
+  auto AddBuffer = [&](size_t Task, Twine ModuleName,
+   std::unique_ptr MB) {
+*AddStream(Task, ModuleName)->OS << MB->getBuffer();
   };
 
   FileCache Cache;
Index: llvm/tools/llvm-lto/llvm-lto.cpp
===
--- llvm/tools/llvm-lto/llvm-lto.cpp
+++ llvm/tools/llvm-lto/llvm-lto.cpp
@@ -317,11 +317,11 @@
   if (!CurrentActivity.empty())
 OS << ' ' << CurrentActivity;
   OS << ": ";
-  
+
   DiagnosticPrinterRawOStream DP(OS);
   DI.print(DP);
   OS << '\n';
-  
+
   if (DI.getSeverity() == DS_Error)
 exit(1);
   return true;
@@ -1099,7 +1099,9 @@
 error("writing merged module failed.");
 }
 
-auto AddStream = [&](size_t Task) -> std::unique_ptr {
+auto AddStream =
+[&](size_t Task,
+Twine ModuleName) -> std::unique_ptr {
   std::string PartFilename = OutputFilename;
   if (Parallelism != 1)
 PartFilename += "." + utostr(Task);
Index: llvm/lib/Support/Caching.cpp
===
--- llvm/lib/Support/Caching.cpp
+++ llvm/lib/Support/Caching.cpp
@@ -37,7 +37,8 @@
   TempFilePrefixRef.toVector(TempFilePrefix);
   CacheDirectoryPathRef.toVector(CacheDirectoryPath);
 
-  return [=](unsigned Task, StringRef Key) -> Expected {
+  return [=](unsigned Task, StringRef Key,
+ Twine ModuleName) -> Expected {
 // This choice of file name allows the cache to be pruned (see pruneCache()
 // in include/llvm/Support/CachePruning.h).
 SmallString<64> EntryPath;
@@ -54,7 +55,7 @@
 /*RequiresNullTerminator=*/false);
   sys::fs::closeFile(*FDOrErr);
   if (MBOrErr) {
-AddBuffer(Task, std::move(*MBOrErr));
+AddBuffer(Task, ModuleName, std::move(*MBOrErr));
 return AddStreamFn();
   }
   EC = MBOrErr.getError();
@@ -77,14 +78,15 @@
 struct CacheStream : CachedFileStream {
   AddBufferFn AddBuffer;
   sys::fs::TempFile TempFile;
+  std::string ModuleName;
   unsigned Task;
 
   CacheStream(std::unique_ptr OS, AddBufferFn AddBuffer,
   sys::fs::TempFile TempFile, std::string EntryPath,
-  unsigned Task)
+  std::string ModuleName, unsigned Task)
   : CachedFileStream(std::move(OS), std::move(EntryPath)),
 AddBuffer(std::move(AddBuffer)), TempFile(std::move(TempFile)),
-Task(Task) {}
+ModuleName(ModuleName), Task(Task) {}
 
   ~CacheStream() {
 // TODO: Manually commit rather than using non-trivial destructor,
@@ -133,11 +135,12 @@
  TempFile.TmpName + " to " + ObjectPathName + ": " +
  toString(std::move(E)) + "\n");
 
-AddBuffer(Task, std::move(*MBOrErr));
+AddBuffer(Task, ModuleName, std::move(*MBOrErr));
   }
 };
 
-return [=](size_t Task) -> Expected> {
+return [=](size_t Task, Twine ModuleName)
+   -> Expected> {
   // Create the cache directory if not already done. Doing this lazily
   // ensures the filesystem isn't mutated until the cache is.
   if (std::error_code EC = sys::fs::create_directories(
@@ -158,7 +161,8 @@
   // This CacheStream will move the temporary file into the cache when done.
   return std::make_unique(
   

[Lldb-commits] [PATCH] D138077: Send statistics in initialized event

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Yeah, sending this data at the entry point is not a good way to tell anything 
about the debug session having debug info. Some shared libraries might be 
loaded on macOS, but not on linux or android.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138077

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

[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type if it has empty debug info and is required to have complete type.

[Zequan Wu via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6435fe699c29: [LLDB][NativePDB] Forcefully complete a record 
type if it has empty debug info… (authored by zequanwu).

Changed prior to commit:
  https://reviews.llvm.org/D134066?vs=475517=475879#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
  lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
@@ -0,0 +1,48 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// RUN: %clang_cl --target=x86_64-windows-msvc -c /Fo%t1.obj -- %p/Inputs/incomplete-tag-type.cpp
+// RUN: %clang_cl --target=x86_64-windows-msvc /O1 /Z7 -c /Fo%t2.obj -- %s
+// RUN: lld-link /debug:full /nodefaultlib /entry:main %t1.obj %t2.obj /out:%t.exe /pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -o \
+// RUN:   "settings set interpreter.stop-command-source-on-error false" \
+// RUN:   -o "p b" -o "p d" -o "p static_e_ref" -o "exit" 2>&1 | FileCheck %s
+
+// CHECK: (lldb) p b
+// CHECK: (B) $0 = {}
+// CHECK: (lldb) p d
+// CHECK: (D) $1 = {}
+// CHECK: (lldb) p static_e_ref
+// CHECK: error: expression failed to parse:
+// CHECK: error: {{.*}}: incomplete type 'E' where a complete type is required
+// CHECK: static_e_ref
+// CHECK: ^
+
+// Complete base class.
+struct A { int x; A(); };
+struct B : A {};
+B b;
+
+// Complete data member.
+struct C {
+  C();
+};
+
+struct D {
+  C c;
+};
+D d;
+
+// Incomplete static data member should return error.
+struct E {
+  E();
+};
+
+struct F {
+  static E static_e;
+};
+
+E F::static_e = E();
+E& static_e_ref = F::static_e;
+
+int main(){}
Index: lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
@@ -0,0 +1,15 @@
+struct A {
+  int x;
+  A();
+};
+A::A() : x(47) {}
+
+struct C {
+  C();
+};
+C::C() = default;
+
+struct E {
+  E();
+};
+E::E() = default;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1045,6 +1045,12 @@
 return m_source_manager_up.get();
   }
 
+  /// Complete a type from debug info, or mark it as forcefully completed if
+  /// there is no definition of the type in the current Module. Call this
+  /// function in contexts where the usual C++ rules require a type to be
+  /// complete (base class, member, etc.).
+  static void RequireCompleteType(CompilerType type);
+
 private:
   /// Returns the PrintingPolicy used when generating the internal type names.
   /// These type names are mostly used for the formatter selection.
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9812,6 +9812,29 @@
   return nullptr;
 }
 
+void TypeSystemClang::RequireCompleteType(CompilerType type) {
+  // Technically, enums can be incomplete too, but we don't handle those as they
+  // are emitted even under -flimit-debug-info.
+  if (!TypeSystemClang::IsCXXClassType(type))
+return;
+
+  if (type.GetCompleteType())
+return;
+
+  // No complete definition in this module.  Mark the class as complete to
+  // satisfy local ast invariants, but make a note of the fact that
+  // it is not _really_ complete so we can later search for a definition in a
+  // different module.
+  // Since we provide layout assistance, layouts of types containing this class
+  // will be correct even if we  are not able to find the definition elsewhere.
+  bool started = TypeSystemClang::StartTagDeclarationDefinition(type);
+  lldbassert(started && "Unable to start a class type definition.");
+  TypeSystemClang::CompleteTagDeclarationDefinition(type);
+  const clang::TagDecl *td = ClangUtil::GetAsTagDecl(type);
+  auto  = llvm::cast(*type.GetTypeSystem());
+  ts.GetMetadata(td)->SetIsForcefullyCompleted();
+}
+
 namespace {
 /// A specialized scratch AST used within ScratchTypeSystemClang.
 /// These are the ASTs backing the different IsolatedASTKinds. They behave
Index: 

[Lldb-commits] [lldb] 6435fe6 - [LLDB][NativePDB] Forcefully complete a record type if it has empty debug info and is required to have complete type.

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-11-16T11:17:07-08:00
New Revision: 6435fe699c2978aa8a91dc1cf5daa82fb4b28d95

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

LOG: [LLDB][NativePDB] Forcefully complete a record type if it has empty debug 
info and is required to have complete type.

It's required in following situations:
1. As a base class.
2. As a data member.
3. As an array element type.

Reviewed By: labath

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

Added: 
lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 037e11895b106..80709d1f0b0dd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -227,28 +227,6 @@ static void ForcefullyCompleteType(CompilerType type) {
   ts.GetMetadata(td)->SetIsForcefullyCompleted();
 }
 
-/// Complete a type from debug info, or mark it as forcefully completed if
-/// there is no definition of the type in the current Module. Call this 
function
-/// in contexts where the usual C++ rules require a type to be complete (base
-/// class, member, etc.).
-static void RequireCompleteType(CompilerType type) {
-  // Technically, enums can be incomplete too, but we don't handle those as 
they
-  // are emitted even under -flimit-debug-info.
-  if (!TypeSystemClang::IsCXXClassType(type))
-return;
-
-  if (type.GetCompleteType())
-return;
-
-  // No complete definition in this module.  Mark the class as complete to
-  // satisfy local ast invariants, but make a note of the fact that
-  // it is not _really_ complete so we can later search for a definition in a
-  // 
diff erent module.
-  // Since we provide layout assistance, layouts of types containing this class
-  // will be correct even if we  are not able to find the definition elsewhere.
-  ForcefullyCompleteType(type);
-}
-
 /// This function serves a similar purpose as RequireCompleteType above, but it
 /// avoids completing the type if it is not immediately necessary. It only
 /// ensures we _can_ complete the type later.
@@ -1323,7 +1301,7 @@ DWARFASTParserClang::ParseArrayType(const DWARFDIE ,
   if (byte_stride == 0 && bit_stride == 0)
 byte_stride = element_type->GetByteSize(nullptr).value_or(0);
   CompilerType array_element_type = element_type->GetForwardCompilerType();
-  RequireCompleteType(array_element_type);
+  TypeSystemClang::RequireCompleteType(array_element_type);
 
   uint64_t array_element_bit_stride = byte_stride * 8 + bit_stride;
   CompilerType clang_type;
@@ -2212,7 +2190,8 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE ,
 clang::TypeSourceInfo *type_source_info =
 base_class->getTypeSourceInfo();
 if (type_source_info)
-  RequireCompleteType(m_ast.GetType(type_source_info->getType()));
+  TypeSystemClang::RequireCompleteType(
+  m_ast.GetType(type_source_info->getType()));
   }
 
   m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
@@ -3018,7 +2997,7 @@ void DWARFASTParserClang::ParseSingleMember(
 }
   }
 
-  RequireCompleteType(member_clang_type);
+  TypeSystemClang::RequireCompleteType(member_clang_type);
 
   clang::FieldDecl *field_decl = TypeSystemClang::AddFieldToRecordType(
   class_clang_type, attrs.name, member_clang_type, attrs.accessibility,

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 2a0954a20aaa7..b4125ca26d04f 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -92,9 +92,10 @@ void UdtRecordCompleter::AddMethod(llvm::StringRef name, 
TypeIndex type_idx,
   m_ast_builder.GetOrCreateType(PdbTypeSymId(type_idx));
   if (method_qt.isNull())
 return;
-  m_ast_builder.CompleteType(method_qt);
   CompilerType method_ct = m_ast_builder.ToCompilerType(method_qt);
-  lldb::opaque_compiler_type_t derived_opaque_ty = 
m_derived_ct.GetOpaqueQualType();
+  TypeSystemClang::RequireCompleteType(method_ct);
+  lldb::opaque_compiler_type_t derived_opaque_ty =
+  m_derived_ct.GetOpaqueQualType();
   auto iter = m_cxx_record_map.find(derived_opaque_ty);
   if (iter != 

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:410
+  LLDB_LOG(log, "xcrun returned exit code %d", status);
+  return "";
+}

JDevlieghere wrote:
> Why not return an Error?
For the caller of this lambda "there is no such SDK" is not an error that 
should be surfaced to the user. There's a loop that will try multiple alternate 
spellings until it finds a matching SDK.


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

https://reviews.llvm.org/D138060

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

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 475857.
aprantl added a comment.

Address feedback


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

https://reviews.llvm.org/D138060

Files:
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/HostTest.cpp

Index: lldb/unittests/Host/HostTest.cpp
===
--- lldb/unittests/Host/HostTest.cpp
+++ lldb/unittests/Host/HostTest.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "lldb/Host/Host.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfoBase.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
@@ -25,3 +27,25 @@
   ASSERT_EQ("Host::GetEnvironment",
 Host::GetEnvironment().lookup("LLDB_TEST_ENVIRONMENT_VAR"));
 }
+
+#if defined(__APPLE__)
+TEST(Host, RunShellCommand) {
+  HostInfoBase::Initialize();
+  FileSystem::Initialize();
+  std::string shell = std::string("SHELL=") + getenv("SHELL");
+  putenv(const_cast("SHELL=/bin/LLDB_TEST_this-file-does-not-exist"));
+  int status, signo;
+  std::string out;
+  auto timeout = std::chrono::seconds(60);
+  Status error = Host::RunShellCommand("/usr/bin/true", FileSpec(), ,
+   , , timeout);
+  ASSERT_TRUE(error.Fail());
+  putenv(const_cast(shell.c_str()));
+
+  error = Host::RunShellCommand("/usr/bin/false", FileSpec(), , ,
+, timeout);
+  ASSERT_FALSE(error.Fail());
+  HostInfoBase::Terminate();
+  FileSystem::Terminate();
+}
+#endif
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -32,6 +32,7 @@
   ${FILES}
   LINK_LIBS
 lldbHost
+lldbCore
 lldbUtilityHelpers
 lldbHostHelpers
 LLVMTestingSupport
Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -6,15 +6,17 @@
 //
 //===--===//
 
+#include "lldb/Host/macosx/HostInfoMacOSX.h"
+#include "Utility/UuidCompatibility.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
-#include "lldb/Host/macosx/HostInfoMacOSX.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
-#include "Utility/UuidCompatibility.h"
 
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
@@ -365,12 +367,16 @@
   return g_developer_directory;
 }
 
-static std::string GetXcodeSDK(XcodeSDK sdk) {
+llvm::Expected GetXcodeSDK(XcodeSDK sdk) {
   XcodeSDK::Info info = sdk.Parse();
   std::string sdk_name = XcodeSDK::GetCanonicalName(info);
+  Progress progress(
+  llvm::formatv("Invoking xcrun to search for SDK {0}", sdk_name));
+  Log *log = GetLog(LLDBLog::Host);
 
   auto xcrun = [](const std::string ,
-  llvm::StringRef developer_dir = "") -> std::string {
+  llvm::StringRef developer_dir =
+  "") -> llvm::Expected {
 Args args;
 if (!developer_dir.empty()) {
   args.AppendArgument("/usr/bin/env");
@@ -391,13 +397,29 @@
 int status = 0;
 int signo = 0;
 std::string output_str;
-lldb_private::Status error =
-Host::RunShellCommand(args, FileSpec(), , , _str,
-  std::chrono::seconds(15));
-
-// Check that xcrun return something useful.
-if (status != 0 || output_str.empty())
-  return {};
+// The first time after Xcode was updated or freshly installed,
+// xcrun can take surprisingly long to build up its database.
+auto timeout = std::chrono::seconds(60);
+bool run_in_shell = false;
+lldb_private::Status error = Host::RunShellCommand(
+args, FileSpec(), , , _str, timeout, run_in_shell);
+
+// Check that xcrun returned something useful.
+if (error.Fail()) {
+  // Catastrophic error.
+  LLDB_LOG(log, "xcrun failed to execute: %s", error.AsCString());
+  return error.ToError();
+}
+if (status != 0) {
+  // xcrun didn't find a matching SDK. Not an error, we'll try
+  // different spellings.
+  LLDB_LOG(log, "xcrun returned exit code %d", status);
+  return "";
+}
+if (output_str.empty()) {
+  LLDB_LOG(log, "xcrun returned no results");
+  return "";
+}
 
 // Convert to a StringRef so we can manipulate the string without modifying
 // the underlying data.
@@ -414,7 +436,8 @@
 return output.str();
   };
 
-  auto find_sdk = [](const std::string _name) -> std::string {
+  auto 

[Lldb-commits] [PATCH] D137983: [lldb] Disable looking at pointee types to find synthetic value for non-ObjC

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

We have tests for shared libraries (grep for DYLIB_C(XX)_SOURCES)), but it's 
easier to reproduce this problem by compiling one CU with -g0 -- see inline 
comment. (If you are creating an "API" test for this, beware that their default 
is to build everything with -fstandalone-debug, and you'll need to explicitly 
change that (see LIMIT_DEBUG_INFO_FLAGS))

This behavior (bug) is triggered by the `FrontEndWantsDereference` formatter 
flag, which is why it manifests itself for (libc++) vector and friends. The 
ObjC formatters don't set that flag so they should be safe. The libstdc++ 
formatters don't set it either. Furthermore there doesn't seem to be a way to 
set this flag by the user, so it is not possible to create a libc++-independent 
reproducer (which is what I was going to suggest).

I've been looking for a better way to fix this today (sorry about the delay), 
but I haven't figured out anything better. There is this fundamental recursion 
between the pretty printer (which wants to dereference a value) and the 
dereferencing code (which wants to know if it has a pretty printer -- so it can 
avoid dereferencing). Just removing the `HasSyntheticValue()`fixed the bug for 
me -- but then we were able to "dereference" incomplete structs. It's possible 
we may be able to allow the dereference to succeed here, but then refuse to 
print the value somewhere down the line. I would like to hear what @jingham 
things about all this.




Comment at: lldb/source/Core/ValueObject.cpp:2676-2677
 if (!m_deref_valobj) {
-  if (HasSyntheticValue()) {
+  // FIXME: C++ stdlib formatters break with incomplete types (e.g.
+  // `std::vector &`). Remove ObjC restriction once that's resolved.
+  if (Language::LanguageIsObjC(GetPreferredDisplayLanguage()) &&

aeubanks wrote:
> dblaikie wrote:
> > Maybe worth filing a bug and referencing it here?
> > 
> > Is this limitation still necessary if the incomplete type has template 
> > parameter DIEs? (I guess probably yes, because it'll be missing member 
> > descriptions, etc)
> > 
> > & does this path get hit if the type is declared in one CU but defined in 
> > another? (& does the inf recurse/crash loop still get hit in that case, 
> > without this patch?)
> > Maybe worth filing a bug and referencing it here?
> Filed https://github.com/llvm/llvm-project/issues/59012, added here
> 
> > Is this limitation still necessary if the incomplete type has template 
> > parameter DIEs? (I guess probably yes, because it'll be missing member 
> > descriptions, etc)
> yes (I incorrectly mentioned in person that this works with 
> `-gsimple-template-names`, it actually still infinite recurses)
> 
> > & does this path get hit if the type is declared in one CU but defined in 
> > another? (& does the inf recurse/crash loop still get hit in that case, 
> > without this patch?)
> if the declaration is in a shared library and the main binary has the 
> definition, we hit this
> if we have two CUs, one with a declaration, one with a definition, but both 
> linked into the same binary, we don't hit the issue
> AFAICT lldb restricts looking up debug info to the binary/shared library, but 
> otherwise prefers definitions over declarations?
Yes, but if one of those CUs (the one that was supposed to contain the 
definition) is built without debug info, then we end up exactly in the same 
situation (a binary without a definition of a type), but without the 
complications that shared libraries bring.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137983

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

[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

[Teresa Johnson via Phabricator via lldb-commits]

tejohnson added inline comments.



Comment at: lld/COFF/LTO.cpp:238
+  sys::path::append(path, directory,
+outputFileBaseName + ".lto." + baseName);
+  sys::path::remove_dots(path, true);

MaskRay wrote:
> What if two input bitcode files have the same basename, e.g. `dir1/a.obj` and 
> `dir2/a.obj`?
I think that should be ok as the output file path created here includes the 
directory. So you should get dir1/a.out.lto.a.obj and dir2/a.out.lto.a.obj.



Comment at: lld/COFF/LTO.cpp:229
+StringRef ltoObjName;
+if (bitcodeFilePath == "ld-temp.o") {
+  ltoObjName =

MaskRay wrote:
> MaskRay wrote:
> > tejohnson wrote:
> > > zequanwu wrote:
> > > > tejohnson wrote:
> > > > > This case should always be i==0 I think?
> > > > IIUC, "ld-temp.o" is the name of combined module. Do you mean there 
> > > > will be only 1 combined module and it will always be the first task?
> > > Yes. So you don't need the handling for i==0 in the name in this case 
> > > (you could probably assert that i==0).
> > This looks like a hack. `assert(i == 0)` will fail with 
> > `-opt:lldltopartitions=2`: `buf[1]` will be called `ld-temp.o` as well.
> > 
> > In addition, if an input bitcode file is called `ld-temp.o` (for some 
> > reason they don't use `.obj`, just `ld-temp.o`), `assert(i == 0)` will fail 
> > as well.
> I guess `if (i < config->ltoPartitions)` may fix the issue.
Ah ok, forgot about the lto partitions case. Sorry, @zequanwu, looks like you 
will need to go back to your old handling that appends i if it is non-zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137217

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

[Lldb-commits] [PATCH] D137873: [LLDB][Minidump] Set abi environment for windows.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Adding a new setting isn't exactly what I had in mind. I was actually hoping 
that it would be enough to fiddle with that setting and leave the minidump 
environment blank (because we have code 

 to fill in the missing pieces of a target ArchSpec from other sources.

If that doesn't work then (besides knowing why), I'd like us try some kind of a 
single-setting solution, as I don't think it makes sense to have two different 
settings like this (happy to hear counterarguments to that). I'd consider 
either moving that environment setting to some place more generic (so it can be 
accessed from both plugins) or just having the minidump plugin fetch the 
setting from inside ObjectFilePECOFF (I'd say that a process->object dependency 
is kinda OK, and we already have ProcessMachCore depending on ObjectFileMachO).




Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:452
 ArchSpec ProcessMinidump::GetArchitecture() {
+  // "settings set plugin.process.minidump.abi" overrides the env in minidump.
+  ArchSpec arch = m_minidump_parser->GetArchitecture();

Is "overrides" the correct word here? Are there any circumstances in which we 
are able to determine the environment from the minidump file? Because if it is, 
then I would expect this to be the other way around (that the environment from 
a specific file overrides the generic catch-all setting)...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137873

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

[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type if it has empty debug info and is required to have complete type.

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:9816-9819
+/// Complete a type from debug info, or mark it as forcefully completed if
+/// there is no definition of the type in the current Module. Call this 
function
+/// in contexts where the usual C++ rules require a type to be complete (base
+/// class, member, etc.).

Please move this comment to the header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066

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

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Some more comments inline, but I think this is essentially as good as we can 
make it.




Comment at: lldb/include/lldb/Symbol/Type.h:300
 
-  TypeSystem *GetTypeSystem(bool prefer_dynamic);
+  CompilerType::TypeSystemSPWrapper GetTypeSystem(bool prefer_dynamic);
 

Maybe the wrapper type should just be its own top-level entity? Or 
`TypeSystem::SPWrapper`?



Comment at: lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp:193-194
+if (!m_type_system_sp) {
+  m_type_system = new TypeSystemClang("siginfo", triple);
+  m_type_system_sp.reset(m_type_system);
+}

Remove `m_type_system`, change `m_type_system_sp` to 
`std::shared_ptr`



Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:316-321
+std::lock_guard guard(m_mutex);
+if (!m_type_system_sp) {
+  m_type_system = new TypeSystemClang("siginfo", triple);
+  m_type_system_sp.reset(m_type_system);
+}
+  }

Same here (but also remove the `_wp` variable).



Comment at: lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp:211
+std::lock_guard guard(m_mutex);
+if (!m_type_system_sp) {
+  m_type_system = new TypeSystemClang("siginfo", triple);

and here



Comment at: lldb/source/Symbol/TypeSystem.cpp:278-279
   m_map[language] = type_system_sp;
-  if (type_system_sp.get())
-return *type_system_sp.get();
+  if (type_system_sp) {
+assert(!type_system_sp->weak_from_this().expired());
+return type_system_sp;

With the current implementation, I don't think these can ever fire.



Comment at: lldb/source/Target/Target.cpp:2461
+Language::GetNameForLanguageType(language) +
+llvm::StringRef("is no longer live"),
+llvm::inconvertibleErrorCode());

Maybe this error needs some updating? (also, missing spaces -- the language 
will get glued to the error msg)



Comment at: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h:48
+/// Simulates a Clang type system owned by a TypeSystemMap.
+class TypeSystemClangHolder {
+  TypeSystemClang *m_ast;

Can this be replaced by a `shared_ptr` now?


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

https://reviews.llvm.org/D136650

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

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:13
 #include "lldb/Host/macosx/HostInfoMacOSX.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Utility/Args.h"

We shouldn't have Host->Core dependencies. Could you report the error in the 
caller?



Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:401
+lldb_private::Status error = Host::RunShellCommand(
+args, FileSpec(), , , _str, timeout);
+

why not just pass `run_in_shell=false` (the optional argument coming after the 
timeout)?



Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:527
+}
+Debugger::ReportError(msg);
+  }

`ReportError(toString(path_or_err.takeError()))` ?



Comment at: lldb/unittests/Host/HostTest.cpp:40
+  auto timeout = std::chrono::seconds(60);
+  Status error = Host::RunShellCommand("/usr/bin/true", FileSpec(), ,
+   , , timeout);

On one (linux) machine I have access to, this is present in `/bin/true`. A 
different one has the same file in both paths.


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

https://reviews.llvm.org/D138060

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

[Lldb-commits] [PATCH] D138077: Send statistics in initialized event

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It fails on all linux bots, so I've reverted it for now.

From what I can tell, the problem is that linux does not immediately (before 
running the process) load the shared library, and so the overall startup debug 
info size is reported as zero (because the main executable is stripped). It 
doesn't seem like this test really needs shared libraries, so maybe the 
simplest way to fix it would be to use an unstripped executable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138077

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

[Lldb-commits] [lldb] 22887ff - Revert "Send statistics in initialized event"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-11-16T12:20:21+01:00
New Revision: 22887ff964ee9135417f7c0f99915296b710934f

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

LOG: Revert "Send statistics in initialized event"

The test is failing on linux.

This reverts commits 7fe3586cda5b683766ec6b6d5ca2d98c2baaf162 and
d599ac41aabddeb2442db7b31faacf143d63abe4.

Added: 
lldb/test/API/tools/lldb-vscode/terminated-event/Makefile

lldb/test/API/tools/lldb-vscode/terminated-event/TestVSCode_terminatedEvent.py
lldb/test/API/tools/lldb-vscode/terminated-event/foo.cpp
lldb/test/API/tools/lldb-vscode/terminated-event/foo.h
lldb/test/API/tools/lldb-vscode/terminated-event/main.cpp

Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/JSONUtils.h
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 
lldb/test/API/tools/lldb-vscode/eventStatistic/Makefile
lldb/test/API/tools/lldb-vscode/eventStatistic/TestVSCode_eventStatistic.py
lldb/test/API/tools/lldb-vscode/eventStatistic/foo.cpp
lldb/test/API/tools/lldb-vscode/eventStatistic/foo.h
lldb/test/API/tools/lldb-vscode/eventStatistic/main.cpp



diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 49f268ae28793..c2de4ad5c7d9a 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -134,7 +134,6 @@ def __init__(self, recv, send, init_commands, 
log_file=None):
 self.configuration_done_sent = False
 self.frame_scopes = {}
 self.init_commands = init_commands
-self.initialized_event = None
 
 @classmethod
 def encode_content(cls, s):
@@ -232,8 +231,6 @@ def handle_recv_packet(self, packet):
 self._process_stopped()
 tid = body['threadId']
 self.thread_stop_reasons[tid] = body
-elif event == 'initialized':
-self.initialized_event = packet
 elif event == 'breakpoint':
 # Breakpoint events come in when a breakpoint has locations
 # added or removed. Keep track of them so we can look for them

diff  --git a/lldb/test/API/tools/lldb-vscode/eventStatistic/Makefile 
b/lldb/test/API/tools/lldb-vscode/terminated-event/Makefile
similarity index 100%
rename from lldb/test/API/tools/lldb-vscode/eventStatistic/Makefile
rename to lldb/test/API/tools/lldb-vscode/terminated-event/Makefile

diff  --git 
a/lldb/test/API/tools/lldb-vscode/eventStatistic/TestVSCode_eventStatistic.py 
b/lldb/test/API/tools/lldb-vscode/terminated-event/TestVSCode_terminatedEvent.py
similarity index 63%
rename from 
lldb/test/API/tools/lldb-vscode/eventStatistic/TestVSCode_eventStatistic.py
rename to 
lldb/test/API/tools/lldb-vscode/terminated-event/TestVSCode_terminatedEvent.py
index 2b7630795dcab..ae364a5fe1f0d 100644
--- 
a/lldb/test/API/tools/lldb-vscode/eventStatistic/TestVSCode_eventStatistic.py
+++ 
b/lldb/test/API/tools/lldb-vscode/terminated-event/TestVSCode_terminatedEvent.py
@@ -10,27 +10,7 @@
 import re
 import json
 
-class TestVSCode_eventStatistic(lldbvscode_testcase.VSCodeTestCaseBase):
-
-def check_statistic(self, statistics):
-self.assertTrue(statistics['totalDebugInfoByteSize'] > 0)
-self.assertTrue(statistics['totalDebugInfoEnabled'] > 0)
-self.assertTrue(statistics['totalModuleCountHasDebugInfo'] > 0)
-
-self.assertIsNotNone(statistics['memory'])
-self.assertNotIn('modules', statistics.keys())
-
-def check_target(self, statistics):
-# lldb-vscode debugs one target at a time
-target = json.loads(statistics['targets'])[0]
-self.assertTrue(target['totalBreakpointResolveTime'] > 0)
-
-breakpoints = target['breakpoints']
-self.assertIn('foo',
-  
breakpoints[0]['details']['Breakpoint']['BKPTResolver']['Options']['SymbolNames'],
-  'foo is a symbol breakpoint')
-
self.assertTrue(breakpoints[1]['details']['Breakpoint']['BKPTResolver']['Options']['FileName'].endswith('main.cpp'),
-'target has source line breakpoint in main.cpp')
+class TestVSCode_terminatedEvent(lldbvscode_testcase.VSCodeTestCaseBase):
 
 @skipIfWindows
 @skipIfRemote
@@ -65,34 +45,20 @@ def test_terminated_event(self):
 self.continue_to_exit()
 
 statistics = self.vscode.wait_for_terminated()['statistics']
-self.check_statistic(statistics)
-self.check_target(statistics)
+self.assertTrue(statistics['totalDebugInfoByteSize'] > 0)
+

[Lldb-commits] [PATCH] D136809: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[LJC via Phabricator via lldb-commits]

paperchalice added a comment.

Ping @Ericson2314


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136809

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

[Lldb-commits] [PATCH] D136809: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[LJC via Phabricator via lldb-commits]

paperchalice updated this revision to Diff 475706.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136809

Files:
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  cmake/Modules/GetClangResourceDir.cmake
  compiler-rt/cmake/base-config-ix.cmake
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  openmp/CMakeLists.txt

Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -79,8 +79,8 @@
 if(${OPENMP_STANDALONE_BUILD})
   set(LIBOMP_HEADERS_INSTALL_PATH "${CMAKE_INSTALL_INCLUDEDIR}")
 else()
-  string(REGEX MATCH "[0-9]+" CLANG_VERSION ${PACKAGE_VERSION})
-  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION}/include")
+  include(GetClangResourceDir)
+  get_clang_resource_dir(LIBOMP_HEADERS_INSTALL_PATH SUBDIR include)
 endif()
 
 # Build host runtime library, after LIBOMPTARGET variables are set since they are needed
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -257,7 +257,11 @@
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR
  ${PACKAGE_VERSION})
-  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
+  if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "")
+set(resource_dir ${LLVM_TOOLS_BINARY_DIR}/${CLANG_RESOURCE_DIR})
+  else()
+set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
+  endif()
   set(flag_types ASM C CXX MODULE_LINKER SHARED_LINKER EXE_LINKER)
   foreach(type ${flag_types})
 set(${type}_flag -DCMAKE_${type}_FLAGS=-resource-dir=${resource_dir})
Index: lldb/unittests/Expression/ClangParserTest.cpp
===
--- lldb/unittests/Expression/ClangParserTest.cpp
+++ lldb/unittests/Expression/ClangParserTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Basic/Version.h"
+#include "clang/Config/config.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangHost.h"
 #include "TestingSupport/SubsystemRAII.h"
@@ -37,12 +38,16 @@
 TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #if !defined(_WIN32)
   std::string path_to_liblldb = "/foo/bar/lib/";
-  std::string path_to_clang_dir =
-  "/foo/bar/" LLDB_INSTALL_LIBDIR_BASENAME "/clang/" CLANG_VERSION_MAJOR_STRING;
+  std::string path_to_clang_dir = CLANG_RESOURCE_DIR[0]
+  ? "/foo/bar/bin/" CLANG_RESOURCE_DIR
+  : "/foo/bar/" LLDB_INSTALL_LIBDIR_BASENAME
+"/clang/" CLANG_VERSION_MAJOR_STRING;
 #else
   std::string path_to_liblldb = "C:\\foo\\bar\\lib";
   std::string path_to_clang_dir =
-  "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
+  CLANG_RESOURCE_DIR[0]
+  ? "C:\\foo\\bar\\bin\\" CLANG_RESOURCE_DIR
+  : "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
 #endif
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -54,8 +54,11 @@
 
   static const llvm::StringRef kResourceDirSuffixes[] = {
   // LLVM.org's build of LLDB uses the clang resource directory placed
-  // in $install_dir/lib{,64}/clang/$clang_version.
-  CLANG_INSTALL_LIBDIR_BASENAME "/clang/" CLANG_VERSION_MAJOR_STRING,
+  // in $install_dir/lib{,64}/clang/$clang_version or
+  // $install_dir/bin/$CLANG_RESOURCE_DIR
+  CLANG_RESOURCE_DIR[0] ? "bin/" CLANG_RESOURCE_DIR
+: CLANG_INSTALL_LIBDIR_BASENAME
+  "/clang/" CLANG_VERSION_MAJOR_STRING,
   // swift-lldb uses the clang resource directory copied from swift, which
   // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
   // it there, so we use LLDB_INSTALL_LIBDIR_BASENAME.
Index: compiler-rt/cmake/base-config-ix.cmake
===
--- compiler-rt/cmake/base-config-ix.cmake
+++ compiler-rt/cmake/base-config-ix.cmake
@@ -7,6 +7,7 @@
 include(CheckIncludeFile)
 include(CheckCXXSourceCompiles)
 include(GNUInstallDirs)
+include(GetClangResourceDir)
 include(ExtendPath)
 
 check_include_file(unwind.h HAVE_UNWIND_H)
@@