date:20230314

[Lldb-commits] [PATCH] D145950: [lldb] Remove MipsLinuxSignals

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett abandoned this revision.
DavidSpickett added a comment.

Someone else had the same idea: 
https://github.com/llvm/llvm-project/commit/93a455375c0fa1dd014a3b4571b22e307d15bbf7


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145950

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

[Lldb-commits] [PATCH] D145377: [LLDB] Show sub type of memory tagging SEGV when reading a core file

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett abandoned this revision.
DavidSpickett added a comment.

This will be back as part of a series.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145377

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

[Lldb-commits] [PATCH] D146041: initial commit

2023-03-14 Thread Aryan Godara via Phabricator via lldb-commits

AryanGodara created this revision.
Herald added a reviewer: bollu.
Herald added subscribers: steakhal, martong, arphaman.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: NoQ.
Herald added a project: All.
AryanGodara requested review of this revision.
Herald added projects: clang, LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits.

Signed-off-by: aryan 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146041

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/CXX/drs/dr16xx.cpp
  clang/test/Lexer/SourceLocationsOverflow.c
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/www/demo/index.cgi
  lldb/examples/synthetic/libcxx.py
  llvm/include/llvm/CodeGen/MachinePassManager.h
  llvm/tools/bugpoint/ExecutionDriver.cpp
  llvm/tools/bugpoint/ExtractFunction.cpp
  llvm/utils/check-each-file
  polly/lib/External/isl/imath/tests/test.sh

Index: polly/lib/External/isl/imath/tests/test.sh
===
--- polly/lib/External/isl/imath/tests/test.sh
+++ polly/lib/External/isl/imath/tests/test.sh
@@ -9,8 +9,8 @@
 set -o pipefail
 
 if [ ! -f ../imtest ] ; then
-  echo "I can't find the imath test driver 'imtest', did you build it?"
-  echo "I can't proceed with the unit tests until you do so, sorry."
+  echo "The imath test driver 'imtest' was not found."
+  echo "It needs to be build before proceeding with the unit tests."
   exit 2
 fi
 
@@ -25,8 +25,8 @@
 echo ""
 echo "-- Running test to compute 1024 decimal digits of pi"
 if [ ! -f ../pi ] ; then
-  echo "I can't find the pi computing program, did you build it?"
-  echo "I can't proceed with the pi test until you do so, sorry."
+  echo "The pi computing program was not found."
+  echo "It needs to be built before proceeding with the pi test."
   exit 1
 fi
 
Index: llvm/utils/check-each-file
===
--- llvm/utils/check-each-file
+++ llvm/utils/check-each-file
@@ -120,7 +120,7 @@
 $linker
 if $checker
 then
-	echo "Sorry, I can't help you, $program is OK when compiled with llvm-native-gcc"
+	echo "I cannot help you, $program is OK when compiled with llvm-native-gcc"
 	exit 1
 fi
 for f in $files
Index: llvm/tools/bugpoint/ExtractFunction.cpp
===
--- llvm/tools/bugpoint/ExtractFunction.cpp
+++ llvm/tools/bugpoint/ExtractFunction.cpp
@@ -119,7 +119,7 @@
   Passes.push_back("verify");
   std::unique_ptr New = runPassesOn(Clone.get(), Passes);
   if (!New) {
-errs() << "Instruction removal failed.  Sorry. :(  Please report a bug!\n";
+errs() << "Instruction removal failed. Please report a bug!\n";
 exit(1);
   }
   return New;
@@ -141,7 +141,7 @@
 
   std::unique_ptr New = runPassesOn(M.get(), CleanupPasses);
   if (!New) {
-errs() << "Final cleanups failed.  Sorry. :(  Please report a bug!\n";
+errs() << "Final cleanups failed. Please report a bug!\n";
 return nullptr;
   }
   return New;
@@ -155,7 +155,7 @@
   if (!NewM) {
 outs() << "*** Loop extraction failed: ";
 EmitProgressBitcode(*M, "loopextraction", true);
-outs() << "*** Sorry. :(  Please report a bug!\n";
+outs() << "*** Please report a bug!\n";
 return nullptr;
   }
 
Index: llvm/tools/bugpoint/ExecutionDriver.cpp
===
--- llvm/tools/bugpoint/ExecutionDriver.cpp
+++ llvm/tools/bugpoint/ExecutionDriver.cpp
@@ -171,7 +171,7 @@
 }
 if (!Interpreter) {
   InterpreterSel = AutoPick;
-  Message = "Sorry, I can't automatically select an interpreter!\n";
+  Message = "I cannot automatically select an interpreter!\n";
 }
 break;
   case RunLLI:
@@ -216,7 +216,7 @@
   Path.c_str(), Message, CCBinary, &SafeToolArgs, &CCToolArgv);
 } else if (InterpreterSel != CompileCustom) {
   SafeInterpreterSel = AutoPick;
-  Message = "Sorry, I can't automatically select a safe interpreter!\n";
+  Message = "I cannot automatically select a safe interpreter!\n";
 }
 break;
   case RunLLC:
@@ -231,7 +231,7 @@
 getToolName(), Message, CustomExecCommand);
 break;
   default:
-Message = "Sorry, this back-end is not supported by bugpoint as the "
+Message = "This back-end is not supported by bugpoint as the "
   "\"safe\" backend right now!\n";
 break;
   }
