[Lldb-commits] [PATCH] D134877: [lldb] Get rid of __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS

[Pavel Labath via Phabricator via lldb-commits]

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

ship it.


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

https://reviews.llvm.org/D134877

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

[Lldb-commits] [PATCH] D134927: Make the sanitizer Notify callbacks asynchronous

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: JDevlieghere, kubamracek.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

lldb handles breakpoints in two phases, the "sync" and "async" phase.  The 
synchronous phase happens early in the event handling, and is for when you need 
to do some work before anyone else gets a chance to look at the breakpoint.  
It's used by the dynamic loader plugins, for instance.

As such, we're in a place where we haven't quite figured out about the current 
event, and are not in a state to handle another one.  That makes running the 
expression evaluator in synchronous callbacks flakey.  The Sanitizer callbacks 
were all registered as synchronous, even though their primary job is to call 
report generation functions in the target.  , calling functions works 
better than I would have expected, but if you rapidly continue enough times 
over a sequence of sanitizer reports, you eventually confuse the event state 
machine.

I bet with enough thinking we could make expression evaluation in sync 
callbacks work, but it would further complicate an already complex system.  And 
there's no reason for the sanitizer callbacks to be synchronous, they are very 
like user report functions in a breakpoint, and should run in the same way.  So 
the simpler path of converting these to async seems the better one.

This patch switches the report functions from sync to async, adds some testing 
for continuing past sequential UBSan reports, and adds a comment telling people 
not to use expressions in sync callbacks.   The test is not great, in that I 
can get this same test code to misbehave quite regularly in Xcode, but I 
couldn't reproduce enough of the Xcode environment in a test to produce a 
reliably failing test, so this test for the most part passes even without the 
fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134927

Files:
  lldb/include/lldb/Breakpoint/Breakpoint.h
  lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
  lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
  lldb/test/API/functionalities/ubsan/basic/main.c

Index: lldb/test/API/functionalities/ubsan/basic/main.c
===
--- lldb/test/API/functionalities/ubsan/basic/main.c
+++ lldb/test/API/functionalities/ubsan/basic/main.c
@@ -1,4 +1,16 @@
 int main() {
   int data[4];
-  return *(int *)(((char *)[0]) + 2); // align line
+  int result = *(int *)(((char *)[0]) + 2); // align line
+
+  int *p = data + 5;  // Index 5 out of bounds for type 'int [4]'
+  *p = data + 5;
+  *p = data + 5;
+  *p = data + 5;
+  *p = data + 5;
+  *p = data + 5;
+  *p = data + 5;
+  *p = data + 5;
+  *p = data + 5;
+
+  return 0;
 }
Index: lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
===
--- lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
+++ lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
@@ -86,4 +86,9 @@
 self.assertEqual(os.path.basename(data["filename"]), "main.c")
 self.assertEqual(data["line"], self.line_align)
 
-self.runCmd("continue")
+for count in range(0,8):
+process.Continue()
+stop_reason = thread.GetStopReason()
+self.assertEqual(stop_reason, lldb.eStopReasonInstrumentation,
+ "Round {0} wasn't instrumentation".format(count))
+
Index: lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
===
--- lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -276,7 +276,7 @@
 /*hardware=*/false)
   .get();
   breakpoint->SetCallback(InstrumentationRuntimeUBSan::NotifyBreakpointHit,
-  this, true);
+  this, false);
   breakpoint->SetBreakpointKind("undefined-behavior-sanitizer-report");
   SetBreakpointID(breakpoint->GetID());
 
Index: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
===
--- lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -922,7 +922,7 @@
   .CreateBreakpoint(symbol_address, internal, hardware)
   .get();
   

[Lldb-commits] [PATCH] D134842: Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core files.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 464097.
clayborg added a comment.

Fix typo in comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134842

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -91,6 +91,9 @@
   std::map>
   m_loaded_modules;
 
+  /// Returns true if the process is for a core file.
+  bool IsCoreFile() const;
+
   /// If possible sets a breakpoint on a function called by the runtime
   /// linker each time a module is loaded or unloaded.
   bool SetRendezvousBreakpoint();
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -213,6 +213,10 @@
 void DynamicLoaderPOSIXDYLD::ProbeEntry() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return;
+
   const addr_t entry = GetEntryPoint();
   if (entry == LLDB_INVALID_ADDRESS) {
 LLDB_LOGF(
@@ -297,6 +301,11 @@
 
 bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
+
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return false;
+
   if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
 LLDB_LOG(log,
  "Rendezvous breakpoint breakpoint id {0} for pid {1}"
@@ -829,3 +838,7 @@
 
   return module_sp->GetFileSpec().GetPath() == "[vdso]";
 }
+
+bool DynamicLoaderPOSIXDYLD::IsCoreFile() const {
+  return !m_process->IsLiveDebugSession();
+}
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
@@ -267,6 +267,8 @@
 
   bool FindMetadata(const char *name, PThreadField field, uint32_t );
 
+  bool IsCoreFile() const;
+
   enum RendezvousAction {
 eNoAction,
 eTakeSnapshot,
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -190,6 +190,14 @@
 }
 
 DYLDRendezvous::RendezvousAction DYLDRendezvous::GetAction() const {
+  // If we have a core file, we will read the current rendezvous state
+  // from the core file's memory into m_current which can be in an inconsistent
+  // state, so we can't rely on its state to determine what we should do. We
+  // always need it to load all of the shared libraries one time when we attach
+  // to a core file.
+  if (IsCoreFile())
+return eTakeSnapshot;
+
   switch (m_current.state) {
 
   case eConsistent:
@@ -664,3 +672,7 @@
 LLDB_LOGF(log, "  Prev : %" PRIx64, I->prev);
   }
 }
+
+bool DYLDRendezvous::IsCoreFile() const {
+  return !m_process->IsLiveDebugSession();
+}


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -91,6 +91,9 @@
   std::map>
   m_loaded_modules;
 
+  /// Returns true if the process is for a core file.
+  bool IsCoreFile() const;
+
   /// If possible sets a breakpoint on a function called by the runtime
   /// linker each time a module is loaded or unloaded.
   bool SetRendezvousBreakpoint();
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -213,6 +213,10 @@
 void DynamicLoaderPOSIXDYLD::ProbeEntry() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return;
+
   const addr_t entry = GetEntryPoint();
   if (entry == LLDB_INVALID_ADDRESS) {
 LLDB_LOGF(
@@ -297,6 +301,11 @@
 
 bool 

[Lldb-commits] [PATCH] D134922: [lldb] Remove scoped timer from high firing and fast running SymbolFileDWARF::FindFunctions

[Dave Lee via Phabricator via lldb-commits]

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

Profiles show that `SymbolFileDWARF::FindFunctions` is both high firing (many 
thousands of calls) and fast running (35 µs mean).

Timers like this are noise and load for profiling systems, and can be removed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134922

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2336,8 +2336,6 @@
   std::lock_guard guard(GetModuleMutex());
   ConstString name = lookup_info.GetLookupName();
   FunctionNameType name_type_mask = lookup_info.GetNameTypeMask();
-  LLDB_SCOPED_TIMERF("SymbolFileDWARF::FindFunctions (name = '%s')",
- name.AsCString());
 
   // eFunctionNameTypeAuto should be pre-resolved by a call to
   // Module::LookupInfo::LookupInfo()


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2336,8 +2336,6 @@
   std::lock_guard guard(GetModuleMutex());
   ConstString name = lookup_info.GetLookupName();
   FunctionNameType name_type_mask = lookup_info.GetNameTypeMask();
-  LLDB_SCOPED_TIMERF("SymbolFileDWARF::FindFunctions (name = '%s')",
- name.AsCString());
 
   // eFunctionNameTypeAuto should be pre-resolved by a call to
   // Module::LookupInfo::LookupInfo()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134920: [lldb] Remove scoped timer from high firing and fast running ExtractUnitDIENoDwoIfNeeded

[Dave Lee via Phabricator via lldb-commits]

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

Profiles show that `DWARFUnit::ExtractUnitDIENoDwoIfNeeded` is both high firing 
(tens of thousands of calls) and fast running (15 µs mean).

Timers like this are noise and load for profiling systems, and can be removed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134920

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -51,8 +51,6 @@
 return; // Already parsed
 
   ElapsedTime elapsed(m_dwarf.GetDebugInfoParseTimeRef());
-  LLDB_SCOPED_TIMERF("%8.8x: DWARFUnit::ExtractUnitDIENoDwoIfNeeded()",
- GetOffset());
 
   // Set the offset to that of the first DIE and calculate the start of the
   // next compilation unit header.


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -51,8 +51,6 @@
 return; // Already parsed
 
   ElapsedTime elapsed(m_dwarf.GetDebugInfoParseTimeRef());
-  LLDB_SCOPED_TIMERF("%8.8x: DWARFUnit::ExtractUnitDIENoDwoIfNeeded()",
- GetOffset());
 
   // Set the offset to that of the first DIE and calculate the start of the
   // next compilation unit header.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:140
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the

aaron.ballman wrote:
> ldionne wrote:
> > I wonder whether `Announcements` is truly a lower-traffic alternative to 
> > `vendors` groups, if we end up posting each potentially breaking change to 
> > the list and tagging vendors on each such review. I'm not against posting 
> > on Discourse, however it seems to me like basically another equivalent 
> > channel of communication for these changes (which might be beneficial, I'm 
> > neutral on that).
> The reason we have a split like that is for timing and chattiness. If you're 
> a downstream like Intel has with ICX, you might want to be in `clang-vendors` 
> so that you are involved in conversations about potentially breaking changes. 
> You'll be getting emails for all review comments on that review. But if 
> you're a downstream like a Gentoo package maintainer, you might not want to 
> be *that* involved in the development of the compiler, but still want to know 
> when changes are coming down the pipeline to do early pre-release testing 
> while there's still time to put the brakes on before a release goes out.
Okay, makes sense. Let's go for it.


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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D134906: Have MemoryCache cache addresses that were unreadable, so duplicate read attempts can be suppressed

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: jingham, JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch is to address an issue hit by one of our heavy SB API scripting 
developers; they have some code iterating over a large number of objects in 
SBValue's and one of the elements of the object has a type of void*; the 
Itanium ABI is trying to sniff the memory there to see if it might have a 
dynamic type.  In this case, the elements often have the value 0, so lldb is 
trying to read memory at address 0, which fails, and the number of reads is 
quite large.  Each failing memory read takes long enough in a JTAG like 
environment that this is a big perf issue.

We have a MemoryCache subsystem that saves blocks of inferior memory when we do 
a read, so repeated reads of the same address, or reads near the address that 
were cached, are saved at a single point in time.  These memory cache buffers 
are flushed when execution is resumed.

This patch adds a new list of addresses we've tried to read from at this 
execution stop, which returned an error, and will not try to read from those 
addresses again.  If lldb allocates memory in the inferior, or if we resume 
execution, this list of addresses is flushed.

I settled on using an llvm::SmallSet container for these addr_t's, but I'd 
appreciate if people have a different suggestion for the most appropriate 
container.  I expect this list to be relatively small -- it is not common that 
lldb tries to read from addresses that are unreadable -- and my primary 
optimization concern is quick lookup because I will consult this list before I 
read from any address in memory.

When I was outlining my idea for this, Jim pointed out that MemoryCache has a 
`InvalidRanges m_invalid_ranges` ivar already, and could I reuse this.  This is 
called by the DynamicLoader to mark a region of memory as never accessible 
(e.g. the lower 4GB on a 64-bit Darwin process), and is not flushed on 
execution resume / memory allocation.  It is expressed in terms of memory 
ranges, when I don't know the range of memory that is inaccessible beyond an 
address.  I could assume the range extends to the end of a page boundary, if I 
knew the page boundary size, but that still doesn't address the fact that 
`m_invalid_ranges` is intended to track inaccessible memory that is invariant 
during the process lifetime.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134906

Files:
  lldb/include/lldb/Target/Memory.h
  lldb/source/Target/Memory.cpp
  lldb/source/Target/Process.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestMemoryReadFailCache.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestMemoryReadFailCache.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestMemoryReadFailCache.py
@@ -0,0 +1,55 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestMemoryReadFailCache(GDBRemoteTestBase):
+
+@skipIfXmlSupportMissing
+def test(self):
+class MyResponder(MockGDBServerResponder):
+first_read = True
+
+def qHostInfo (self):
+return "cputype:16777228;cpusubtype:2;addressing_bits:47;ostype:macosx;vendor:apple;os_version:12.6.0;endian:little;ptrsize:8;"
+
+def readMemory(self, addr, length):
+if self.first_read:
+  self.first_read = False
+  return "E05"
+return "AA" * length
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+process = self.connect(target)
+
+err = lldb.SBError()
+uint = process.ReadUnsignedFromMemory(0x1000, 4, err)
+self.assertTrue (err.Fail(), 
+  "lldb should fail to read memory at 0x100 the first time, "
+  "after checking with stub")
+
+# Now the remote stub will return a non-zero number if 
+# we ask again, unlike a real stub.
+# Confirm that reading from that address still fails - 
+# that we cached the unreadable address
+uint = process.ReadUnsignedFromMemory(0x1000, 4, err)
+self.assertTrue (err.Fail(), 
+  "lldb should fail to read memory at 0x100 the second time, "
+  "because we did not read from the remote stub.")
+
+# Allocate memory in the inferior, which will flush
+# the unreadable address cache.
+process.AllocateMemory(0x100, 3, 

[Lldb-commits] [PATCH] D134877: [lldb] Get rid of __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS

[serge via Phabricator via lldb-commits]

serge-sans-paille updated this revision to Diff 463999.
serge-sans-paille retitled this revision from "[lldb] Fixes for swig-4.1.0 
macro definition correction" to "[lldb] Get rid of __STDC_LIMIT_MACROS and 
__STDC_CONSTANT_MACROS".
serge-sans-paille edited the summary of this revision.
serge-sans-paille added a comment.

It turns out that the culprint macro are actually obsolete in C++11, so just 
get rid of them.


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

https://reviews.llvm.org/D134877

Files:
  lldb/bindings/CMakeLists.txt
  lldb/bindings/interfaces.swig


Index: lldb/bindings/interfaces.swig
===
--- lldb/bindings/interfaces.swig
+++ lldb/bindings/interfaces.swig
@@ -1,8 +1,5 @@
 /* Various liblldb typedefs that SWIG needs to know about.  */
 #define __extension__ /* Undefine GCC keyword to make Swig happy when 
processing glibc's stdint.h. */
-/* The ISO C99 standard specifies that in C++ implementations limit macros such
-   as INT32_MAX should only be defined if __STDC_LIMIT_MACROS is. */
-#define __STDC_LIMIT_MACROS
 %include "stdint.i"
 
 %include "lldb/lldb-defines.h"
Index: lldb/bindings/CMakeLists.txt
===
--- lldb/bindings/CMakeLists.txt
+++ lldb/bindings/CMakeLists.txt
@@ -26,8 +26,6 @@
   -features autodoc
   -I${LLDB_SOURCE_DIR}/include
   -I${CMAKE_CURRENT_SOURCE_DIR}
-  -D__STDC_LIMIT_MACROS
-  -D__STDC_CONSTANT_MACROS
   ${DARWIN_EXTRAS}
 )
 


Index: lldb/bindings/interfaces.swig
===
--- lldb/bindings/interfaces.swig
+++ lldb/bindings/interfaces.swig
@@ -1,8 +1,5 @@
 /* Various liblldb typedefs that SWIG needs to know about.  */
 #define __extension__ /* Undefine GCC keyword to make Swig happy when processing glibc's stdint.h. */
-/* The ISO C99 standard specifies that in C++ implementations limit macros such
-   as INT32_MAX should only be defined if __STDC_LIMIT_MACROS is. */
-#define __STDC_LIMIT_MACROS
 %include "stdint.i"
 
 %include "lldb/lldb-defines.h"
Index: lldb/bindings/CMakeLists.txt
===
--- lldb/bindings/CMakeLists.txt
+++ lldb/bindings/CMakeLists.txt
@@ -26,8 +26,6 @@
   -features autodoc
   -I${LLDB_SOURCE_DIR}/include
   -I${CMAKE_CURRENT_SOURCE_DIR}
-  -D__STDC_LIMIT_MACROS
-  -D__STDC_CONSTANT_MACROS
   ${DARWIN_EXTRAS}
 )
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm not sure what kind of guarantees are you looking for. `m_process_sp` is a 
private member of the Target class, so it's not like just anyone can come in 
and change it. There's no way to stop code from inside the Target class from 
changing it without going through the `CreateProcess` method, but the same can 
be said about the calls to `WillXXX` methods in the process class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134882

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

[Lldb-commits] [lldb] 4017d86 - When there are variable errors, display an error in VS Code's local variables view.

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2022-09-29T10:55:16-07:00
New Revision: 4017d86df98faa53bbb1a0acc4e8e58d2b6f2608

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

LOG: When there are variable errors, display an error in VS Code's local 
variables view.

After recent diffs that enable variable errors that stop variables from being 
correctly displayed when debugging, allow users to see these errors in the 
LOCALS variables in the VS Code UI. We do this by detecting when no variables 
are available and when there is an error to be displayed, and we add a single 
variable named "" whose value is a string error that the user can read. 
This allows the user to be aware of the reason variables are not available and 
fix the issue. Previously if someone enabled "-gline-tables-only" or was 
debugging with DWARF in .o files or with .dwo files and those separate object 
files were missing or they were out of date, the user would see nothing in the 
variables view. Communicating these errors to the user is essential to a good 
debugging experience.

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

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py 
b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
index e4cb103010fde..9b9195561606b 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -7,7 +7,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
-
+import os
 
 def make_buffer_verify_dict(start_idx, count, offset=0):
 verify_dict = {}
@@ -38,6 +38,14 @@ def verify_values(self, verify_dict, actual, 
varref_dict=None, expression=None):
  ' "%s")') % (
 key, actual_value,
 verify_value))
+if 'contains' in verify_dict:
+verify = verify_dict['contains']
+for key in verify:
+contains_array = verify[key]
+actual_value = actual[key]
+self.assertTrue(isinstance(contains_array, list))
+for verify_value in contains_array:
+self.assertIn(verify_value, actual_value)
 if 'missing' in verify_dict:
 missing = verify_dict['missing']
 for key in missing:
@@ -508,3 +516,46 @@ def test_registers(self):
 self.assertTrue(value.startswith('0x'))
 self.assertTrue('a.out`main + ' in value)
 self.assertTrue('at main.cpp:' in value)
+
+@no_debug_info_test
+@skipUnlessDarwin
+def test_darwin_dwarf_missing_obj(self):
+'''
+Test that if we build a binary with DWARF in .o files and we remove
+the .o file for main.cpp, that we get a variable named ""
+whose value matches the appriopriate error. Errors when getting
+variables are returned in the LLDB API when the user should be
+notified of issues that can easily be solved by rebuilding or
+changing compiler options and are designed to give better feedback
+to the user.
+'''
+self.build(debug_info="dwarf")
+program = self.getBuildArtifact("a.out")
+main_obj = self.getBuildArtifact("main.o")
+self.assertTrue(os.path.exists(main_obj))
+# Delete the main.o file that contains the debug info so we force an
+# error when we run to main and try to get variables
+os.unlink(main_obj)
+
+self.create_debug_adaptor()
+self.assertTrue(os.path.exists(program), 'executable must exist')
+self.launch(program)
+
+functions = ['main']
+breakpoint_ids = self.set_function_breakpoints(functions)
+self.assertEquals(len(breakpoint_ids), len(functions), "expect one 
breakpoint")
+self.continue_to_breakpoints(breakpoint_ids)
+
+locals = self.vscode.get_local_variables()
+
+verify_locals = {
+'': {
+'equals': {'type': 'const char *'},
+'contains': { 'value': [
+'debug map object file ',
+'main.o" containing debug info does not exist, debug info 
will not be loaded']
+}
+},
+}
+varref_dict = {}
+self.verify_variables(verify_locals, locals, varref_dict)

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

[Greg Clayton via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4017d86df98f: When there are variable errors, display an 
error in VS Codes local variables… (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134333

Files:
  lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2953,6 +2953,31 @@
 }
 
 num_children = top_scope->GetSize();
+if (num_children == 0 && variablesReference == VARREF_LOCALS) {
+  // Check for an error in the SBValueList that might explain why we don't
+  // have locals. If we have an error display it as the sole value in the
+  // the locals.
+
+  // "error" owns the error string so we must keep it alive as long as we
+  // want to use the returns "const char *"
+  lldb::SBError error = top_scope->GetError();
+  const char *var_err = error.GetCString();
+  if (var_err) {
+// Create a fake variable named "error" to explain why variables were
+// not available. This new error will help let users know when there was
+// a problem that kept variables from being available for display and
+// allow users to fix this issue instead of seeing no variables. The
+// errors are only set when there is a problem that the user could
+// fix, so no error will show up when you have no debug info, only when
+// we do have debug info and something that is fixable can be done.
+llvm::json::Object object;
+EmplaceSafeString(object, "name", "");
+EmplaceSafeString(object, "type", "const char *");
+EmplaceSafeString(object, "value", var_err);
+object.try_emplace("variablesReference", (int64_t)0);
+variables.emplace_back(std::move(object));
+  }
+}
 const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
 
 // We first find out which variable names are duplicated
Index: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
===
--- lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -7,7 +7,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
-
+import os
 
 def make_buffer_verify_dict(start_idx, count, offset=0):
 verify_dict = {}
@@ -38,6 +38,14 @@
  ' "%s")') % (
 key, actual_value,
 verify_value))
+if 'contains' in verify_dict:
+verify = verify_dict['contains']
+for key in verify:
+contains_array = verify[key]
+actual_value = actual[key]
+self.assertTrue(isinstance(contains_array, list))
+for verify_value in contains_array:
+self.assertIn(verify_value, actual_value)
 if 'missing' in verify_dict:
 missing = verify_dict['missing']
 for key in missing:
@@ -508,3 +516,46 @@
 self.assertTrue(value.startswith('0x'))
 self.assertTrue('a.out`main + ' in value)
 self.assertTrue('at main.cpp:' in value)
+
+@no_debug_info_test
+@skipUnlessDarwin
+def test_darwin_dwarf_missing_obj(self):
+'''
+Test that if we build a binary with DWARF in .o files and we remove
+the .o file for main.cpp, that we get a variable named ""
+whose value matches the appriopriate error. Errors when getting
+variables are returned in the LLDB API when the user should be
+notified of issues that can easily be solved by rebuilding or
+changing compiler options and are designed to give better feedback
+to the user.
+'''
+self.build(debug_info="dwarf")
+program = self.getBuildArtifact("a.out")
+main_obj = self.getBuildArtifact("main.o")
+self.assertTrue(os.path.exists(main_obj))
+# Delete the main.o file that contains the debug info so we force an
+# error when we run to main and try to get variables
+os.unlink(main_obj)
+
+self.create_debug_adaptor()
+self.assertTrue(os.path.exists(program), 'executable must exist')
+self.launch(program)
+
+functions = ['main']
+breakpoint_ids = self.set_function_breakpoints(functions)
+self.assertEquals(len(breakpoint_ids), len(functions), "expect one breakpoint")
+

[Lldb-commits] [lldb] b3a0bed - [NFC] Add header documentation to the SBError::GetCString() to clarify ownwership of the returned string.

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2022-09-29T10:54:31-07:00
New Revision: b3a0bed5fb8766dcf27583ab1f73edc6e7232657

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

LOG: [NFC] Add header documentation to the SBError::GetCString() to clarify 
ownwership of the returned string.

Title says it all!

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

Added: 


Modified: 
lldb/include/lldb/API/SBError.h

Removed: 




diff  --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index b34014aecc441..30214e37c8e5c 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -27,6 +27,10 @@ class LLDB_API SBError {
 
   const SBError =(const lldb::SBError );
 
+  /// Get the error string as a NULL terminated UTF8 c-string.
+  ///
+  /// This SBError object owns the returned string and this object must be kept
+  /// around long enough to use the returned string.
   const char *GetCString() const;
 
   void Clear();



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

[Lldb-commits] [PATCH] D134846: [NFC] Add header documentation to the SBError::GetCString() to clarify ownwership of the returned string.

[Greg Clayton via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3a0bed5fb87: [NFC] Add header documentation to the 
SBError::GetCString() to clarify… (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134846

Files:
  lldb/include/lldb/API/SBError.h


Index: lldb/include/lldb/API/SBError.h
===
--- lldb/include/lldb/API/SBError.h
+++ lldb/include/lldb/API/SBError.h
@@ -27,6 +27,10 @@
 
   const SBError =(const lldb::SBError );
 
+  /// Get the error string as a NULL terminated UTF8 c-string.
+  ///
+  /// This SBError object owns the returned string and this object must be kept
+  /// around long enough to use the returned string.
   const char *GetCString() const;
 
   void Clear();


Index: lldb/include/lldb/API/SBError.h
===
--- lldb/include/lldb/API/SBError.h
+++ lldb/include/lldb/API/SBError.h
@@ -27,6 +27,10 @@
 
   const SBError =(const lldb::SBError );
 
+  /// Get the error string as a NULL terminated UTF8 c-string.
+  ///
+  /// This SBError object owns the returned string and this object must be kept
+  /// around long enough to use the returned string.
   const char *GetCString() const;
 
   void Clear();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134842: Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core files.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 463962.
clayborg added a comment.

Switch DYLDRendezvous::GetAction() to return eTakeSnapshot when we have a core 
file and don't modify m_current.state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134842

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -91,6 +91,9 @@
   std::map>
   m_loaded_modules;
 
+  /// Returns true if the process is for a core file.
+  bool IsCoreFile() const;
+
   /// If possible sets a breakpoint on a function called by the runtime
   /// linker each time a module is loaded or unloaded.
   bool SetRendezvousBreakpoint();
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -213,6 +213,10 @@
 void DynamicLoaderPOSIXDYLD::ProbeEntry() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return;
+
   const addr_t entry = GetEntryPoint();
   if (entry == LLDB_INVALID_ADDRESS) {
 LLDB_LOGF(
@@ -297,6 +301,11 @@
 
 bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
+
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return false;
+
   if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
 LLDB_LOG(log,
  "Rendezvous breakpoint breakpoint id {0} for pid {1}"
@@ -829,3 +838,7 @@
 
   return module_sp->GetFileSpec().GetPath() == "[vdso]";
 }
+
+bool DynamicLoaderPOSIXDYLD::IsCoreFile() const {
+  return !m_process->IsLiveDebugSession();
+}
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
@@ -267,6 +267,8 @@
 
   bool FindMetadata(const char *name, PThreadField field, uint32_t );
 
+  bool IsCoreFile() const;
+
   enum RendezvousAction {
 eNoAction,
 eTakeSnapshot,
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -190,6 +190,13 @@
 }
 
 DYLDRendezvous::RendezvousAction DYLDRendezvous::GetAction() const {
+  // If we have a core file, we will read the current rendezvous state
+  // from the core file's memory info m_current which can be in an inconsistent
+  // state, so we can't rely on its state and we always need it to load all of
+  // the shared libraries.
+  if (IsCoreFile())
+return eTakeSnapshot;
+
   switch (m_current.state) {
 
   case eConsistent:
@@ -664,3 +671,7 @@
 LLDB_LOGF(log, "  Prev : %" PRIx64, I->prev);
   }
 }
+
+bool DYLDRendezvous::IsCoreFile() const {
+  return !m_process->IsLiveDebugSession();
+}


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -91,6 +91,9 @@
   std::map>
   m_loaded_modules;
 
+  /// Returns true if the process is for a core file.
+  bool IsCoreFile() const;
+
   /// If possible sets a breakpoint on a function called by the runtime
   /// linker each time a module is loaded or unloaded.
   bool SetRendezvousBreakpoint();
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -213,6 +213,10 @@
 void DynamicLoaderPOSIXDYLD::ProbeEntry() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return;
+
   const addr_t entry = GetEntryPoint();
   if (entry == LLDB_INVALID_ADDRESS) {
 LLDB_LOGF(
@@ -297,6 +301,11 @@
 
 bool 

[Lldb-commits] [PATCH] D134842: Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core files.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D134842#3823497 , @labath wrote:

> In D134842#3822740 , @yinghuitan 
> wrote:
>
>> I am surprised other major companies did not hit this issue.
>
> That could be because this is something specific to your environment. Just to 
> be clear, is this happening for *all* core files or only for some of them? If 
> only some, is there anything special about the state of the applications that 
> produced those core files (e.g. are they in the middle of loading a shared 
> library?)

These are produced by the linux kernel. If we produce any core files manually 
we make Minidumps as they contain more useful information. ELF core files are 
kind of a joke as they don't include any build IDs for the shared libraries so 
many people often will try to load a core file on another system and they end 
up just loading the shared libraries from the current machine (like 
/usr/lib/libc.so etc) and they will never know. ELF core file are only really 
useful on the same machine or for looking at the main binary's stack frames.

> Even though this may very well be the right fix for middle-of-dlopen core 
> dumps (we can't really wait for the loading to finish), I suspect this is 
> actually masking some other problem, as the amount of time an application 
> spends in the RT_ADD state is very brief, and it shouldn't be doing anything 
> crash-prone while in there.

Thanks for the background on the eAdd state, makes more sense, there are no 
docs on what these states mean so I didn't know what the normal values were. I 
will check our small core files that are checked in and see if they end up in 
these states as well and report back. I will also check out the core file that 
has this issue to see if one of the threads is doing dlopen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134842

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

[Lldb-commits] [PATCH] D134842: Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core files.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:174-181
+  // If we have a core file, we will read the current rendezvous state
+  // from the core file's memory which will indicate there is nothing
+  // to do, but we need it to load all of the shared libraries. If we
+  // don't change this, then "info.state" will be set to eAdd and the
+  // m_previous.state will be eConsistent and GetAction() will return
+  // eNoAction when we need it to return eTakeSnapshot.
+  if (IsCoreFile())

labath wrote:
> How about we just change `GetAction` to directly return `eTakeSnapshot` in 
> this case?
that is a good idea


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134842

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Mark de Wever via Phabricator via lldb-commits]

Mordante added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:129
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.

aaron.ballman wrote:
> Mordante wrote:
> > 
> I didn't apply this one because I think it's grammatically correct without 
> the comma.
I looks slightly odd to me, but I'm not a native speaker.


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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

If there's some guarantee that Target::CreateProcess HAS to be the way that a 
process is created, then this is fine.  Is there such a guarantee - the 
function is pretty trivial so somebody might be tempted to do the work in some 
other place but miss the breakpoint resetting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134882

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:129
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.

Mordante wrote:
> 
I didn't apply this one because I think it's grammatically correct without the 
comma.


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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman updated this revision to Diff 463949.
aaron.ballman marked 3 inline comments as done.
aaron.ballman added a comment.

Applying review feedback.


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

https://reviews.llvm.org/D134878

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's 
code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading 
a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" group to the review for their awareness. The purpose of these
+  groups is to give vendors early notice that potentially disruptive changes
+  are being considered but have not yet been accepted. Vendors can give early
+  testing feedback on the changes to alert us to unacceptable breakages. The
+  current list of vendor groups is:
+
+  * `Clang vendors `_
+  * `libc++ vendors `_
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.
+
+* When committing the change to the repository, add appropriate information
+  about the potentially breaking changes to the ``Potentially Breaking 
Changes``
+  section of the project's release notes. The release note should have
+  information about what the change is, what is potentially disruptive about
+  it, as well as any code examples, links, and motivation that is appropriate
+  to share with users. This helps users to learn about potential issues with
+  upgrading to that release.
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the
+  `Announcements `_ channel on
+  Discourse. The post should be tagged with the ``potentially-breaking`` label
+  and a label specific to the project (such as ``clang``, ``llvm``, etc). This
+  is another mechanism by which we can give pre-release notice to users about
+  potentially disruptive changes. It is a lower-traffic alternative to the
+  joining "vendors" group. To automatically be notified of new announcements
+  with the ``potentially-breaking`` label, go to your user preferences page in
+  Discourse, and add the label to one of the watch categories under
+  ``Notifications->Tags``.
+
 .. _code owners:
 
 Code Owners
@@ -181,7 +226,12 @@
   programming paradigms.
 * Modifying a C stable API.
 * Notifying users about a potentially disruptive change expected to be made in
-  a future release, such as removal of a deprecated feature.
+  a future release, such as removal of a deprecated feature. In this case, the
+  release note should be added to a ``Potentially Breaking Changes`` section of
+  the notes with sufficient information and examples to demonstrate the
+  potential disruption. Additionally, any new entries to this section should be
+  announced in the `Announcements `_
+  channel on Discourse. See :ref:`breaking` for more details.
 
 Code reviewers are encouraged to request a release note if they think one is
 warranted when performing a code review.


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" group to the review for their awareness. The purpose of these
+  groups is 

[Lldb-commits] [PATCH] D134873: [LLDB] Add "process status" as equivalent of GDB's "frame" command

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

`f` with no input does this job:

  (lldb) f
  frame #1: 0x00013f70 callem`nothing at callem.c:3
 1  int nothing(int input) {
 2if (input < 10) {
  -> 3  return nothing(++input);
   ^
 4}
 5return input;
 6  }
 7  
  (lldb) up
  frame #2: 0x00013f70 callem`nothing at callem.c:3
 1  int nothing(int input) {
 2if (input < 10) {
  -> 3  return nothing(++input);
   ^
 4}
 5return input;
 6  }
 7  
  (lldb) f
  frame #2: 0x00013f70 callem`nothing at callem.c:3
 1  int nothing(int input) {
 2if (input < 10) {
  -> 3  return nothing(++input);
   ^
 4}
 5return input;
 6  }
 7  
  (lldb) 

etc...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134873

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Mark de Wever via Phabricator via lldb-commits]

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

Thanks a lot for working on this! A few small nits, otherwise LGTM.




Comment at: llvm/docs/DeveloperPolicy.rst:112
+
+Please help notify users of potential disruptions when upgrading to a newer
+version of a tool. For example, deprecating a feature that is expected to be





Comment at: llvm/docs/DeveloperPolicy.rst:129
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.





Comment at: llvm/docs/DeveloperPolicy.rst:150
+  Discourse, and add the label to one of the watch categories under
+  ``Notifications->Tags``
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D134877: [lldb] Fixes for swig-4.1.0 macro definition correction

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

sure, sounds good. TBH, I am not sure if either of those is really needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134877

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

[Lldb-commits] [PATCH] D134877: [lldb] Fixes for swig-4.1.0 macro definition correction

[serge via Phabricator via lldb-commits]

serge-sans-paille added a comment.

In D134877#3823855 , @labath wrote:

> Looking at https://bugzilla.redhat.com/show_bug.cgi?id=2128646, I'd say that 
> the real bug is that we're defining this macro in two places. How about we 
> leave these definitions as they are, and remove the one at 
> `bindings/interfaces.swig:5` ?

In that case I would advocate for keeping the definition in the header, as it's 
commented (which means adding a definition for __STDC_CONSTANT_MACROS in the 
interface too).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134877

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:140
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the

ldionne wrote:
> I wonder whether `Announcements` is truly a lower-traffic alternative to 
> `vendors` groups, if we end up posting each potentially breaking change to 
> the list and tagging vendors on each such review. I'm not against posting on 
> Discourse, however it seems to me like basically another equivalent channel 
> of communication for these changes (which might be beneficial, I'm neutral on 
> that).
The reason we have a split like that is for timing and chattiness. If you're a 
downstream like Intel has with ICX, you might want to be in `clang-vendors` so 
that you are involved in conversations about potentially breaking changes. 
You'll be getting emails for all review comments on that review. But if you're 
a downstream like a Gentoo package maintainer, you might not want to be *that* 
involved in the development of the compiler, but still want to know when 
changes are coming down the pipeline to do early pre-release testing while 
there's still time to put the brakes on before a release goes out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

Thanks for working on this! FWIW, this more or less standardizes what we've 
been doing in libc++ for the past ~1.5 years and it's been pretty low effort 
for us to do. And putting my vendor hat on, it's been extremely useful for me 
to track down potential issues when trying to ship a new version of libc++. 
This LGTM, although I'm somewhat neutral on whether to post on Discourse as 
well as having the vendor groups on Phabricator. I'm not sure I understand the 
benefit of doing both, but I will happily conform if folks see value in it.




Comment at: llvm/docs/DeveloperPolicy.rst:140
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the

I wonder whether `Announcements` is truly a lower-traffic alternative to 
`vendors` groups, if we end up posting each potentially breaking change to the 
list and tagging vendors on each such review. I'm not against posting on 
Discourse, however it seems to me like basically another equivalent channel of 
communication for these changes (which might be beneficial, I'm neutral on 
that).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a reviewer: JDevlieghere.
aaron.ballman added a comment.

Adding someone from lldb as a review for awareness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: fdeazeve, jingham.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

This ensures it is run regardless of the method we use to initiate the
session (previous version did not handle connects).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134882

Files:
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -63,3 +63,54 @@
 self.dbg.GetSelectedTarget().GetProcess().Kill()
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),
   self.process(), [lldb.eStateExited])
+def test_breakpoint_count(self):
+"""
+Test that breakpoint count gets reset for each new connection.
+"""
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self):
+super().__init__()
+self.continued = False
+
+def qfThreadInfo(self):
+return "m47"
+
+def qsThreadInfo(self):
+return "l"
+
+def setBreakpoint(self, packet):
+return "OK"
+
+def readRegister(self, reg):
+# Pretend we're at the breakpoint after we've been resumed.
+return "3412" if self.continued else 
"4747"
+
+def cont(self):
+self.continued = True
+return "T05thread=47;reason:breakpoint"
+
+# Connect to the first process and set our breakpoint.
+self.server.responder = MyResponder()
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+bkpt = target.BreakpointCreateByAddress(0x1234)
+self.assertTrue(bkpt.IsValid())
+self.assertEqual(bkpt.GetNumLocations(), 1)
+
+# "continue" the process. It should hit our breakpoint.
+process.Continue()
+self.assertState(process.GetState(), lldb.eStateStopped)
+self.assertEqual(bkpt.GetHitCount(), 1)
+
+# Now kill it, and connect again.
+process.Kill()
+self.server.stop()
+self.server = MockGDBServer(self.server_socket_class())
+self.server.start()
+
+process = self.connect(target)
+
+# The hit count should be reset.
+self.assertEqual(bkpt.GetHitCount(), 0)
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -209,6 +209,7 @@
   if (!listener_sp)
 listener_sp = GetDebugger().GetListener();
   DeleteCurrentProcess();
+  ResetBreakpointHitCounts();
   m_process_sp = Process::FindPlugin(shared_from_this(), plugin_name,
  listener_sp, crash_file, can_connect);
   return m_process_sp;
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2761,18 +2761,15 @@
 }
 
 Status Process::WillLaunch(Module *module) {
-  GetTarget().ResetBreakpointHitCounts();
   return DoWillLaunch(module);
 }
 
 Status Process::WillAttachToProcessWithID(lldb::pid_t pid) {
-  GetTarget().ResetBreakpointHitCounts();
   return DoWillAttachToProcessWithID(pid);
 }
 
 Status Process::WillAttachToProcessWithName(const char *process_name,
 bool wait_for_launch) {
-  GetTarget().ResetBreakpointHitCounts();
   return DoWillAttachToProcessWithName(process_name, wait_for_launch);
 }
 


Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -63,3 +63,54 @@
 self.dbg.GetSelectedTarget().GetProcess().Kill()
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),
   self.process(), [lldb.eStateExited])
+def test_breakpoint_count(self):
+"""
+Test that breakpoint count gets reset for each new connection.
+"""
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self):
+super().__init__()
+self.continued = False
+
+def qfThreadInfo(self):
+return "m47"
+
+def qsThreadInfo(self):
+return "l"
+
+def 

[Lldb-commits] [PATCH] D134877: [lldb] Fixes for swig-4.1.0 macro definition correction

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looking at https://bugzilla.redhat.com/show_bug.cgi?id=2128646, I'd say that 
the real bug is that we're defining this macro in two places. How about we 
leave these definitions as they are, and remove the one at 
`bindings/interfaces.swig:5` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134877

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a subscriber: mehdi_amini.
aaron.ballman added a comment.

Note: I would love to add a vendors group to Phabricator for LLVM and lldb (and 
any other project we think has vendors), but this requires admin privileges to 
do. @mehdi_amini, is this something you could help out with assuming folks are 
in favor of the developer policy changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134878

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

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rnk, lattner, ldionne, rjmccall, Mordante, 
tahonermann, bruno, tonic.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: LLVM.

We've recently had issues appropriately notifying users and stakeholders of 
changes to projects that may be potentially disruptive when upgrading. This led 
to discussion on Discourse about how to improve the situation, which can be 
found at: 
https://discourse.llvm.org/t/rfc-add-new-discourse-channel-for-potentially-breaking-disruptive-changes-for-clang/65251

Ultimately, it sounds like we want to encourage three things:

1. Alert vendors during the code review so they can provide early feedback on 
potentially breaking changes that would be unacceptable for them.
2. Alert vendors and users after committing the changes so they can perform 
pre-release testing on a completed change to determine if it causes 
unreasonable problems for them.
3. Alert users more clearly through the release notes so that it's easier to 
determine how disruptive an upgrade might be.

This updates the developer policy accordingly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134878

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's 
code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users of potential disruptions when upgrading to a newer
+version of a tool. For example, deprecating a feature that is expected to be
+removed in the future, removing an already-deprecated feature, upgrading a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" group to the review for their awareness. The purpose of these
+  groups is to give vendors early notice that potentially disruptive changes
+  are being considered but have not yet been accepted. Vendors can give early
+  testing feedback on the changes to alert us to unacceptable breakages. The
+  current list of vendor groups is:
+
+  * `Clang vendors `_
+  * `libc++ vendors `_
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.
+
+* When committing the change to the repository, add appropriate information
+  about the potentially breaking changes to the ``Potentially Breaking 
Changes``
+  section of the project's release notes. The release note should have
+  information about what the change is, what is potentially disruptive about
+  it, as well as any code examples, links, and motivation that is appropriate
+  to share with users. This helps users to learn about potential issues with
+  upgrading to that release.
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the
+  `Announcements `_ channel on
+  Discourse. The post should be tagged with the ``potentially-breaking`` label
+  and a label specific to the project (such as ``clang``, ``llvm``, etc). This
+  is another mechanism by which we can give pre-release notice to users about
+  potentially disruptive changes. It is a lower-traffic alternative to the
+  joining "vendors" group. To automatically be notified of new announcements
+  with the ``potentially-breaking`` label, go to your user preferences page in
+  Discourse, and add the label to one of the watch categories under
+  ``Notifications->Tags``
+
 .. _code owners:
 
 Code Owners
@@ -181,7 +226,12 @@
   programming paradigms.
 * Modifying a C stable API.
 * Notifying users about a potentially disruptive change expected to be made in
-  a future release, such as removal of a deprecated feature.
+  a future release, such as removal of a deprecated feature. In this case, the
+  release note should be added to a ``Potentially Breaking Changes`` section of
+  the notes with sufficient information and examples to demonstrate the
+  potential disruption. Additionally, any new entries to this section should be
+  announced in the `Announcements `_
+  channel on Discourse. See :ref:`breaking` for more details.
 
 Code reviewers are encouraged to request a release note if they think one is
 warranted when performing a code review.


Index: llvm/docs/DeveloperPolicy.rst

[Lldb-commits] [PATCH] D134877: [lldb] Fixes for swig-4.1.0 macro definition correction

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: clayborg, jingham.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

For swig-4.1.0 change:

  #2193 -DFOO on the SWIG command line now sets FOO to 1 for
  consistency with C/C++ compiler preprocessors.  Previously
  SWIG set FOO to an empty value.
  
  Existing invocations of SWIG with `-DFOO` where the empty value
  matters can be updated to `-DFOO=` which should work with both
  old and new releases of SWIG.
  
  *** POTENTIAL INCOMPATIBILITY ***

See https://github.com/swig/swig/issues/2193

This patch is backwards compatible with older versions of SWIG.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134877

Files:
  lldb/bindings/CMakeLists.txt


Index: lldb/bindings/CMakeLists.txt
===
--- lldb/bindings/CMakeLists.txt
+++ lldb/bindings/CMakeLists.txt
@@ -26,8 +26,8 @@
   -features autodoc
   -I${LLDB_SOURCE_DIR}/include
   -I${CMAKE_CURRENT_SOURCE_DIR}
-  -D__STDC_LIMIT_MACROS
-  -D__STDC_CONSTANT_MACROS
+  -D__STDC_LIMIT_MACROS=
+  -D__STDC_CONSTANT_MACROS=
   ${DARWIN_EXTRAS}
 )
 