Index: llvm/include/llvm/CodeGen/MachinePassManager.h
===
--- llvm/include/llvm/CodeGen/MachinePassManager.h
+++ llvm/include/llvm/CodeGen/MachinePassManager.h
@@ -226,7 +226,7 @@
   addRunOnModule(PassConceptT *Pass) {
 static_assert(is_detected::value,

[Lldb-commits] [PATCH] D146041: initial commit

2023-03-14 Thread Mehdi AMINI via Phabricator via lldb-commits

mehdi_amini added a comment.
Herald added a subscriber: JDevlieghere.

You should look into the title and description of the commit: 
https://cbea.ms/git-commit/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041

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

[Lldb-commits] [PATCH] D146043: [lldb] Refactor CrashReason

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

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

So that there is only one function that NativeThreads call,
which takes a siginfo. Everything else is an internal detail.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146043

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
  lldb/source/Plugins/Process/POSIX/CrashReason.cpp
  lldb/source/Plugins/Process/POSIX/CrashReason.h

Index: lldb/source/Plugins/Process/POSIX/CrashReason.h
===
--- lldb/source/Plugins/Process/POSIX/CrashReason.h
+++ lldb/source/Plugins/Process/POSIX/CrashReason.h
@@ -15,45 +15,6 @@
 
 #include 
 
-enum class CrashReason {
-  eInvalidCrashReason,
-
-  // SIGSEGV crash reasons.
-  eInvalidAddress,
-  ePrivilegedAddress,
-  eBoundViolation,
-  eAsyncTagCheckFault,
-  eSyncTagCheckFault,
-
-  // SIGILL crash reasons.
-  eIllegalOpcode,
-  eIllegalOperand,
-  eIllegalAddressingMode,
-  eIllegalTrap,
-  ePrivilegedOpcode,
-  ePrivilegedRegister,
-  eCoprocessorError,
-  eInternalStackError,
-
-  // SIGBUS crash reasons,
-  eIllegalAlignment,
-  eIllegalAddress,
-  eHardwareError,
-
-  // SIGFPE crash reasons,
-  eIntegerDivideByZero,
-  eIntegerOverflow,
-  eFloatDivideByZero,
-  eFloatOverflow,
-  eFloatUnderflow,
-  eFloatInexactResult,
-  eFloatInvalidOperation,
-  eFloatSubscriptRange
-};
-
-std::string GetCrashReasonString(CrashReason reason, lldb::addr_t fault_addr);
-std::string GetCrashReasonString(CrashReason reason, const siginfo_t &info);
-
-CrashReason GetCrashReason(const siginfo_t &info);
+std::string GetCrashReasonString(const siginfo_t &info);
 
 #endif // #ifndef liblldb_CrashReason_H_
Index: lldb/source/Plugins/Process/POSIX/CrashReason.cpp
===
--- lldb/source/Plugins/Process/POSIX/CrashReason.cpp
+++ lldb/source/Plugins/Process/POSIX/CrashReason.cpp
@@ -12,6 +12,42 @@
 
 #include 
 
+enum class CrashReason {
+  eInvalidCrashReason,
+
+  // SIGSEGV crash reasons.
+  eInvalidAddress,
+  ePrivilegedAddress,
+  eBoundViolation,
+  eAsyncTagCheckFault,
+  eSyncTagCheckFault,
+
+  // SIGILL crash reasons.
+  eIllegalOpcode,
+  eIllegalOperand,
+  eIllegalAddressingMode,
+  eIllegalTrap,
+  ePrivilegedOpcode,
+  ePrivilegedRegister,
+  eCoprocessorError,
+  eInternalStackError,
+
+  // SIGBUS crash reasons,
+  eIllegalAlignment,
+  eIllegalAddress,
+  eHardwareError,
+
+  // SIGFPE crash reasons,
+  eIntegerDivideByZero,
+  eIntegerOverflow,
+  eFloatDivideByZero,
+  eFloatOverflow,
+  eFloatUnderflow,
+  eFloatInexactResult,
+  eFloatInvalidOperation,
+  eFloatSubscriptRange
+};
+
 static void AppendFaultAddr(std::string &str, lldb::addr_t addr) {
   std::stringstream ss;
   ss << " (fault address: 0x" << std::hex << addr << ")";
@@ -37,10 +73,8 @@
 }
 #endif
 
-static CrashReason GetCrashReasonForSIGSEGV(const siginfo_t &info) {
-  assert(info.si_signo == SIGSEGV);
-
-  switch (info.si_code) {
+static CrashReason GetCrashReasonForSIGSEGV(int code) {
+  switch (code) {
 #ifdef SI_KERNEL
   case SI_KERNEL:
 // Some platforms will occasionally send nonstandard spurious SI_KERNEL
@@ -73,10 +107,8 @@
   return CrashReason::eInvalidCrashReason;
 }
 
-static CrashReason GetCrashReasonForSIGILL(const siginfo_t &info) {
-  assert(info.si_signo == SIGILL);
-
-  switch (info.si_code) {
+static CrashReason GetCrashReasonForSIGILL(int code) {
+  switch (code) {
   case ILL_ILLOPC:
 return CrashReason::eIllegalOpcode;
   case ILL_ILLOPN:
@@ -98,10 +130,8 @@
   return CrashReason::eInvalidCrashReason;
 }
 
-static CrashReason GetCrashReasonForSIGFPE(const siginfo_t &info) {
-  assert(info.si_signo == SIGFPE);
-
-  switch (info.si_code) {
+static CrashReason GetCrashReasonForSIGFPE(int code) {
+  switch (code) {
   case FPE_INTDIV:
 return CrashReason::eIntegerDivideByZero;
   case FPE_INTOVF:
@@ -123,10 +153,8 @@
   return CrashReason::eInvalidCrashReason;
 }
 
-static CrashReason GetCrashReasonForSIGBUS(const siginfo_t &info) {
-  assert(info.si_signo == SIGBUS);
-
-  switch (info.si_code) {
+static CrashReason GetCrashReasonForSIGBUS(int code) {
+  switch (code) {
   case BUS_ADRALN:
 return CrashReason::eIllegalAlignment;
   case BUS_ADRERR:
@@ -138,25 +166,8 @@
   return CrashReason::eInvalidCrashReason;
 }
 
-std::string GetCrashReasonString(CrashReason reason, const siginfo_t &info) {
-  std::string str;
-
-// make sure that siginfo_t has the bound fields available.
-#if defined(si_lower) && defined(si_upper)
-  if (reason == CrashReason::eBoundViolation) {
-str = "signal SIGSEGV";
-AppendBounds(str, reinte

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett created this revision.
Herald added subscribers: krytarowski, arichardson, emaste.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

By adding signal codes to UnixSignals and adding a new function
where you can get a string with optional address and bounds.

Added signal codes to the Linux, FreeBSD and NetBSD signal sets.
I've checked the numbers against the relevant sources.

Each signal code has a code number, description and printing options.
By default you just get the descripton, you can opt into adding either
a fault address or bounds information.

Bounds signals we'll use the description, unless we have the bounds
values in which case we say whether it is an upper or lower bound
issue.

GetCrashReasonString remains in CrashReason because we need it to
be compiled only for platforms with siginfo_t. Ideally it would
move into NativeProcessProtocol, but that is also used
by NativeRegisterContextWindows, where there would be no siginfo_t.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146044

Files:
  lldb/include/lldb/Target/UnixSignals.h
  lldb/source/Plugins/Process/POSIX/CrashReason.cpp
  lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
  lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
  lldb/source/Plugins/Process/Utility/NetBSDSignals.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/unittests/Signals/UnixSignalsTest.cpp

Index: lldb/unittests/Signals/UnixSignalsTest.cpp
===
--- lldb/unittests/Signals/UnixSignalsTest.cpp
+++ lldb/unittests/Signals/UnixSignalsTest.cpp
@@ -23,6 +23,10 @@
 AddSignal(4, "SIG4", true, false, true, "DESC4");
 AddSignal(8, "SIG8", true, true, true, "DESC8");
 AddSignal(16, "SIG16", true, false, false, "DESC16");
+AddSignalCode(16, 1, "a specific type of SIG16");
+AddSignalCode(16, 2, "SIG16 with a fault address",
+  SignalCodePrintOption::Address);
+AddSignalCode(16, 3, "bounds violation", SignalCodePrintOption::Bounds);
   }
 };
 
@@ -93,6 +97,50 @@
   EXPECT_EQ(name, signals.GetSignalAsCString(signo));
 }
 
+TEST(UnixSignalsTest, GetAsCString) {
+  TestSignals signals;
+
+  EXPECT_EQ(nullptr, signals.GetSignalAsCString(100));
+  std::string name = signals.GetSignalAsCString(16);
+  EXPECT_EQ("SIG16", name);
+}
+
+TEST(UnixSignalsTest, GetAsString) {
+  TestSignals signals;
+
+  EXPECT_EQ("", signals.GetSignalAsString(100, std::nullopt));
+  EXPECT_EQ("SIG16", signals.GetSignalAsString(16, std::nullopt));
+  EXPECT_EQ("", signals.GetSignalAsString(100, 100));
+  EXPECT_EQ("SIG16", signals.GetSignalAsString(16, 100));
+  EXPECT_EQ("SIG16: a specific type of SIG16",
+signals.GetSignalAsString(16, 1));
+
+  // Unknown code, won't use the address.
+  EXPECT_EQ("SIG16", signals.GetSignalAsString(16, 100, 0xCAFEF00D));
+  // Known code, that shouldn't print fault address.
+  EXPECT_EQ("SIG16: a specific type of SIG16",
+signals.GetSignalAsString(16, 1, 0xCAFEF00D));
+  // Known code that should.
+  EXPECT_EQ("SIG16: SIG16 with a fault address (fault address: 0xcafef00d)",
+signals.GetSignalAsString(16, 2, 0xCAFEF00D));
+  // No address given just print the code description.
+  EXPECT_EQ("SIG16: SIG16 with a fault address",
+signals.GetSignalAsString(16, 2));
+
+  const char *expected = "SIG16: bounds violation";
+  // Must pass all needed info to get full output.
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3));
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3, 0xcafef00d));
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3, 0xcafef00d, 0x1234));
+
+  EXPECT_EQ("SIG16: upper bound violation (fault address: 0x5679, lower bound: "
+"0x1234, upper bound: 0x5678)",
+signals.GetSignalAsString(16, 3, 0x5679, 0x1234, 0x5678));
+  EXPECT_EQ("SIG16: lower bound violation (fault address: 0x1233, lower bound: "
+"0x1234, upper bound: 0x5678)",
+signals.GetSignalAsString(16, 3, 0x1233, 0x1234, 0x5678));
+}
+
 TEST(UnixSignalsTest, VersionChange) {
   TestSignals signals;
 
Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/ArchSpec.h"
 #include 
+#include 
 
 using namespace lldb_private;
 using namespace llvm;
@@ -112,6 +113,16 @@
   ++m_version;
 }
 
+void UnixSignals::AddSignalCode(int signo, int code, const char *description,
+SignalCodePrintOption print_option) {
+  collection::iterator signal = m_signals.find(signo);
+  assert(signal != m_signals.end() &&
+ "Tried to add code to signal that does not exist.");
+  signal->second.m_codes.insert(
+  std::pair{code, SignalCode{Con

[Lldb-commits] [PATCH] D146045: [LLDB] Show sub type of signals when debugging a core file

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett created this revision.
Herald added subscribers: atanasyan, arichardson, sdardis.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

Previously we only looked at the si_signo field, so you got:

  (lldb) bt
  * thread #1, name = 'a.out.mte', stop reason = signal SIGSEGV
* frame #0: 0x004007f4

This patch adds si_code so we can show:

  (lldb) bt
  * thread #1, name = 'a.out.mte', stop reason = signal SIGSEGV: sync tag check 
fault
* frame #0: 0x004007f4

The order of errno and code was incorrect in ElfLinuxSigInfo::Parse.
It was the order that a "swapped" siginfo arch would use, which for Linux,
is only MIPS. We removed MIPS Linux support some time ago.

See:
https://github.com/torvalds/linux/blob/fe15c26ee26efa11741a7b632e9f23b01aca4cc6/include/uapi/asm-generic/siginfo.h#L121

A test is added using memory tagging faults. Which were the original
motivation for the changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146045

Files:
  lldb/include/lldb/Target/StopInfo.h
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
  lldb/source/Target/StopInfo.cpp
  
lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -192,6 +192,9 @@
   omit defaulted template parameters. The full template parameter list can still be
   viewed with ``expr --raw-output``/``frame var --raw-output``. (`D141828 `_)
 
+* LLDB is now able to show the subtype of signals found in a core file. For example
+  memory tagging specific segfaults such as ``SIGSEGV: sync tag check fault``.
+
 Changes to Sanitizers
 -
 
Index: lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
===
--- lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
+++ lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
@@ -166,3 +166,14 @@
 # the MTE core file which does support it but does not allow writing tags.
 self.expect("memory tag write 0 1",
 substrs=["error: Process does not support memory tagging"], error=True)
+
+@skipIfLLVMTargetMissing("AArch64")
+def test_mte_tag_fault_reason(self):
+""" Test that we correctly report the fault reason. """
+self.runCmd("target create --core core.mte")
+
+# There is no fault address shown here because core files do not include
+# si_addr.
+self.expect("bt", substrs=[
+"* thread #1, name = 'a.out.mte', stop reason = signal SIGSEGV: "
+"sync tag check fault"])
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -1039,8 +1039,9 @@
 
 class StopInfoUnixSignal : public StopInfo {
 public:
-  StopInfoUnixSignal(Thread &thread, int signo, const char *description)
-  : StopInfo(thread, signo) {
+  StopInfoUnixSignal(Thread &thread, int signo, const char *description,
+ std::optional code)
+  : StopInfo(thread, signo), m_code(code) {
 SetDescription(description);
   }
 
@@ -1095,19 +1096,26 @@
 if (m_description.empty()) {
   ThreadSP thread_sp(m_thread_wp.lock());
   if (thread_sp) {
+UnixSignalsSP unix_signals = thread_sp->GetProcess()->GetUnixSignals();
 StreamString strm;
-const char *signal_name =
-thread_sp->GetProcess()->GetUnixSignals()->GetSignalAsCString(
-m_value);
-if (signal_name)
-  strm.Printf("signal %s", signal_name);
+strm << "signal ";
+
+std::string signal_name =
+unix_signals->GetSignalAsString(m_value, m_code);
+if (signal_name.size())
+  strm << signal_name;
 else
-  strm.Printf("signal %" PRIi64, m_value);
+  strm.Printf("%" PRIi64, m_value);
+
 m_description = std::string(strm.GetString());
   }
 }
 return m_description.c_str();
   }
+
+private:
+  // In siginfo_t terms, if m_value is si_signo, m_code is si_code.
+  std::optional m_code;
 };
 
 // StopInfoTrace
@@ -1366,9 +1374,10 @@
 }
 
 StopInfoSP StopInfo::CreateStopReasonWithSignal(Thread &thread, int signo,
-const char *description) {
+const char *description,
+std::o

[Lldb-commits] [PATCH] D146043: [lldb] Refactor CrashReason

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added reviewers: labath, JDevlieghere.
DavidSpickett added a comment.

Or in other words "make it look like UnixSignals on the inside".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146043

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

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a subscriber: mgorny.
DavidSpickett added a comment.

@emaste , @mgorny maybe you want to glance at the numbers for the BSDs. I got 
the codes from the FreeBSD/NetBSD sources, and the signal numbers are as 
before, using the Darwin numbers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146044

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

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp:38
+  AddSignalCode(8, 8 /*FPE_FLTSUB*/, "invalid floating point subscript range");
+  AddSignalCode(8, 9 /*FPE_FLTIDO*/, "input denormal operation");
+

This one is only in FreeBSD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146044

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

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread Ed Maste via Phabricator via lldb-commits

emaste added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp:20
+  // SIGILL
+  AddSignalCode(4, 1 /*ILL_ILLOPC*/, "illegal instruction");
+  AddSignalCode(4, 2 /*ILL_ILLOPN*/, "illegal instruction operand");

FreeBSD comment uses "illegal opcode" fwiw



Comment at: lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp:29
+
+  // SIGFPE
+  AddSignalCode(8, 1 /*FPE_INTDIV*/, "integer divide by zero");

looks like the first two are swapped?
```
/* codes for SIGFPE */
#define FPE_INTOVF  1   /* Integer overflow.*/
#define FPE_INTDIV  2   /* Integer divide by zero.  */
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146044

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

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett updated this revision to Diff 505082.
DavidSpickett added a comment.

Corect FreeBSD SIGFPE integer overflow/divide by zero order.

FreeBSD is overflow then divide by zero.
NetBSD is divide by zero then overflow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146044

Files:
  lldb/include/lldb/Target/UnixSignals.h
  lldb/source/Plugins/Process/POSIX/CrashReason.cpp
  lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
  lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
  lldb/source/Plugins/Process/Utility/NetBSDSignals.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/unittests/Signals/UnixSignalsTest.cpp

Index: lldb/unittests/Signals/UnixSignalsTest.cpp
===
--- lldb/unittests/Signals/UnixSignalsTest.cpp
+++ lldb/unittests/Signals/UnixSignalsTest.cpp
@@ -23,6 +23,10 @@
 AddSignal(4, "SIG4", true, false, true, "DESC4");
 AddSignal(8, "SIG8", true, true, true, "DESC8");
 AddSignal(16, "SIG16", true, false, false, "DESC16");
+AddSignalCode(16, 1, "a specific type of SIG16");
+AddSignalCode(16, 2, "SIG16 with a fault address",
+  SignalCodePrintOption::Address);
+AddSignalCode(16, 3, "bounds violation", SignalCodePrintOption::Bounds);
   }
 };
 
@@ -93,6 +97,50 @@
   EXPECT_EQ(name, signals.GetSignalAsCString(signo));
 }
 
+TEST(UnixSignalsTest, GetAsCString) {
+  TestSignals signals;
+
+  EXPECT_EQ(nullptr, signals.GetSignalAsCString(100));
+  std::string name = signals.GetSignalAsCString(16);
+  EXPECT_EQ("SIG16", name);
+}
+
+TEST(UnixSignalsTest, GetAsString) {
+  TestSignals signals;
+
+  EXPECT_EQ("", signals.GetSignalAsString(100, std::nullopt));
+  EXPECT_EQ("SIG16", signals.GetSignalAsString(16, std::nullopt));
+  EXPECT_EQ("", signals.GetSignalAsString(100, 100));
+  EXPECT_EQ("SIG16", signals.GetSignalAsString(16, 100));
+  EXPECT_EQ("SIG16: a specific type of SIG16",
+signals.GetSignalAsString(16, 1));
+
+  // Unknown code, won't use the address.
+  EXPECT_EQ("SIG16", signals.GetSignalAsString(16, 100, 0xCAFEF00D));
+  // Known code, that shouldn't print fault address.
+  EXPECT_EQ("SIG16: a specific type of SIG16",
+signals.GetSignalAsString(16, 1, 0xCAFEF00D));
+  // Known code that should.
+  EXPECT_EQ("SIG16: SIG16 with a fault address (fault address: 0xcafef00d)",
+signals.GetSignalAsString(16, 2, 0xCAFEF00D));
+  // No address given just print the code description.
+  EXPECT_EQ("SIG16: SIG16 with a fault address",
+signals.GetSignalAsString(16, 2));
+
+  const char *expected = "SIG16: bounds violation";
+  // Must pass all needed info to get full output.
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3));
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3, 0xcafef00d));
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3, 0xcafef00d, 0x1234));
+
+  EXPECT_EQ("SIG16: upper bound violation (fault address: 0x5679, lower bound: "
+"0x1234, upper bound: 0x5678)",
+signals.GetSignalAsString(16, 3, 0x5679, 0x1234, 0x5678));
+  EXPECT_EQ("SIG16: lower bound violation (fault address: 0x1233, lower bound: "
+"0x1234, upper bound: 0x5678)",
+signals.GetSignalAsString(16, 3, 0x1233, 0x1234, 0x5678));
+}
+
 TEST(UnixSignalsTest, VersionChange) {
   TestSignals signals;
 
Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/ArchSpec.h"
 #include 
+#include 
 
 using namespace lldb_private;
 using namespace llvm;
@@ -112,6 +113,16 @@
   ++m_version;
 }
 
+void UnixSignals::AddSignalCode(int signo, int code, const char *description,
+SignalCodePrintOption print_option) {
+  collection::iterator signal = m_signals.find(signo);
+  assert(signal != m_signals.end() &&
+ "Tried to add code to signal that does not exist.");
+  signal->second.m_codes.insert(
+  std::pair{code, SignalCode{ConstString(description), print_option}});
+  ++m_version;
+}
+
 void UnixSignals::RemoveSignal(int signo) {
   collection::iterator pos = m_signals.find(signo);
   if (pos != m_signals.end())
@@ -127,6 +138,58 @@
 return pos->second.m_name.GetCString();
 }
 
+std::string
+UnixSignals::GetSignalAsString(int32_t signo, std::optional code,
+   std::optional addr,
+   std::optional lower,
+   std::optional upper) const {
+  std::string str;
+
+  collection::const_iterator pos = m_signals.find(signo);
+  if (pos != m_signals.end()) {
+str = pos->second.m_name.GetCString();
+
+if (code) {
+  std::map::const_iterator cpos =
+  pos->second.m_codes.find(*code);
+  if

[Lldb-commits] [lldb] 55aa4bf - [lldb] Fix -Wswitch in TypeSystemClang.cpp ('SveBoolx2' and 'SveBoolx4' not handled in switch) (NFC)

2023-03-14 Thread Jie Fu via lldb-commits


Author: Jie Fu
Date: 2023-03-14T22:24:37+08:00
New Revision: 55aa4bfaee8a2bd5c56765b4d54c4115dcc5d6f3

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

LOG: [lldb] Fix -Wswitch in TypeSystemClang.cpp ('SveBoolx2' and 'SveBoolx4' 
not handled in switch) (NFC)

/data/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:4859:13:
 error: enumeration values 'SveBoolx2' and 'SveBoolx4' not handled in switch 
[-Werror,-Wswitch]
switch (llvm::cast(qual_type)->getKind()) {
^~~~
1 error generated.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 1d4d66d6e6dc..a739494cffcc 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -5003,6 +5003,8 @@ lldb::Encoding 
TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
 
 // ARM -- Scalable Vector Extension
 case clang::BuiltinType::SveBool:
+case clang::BuiltinType::SveBoolx2:
+case clang::BuiltinType::SveBoolx4:
 case clang::BuiltinType::SveCount:
 case clang::BuiltinType::SveInt8:
 case clang::BuiltinType::SveInt8x2:



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

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett updated this revision to Diff 505090.
DavidSpickett added a comment.

Update the descriptions to match the platform's.

Add a couple of FreeBSD signals I missed the first time.

We're missing a bunch of Linux ones too but since that file
already existed, I don't want to make the diff more messy.
I'll add them later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146044

Files:
  lldb/include/lldb/Target/UnixSignals.h
  lldb/source/Plugins/Process/POSIX/CrashReason.cpp
  lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp
  lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
  lldb/source/Plugins/Process/Utility/NetBSDSignals.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/unittests/Signals/UnixSignalsTest.cpp

Index: lldb/unittests/Signals/UnixSignalsTest.cpp
===
--- lldb/unittests/Signals/UnixSignalsTest.cpp
+++ lldb/unittests/Signals/UnixSignalsTest.cpp
@@ -23,6 +23,10 @@
 AddSignal(4, "SIG4", true, false, true, "DESC4");
 AddSignal(8, "SIG8", true, true, true, "DESC8");
 AddSignal(16, "SIG16", true, false, false, "DESC16");
+AddSignalCode(16, 1, "a specific type of SIG16");
+AddSignalCode(16, 2, "SIG16 with a fault address",
+  SignalCodePrintOption::Address);
+AddSignalCode(16, 3, "bounds violation", SignalCodePrintOption::Bounds);
   }
 };
 
@@ -93,6 +97,50 @@
   EXPECT_EQ(name, signals.GetSignalAsCString(signo));
 }
 
+TEST(UnixSignalsTest, GetAsCString) {
+  TestSignals signals;
+
+  EXPECT_EQ(nullptr, signals.GetSignalAsCString(100));
+  std::string name = signals.GetSignalAsCString(16);
+  EXPECT_EQ("SIG16", name);
+}
+
+TEST(UnixSignalsTest, GetAsString) {
+  TestSignals signals;
+
+  EXPECT_EQ("", signals.GetSignalAsString(100, std::nullopt));
+  EXPECT_EQ("SIG16", signals.GetSignalAsString(16, std::nullopt));
+  EXPECT_EQ("", signals.GetSignalAsString(100, 100));
+  EXPECT_EQ("SIG16", signals.GetSignalAsString(16, 100));
+  EXPECT_EQ("SIG16: a specific type of SIG16",
+signals.GetSignalAsString(16, 1));
+
+  // Unknown code, won't use the address.
+  EXPECT_EQ("SIG16", signals.GetSignalAsString(16, 100, 0xCAFEF00D));
+  // Known code, that shouldn't print fault address.
+  EXPECT_EQ("SIG16: a specific type of SIG16",
+signals.GetSignalAsString(16, 1, 0xCAFEF00D));
+  // Known code that should.
+  EXPECT_EQ("SIG16: SIG16 with a fault address (fault address: 0xcafef00d)",
+signals.GetSignalAsString(16, 2, 0xCAFEF00D));
+  // No address given just print the code description.
+  EXPECT_EQ("SIG16: SIG16 with a fault address",
+signals.GetSignalAsString(16, 2));
+
+  const char *expected = "SIG16: bounds violation";
+  // Must pass all needed info to get full output.
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3));
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3, 0xcafef00d));
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3, 0xcafef00d, 0x1234));
+
+  EXPECT_EQ("SIG16: upper bound violation (fault address: 0x5679, lower bound: "
+"0x1234, upper bound: 0x5678)",
+signals.GetSignalAsString(16, 3, 0x5679, 0x1234, 0x5678));
+  EXPECT_EQ("SIG16: lower bound violation (fault address: 0x1233, lower bound: "
+"0x1234, upper bound: 0x5678)",
+signals.GetSignalAsString(16, 3, 0x1233, 0x1234, 0x5678));
+}
+
 TEST(UnixSignalsTest, VersionChange) {
   TestSignals signals;
 
Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/ArchSpec.h"
 #include 
+#include 
 
 using namespace lldb_private;
 using namespace llvm;
@@ -112,6 +113,16 @@
   ++m_version;
 }
 
+void UnixSignals::AddSignalCode(int signo, int code, const char *description,
+SignalCodePrintOption print_option) {
+  collection::iterator signal = m_signals.find(signo);
+  assert(signal != m_signals.end() &&
+ "Tried to add code to signal that does not exist.");
+  signal->second.m_codes.insert(
+  std::pair{code, SignalCode{ConstString(description), print_option}});
+  ++m_version;
+}
+
 void UnixSignals::RemoveSignal(int signo) {
   collection::iterator pos = m_signals.find(signo);
   if (pos != m_signals.end())
@@ -127,6 +138,58 @@
 return pos->second.m_name.GetCString();
 }
 
+std::string
+UnixSignals::GetSignalAsString(int32_t signo, std::optional code,
+   std::optional addr,
+   std::optional lower,
+   std::optional upper) const {
+  std::string str;
+
+  collection::const_iterator pos = m_signals.find(signo);
+  if (pos != m_signals.end()) {
+str = pos->second.m_name.GetCString();
+
+

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/FreeBSDSignals.cpp:20
+  // SIGILL
+  AddSignalCode(4, 1 /*ILL_ILLOPC*/, "illegal instruction");
+  AddSignalCode(4, 2 /*ILL_ILLOPN*/, "illegal instruction operand");

emaste wrote:
> FreeBSD comment uses "illegal opcode" fwiw
Turns out, so does Linux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146044

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

[Lldb-commits] [PATCH] D146041: Fix weirdly apologetic diagnostic messages

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

Is there some standard for writing warning messages? For llvm that is, it would 
be worth looking through the getting started guides to see. I think the 
majority of warnings are "formal" in that sense so this seems fine.

Personally I agree with making the warnings more succinct but aside from that I 
don't see the need to change comments or testing scripts.

You may consider splitting this change into 2. One that only changes warnings 
and errors (a less controversial change) and the rest (that is up to the 
reviewers of each bit).




Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:51
 template<_Complex int> struct ComplexInt {};
-using CI = ComplexInt<1 + 3i>; // FIXME: expected-error {{sorry}}
-using CI = ComplexInt<1 + 3i>; // FIXME: expected-error {{sorry}}

Please find out what the {{sorry}} here was for. Did someone literally write it 
as an apology, or is it indicating some warning that should be emitted here, if 
the FIXME is fixed?

You could git blame this file and find the commit that introduced it, as a 
starting point.



Comment at: polly/lib/External/isl/imath/tests/test.sh:12
 if [ ! -f ../imtest ] ; then
-  echo "I can't find the imath test driver 'imtest', did you build it?"
-  echo "I can't proceed with the unit tests until you do so, sorry."
+  echo "The imath test driver 'imtest' was not found."
+  echo "It needs to be build before proceeding with the unit tests."

Given that this is a developer facing script, I think the "did you build it" is 
in fact quite useful here. Perhaps it could say:
"The imath test driver 'imtest' was not found. Did you build it? It is required 
for running the tests."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041

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

[Lldb-commits] [PATCH] D146041: Fix weirdly apologetic diagnostic messages

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

Ah, here it is: 
https://llvm.org/docs/CodingStandards.html#error-and-warning-messages

Seems to back you up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041

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

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread Ed Maste via Phabricator via lldb-commits

emaste added a comment.

FreeBSD LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146044

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

[Lldb-commits] [PATCH] D146041: Fix weirdly apologetic diagnostic messages

2023-03-14 Thread Jun Zhang via Phabricator via lldb-commits

junaire added a comment.

I'm not certain if it's bad to say 'sorry', IMHO it's fine?

Anyway, you can't just simply delete those words (in diagnostic messages and 
regression tests), that doesn't work. To verify the patch is good, you can run 
`ninja check-all` to make sure the testsuite passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz created this revision.
sgraenitz added reviewers: theraven, triplef, mgorny, labath, omjavaid, 
xiaobai, aleksandr.urakov, zturner.
Herald added a project: All.
sgraenitz requested review of this revision.
Herald added a project: LLDB.

We are planning to add basic ObjC debug support for the GNUstep ObjC runtime on 
Linux and Windows: https://github.com/gnustep/libobjc2 On Linux much of the 
desired functionality is there already. It seems accidental in parts, but it's 
a good early baseline. On Windows nothing works yet.

In this first patch I want to add the necessary test infrastructure to utilize 
the GNUstep runtime in the LLDB test suite. The minimal test demonstrates that 
it works on Linux. The next goal is to extend the PDB parser and get Windows to 
this baseline as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146058

Files:
  lldb/test/CMakeLists.txt
  lldb/test/Shell/Expr/objc-gnustep-print.m
  lldb/test/Shell/helper/build.py
  lldb/test/Shell/helper/toolchain.py
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.objc_gnustep_dir = "@LLDB_TEST_OBJC_GNUSTEP_DIR@"
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -24,7 +24,7 @@
 
 # suffixes: A list of file extensions to treat as test files. This is overriden
 # by individual lit.local.cfg files in the test subdirectories.
-config.suffixes = ['.test', '.cpp', '.s']
+config.suffixes = ['.test', '.cpp', '.s', '.m']
 
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
@@ -135,6 +135,14 @@
 if config.have_lldb_server:
 config.available_features.add('lldb-server')
 
+if config.objc_gnustep_dir:
+config.available_features.add('objc-gnustep')
+if platform.system() == 'Windows':
+# objc.dll must be in PATH since Windows has no rpath
+config.environment['PATH'] = os.path.pathsep.join((
+os.path.join(config.objc_gnustep_dir, 'lib'),
+config.environment.get('PATH','')))
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True
Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -42,6 +42,8 @@
 build_script_args.append('--tools-dir={0}'.format(config.lldb_tools_dir))
 if config.llvm_libs_dir:
 build_script_args.append('--libs-dir={0}'.format(config.llvm_libs_dir))
+if config.objc_gnustep_dir:
+build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir))
 
 lldb_init = _get_lldb_init_path(config)
 
Index: lldb/test/Shell/helper/build.py
===
--- lldb/test/Shell/helper/build.py
+++ lldb/test/Shell/helper/build.py
@@ -49,6 +49,18 @@
 action='append',
 help='If specified, a path to search in addition to PATH when --compiler is not an exact path')
 
+parser.add_argument('--objc-gnustep-dir',
+metavar='directory',
+dest='objc_gnustep_dir',
+required=False,
+help='If specified, a path to GNUstep libobjc2 runtime for use on Windows and Linux')
+
+parser.add_argument('--objc-gnustep',
+dest='objc_gnustep',
+action='store_true',
+default=False,
+help='Windows and Linux include/link GNUstep libobjc2 for this build')
+
 if sys.platform == 'darwin':
 parser.add_argument('--apple-sdk',
 metavar='apple_sdk',
@@ -238,6 +250,10 @@
 self.obj_ext = obj_ext
 self.lib_paths = args.libs_dir
 self.std = args.std
+if args.objc_gnustep:
+assert args.objc_gnustep_dir, "GNUstep libobjc2 runtime for Linux and Windows"
+self.objc_gnustep_inc = os.path.join(args.objc_gnustep_dir, 'include')
+self.objc_gnustep_lib = os.path.join(args.objc_gnustep_dir, 'lib')
 
 def _exe_file_name(self):
 assert self.mode != 'compile'
@@ -656,15 +672,20 @@
 args.append('-static')
 args.append('-c')
 
-args.extend(['-o', obj])
-args.append(source

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added a comment.

I am not in touch with any of the build bot maintainers for Windows/Linux LLDB. 
Do you think it's worth asking for mainline test support? They'd basically need 
to build and install libobjc2 once. It's a few hundred kilobytes.

For me the test expectedly fails on Windows 10 and passes on Ubuntu 22.04 LTS 
right now:

  > bin/llvm-lit --filter=objc-gnustep -v tools/lldb/test
  -- Testing: 1 of 1979 tests, 1 workers --
  PASS: lldb-shell :: Expr/objc-gnustep-print.m (1 of 1)
  
  Testing Time: 1.26s
Excluded: 1583
Passed  :1




Comment at: lldb/test/CMakeLists.txt:39
+if (LLDB_TEST_OBJC_GNUSTEP_DIR)
+  message(STATUS "Found GNUstep ObjC runtime: 
${LLDB_TEST_OBJC_GNUSTEP_DIR}/${gnustep_info}")
+endif()

Might be worth a `FindGNUstep.cmake` at some point? (Or is there one somewhere?)

Windows default install dir:
```
-- Found GNUstep ObjC runtime: C:/Program Files (x86)/libobjc/lib/objc.dll
```

Linux default install dir:
```
-- Found GNUstep ObjC runtime: /usr/local/lib/libobjc.so
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett added a comment.

I am not familiar with Objective C, especially beyond Apple platforms. Are you 
aiming to run all the existing Objective C test cases with this different 
runtime or will it be mainly new tests?
(would be very neat if you got all the existing stuff for free)

Comment at: lldb/test/CMakeLists.txt:32
+  set(gnustep_info lib/libobjc.so)
+elseif (WIN32 AND EXISTS "C:/Program Files (x86)/libobjc/lib/objc.dll"
+  AND EXISTS "C:/Program Files 
(x86)/libobjc/include/objc/runtime.h")

Does this library work on Windows on Arm (AArch64 Windows)?

Fine not to handle that in this change, just curious. We (Linaro) do have an 
lldb bot for it, so it could be added at some point.

Comment at: lldb/test/CMakeLists.txt:39
+if (LLDB_TEST_OBJC_GNUSTEP_DIR)
+  message(STATUS "Found GNUstep ObjC runtime: 
${LLDB_TEST_OBJC_GNUSTEP_DIR}/${gnustep_info}")
+endif()

sgraenitz wrote:
> Might be worth a `FindGNUstep.cmake` at some point? (Or is there one 
> somewhere?)
> 
> Windows default install dir:
> ```
> -- Found GNUstep ObjC runtime: C:/Program Files (x86)/libobjc/lib/objc.dll
> ```
> 
> Linux default install dir:
> ```
> -- Found GNUstep ObjC runtime: /usr/local/lib/libobjc.so
> ```
I think we generally use UPPERCASE names for cmake variables.

Comment at: lldb/test/Shell/helper/build.py:62
+default=False,
+help='Windows and Linux include/link GNUstep libobjc2 for 
this build')
+

How about "Include and link GNUstep libobjc2 (Windows and Linux only)" ? It is 
a bit clearer imo.

Comment at: lldb/test/Shell/helper/build.py:687
+args.extend(['-o', obj])
+args.append(source)
+

Why did these 2 need to move?

Comment at: lldb/test/Shell/lit.cfg.py:27
 # by individual lit.local.cfg files in the test subdirectories.
-config.suffixes = ['.test', '.cpp', '.s']
+config.suffixes = ['.test', '.cpp', '.s', '.m']

There are a couple of .m files in the Shell dir already, I assume the related 
tests still pass for those?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

> I am not in touch with any of the build bot maintainers for Windows/Linux 
> LLDB. Do you think it's worth asking for mainline test support? They'd 
> basically need to build and install libobjc2 once. It's a few hundred 
> kilobytes.

Myself and Omair both work on Linaro's bots. We run lldb checks on linux 
arm/aarch64 and Windows on Arm. We could look at installing it.

Pavel runs the x86 debian bot and I think the x86 Windows bot may have been 
retired, or at least is offline at the moment 
(https://lab.llvm.org/buildbot/#/builders/83)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

The Windows x86 bot has in fact been removed: 
https://github.com/llvm/llvm-zorg/commit/acb6223670a41a7fe5d9846075e6b92bd157d78f


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

2023-03-14 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

> Also, I realise that all this XML substitution with strings is very brittle. 
> I want to replace that with Python's xml.etree but will do that later in 
> another patch.

Having tried this, it gives you nice indentation but it's not that much 
different to the raw text. Except that you can't see exactly what's going in 
unless you turn on logging. It is useful to have example of the format here at 
a glance and I don't see there being too much churn in this file so I'm leaving 
it as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145580

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread David Chisnall via Phabricator via lldb-commits

theraven added inline comments.



Comment at: lldb/test/CMakeLists.txt:39
+if (LLDB_TEST_OBJC_GNUSTEP_DIR)
+  message(STATUS "Found GNUstep ObjC runtime: 
${LLDB_TEST_OBJC_GNUSTEP_DIR}/${gnustep_info}")
+endif()

DavidSpickett wrote:
> sgraenitz wrote:
> > Might be worth a `FindGNUstep.cmake` at some point? (Or is there one 
> > somewhere?)
> > 
> > Windows default install dir:
> > ```
> > -- Found GNUstep ObjC runtime: C:/Program Files (x86)/libobjc/lib/objc.dll
> > ```
> > 
> > Linux default install dir:
> > ```
> > -- Found GNUstep ObjC runtime: /usr/local/lib/libobjc.so
> > ```
> I think we generally use UPPERCASE names for cmake variables.
This is somewhat complicated.  If `gnustep-config` exists, the runtime supports 
installing itself into a location defined by the gnustep-make install, which 
supports NeXT, Apple, and FHS-style layouts.  It's probably not worth trying to 
autodetect the general case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D145450: [lldb] Respect empty arguments in target.run-args

2023-03-14 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/source/Host/common/ProcessLaunchInfo.cpp:324-325
   std::string safe_arg = Args::GetShellSafeArgument(m_shell, argv[i]);
+  if (safe_arg.empty())
+safe_arg = "\"\"";
   // Add a space to separate this arg from the previous one.

Shouldn't this belong inside `Args::GetShellSafeArgument`? I don't think 
there's any situation where the current behavior of the function would be 
correct (and this is the only caller of the function anyway...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145450

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

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

2023-03-14 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.cpp:112
+symtab.AddSymbol(Symbol(
+/*symID*/ 0, Mangled(symbol.name), eSymbolTypeCode,
+/*is_global*/ true, /*is_debug*/ false,

clayborg wrote:
> JDevlieghere wrote:
> > mib wrote:
> > > Should we increment the `symID` here ?
> > The IDs are arbitrary, and if we start at zero, we'll have conflicts with 
> > the ones already in the symbol table (e.g. the lldb_unnamed_symbols for 
> > stripped binaries). We could check the size of the symtab and continue 
> > counting from there. Or we can use 0 like we did here to indicate that 
> > these values are "special". I went with the latter approach mostly because 
> > that's what SymbolFileBreakpad does too. 
> regardless of where the code goes that converts ObjectFileJSON::Symbol to 
> lldb_private::Symbol, I would vote that we include enough information in 
> ObjectFileJSON::Symbol so that we don't have to make up symbols IDs, or set 
> all symbol IDs to zero. LLDB relies on having unique symbol IDs in each 
> lldb_private::Symbol instance. 
Speaking of breakpad, have you considered using SymbolFileBreakpad directly? I 
think it serves pretty much the same purpose, and it also supports other, more 
advanced functionality (e.g. line tables and unwind info). It sounds like you 
don't need that now, but if you ever did, you wouldn't need to reinvent that 
logic...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145180

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz updated this revision to Diff 505149.
sgraenitz marked 3 inline comments as done.
sgraenitz added a comment.

Improve help text for the build.py --objc-gnustep option


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

Files:
  lldb/test/CMakeLists.txt
  lldb/test/Shell/Expr/objc-gnustep-print.m
  lldb/test/Shell/helper/build.py
  lldb/test/Shell/helper/toolchain.py
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.objc_gnustep_dir = "@LLDB_TEST_OBJC_GNUSTEP_DIR@"
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -24,7 +24,7 @@
 
 # suffixes: A list of file extensions to treat as test files. This is overriden
 # by individual lit.local.cfg files in the test subdirectories.
-config.suffixes = ['.test', '.cpp', '.s']
+config.suffixes = ['.test', '.cpp', '.s', '.m']
 
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
@@ -135,6 +135,14 @@
 if config.have_lldb_server:
 config.available_features.add('lldb-server')
 
+if config.objc_gnustep_dir:
+config.available_features.add('objc-gnustep')
+if platform.system() == 'Windows':
+# objc.dll must be in PATH since Windows has no rpath
+config.environment['PATH'] = os.path.pathsep.join((
+os.path.join(config.objc_gnustep_dir, 'lib'),
+config.environment.get('PATH','')))
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True
Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -42,6 +42,8 @@
 build_script_args.append('--tools-dir={0}'.format(config.lldb_tools_dir))
 if config.llvm_libs_dir:
 build_script_args.append('--libs-dir={0}'.format(config.llvm_libs_dir))
+if config.objc_gnustep_dir:
+build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir))
 
 lldb_init = _get_lldb_init_path(config)
 
Index: lldb/test/Shell/helper/build.py
===
--- lldb/test/Shell/helper/build.py
+++ lldb/test/Shell/helper/build.py
@@ -49,6 +49,18 @@
 action='append',
 help='If specified, a path to search in addition to PATH when --compiler is not an exact path')
 
+parser.add_argument('--objc-gnustep-dir',
+metavar='directory',
+dest='objc_gnustep_dir',
+required=False,
+help='If specified, a path to GNUstep libobjc2 runtime for use on Windows and Linux')
+
+parser.add_argument('--objc-gnustep',
+dest='objc_gnustep',
+action='store_true',
+default=False,
+help='Include and link GNUstep libobjc2 (Windows and Linux only)')
+
 if sys.platform == 'darwin':
 parser.add_argument('--apple-sdk',
 metavar='apple_sdk',
@@ -238,6 +250,10 @@
 self.obj_ext = obj_ext
 self.lib_paths = args.libs_dir
 self.std = args.std
+if args.objc_gnustep:
+assert args.objc_gnustep_dir, "GNUstep libobjc2 runtime for Linux and Windows"
+self.objc_gnustep_inc = os.path.join(args.objc_gnustep_dir, 'include')
+self.objc_gnustep_lib = os.path.join(args.objc_gnustep_dir, 'lib')
 
 def _exe_file_name(self):
 assert self.mode != 'compile'
@@ -656,15 +672,20 @@
 args.append('-static')
 args.append('-c')
 
-args.extend(['-o', obj])
-args.append(source)
-
 if sys.platform == 'darwin':
 args.extend(['-isysroot', self.apple_sdk])
+elif self.objc_gnustep_inc:
+if source.endswith('.m') or source.endswith('.mm'):
+args.extend(['-fobjc-runtime=gnustep-2.0', '-I', self.objc_gnustep_inc])
+if sys.platform == "win32":
+args.extend(['-Xclang', '-gcodeview', '-Xclang', '--dependent-lib=msvcrtd'])
 
 if self.std:
 args.append('-std={0}'.format(self.std))
 
+args.extend(['-o', obj])
+args.ap

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added a subscriber: stella.stamenova.
sgraenitz added a comment.

In D146058#4193470 , @DavidSpickett 
wrote:

> Myself and Omair both work on Linaro's bots. We run lldb checks on linux 
> arm/aarch64 and Windows on Arm. We could look at installing it.

Right now, my focus is x86 Windows and Linux, but thanks for the offer! So, 
this would be https://lab.llvm.org/buildbot/#/builders/219 (Windows AArch64) 
and https://lab.llvm.org/buildbot/#/builders/17 (Linux AArch32)? Let me discuss 
this internally and come back to you soon.

> Pavel runs the x86 debian bot and I think the x86 Windows bot may have been 
> retired, or at least is offline at the moment 
> (https://lab.llvm.org/buildbot/#/builders/83)

Yes, that would be cool. @labath What do you think?

In D146058#4193471 , @DavidSpickett 
wrote:

> The Windows x86 bot has in fact been removed: 
> https://github.com/llvm/llvm-zorg/commit/acb6223670a41a7fe5d9846075e6b92bd157d78f

Oh I still saw that one running a few days back! @stella.stamenova in the 
review you mention a second windows lldb bot, is it x86 as well? I don't see it 
in https://lab.llvm.org/buildbot/#/console




Comment at: lldb/test/CMakeLists.txt:39
+if (LLDB_TEST_OBJC_GNUSTEP_DIR)
+  message(STATUS "Found GNUstep ObjC runtime: 
${LLDB_TEST_OBJC_GNUSTEP_DIR}/${gnustep_info}")
+endif()

theraven wrote:
> DavidSpickett wrote:
> > sgraenitz wrote:
> > > Might be worth a `FindGNUstep.cmake` at some point? (Or is there one 
> > > somewhere?)
> > > 
> > > Windows default install dir:
> > > ```
> > > -- Found GNUstep ObjC runtime: C:/Program Files (x86)/libobjc/lib/objc.dll
> > > ```
> > > 
> > > Linux default install dir:
> > > ```
> > > -- Found GNUstep ObjC runtime: /usr/local/lib/libobjc.so
> > > ```
> > I think we generally use UPPERCASE names for cmake variables.
> This is somewhat complicated.  If `gnustep-config` exists, the runtime 
> supports installing itself into a location defined by the gnustep-make 
> install, which supports NeXT, Apple, and FHS-style layouts.  It's probably 
> not worth trying to autodetect the general case.
> This is somewhat complicated.
> It's probably not worth trying to autodetect the general case.

Configuring with cmake and running the install target gives me these paths. Do 
you mean I should remove them and just let the user set 
`LLDB_TEST_OBJC_GNUSTEP_DIR`?

> If gnustep-config exists, the runtime supports installing itself into a 
> location defined by the gnustep-make install, which supports NeXT, Apple, and 
> FHS-style layouts.

Right now, I am assuming to find the shared library in the `lib` subdirectory 
and headers in `include`. Is that different in the other layouts? Do we need to 
support that?

> I think we generally use UPPERCASE names for cmake variables.

I  think the convention is: UPPERCASE for global variables, lowercase for local 
variables



Comment at: lldb/test/Shell/helper/build.py:687
+args.extend(['-o', obj])
+args.append(source)
+

DavidSpickett wrote:
> Why did these 2 need to move?
The flags wouldn't apply for previous source files otherwise. I opted to move 
them both in order to keep them close in the resulting command.



Comment at: lldb/test/Shell/lit.cfg.py:27
 # by individual lit.local.cfg files in the test subdirectories.
-config.suffixes = ['.test', '.cpp', '.s']
+config.suffixes = ['.test', '.cpp', '.s', '.m']
 

DavidSpickett wrote:
> There are a couple of .m files in the Shell dir already, I assume the related 
> tests still pass for those?
Interesting, it's exactly one .m and one .mm outside `Input` directories right? 
Let me double-check that.
```
SymbolFile/DWARF/clang-ast-from-dwarf-objc-property.m
SymbolFile/DWARF/x86/module-ownership.mm
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D144392: [lldb] Skip signal handlers for ignored signals on lldb-server's side when stepping

2023-03-14 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D144392#4188940 , @kpdev42 wrote:

> Gdb does this for example:
>
>> GDB optimizes for stepping the mainline code. If a signal that has handle 
>> nostop and handle pass set arrives while a stepping command (e.g., stepi, 
>> step, next) is in progress, GDB lets the signal handler run and then resumes 
>> stepping the mainline code once the signal handler returns. In other words, 
>> GDB steps over the signal handler. This prevents signals that you’ve 
>> specified as not interesting (with handle nostop) from changing the focus of 
>> debugging unexpectedly.
>
> (https://sourceware.org/gdb/onlinedocs/gdb/Signals.html), which seems 
> reasonable.

I agree that this is the most useful behavior in most normal situations.

> If this logic does not belong in lldb-server,

Yes,  I believe this is too complex to be done inside lldb-server. I might 
reconsider if you can show that the gdb logic you described above is also 
implemented on the gdbserver side, but I'd rather not.. :)

> then it could be fixed right inside the `StepOverBreakpoint` plan, by 
> enabling the breakpoint being stepped over when a signal is received, waiting 
> for it to hit when the potential handler has returned, and then trying to 
> step off again. But then doing a `step-into` from a line with a breakpoint 
> will step over the signal handler, and from a line without a breakpoint will 
> stop inside the handler, if a signal is received. Then probably creating a 
> new `ThreadPlan` instead with the same logic, executing on top of the plan 
> stack, is the way to go?

Sounds approximately right. The way I'd probably do is it to have the 
`StepOverBreakpoint` step into the signal handler, and then push a `StepOut` 
(which hopefully knows how to step out of a signal handler -- if not it may 
need some tuning, or creating a specialized `StepOutOfSignal` plan). After the 
step out is done, it can retry the step over operation.

@jingham is our thread plan guru, though, so he may have a better idea...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144392

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

[Lldb-commits] [PATCH] D145955: Refactor ObjectFilePlaceholder for sharing

2023-03-14 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

yay


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145955

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

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

It's not exactly what I had in mind, but I kinda like it :)




Comment at: lldb/include/lldb/Target/UnixSignals.h:127
+  struct SignalCode {
+ConstString m_description;
+SignalCodePrintOption m_print_option;

I think we should just make strings out of these, particularly since we now 
also have a map here. It's not like this is extremely performance-sensitive 
code.



Comment at: lldb/source/Plugins/Process/POSIX/CrashReason.cpp:25
+  std::string description =
+  lldb_private::UnixSignals::CreateForHost()->GetSignalAsString(
+  info.si_signo, info.si_code,

I think this function should really be called GetSignalDescription, or 
something similar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146044

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

[Lldb-commits] [PATCH] D145450: [lldb] Respect empty arguments in target.run-args

2023-03-14 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/source/Host/common/ProcessLaunchInfo.cpp:324-325
   std::string safe_arg = Args::GetShellSafeArgument(m_shell, argv[i]);
+  if (safe_arg.empty())
+safe_arg = "\"\"";
   // Add a space to separate this arg from the previous one.

labath wrote:
> Shouldn't this belong inside `Args::GetShellSafeArgument`? I don't think 
> there's any situation where the current behavior of the function would be 
> correct (and this is the only caller of the function anyway...)
As in, we should set safe_arg to `\"\"` when `argv[I]` is the empty string 
(e.g. safe_arg should never be empty)? If that's what you're suggesting, I 
think that would be a reasonable thing to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145450

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Stella Stamenova via Phabricator via lldb-commits

stella.stamenova added a comment.

> Oh I still saw that one running a few days back! @stella.stamenova in the 
> review you mention a second windows lldb bot, is it x86 as well? I don't see 
> it in https://lab.llvm.org/buildbot/#/console

The one I was referring to is https://lab.llvm.org/buildbot/#/builders/219.

Also, the one that was removed was not x86 either, it was x64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/test/Shell/helper/build.py:254
+if args.objc_gnustep:
+assert args.objc_gnustep_dir, "GNUstep libobjc2 runtime for Linux 
and Windows"
+self.objc_gnustep_inc = os.path.join(args.objc_gnustep_dir, 
'include')

The assertion message here is not tremendously helpful. Maybe something like 
"--objc-gnustep specified without path to libobjc2. Use --objc-gnustep-dir to 
specify said path."



Comment at: lldb/test/Shell/helper/toolchain.py:46
+if config.objc_gnustep_dir:
+
build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir))
 

Why does {0} need quotes around it but the args above it don't?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D145136: Add a Debugger interruption mechanism in parallel to the Command Interpreter interruption

2023-03-14 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

polite ping...  I think I've addressed all the concerns folks raised.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145136

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

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

2023-03-14 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM with Pavel's comments.




Comment at: lldb/unittests/Signals/UnixSignalsTest.cpp:132-134
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3));
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3, 0xcafef00d));
+  EXPECT_EQ(expected, signals.GetSignalAsString(16, 3, 0xcafef00d, 0x1234));

Nit: If these all need to pass maybe make these `ASSERT_EQ` so you don't get an 
"expected" failure below. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146044

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

[Lldb-commits] [PATCH] D146043: [lldb] Refactor CrashReason

2023-03-14 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146043

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz updated this revision to Diff 505189.
sgraenitz marked 2 inline comments as done.
sgraenitz added a comment.

Polish assertion message in build.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

Files:
  lldb/test/CMakeLists.txt
  lldb/test/Shell/Expr/objc-gnustep-print.m
  lldb/test/Shell/helper/build.py
  lldb/test/Shell/helper/toolchain.py
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.objc_gnustep_dir = "@LLDB_TEST_OBJC_GNUSTEP_DIR@"
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -24,7 +24,7 @@
 
 # suffixes: A list of file extensions to treat as test files. This is overriden
 # by individual lit.local.cfg files in the test subdirectories.
-config.suffixes = ['.test', '.cpp', '.s']
+config.suffixes = ['.test', '.cpp', '.s', '.m']
 
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
@@ -135,6 +135,14 @@
 if config.have_lldb_server:
 config.available_features.add('lldb-server')
 
+if config.objc_gnustep_dir:
+config.available_features.add('objc-gnustep')
+if platform.system() == 'Windows':
+# objc.dll must be in PATH since Windows has no rpath
+config.environment['PATH'] = os.path.pathsep.join((
+os.path.join(config.objc_gnustep_dir, 'lib'),
+config.environment.get('PATH','')))
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True
Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -42,6 +42,8 @@
 build_script_args.append('--tools-dir={0}'.format(config.lldb_tools_dir))
 if config.llvm_libs_dir:
 build_script_args.append('--libs-dir={0}'.format(config.llvm_libs_dir))
+if config.objc_gnustep_dir:
+build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir))
 
 lldb_init = _get_lldb_init_path(config)
 
Index: lldb/test/Shell/helper/build.py
===
--- lldb/test/Shell/helper/build.py
+++ lldb/test/Shell/helper/build.py
@@ -49,6 +49,18 @@
 action='append',
 help='If specified, a path to search in addition to PATH when --compiler is not an exact path')
 
+parser.add_argument('--objc-gnustep-dir',
+metavar='directory',
+dest='objc_gnustep_dir',
+required=False,
+help='If specified, a path to GNUstep libobjc2 runtime for use on Windows and Linux')
+
+parser.add_argument('--objc-gnustep',
+dest='objc_gnustep',
+action='store_true',
+default=False,
+help='Include and link GNUstep libobjc2 (Windows and Linux only)')
+
 if sys.platform == 'darwin':
 parser.add_argument('--apple-sdk',
 metavar='apple_sdk',
@@ -238,6 +250,10 @@
 self.obj_ext = obj_ext
 self.lib_paths = args.libs_dir
 self.std = args.std
+if args.objc_gnustep:
+assert args.objc_gnustep_dir, "--objc-gnustep specified without path to libobjc2"
+self.objc_gnustep_inc = os.path.join(args.objc_gnustep_dir, 'include')
+self.objc_gnustep_lib = os.path.join(args.objc_gnustep_dir, 'lib')
 
 def _exe_file_name(self):
 assert self.mode != 'compile'
@@ -656,15 +672,20 @@
 args.append('-static')
 args.append('-c')
 
-args.extend(['-o', obj])
-args.append(source)
-
 if sys.platform == 'darwin':
 args.extend(['-isysroot', self.apple_sdk])
+elif self.objc_gnustep_inc:
+if source.endswith('.m') or source.endswith('.mm'):
+args.extend(['-fobjc-runtime=gnustep-2.0', '-I', self.objc_gnustep_inc])
+if sys.platform == "win32":
+args.extend(['-Xclang', '-gcodeview', '-Xclang', '--dependent-lib=msvcrtd'])
 
 if self.std:
 args.append('-std={0}'.format(self.std))
 
+args.extend(['-o', obj])
+args.append(source)
+

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz added a comment.

In D146058#4193795 , 
@stella.stamenova wrote:

> The one I was referring to is https://lab.llvm.org/buildbot/#/builders/219.

Ok, thanks for the update. Then we are lacking a Windows x86 build bot for LLDB 
now!

> Also, the one that was removed was not x86 either, it was x64.

Yes, the naming can be confusing. I think the x86 we talked about mainly means 
64-bit (often called x64 on Windows) and includes the 32-bit legacy. Like the 
`X86` target backend in LLVM.




Comment at: lldb/test/Shell/helper/build.py:254
+if args.objc_gnustep:
+assert args.objc_gnustep_dir, "GNUstep libobjc2 runtime for Linux 
and Windows"
+self.objc_gnustep_inc = os.path.join(args.objc_gnustep_dir, 
'include')

bulbazord wrote:
> The assertion message here is not tremendously helpful. Maybe something like 
> "--objc-gnustep specified without path to libobjc2. Use --objc-gnustep-dir to 
> specify said path."
None of the other `assert`s have messages at all, but ok :)



Comment at: lldb/test/Shell/helper/toolchain.py:46
+if config.objc_gnustep_dir:
+
build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir))
 

bulbazord wrote:
> Why does {0} need quotes around it but the args above it don't?
Good catch! That's due to spaces in Windows paths...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Stefan Gränitz via Phabricator via lldb-commits

sgraenitz marked 2 inline comments as done.
sgraenitz added a comment.

Soo, @DavidSpickett

In D146058#4193446 , @DavidSpickett 
wrote:

> I am not familiar with Objective C, especially beyond Apple platforms. Are 
> you aiming to run all the existing Objective C test cases with this different 
> runtime or will it be mainly new tests?
> (would be very neat if you got all the existing stuff for free)

It would be great to get all the existing Objective C test coverage for free, 
but I guess it'd take a while to support all of the Apple runtime features. 
Literally all existing ObjC tests are API tests and when running locally, they 
are all `UNSUPPORTED` for me on Linux and Windows. I didn't manage to get them 
running quickly and thus, I went for a Shell test here. My impression was that 
API tests are LLDB legacy and new tests would usually be Shell. It might still 
be worth getting (some of) the API tests to work with GNUstep as well, but I 
might need some advice on how to do that.

In D146058#4193470 , @DavidSpickett 
wrote:

> Myself and Omair both work on Linaro's bots. We run lldb checks on linux 
> arm/aarch64 and Windows on Arm. We could look at installing it.

I suspect `lldb-arm-ubuntu` means 32-bit ARM. This should work, but it's not a 
priority for us right now. We could still give it a try and see if it works 
out-of-the-box!

As a side-note: Could the AArch64 bot 
https://lab.llvm.org/buildbot/#/builders/219 pass `LLVM_LIT_ARGS="-v"` as well 
so the log tells us which tests actually ran? That would be great!

Comment at: lldb/test/CMakeLists.txt:32
+  set(gnustep_info lib/libobjc.so)
+elseif (WIN32 AND EXISTS "C:/Program Files (x86)/libobjc/lib/objc.dll"
+  AND EXISTS "C:/Program Files 
(x86)/libobjc/include/objc/runtime.h")

DavidSpickett wrote:
> Does this library work on Windows on Arm (AArch64 Windows)?
> 
> Fine not to handle that in this change, just curious. We (Linaro) do have an 
> lldb bot for it, so it could be added at some point.
Not yet: https://github.com/gnustep/libobjc2/issues/227

Comment at: lldb/test/Shell/lit.cfg.py:27
 # by individual lit.local.cfg files in the test subdirectories.
-config.suffixes = ['.test', '.cpp', '.s']
+config.suffixes = ['.test', '.cpp', '.s', '.m']

sgraenitz wrote:
> DavidSpickett wrote:
> > There are a couple of .m files in the Shell dir already, I assume the 
> > related tests still pass for those?
> Interesting, it's exactly one .m and one .mm outside `Input` directories 
> right? Let me double-check that.
> ```
> SymbolFile/DWARF/clang-ast-from-dwarf-objc-property.m
> SymbolFile/DWARF/x86/module-ownership.mm
> ```
These have been running ever since, because 
https://github.com/llvm/llvm-project/blob/release/16.x/lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg
 Anyway, thanks for your note!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058

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

[Lldb-commits] [PATCH] D146041: Fix weirdly apologetic diagnostic messages

2023-03-14 Thread Aryan Godara via Phabricator via lldb-commits

AryanGodara added a comment.

In D146041#4192816 , @mehdi_amini 
wrote:

> You should look into the title and description of the commit: 
> https://cbea.ms/git-commit/

Is the title and description appropriate now?
I can add more details to the description, as required


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041

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

[Lldb-commits] [PATCH] D146041: Fix weirdly apologetic diagnostic messages

2023-03-14 Thread Aryan Godara via Phabricator via lldb-commits

AryanGodara added a comment.

In D146041#4193045 , @DavidSpickett 
wrote:

> Is there some standard for writing warning messages? For llvm that is, it 
> would be worth looking through the getting started guides to see. I think the 
> majority of warnings are "formal" in that sense so this seems fine.
>
> Personally I agree with making the warnings more succinct but aside from that 
> I don't see the need to change comments or testing scripts.
>
> You may consider splitting this change into 2. One that only changes warnings 
> and errors (a less controversial change) and the rest (that is up to the 
> reviewers of each bit).

I will do go through the changes I made, and first make sure to change only 
warnings and errors.

Since this is my first commit to such a large repository(and project), can you 
please guide me with this  @DavidSpickett !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041

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

[Lldb-commits] [PATCH] D146041: Fix weirdly apologetic diagnostic messages

2023-03-14 Thread Aryan Godara via Phabricator via lldb-commits

AryanGodara added a comment.

In D146041#4193172 , @junaire wrote:

> I'm not certain if it's bad to say 'sorry', IMHO it's fine?
>
> Anyway, you can't just simply delete those words (in diagnostic messages and 
> regression tests), that doesn't work. To verify the patch is good, you can 
> run `ninja check-all` to make sure the testsuite passes.

This was one of the issues for Outreachy '23, and I asked my outreachy mentor 
to work on this issue.
I'm looking for any help to make the patch more meaningful, and get guidance 
along the way.

I'll run `ninja check-all`, before pushing the next commit for revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041

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

[Lldb-commits] [PATCH] D145450: [lldb] Respect empty arguments in target.run-args

2023-03-14 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/source/Host/common/ProcessLaunchInfo.cpp:324-325
   std::string safe_arg = Args::GetShellSafeArgument(m_shell, argv[i]);
+  if (safe_arg.empty())
+safe_arg = "\"\"";
   // Add a space to separate this arg from the previous one.

bulbazord wrote:
> labath wrote:
> > Shouldn't this belong inside `Args::GetShellSafeArgument`? I don't think 
> > there's any situation where the current behavior of the function would be 
> > correct (and this is the only caller of the function anyway...)
> As in, we should set safe_arg to `\"\"` when `argv[I]` is the empty string 
> (e.g. safe_arg should never be empty)? If that's what you're suggesting, I 
> think that would be a reasonable thing to do.
I think it is. What I'm saying is that `Args::GetShellSafeArgument(???, "")` 
should automatically return `"\"\"` instead of needing to fix this up 
afterwards...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145450

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

Re: [Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

2023-03-14 Thread Jim Ingham via lldb-commits



> On Mar 14, 2023, at 12:10 PM, Stefan Gränitz via Phabricator via lldb-commits 
>  wrote:
> 
> sgraenitz marked 2 inline comments as done.
> sgraenitz added a comment.
> 
> Soo, @DavidSpickett
> 
> In D146058#4193446 , @DavidSpickett 
> wrote:
> 
>> I am not familiar with Objective C, especially beyond Apple platforms. Are 
>> you aiming to run all the existing Objective C test cases with this 
>> different runtime or will it be mainly new tests?
>> (would be very neat if you got all the existing stuff for free)
> 
> It would be great to get all the existing Objective C test coverage for free, 
> but I guess it'd take a while to support all of the Apple runtime features. 
> Literally all existing ObjC tests are API tests and when running locally, 
> they are all `UNSUPPORTED` for me on Linux and Windows. I didn't manage to 
> get them running quickly and thus, I went for a Shell test here. My 
> impression was that API tests are LLDB legacy and new tests would usually be 
> Shell. It might still be worth getting (some of) the API tests to work with 
> GNUstep as well, but I might need some advice on how to do that.

That is not true.  The shell tests are generally less accurate than the API 
tests - it's much easier to make a test that isn't doing what you thought 
because they match something unintended.  If the test goes wrong, they are also 
harder to debug.  We use them for things where you really do want the terminal 
type interaction or where they are testing very simple stuff.  For anything 
more complicated than that, the API tests are generally a better way to go, and 
if you're on the fence, I at least always suggest API tests.

Jim


> 
> In D146058#4193470 , @DavidSpickett 
> wrote:
> 
>> Myself and Omair both work on Linaro's bots. We run lldb checks on linux 
>> arm/aarch64 and Windows on Arm. We could look at installing it.
> 
> I suspect `lldb-arm-ubuntu` means 32-bit ARM. This should work, but it's not 
> a priority for us right now. We could still give it a try and see if it works 
> out-of-the-box!
> 
> As a side-note: Could the AArch64 bot 
> https://lab.llvm.org/buildbot/#/builders/219 pass `LLVM_LIT_ARGS="-v"` as 
> well so the log tells us which tests actually ran? That would be great!
> 
> 
> 
> 
> Comment at: lldb/test/CMakeLists.txt:32
> +  set(gnustep_info lib/libobjc.so)
> +elseif (WIN32 AND EXISTS "C:/Program Files (x86)/libobjc/lib/objc.dll"
> +  AND EXISTS "C:/Program Files 
> (x86)/libobjc/include/objc/runtime.h")
> 
> DavidSpickett wrote:
>> Does this library work on Windows on Arm (AArch64 Windows)?
>> 
>> Fine not to handle that in this change, just curious. We (Linaro) do have an 
>> lldb bot for it, so it could be added at some point.
> Not yet: https://github.com/gnustep/libobjc2/issues/227
> 
> 
> 
> Comment at: lldb/test/Shell/lit.cfg.py:27
> # by individual lit.local.cfg files in the test subdirectories.
> -config.suffixes = ['.test', '.cpp', '.s']
> +config.suffixes = ['.test', '.cpp', '.s', '.m']
> 
> 
> sgraenitz wrote:
>> DavidSpickett wrote:
>>> There are a couple of .m files in the Shell dir already, I assume the 
>>> related tests still pass for those?
>> Interesting, it's exactly one .m and one .mm outside `Input` directories 
>> right? Let me double-check that.
>> ```
>> SymbolFile/DWARF/clang-ast-from-dwarf-objc-property.m
>> SymbolFile/DWARF/x86/module-ownership.mm
>> ```
> These have been running ever since, because 
> https://github.com/llvm/llvm-project/blob/release/16.x/lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg
>  Anyway, thanks for your note!
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D146058/new/
> 
> https://reviews.llvm.org/D146058
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [PATCH] D146045: [LLDB] Show sub type of signals when debugging a core file

2023-03-14 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Nice. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146045

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

[Lldb-commits] [lldb] 65a2d6d - [lldb] Use *{Set, Map}::contains (NFC)

2023-03-14 Thread Kazu Hirata via lldb-commits


Author: Kazu Hirata
Date: 2023-03-14T21:41:40-07:00
New Revision: 65a2d6d6904f0b06f0d0cfdfbfa67dbe6599f081

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

LOG: [lldb] Use *{Set,Map}::contains (NFC)

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index c084c3fbefc5d..2826b102625fe 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -113,7 +113,7 @@ class DeclContextOverride {
   llvm::DenseMap m_backups;
 
   void OverrideOne(clang::Decl *decl) {
-if (m_backups.find(decl) != m_backups.end()) {
+if (m_backups.contains(decl)) {
   return;
 }
 

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
index cc3abfdfde394..527055024a768 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -212,7 +212,7 @@ class TokenVerifier {
   /// Returns true iff the given expression body contained a token with the
   /// given content.
   bool hasToken(llvm::StringRef token) const {
-return m_tokens.find(token) != m_tokens.end();
+return m_tokens.contains(token);
   }
 };
 

diff  --git a/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp 
b/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
index 6b78ce5de697a..6e56e29f8c31c 100644
--- a/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
+++ b/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
@@ -23,7 +23,7 @@
 using namespace lldb_private;
 
 bool ClangHighlighter::isKeyword(llvm::StringRef token) const {
-  return keywords.find(token) != keywords.end();
+  return keywords.contains(token);
 }
 
 ClangHighlighter::ClangHighlighter() {

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 391ed99607f77..1f89db0cb5ca0 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -4556,7 +4556,7 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   // Count how many trie symbols we'll add to the symbol table
   int trie_symbol_table_augment_count = 0;
   for (auto &e : external_sym_trie_entries) {
-if (symbols_added.find(e.entry.address) == symbols_added.end())
+if (!symbols_added.contains(e.entry.address))
   trie_symbol_table_augment_count++;
   }
 
@@ -4603,8 +4603,7 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   if (function_starts_count > 0) {
 uint32_t num_synthetic_function_symbols = 0;
 for (i = 0; i < function_starts_count; ++i) {
-  if (symbols_added.find(function_starts.GetEntryRef(i).addr) ==
-  symbols_added.end())
+  if (!symbols_added.contains(function_starts.GetEntryRef(i).addr))
 ++num_synthetic_function_symbols;
 }
 
@@ -4616,7 +4615,7 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
   for (i = 0; i < function_starts_count; ++i) {
 const FunctionStarts::Entry *func_start_entry =
 function_starts.GetEntryAtIndex(i);
-if (symbols_added.find(func_start_entry->addr) == symbols_added.end()) 
{
+if (!symbols_added.contains(func_start_entry->addr)) {
   addr_t symbol_file_addr = func_start_entry->addr;
   uint32_t symbol_flags = 0;
   if (func_start_entry->data)

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index b99e9ec82f126..075d4b042d2aa 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1392,7 +1392,7 @@ bool SymbolFileNativePDB::ParseImportedModules(
 void SymbolFileNativePDB::ParseInlineSite(PdbCompilandSymId id,
   Address func_addr) {
   lldb::user_id_t opaque_uid = toOpaqueUid(id);
-  if (m_inline_sites.find(opaque_uid) != m_inline_sites.end())
+  if (m_inline_sites.contains(opaque_uid))
 return;
 
   addr_t func_base = func_addr.GetFileAddress()

49 matches

Mail list logo