Index: lldb/bindings/CMakeLists.txt
===
--- lldb/bindings/CMakeLists.txt
+++ lldb/bindings/CMakeLists.txt
@@ -26,8 +26,8 @@
   -features autodoc
   -I${LLDB_SOURCE_DIR}/include
   -I${CMAKE_CURRENT_SOURCE_DIR}
-  -D__STDC_LIMIT_MACROS
-  -D__STDC_CONSTANT_MACROS
+  -D__STDC_LIMIT_MACROS=
+  -D__STDC_CONSTANT_MACROS=
   ${DARWIN_EXTRAS}
 )
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134873: [LLDB] Add "process status" as equivalent of GDB's "frame" command

[Andrzej Warzynski via Phabricator via lldb-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134873

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

[Lldb-commits] [PATCH] D134873: [LLDB] Add "process status" as equivalent of GDB's "frame" command

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

https://lldb.llvm.org/use/map.html#examining-thread-state if you want to see 
the current page.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134873

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

[Lldb-commits] [PATCH] D134873: [LLDB] Add "process status" as equivalent of GDB's "frame" command

[David Spickett via Phabricator via lldb-commits]

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

This is useful for answering the question "where am I?" and is surprisingly
difficult to figure out without just doing another step command.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134873

Files:
  lldb/docs/use/map.rst


Index: lldb/docs/use/map.rst
===
--- lldb/docs/use/map.rst
+++ lldb/docs/use/map.rst
@@ -410,6 +410,18 @@

  
 
+ 
+   Show the current frame and 
source line.
+ 
+ 
+   
+  (gdb) frame
+   
+   
+  (lldb) process status
+   
+ 
+
   

 


Index: lldb/docs/use/map.rst
===
--- lldb/docs/use/map.rst
+++ lldb/docs/use/map.rst
@@ -410,6 +410,18 @@

  
 
+ 
+   Show the current frame and source line.
+ 
+ 
+   
+  (gdb) frame
+   
+   
+  (lldb) process status
+   
+ 
+
   

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

[Lldb-commits] [PATCH] D134842: Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core files.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D134842#3822740 , @yinghuitan 
wrote:

> I am surprised other major companies did not hit this issue.

That could be because this is something specific to your environment. Just to 
be clear, is this happening for *all* core files or only for some of them? If 
only some, is there anything special about the state of the applications that 
produced those core files (e.g. are they in the middle of loading a shared 
library?)
Even though this may very well be the right fix for middle-of-dlopen core dumps 
(we can't really wait for the loading to finish), I suspect this is actually 
masking some other problem, as the amount of time an application spends in the 
RT_ADD state is very brief, and it shouldn't be doing anything crash-prone 
while in there.




Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:174-181
+  // If we have a core file, we will read the current rendezvous state
+  // from the core file's memory which will indicate there is nothing
+  // to do, but we need it to load all of the shared libraries. If we
+  // don't change this, then "info.state" will be set to eAdd and the
+  // m_previous.state will be eConsistent and GetAction() will return
+  // eNoAction when we need it to return eTakeSnapshot.
+  if (IsCoreFile())

How about we just change `GetAction` to directly return `eTakeSnapshot` in this 
case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134842

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

[Lldb-commits] [PATCH] D134751: [lldb] Fix running tests when LLVM target triple is different from host

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added a comment.

Thanks for the review.

In D134751#3818743 , @JDevlieghere 
wrote:

> Supporting builds with different host and target triples makes sense. 
> However, I have a few concerns about the current patch:
>
> - I wouldn't cal it the "host triple". The API tests (`dotest.py`) support 
> running the test suite for a different platform/architecture. Take a look at 
> the Darwin builder and you'll see a bunch of support for that.

How about calling it "clang triple"?

> - How does this interact with the `-arch` flag? When the host and target 
> triple differ, which part of the triple is different? In theory it can be any 
> part, but maybe your use case could be covered by setting the `--arch` flag?

If I run Clang with `--target=x86_64-linux-gnu -m32` then it automatically 
targets `i386-linux-gnu`, so I suppose -arch should override the arch part of 
the target, which is correct. `DarwinBuilder::getArchCFlags`on the other hand 
has a different handling which generates a `-target` option from arch and other 
Apple-specific details without considering the new host-triple flag (or 
whatever we end up calling it), so it should behave the same as before this 
change.

For me, my Linux native build has the default target triple configured to 
x86_64-w64-windows-gnu, and linking of lldb tests fail due to missing mingw-w64 
libraries. (Even if they do link the tests probably won't work anyway.) The 
Shell tests that use the %clang_host lit substitution work because they include 
the host --target flag, so I thought the tests using the build scripts should 
do the same.

> - This doesn't update `getTriple` in `builder.py` which is necessary for some 
> of the decorators to work (in particular the ones that invoke the compiler to 
> see if something is supported).

I see, I will change this too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134751

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

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The fact that MSVC does not store all of the anonymous union data is 
unfortunate, though not entirely surprising, given that the goal of debug info 
never was to offer an exact reconstruction of the source AST.

That, I'm not sure if checking for matching initial offsets is sufficient to 
create a semblance of a consistent structure definition. What will this code do 
with structures like this:

  struct S3 {
char x[3];
  };
  struct A {
union {
  struct {
char c1;
S3 s1;
  };
  struct {
S3 s2;
char c2;
  };
};
  } a;

In this case, `a.s1` and `a.c2` overlap, but they don't share the same starting 
offset. If the compiler does not provide data for anonymous structs as well, 
then I think this algorithm will need to be more complicated. I'm not even sure 
they can be reconstructed correctly, as the anonymous structs and unions can 
nest indefinitely. Maybe it would be sufficient to represent this as a "union 
of structs", where each struct gets (greedily) packed with as many members as 
it can hold, and we create as many structs as we need (we can replace the 
struct with a simple member if it would hold just one member)?




Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:321
+  // the artificial anonymous union.
+  lldb::user_id_t au_id = LLDB_INVALID_UID - toOpaqueUid(m_id);
+  uint64_t au_field_bitoffset = 0;

How are these opaque ids computed? Will they produce unique IDs if you have two 
structs like this close together? Might it be better to just make a global 
(local to a symbol file or something) counter and always increment/decrement 
that when creating another type ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

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