[Lldb-commits] [lldb] e1d4fb1 - [lldb] Fix build errors from 3bea7306e8

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-01T09:01:35+02:00
New Revision: e1d4fb1ebfc612d65331f72bb0827d7206d4

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

LOG: [lldb] Fix build errors from 3bea7306e8

The addition of the dummy constructors requires matching changes in os-
and arch-specific files, which I forgot about.

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp 
b/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
index d5052e7d1b3af..9328d606ad26d 100644
--- 
a/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
+++ 
b/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -260,7 +260,7 @@ 
NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
   native_thread, CreateRegisterInfoInterface(target_arch)),
-  m_regset_offsets({0}) {
+  NativeRegisterContextDBReg_x86(native_thread), m_regset_offsets({0}) {
   assert(m_gpr.size() == GetRegisterInfoInterface().GetGPRSize());
   std::array first_regnos;
 

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
index 2bc2a923298d9..91f88280145d2 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -56,8 +56,9 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 
 NativeRegisterContextLinux_arm::NativeRegisterContextLinux_arm(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
-: NativeRegisterContextRegisterInfo(
-  native_thread, new RegisterInfoPOSIX_arm(target_arch)) {
+: NativeRegisterContextRegisterInfo(native_thread,
+new 
RegisterInfoPOSIX_arm(target_arch)),
+  NativeRegisterContextLinux(native_thread) {
   assert(target_arch.GetMachine() == llvm::Triple::arm);
 
   ::memset(&m_fpr, 0, sizeof(m_fpr));

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 09cf72c044822..506b54e44a62f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -76,7 +76,8 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread,
 std::unique_ptr register_info_up)
 : NativeRegisterContextRegisterInfo(native_thread,
-register_info_up.release()) {
+register_info_up.release()),
+  NativeRegisterContextLinux(native_thread) {
   ::memset(&m_fpr, 0, sizeof(m_fpr));
   ::memset(&m_gpr_arm64, 0, sizeof(m_gpr_arm64));
   ::memset(&m_hwp_regs, 0, sizeof(m_hwp_regs));

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
index 4531148c3fdc9..60582e4cc61f6 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
@@ -128,7 +128,8 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 NativeRegisterContextLinux_ppc64le::NativeRegisterContextLinux_ppc64le(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
-  native_thread, new RegisterInfoPOSIX_ppc64le(target_arch)) {
+  native_thread, new RegisterInfoPOSIX_ppc64le(target_arch)),
+  NativeRegisterContextLinux(native_thread) {
   if (target_arch.GetMachine() != llvm::Triple::ppc64le) {
 llvm_unreachable("Unhandled target architecture.");
   }

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
index beae2da756a81..3c0916499f70d 100644
--- a/lldb/source/Plu

[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think this looks fine. Could you also create a gdb-client test case (you can 
ignore the default thread thingy)?




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1131
 
 bool GDBRemoteCommunicationClient::GetDefaultThreadId(lldb::tid_t &tid) {
   StringExtractorGDBRemote response;

It looks like this is only used in the (probably completely broken) non-stop 
mode. I should put that on my to-delete list...



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h:370
+  void GetCurrentProcessAndThreadIDs(
+  std::vector> &ids,
+  bool &sequence_mutex_unavailable);

lets make this an actual return value


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

https://reviews.llvm.org/D99603

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

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D98822#2658780 , @mgorny wrote:

> What about debug registers on FreeBSD? This system is pretty unique because 
> child processes inherit dbregs from parent, and therefore we need to clear 
> them. GDB doesn't do that and watchpoints crash forked children. Should we 
> keep the clearing part as a hack specific to FreeBSD, or have lldb generic 
> clear all hardware breakpoints and watchpoints on fork?

I think we can keep the freebsd-specific clearing code (in the server). 
Clearing all hardware breakpoints in the client would be pretty logical, but if 
we wanted to make that work consistently, then we might need to add code to 
/other/ operating system implementations to /set/ the breakpoints (so that the 
client can clear them properly)...




Comment at: lldb/include/lldb/Host/linux/Host.h:13
+#include "llvm/ADT/Optional.h"
+
+#include "lldb/lldb-types.h"

These spaces are pretty common in lldb, but that's not actually consistent with 
how the rest of llvm formats them...



Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp:738
+::pid_t wait_pid =
+llvm::sys::RetryAfterSignal(-1, waitpid, -1, &status, WNOHANG);
 

And the netbsd comment might apply to freebsd as well...



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:290-291
+#ifdef LLDB_HAS_FREEBSD_WATCHPOINT
+  llvm::Error error = ReadHardwareDebugInfo();
+  if (error)
+return error;





Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:647-665
+  case (SIGTRAP | (PTRACE_EVENT_FORK << 8)): {
+unsigned long data = 0;
+if (GetEventMessage(thread.GetID(), &data).Fail())
+  data = 0;
+
+if (!MonitorClone(data, {{PTRACE_EVENT_FORK, thread.GetID()}}))
+  WaitForCloneNotification(data);

It looks like these two (and probably also PTRACE_EVENT_CLONE) could be bundled 
together.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:659
+unsigned long data = 0;
+if (GetEventMessage(thread.GetID(), &data).Fail())
+  data = 0;

If this fails (I guess that can only happen if the entire process was 
SIGKILLED), how does continuing with pid=0 help?
I'm not sure that resuming the parent (as the clone case does) is significantly 
better, but it seems like we should at least be consistent...



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:906
+
+  auto parent_thread = GetThreadByID(clone_info->parent_tid);
+  assert(parent_thread);

auto *



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:931
+  }
+  // fallthrough
+  case PTRACE_EVENT_FORK: {

I think we have some macro or attribute for that these days...



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:936
+ m_arch,unused_loop,   {child_pid}};
+child_process.m_software_breakpoints = m_software_breakpoints;
+child_process.Detach();

Copying the breakpoints is not needed yet, right? We could just group these two 
(fork/vfork) paths for now...



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:952
+  default:
+assert(false && "unknown clone_info.event");
+  }

llvm_unreachable("unknown clone_info.event");



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:955
+
+  m_pending_pid_map.erase(child_pid);
+  return true;

erase(find_it) would be slightly more efficient. (And it might be better to put 
this before the switch, to make sure it executes in case of early exits.)



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:784
+int status;
+::pid_t wait_pid = llvm::sys::RetryAfterSignal(-1, waitpid, -1, &status,
+   WALLSIG | WNOHANG);

For NetBSD, it might be cleaner to keep waiting for the parent only, and then 
do a separate waitpid for the child once you get its process id. That would 
make the sequence of events (and the relevant code) predictable.

I think this is how this interface was meant to be used, even on linux, but 
linux makes it tricky because it's not possible to wait for all threads 
belonging to a given process in a single call -- one would have to iterate 
through the threads and wait for each one separately (it might be interesting 
to try to implement and benchmark that).



Comment at: lldb/test/Shell/Subprocess/Inputs/clone.c:23
+int main() {
+  char stack[4096];
+

Better include an alignas directive, just in case.



Comment at: lldb/test/Shell/Subproces

Re: [Lldb-commits] [PATCH] D98482: [lldb] [server] Support for multiprocess extension

[Jason Molenda via lldb-commits]

Hi Michał, this commit has caused a test failure on the macos incremental CI 
bot,

http://green.lab.llvm.org/green/blue/organizations/jenkins/lldb-cmake/activity

Failed Tests (3):

  lldb-api :: tools/lldb-server/TestLldbGdbServer.py
  lldb-api :: tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
  lldb-shell :: lldb-server/TestGdbserverPort.test

I know this commit caused the TestLldbGdbServer.py for sure, I'm pretty sure it 
caused TestGdbRemote_vContThreads.py, I haven't tested TestGdbserverPort.test 
yet.  It's probably best to revert the change until we can debug what is 
happening here (I can look tomorrow) so the CI bots aren't all red for macos.


> On Mar 30, 2021, at 6:09 AM, Michał Górny via Phabricator via lldb-commits 
>  wrote:
> 
> This revision was not accepted when it landed; it landed in state "Needs 
> Review".
> This revision was landed with ongoing or failed builds.
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rG6c1a8039de46: [lldb] [server] Support for multiprocess 
> extension (authored by mgorny).
> Herald added a project: LLDB.
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D98482/new/
> 
> https://reviews.llvm.org/D98482
> 
> Files:
>  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
>  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
>  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
>  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
>  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
>  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
>  lldb/source/Utility/StringExtractorGDBRemote.cpp
>  lldb/test/API/tools/lldb-server/TestGdbRemote_vContThreads.py
>  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
>  lldb/unittests/Utility/CMakeLists.txt
>  lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp
> 
> ___
> 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] [lldb] dd2a63e - Revert "Revert "[LLDB] Arm64/Linux test case for MTE and Pointer Authentication regset""

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2021-04-01T14:10:14+05:00
New Revision: dd2a63e1ee53c1178d8e17a3763edc26d23f00a2

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

LOG: Revert "Revert "[LLDB] Arm64/Linux test case for MTE and Pointer 
Authentication regset""

This reverts commit feb6f2c78fa9474e7329c4a809f175b1675d0975.

Added: 
lldb/test/API/commands/register/register/aarch64_dynamic_regset/Makefile

lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
lldb/test/API/commands/register/register/aarch64_dynamic_regset/main.c

Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 958cadd3a7c81..a9928af677a6e 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1269,7 +1269,7 @@ def isPPC64le(self):
 return True
 return False
 
-def isAArch64SVE(self):
+def getCPUInfo(self):
 triple = self.dbg.GetSelectedPlatform().GetTriple()
 
 # TODO other platforms, please implement this function
@@ -1290,7 +1290,16 @@ def isAArch64SVE(self):
 except:
 return False
 
-return " sve " in cpuinfo
+return cpuinfo
+
+def isAArch64SVE(self):
+return "sve" in self.getCPUInfo()
+
+def isAArch64MTE(self):
+return "mte" in self.getCPUInfo()
+
+def isAArch64PAuth(self):
+return "paca" in self.getCPUInfo()
 
 def hasLinuxVmFlags(self):
 """ Check that the target machine has "VmFlags" lines in

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_dynamic_regset/Makefile 
b/lldb/test/API/commands/register/register/aarch64_dynamic_regset/Makefile
new file mode 100644
index 0..5fc881c2db97c
--- /dev/null
+++ b/lldb/test/API/commands/register/register/aarch64_dynamic_regset/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := main.c
+
+CFLAGS_EXTRAS := -march=armv8-a+sve
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
 
b/lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
new file mode 100644
index 0..ecaefb0e657e8
--- /dev/null
+++ 
b/lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
@@ -0,0 +1,109 @@
+"""
+Test AArch64 dynamic register sets
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_register_size(self, set, name, expected):
+reg_value = set.GetChildMemberWithName(name)
+self.assertTrue(reg_value.IsValid(),
+'Expected a register named %s' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected,
+ 'Expected a register %s size == %i bytes' % (name, 
expected))
+
+def sve_regs_read_dynamic(self, sve_registers):
+vg_reg = sve_registers.GetChildMemberWithName("vg")
+vg_reg_value = sve_registers.GetChildMemberWithName(
+"vg").GetValueAsUnsigned()
+
+z_reg_size = vg_reg_value * 8
+p_reg_size = int(z_reg_size / 8)
+
+for i in range(32):
+z_regs_value = '{' + \
+' '.join('0x{:02x}'.format(i + 1)
+ for _ in range(z_reg_size)) + '}'
+self.expect('register read z%i' %
+(i), substrs=[z_regs_value])
+
+# Set P registers with random test values. The P registers are 
predicate
+# registers, which hold one bit for each byte available in a Z 
register.
+# For below mentioned values of P registers, P(0,5,10,15) will have all
+# Z register lanes set while P(4,9,14) will have no lanes set.
+p_value_bytes = ['0xff', '0x55', '0x11', '0x01', '0x00']
+for i in range(16):
+p_regs_value = '{' + \
+' '.join(p_value_bytes[i % 5] for _ in range(p_reg_size)) + '}'
+self.expect('register read p%i' % (i), substrs=[p_regs_value])
+
+self.expect("register read ffr", substrs=[p_regs_value])
+
+for i in range(32):
+z_regs_value = '{' + \
+' '.join('0x{:02x}'.format(32 - i)
+ for _ in range(z_reg_size)) + '}'
+self.runCmd("register write z%i '%s'" % (i, z_regs_value))
+self.expect('register read z%i' % (i), substrs=[z_regs_value])
+
+for i in range(16):
+p_regs_value = '{' + \
+' '.join('0x{:02x}'.for

[Lldb-commits] [lldb] 88a5b35 - Revert "Revert "[LLDB] Arm64/Linux Add MTE and Pointer Authentication registers""

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2021-04-01T14:07:50+05:00
New Revision: 88a5b35d63f927db69ec953ff487a7ba2504a610

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

LOG: Revert "Revert "[LLDB] Arm64/Linux Add MTE and Pointer Authentication 
registers""

This reverts commit 71b648f7158c7a0b4918eaa3e94d307e4bbfce97.

There was a typo in the last commit which was causing LLDB AArch64 Linux
buildbot testsuite failures. Now fixed in current version.

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
lldb/source/Plugins/Process/POSIX/NativeProcessELF.h
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 506b54e44a62f..1a85ea0f697f7 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -33,6 +33,17 @@
 #define NT_ARM_SVE 0x405 /* ARM Scalable Vector Extension */
 #endif
 
+#ifndef NT_ARM_PAC_MASK
+#define NT_ARM_PAC_MASK 0x406 /* Pointer authentication code masks */
+#endif
+
+#ifndef NT_ARM_TAGGED_ADDR_CTRL
+#define NT_ARM_TAGGED_ADDR_CTRL 0x409 /* Tagged address control register */
+#endif
+
+#define HWCAP_PACA (1 << 30)
+#define HWCAP2_MTE (1 << 18)
+
 #define REG_CONTEXT_SIZE (GetGPRSize() + GetFPRSize())
 
 using namespace lldb;
@@ -62,6 +73,18 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 .Success())
   opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskSVE);
 
+NativeProcessLinux &process = native_thread.GetProcess();
+
+llvm::Optional auxv_at_hwcap =
+process.GetAuxValue(AuxVector::AUXV_AT_HWCAP);
+if (auxv_at_hwcap && (*auxv_at_hwcap & HWCAP_PACA))
+  opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskPAuth);
+
+llvm::Optional auxv_at_hwcap2 =
+process.GetAuxValue(AuxVector::AUXV_AT_HWCAP2);
+if (auxv_at_hwcap2 && (*auxv_at_hwcap2 & HWCAP2_MTE))
+  opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
+
 auto register_info_up =
 std::make_unique(target_arch, opt_regsets);
 return std::make_unique(
@@ -83,6 +106,9 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   ::memset(&m_hwp_regs, 0, sizeof(m_hwp_regs));
   ::memset(&m_hbp_regs, 0, sizeof(m_hbp_regs));
   ::memset(&m_sve_header, 0, sizeof(m_sve_header));
+  ::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
+
+  m_mte_ctrl_reg = 0;
 
   // 16 is just a maximum value, query hardware for actual watchpoint count
   m_max_hwp_supported = 16;
@@ -94,6 +120,8 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_fpu_is_valid = false;
   m_sve_buffer_is_valid = false;
   m_sve_header_is_valid = false;
+  m_pac_mask_is_valid = false;
+  m_mte_ctrl_is_valid = false;
 
   if (GetRegisterInfo().IsSVEEnabled())
 m_sve_state = SVEState::Unknown;
@@ -230,6 +258,22 @@ NativeRegisterContextLinux_arm64::ReadRegister(const 
RegisterInfo *reg_info,
 src = (uint8_t *)GetSVEBuffer() + offset;
   }
 }
+  } else if (IsPAuth(reg)) {
+error = ReadPAuthMask();
+if (error.Fail())
+  return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetPAuthOffset();
+assert(offset < GetPACMaskSize());
+src = (uint8_t *)GetPACMask() + offset;
+  } else if (IsMTE(reg)) {
+error = ReadMTEControl();
+if (error.Fail())
+  return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetMTEOffset();
+assert(offset < GetMTEControlSize());
+src = (uint8_t *)GetMTEControl() + offset;
   } else
 return Status("failed - register wasn't recognized to be a GPR or an FPR, "
   "write strategy unknown");
@@ -388,6 +432,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
 return WriteAllSVE();
   }
 }
+  } else if (IsMTE(reg)) {
+error = ReadMTEControl();
+if (error.Fail())
+  return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetMTEOffset();
+assert(offset < GetMTEControlSize());
+dst = (uint8_t *)GetMTEControl() + offset;
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+return WriteMTEControl();
   }
 
   return Status("Failed to write register value");
@@ -476,6 +531,14 @@ bool NativeRegisterContextLinux_arm64::IsSVE(unsigned reg) 
c

Re: [Lldb-commits] [PATCH] D98482: [lldb] [server] Support for multiprocess extension

[Pavel Labath via lldb-commits]

I think the first two test just need the "llgs" test category, since 
debugserver (obviously) does not support the extension. I don't see 
TestGdbserverPort.test failing in the latest build on that page (and I'd 
be surprised if this was related to that, so I suspect that was just flake.


pl

On 01/04/2021 11:06, Jason Molenda wrote:

Hi Michał, this commit has caused a test failure on the macos incremental CI 
bot,

http://green.lab.llvm.org/green/blue/organizations/jenkins/lldb-cmake/activity

Failed Tests (3):

   lldb-api :: tools/lldb-server/TestLldbGdbServer.py
   lldb-api :: tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
   lldb-shell :: lldb-server/TestGdbserverPort.test

I know this commit caused the TestLldbGdbServer.py for sure, I'm pretty sure it 
caused TestGdbRemote_vContThreads.py, I haven't tested TestGdbserverPort.test 
yet.  It's probably best to revert the change until we can debug what is 
happening here (I can look tomorrow) so the CI bots aren't all red for macos.



On Mar 30, 2021, at 6:09 AM, Michał Górny via Phabricator via lldb-commits 
 wrote:

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c1a8039de46: [lldb] [server] Support for multiprocess 
extension (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98482

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemote_vContThreads.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp

___
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] D99497: [LLDB] Fix sync issue in TestVSCode_launch.test_progress_events

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

I am going ahead and commit this change till we find a better way to fix the 
sync issue. sleep time set to 2s now.


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

https://reviews.llvm.org/D99497

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

[Lldb-commits] [lldb] b468f0e - [LLDB] Fix sync issue in TestVSCode_launch.test_progress_events

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2021-04-01T14:16:54+05:00
New Revision: b468f0e165ed67c5b1046b295b65e446afee62aa

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

LOG: [LLDB] Fix sync issue in TestVSCode_launch.test_progress_events

This fixes flakiness in TestVSCode_launch.test_progress_events
vscode.progress_events some times failed to populate in time for
follow up iterations.

Adding a minor delay before the the for the loop fixes the issue.

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py 
b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
index aceed56fe2491..dc4549cd3251c 100644
--- a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -454,7 +454,6 @@ def test_terminate_commands(self):
 
 @skipIfWindows
 @skipIfRemote
-@skipIf(oslist=["linux"])
 def test_progress_events(self):
 '''
 Tests the progress events to ensure we are receiving them.
@@ -486,6 +485,8 @@ def test_progress_events(self):
 # Iterate over all progress events and save all start and end IDs, and
 # remember any shared libraries that got symbol table parsing progress
 # events.
+# Sleep for 2 seconds to make sure progress_events gets populated
+time.sleep(2)
 for progress_event in self.vscode.progress_events:
 event_type = progress_event['event']
 if event_type == 'progressStart':



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

[Lldb-commits] [PATCH] D99497: [LLDB] Fix sync issue in TestVSCode_launch.test_progress_events

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb468f0e165ed: [LLDB] Fix sync issue in 
TestVSCode_launch.test_progress_events (authored by omjavaid).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D99497?vs=333819&id=334622#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99497

Files:
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py


Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -454,7 +454,6 @@
 
 @skipIfWindows
 @skipIfRemote
-@skipIf(oslist=["linux"])
 def test_progress_events(self):
 '''
 Tests the progress events to ensure we are receiving them.
@@ -486,6 +485,8 @@
 # Iterate over all progress events and save all start and end IDs, and
 # remember any shared libraries that got symbol table parsing progress
 # events.
+# Sleep for 2 seconds to make sure progress_events gets populated
+time.sleep(2)
 for progress_event in self.vscode.progress_events:
 event_type = progress_event['event']
 if event_type == 'progressStart':


Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -454,7 +454,6 @@
 
 @skipIfWindows
 @skipIfRemote
-@skipIf(oslist=["linux"])
 def test_progress_events(self):
 '''
 Tests the progress events to ensure we are receiving them.
@@ -486,6 +485,8 @@
 # Iterate over all progress events and save all start and end IDs, and
 # remember any shared libraries that got symbol table parsing progress
 # events.
+# Sleep for 2 seconds to make sure progress_events gets populated
+time.sleep(2)
 for progress_event in self.vscode.progress_events:
 event_type = progress_event['event']
 if event_type == 'progressStart':
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D98482: [lldb] [server] Support for multiprocess extension

[Michał Górny via lldb-commits]

On Thu, 2021-04-01 at 11:14 +0200, Pavel Labath wrote:
> I think the first two test just need the "llgs" test category, since 
> debugserver (obviously) does not support the extension. I don't see 
> TestGdbserverPort.test failing in the latest build on that page (and I'd 
> be surprised if this was related to that, so I suspect that was just flake.
> 
> pl
> 
> On 01/04/2021 11:06, Jason Molenda wrote:
> > Hi Michał, this commit has caused a test failure on the macos incremental 
> > CI bot,
> > 
> > http://green.lab.llvm.org/green/blue/organizations/jenkins/lldb-cmake/activity
> > 
> > Failed Tests (3):
> > 
> >    lldb-api :: tools/lldb-server/TestLldbGdbServer.py
> >    lldb-api :: tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
> >    lldb-shell :: lldb-server/TestGdbserverPort.test
> > 
> > I know this commit caused the TestLldbGdbServer.py for sure, I'm pretty 
> > sure it caused TestGdbRemote_vContThreads.py, I haven't tested 
> > TestGdbserverPort.test yet.  It's probably best to revert the change until 
> > we can debug what is happening here (I can look tomorrow) so the CI bots 
> > aren't all red for macos.

I'm going to try adding the category.  If that doesn't help, I'll revert
it afterwards.

-- 
Best regards,
Michał Górny


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

[Lldb-commits] [lldb] fcea418 - [lldb] [test] Mark lldb-server multiprocess tests as LLGS cat

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-04-01T14:17:47+02:00
New Revision: fcea4181bbfbc15a27ad4d3c06a09b706b1d6c47

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

LOG: [lldb] [test] Mark lldb-server multiprocess tests as LLGS cat

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py 
b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
index 0764e3d6c75a6..7090abf00fc5a 100644
--- a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -398,6 +398,7 @@ def test_Hg_switches_to_3_threads_attach(self):
 self.Hg_switches_to_3_threads()
 
 @expectedFailureAll(oslist=["windows"]) # expect 4 threads
+@add_test_categories(["llgs"])
 def test_Hg_switches_to_3_threads_attach_pass_correct_pid(self):
 self.build()
 self.set_inferior_startup_attach()
@@ -427,18 +428,21 @@ def Hg_fails_on_pid(self, pass_pid):
 self.expect_gdbremote_sequence()
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_Hg_fails_on_another_pid(self):
 self.build()
 self.set_inferior_startup_launch()
 self.Hg_fails_on_pid(1)
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_Hg_fails_on_zero_pid(self):
 self.build()
 self.set_inferior_startup_launch()
 self.Hg_fails_on_pid(0)
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_Hg_fails_on_minus_one_pid(self):
 self.build()
 self.set_inferior_startup_launch()

diff  --git 
a/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py 
b/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
index c7ced621c3f90..86a1693721774 100644
--- 
a/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
+++ 
b/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
@@ -96,6 +96,7 @@ def test_signal_all_threads(self):
 
 @skipIfWindows
 @expectedFailureNetBSD
+@add_test_categories(["llgs"])
 def test_signal_process_by_pid(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -109,6 +110,7 @@ def test_signal_process_by_pid(self):
 
 @skipIfWindows
 @expectedFailureNetBSD
+@add_test_categories(["llgs"])
 def test_signal_process_minus_one(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -121,6 +123,7 @@ def test_signal_process_minus_one(self):
 
 @skipIfWindows
 @expectedFailureNetBSD
+@add_test_categories(["llgs"])
 def test_signal_minus_one(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -132,6 +135,7 @@ def test_signal_minus_one(self):
 
 @skipIfWindows
 @expectedFailureNetBSD
+@add_test_categories(["llgs"])
 def test_signal_all_threads_by_pid(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -147,6 +151,7 @@ def test_signal_all_threads_by_pid(self):
 
 @skipIfWindows
 @expectedFailureNetBSD
+@add_test_categories(["llgs"])
 def test_signal_minus_one_by_pid(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -160,6 +165,7 @@ def test_signal_minus_one_by_pid(self):
 
 @skipIfWindows
 @expectedFailureNetBSD
+@add_test_categories(["llgs"])
 def test_signal_minus_one_by_minus_one(self):
 self.build()
 self.set_inferior_startup_launch()



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

[Lldb-commits] [lldb] 48e3da1 - [lldb] Rewrite TestAutoInstallMainExecutable logic

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-01T14:20:20+02:00
New Revision: 48e3da13519dea3bd91ab7de656c7d46105c2c01

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

LOG: [lldb] Rewrite TestAutoInstallMainExecutable logic

The test uses debug info from one binary to debug a different one. This
does not work on macos, and its pure luck that it works elsewhere (the
variable that it inspects happens to have the same address in both).

The purpose of this test is to verify that lldb has not overwritten the
target executable. That can be more easily achieved by checking the exit
code of the binary, so change the test to do that.

Also remove the llgs_test decorator, as it's preventing the test from
running on macos. All the test needs is the platform functionality of
lldb-server, which is available everywhere.

Added: 


Modified: 
lldb/test/API/commands/target/auto-install-main-executable/Makefile

lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
lldb/test/API/commands/target/auto-install-main-executable/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/commands/target/auto-install-main-executable/Makefile 
b/lldb/test/API/commands/target/auto-install-main-executable/Makefile
index 1354ec49464a8..07e6c9a1d0f15 100644
--- a/lldb/test/API/commands/target/auto-install-main-executable/Makefile
+++ b/lldb/test/API/commands/target/auto-install-main-executable/Makefile
@@ -1,9 +1,9 @@
 CXX_SOURCES := main.cpp
-CXXFLAGS := -DBUILD=\"stock\"
+CXXFLAGS := -DBUILD=47
 
 a.out: a.device.out
 
 include Makefile.rules
 
 a.device.out:
-   $(CXX) $(CXXFLAGS) -DBUILD=\"device\" -o $@ $(SRCDIR)/main.cpp
+   $(CXX) $(CXXFLAGS) -DBUILD=74 -o $@ $(SRCDIR)/main.cpp

diff  --git 
a/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
 
b/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
index 862943c41aa37..410e1f108c16a 100644
--- 
a/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
+++ 
b/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
@@ -15,10 +15,11 @@ class TestAutoInstallMainExecutable(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
-@llgs_test
 @skipIfRemote
 @expectedFailureAll(oslist=["windows"]) # process modules not loaded
 def test_target_auto_install_main_executable(self):
+if lldbgdbserverutils.get_lldb_server_exe() is None:
+  self.skipTest("lldb-server not found")
 self.build()
 
 hostname = socket.getaddrinfo("localhost", 0, 
proto=socket.IPPROTO_TCP)[0][4][0]
@@ -69,23 +70,4 @@ def test_target_auto_install_main_executable(self):
 (dest.fullpath,
 self.getBuildArtifact("a.out")))
 
-target = self.dbg.GetSelectedTarget()
-breakpoint = target.BreakpointCreateByName("main")
-
-launch_info = target.GetLaunchInfo()
-error = lldb.SBError()
-process = target.Launch(launch_info, error)
-self.assertTrue(process, PROCESS_IS_VALID)
-
-thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
-self.assertTrue(
-thread.IsValid(),
-"There should be a thread stopped due to breakpoint")
-
-frame = thread.GetFrameAtIndex(0)
-self.assertEqual(frame.GetFunction().GetName(), "main")
-
-self.expect("target variable build", substrs=['"device"'],
-msg="Magic in the binary is wrong")
-
-process.Continue()
+self.expect("process launch", substrs=["exited with status = 74"])

diff  --git 
a/lldb/test/API/commands/target/auto-install-main-executable/main.cpp 
b/lldb/test/API/commands/target/auto-install-main-executable/main.cpp
index 373f1a724e288..f9fb9978a208e 100644
--- a/lldb/test/API/commands/target/auto-install-main-executable/main.cpp
+++ b/lldb/test/API/commands/target/auto-install-main-executable/main.cpp
@@ -1,8 +1,5 @@
-#include 
+int build = BUILD;
 
-const char* build = BUILD;
-
-int main(int argc, char **argv) {
-  printf("argc: %d\n", argc);
-  return argv[0][0];
+int main() {
+  return BUILD;
 }



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

[Lldb-commits] [lldb] bad5ee1 - [lldb] Make TestLoadUsingLazyBind work on linux

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-01T14:49:42+02:00
New Revision: bad5ee15ea2e5a5aaaed9c9a5d9982e23cedba55

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

LOG: [lldb] Make TestLoadUsingLazyBind work on linux

and probably other posix oses. Use extra_images to ensure
LD_LIBRARY_PATH is set correctly.

Also take the opportunity to remove hand-rolled library extension
management code in favor of the existing one.

Added: 


Modified: 
lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py

Removed: 




diff  --git a/lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py 
b/lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py
index a32e589884ce6..9ca6229943061 100644
--- a/lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py
+++ b/lldb/test/API/functionalities/load_lazy/TestLoadUsingLazyBind.py
@@ -21,25 +21,23 @@ class LoadUsingLazyBind(TestBase):
 @skipIfWindows # The Windows platform doesn't implement DoLoadImage.
 # Failing for unknown reasons on Linux, see
 # https://bugs.llvm.org/show_bug.cgi?id=49656.
-@skipUnlessDarwin
 def test_load_using_lazy_bind(self):
 """Test that we load using RTLD_LAZY"""
 
 self.build()
 wd = os.path.realpath(self.getBuildDir())
 
-ext = '.so'
-if self.platformIsDarwin():
-ext = '.dylib'
+def make_lib_name(name):
+return (self.platformContext.shlib_prefix + name + "." +
+self.platformContext.shlib_extension)
 
 def make_lib_path(name):
-libpath = os.path.join(wd, name + ext)
+libpath = os.path.join(wd, make_lib_name(name))
 self.assertTrue(os.path.exists(libpath))
 return libpath
 
-libt1 = make_lib_path('libt1')
-libt2_0 = make_lib_path('libt2_0')
-libt2_1 = make_lib_path('libt2_1')
+libt2_0 = make_lib_path('t2_0')
+libt2_1 = make_lib_path('t2_1')
 
 # Overwrite t2_0 with t2_1 to delete the definition of `use`.
 shutil.copy(libt2_1, libt2_0)
@@ -47,11 +45,12 @@ def make_lib_path(name):
 # Launch a process and break
 (target, process, thread, _) = lldbutil.run_to_source_breakpoint(self,
 "break here",
-lldb.SBFileSpec("main.cpp"))
+lldb.SBFileSpec("main.cpp"),
+extra_images=["t1"])
 
 # Load libt1; should fail unless we use RTLD_LAZY
 error = lldb.SBError()
-lib_spec = lldb.SBFileSpec('libt1' + ext)
+lib_spec = lldb.SBFileSpec(make_lib_name('t1'))
 paths = lldb.SBStringList()
 paths.AppendString(wd)
 out_spec = lldb.SBFileSpec()



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

[Lldb-commits] [PATCH] D97284: [lldb][AArch64] Add MTE CPU feature test predicate

[David Spickett via Phabricator via lldb-commits]

DavidSpickett abandoned this revision.
DavidSpickett added a comment.

This is now redundant since Omair landed his changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97284

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

[Lldb-commits] [PATCH] D99729: [lldb] Improve CPUInfo test predicate

[David Spickett via Phabricator via lldb-commits]

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

Use a with block for reading the cpuinfo file.

When loading the file fails (or we're not on Linux)
return an empty string. Since all the callers are
going to do "x in self.getCPUInfo()".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99729

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1274,7 +1274,7 @@
 
 # TODO other platforms, please implement this function
 if not re.match(".*-.*-linux", triple):
-return False
+return ""
 
 # Need to do something different for non-Linux/Android targets
 cpuinfo_path = self.getBuildArtifact("cpuinfo")
@@ -1284,11 +1284,10 @@
 cpuinfo_path = "/proc/cpuinfo"
 
 try:
-f = open(cpuinfo_path, 'r')
-cpuinfo = f.read()
-f.close()
+with open(cpuinfo_path, 'r') as f:
+cpuinfo = f.read()
 except:
-return False
+return ""
 
 return cpuinfo
 


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1274,7 +1274,7 @@
 
 # TODO other platforms, please implement this function
 if not re.match(".*-.*-linux", triple):
-return False
+return ""
 
 # Need to do something different for non-Linux/Android targets
 cpuinfo_path = self.getBuildArtifact("cpuinfo")
@@ -1284,11 +1284,10 @@
 cpuinfo_path = "/proc/cpuinfo"
 
 try:
-f = open(cpuinfo_path, 'r')
-cpuinfo = f.read()
-f.close()
+with open(cpuinfo_path, 'r') as f:
+cpuinfo = f.read()
 except:
-return False
+return ""
 
 return cpuinfo
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 334686.
mgorny marked an inline comment as done.
mgorny added a comment.

Changed `GetCurrentProcessAndThreadIDs()` to return the vector, and reformatted.

Tests next.


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

https://reviews.llvm.org/D99603

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -335,7 +335,7 @@
 
   size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string &value);
 
-  size_t UpdateThreadIDsFromStopReplyThreadsValue(std::string &value);
+  size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
 
   bool HandleNotifyPacket(StringExtractorGDBRemote &packet);
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1487,22 +1487,22 @@
   m_thread_pcs.clear();
 }
 
-size_t
-ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string &value) {
+size_t ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(
+llvm::StringRef value) {
   m_thread_ids.clear();
-  size_t comma_pos;
-  lldb::tid_t tid;
-  while ((comma_pos = value.find(',')) != std::string::npos) {
-value[comma_pos] = '\0';
-// thread in big endian hex
-tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-if (tid != LLDB_INVALID_THREAD_ID)
-  m_thread_ids.push_back(tid);
-value.erase(0, comma_pos + 1);
-  }
-  tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-  if (tid != LLDB_INVALID_THREAD_ID)
-m_thread_ids.push_back(tid);
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
+  StringExtractorGDBRemote thread_ids{value};
+
+  do {
+auto pid_tid = thread_ids.GetPidTid(pid);
+if (pid_tid && pid_tid->first == pid) {
+  lldb::tid_t tid = pid_tid->second;
+  if (tid != LLDB_INVALID_THREAD_ID &&
+  tid != StringExtractorGDBRemote::AllProcesses)
+m_thread_ids.push_back(tid);
+}
+  } while (thread_ids.GetChar() == ',');
+
   return m_thread_ids.size();
 }
 
@@ -1519,7 +1519,7 @@
 value.erase(0, comma_pos + 1);
   }
   pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
-  if (pc != LLDB_INVALID_THREAD_ID)
+  if (pc != LLDB_INVALID_ADDRESS)
 m_thread_pcs.push_back(pc);
   return m_thread_pcs.size();
 }
@@ -2141,6 +2141,7 @@
 }
 
 StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
   stop_packet.SetFilePos(0);
   const char stop_type = stop_packet.GetChar();
   switch (stop_type) {
@@ -2155,14 +2156,12 @@
 if (stop_id == 0) {
   // Our first stop, make sure we have a process ID, and also make sure we
   // know about our registers
-  if (GetID() == LLDB_INVALID_PROCESS_ID) {
-lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
-if (pid != LLDB_INVALID_PROCESS_ID)
-  SetID(pid);
-  }
+  if (GetID() == LLDB_INVALID_PROCESS_ID && pid != LLDB_INVALID_PROCESS_ID)
+SetID(pid);
   BuildDynamicRegisterInfo(true);
 }
 // Stop with signal and thread info
+lldb::pid_t stop_pid = LLDB_INVALID_PROCESS_ID;
 lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
 const uint8_t signo = stop_packet.GetHexU8();
 llvm::StringRef key;
@@ -2191,24 +2190,18 @@
 value.getAsInteger(16, x);
 exc_data.push_back(x);
   } else if (key.compare("thread") == 0) {
-// thread in big endian hex
-if (value.getAsInteger(16, tid))
+// thread-id
+StringExtractorGDBRemote thread_id{value};
+auto pid_tid = thread_id.GetPidTid(pid);
+if (pid_tid) {
+  stop_pid = pid_tid->first;
+  tid = pid_tid->second;
+} else
   tid = LLDB_INVALID_THREAD_ID;
   } else if (key.compare("threads") == 0) {
 std::lock_guard guard(
 m_thread_list_real.GetMutex());
-
-m_thread_ids.clear();
-// A comma separated list of all threads in the current
-// process that includes the thread for this stop reply packet
-lldb::tid_t tid;
-while (!value.empty()) {
-  llvm::StringRef tid_str;
-  std::tie(tid_str, value) = value.split(',');
-  if (tid_str.getAsInteger(16, tid))
-tid = LLDB_INVALID_THREAD_ID;
-  m_thread_ids.push_back(tid);
-

[Lldb-commits] [PATCH] D99653: [nfc] [lldb] 1/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 334690.
jankratochvil added a comment.

The patch now requires D99731 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99653

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


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -235,15 +235,7 @@
   /// Return a rangelist's offset based on an index. The index designates
   /// an entry in the rangelist table's offset array and is supplied by
   /// DW_FORM_rnglistx.
-  llvm::Optional GetRnglistOffset(uint32_t Index) const {
-if (!m_rnglist_table)
-  return llvm::None;
-if (llvm::Optional off = m_rnglist_table->getOffsetEntry(
-m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(),
-Index))
-  return *off + m_ranges_base;
-return llvm::None;
-  }
+  llvm::Optional GetRnglistOffset(uint32_t Index);
 
   llvm::Optional GetLoclistOffset(uint32_t Index) {
 if (!m_loclist_table_header)
@@ -291,6 +283,8 @@
 return &m_die_array[0];
   }
 
+  const llvm::Optional &GetRnglist();
+
   SymbolFileDWARF &m_dwarf;
   std::shared_ptr m_dwo;
   DWARFUnitHeader m_header;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -495,6 +495,19 @@
 ranges_base, toString(table_or_error.takeError()).c_str());
 }
 
+const llvm::Optional &DWARFUnit::GetRnglist() {
+  return m_rnglist_table;
+}
+
+llvm::Optional DWARFUnit::GetRnglistOffset(uint32_t Index) {
+  if (!GetRnglist())
+return llvm::None;
+  if (llvm::Optional off = GetRnglist()->getOffsetEntry(
+  m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(), 
Index))
+return *off + m_ranges_base;
+  return llvm::None;
+}
+
 void DWARFUnit::SetStrOffsetsBase(dw_offset_t str_offsets_base) {
   m_str_offsets_base = str_offsets_base;
 }
@@ -936,11 +949,11 @@
 return ranges;
   }
 
-  if (!m_rnglist_table)
+  if (!GetRnglist())
 return llvm::createStringError(errc::invalid_argument,
"missing or invalid range list table");
 
-  auto range_list_or_error = m_rnglist_table->findList(
+  auto range_list_or_error = GetRnglist()->findList(
   m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(), offset);
   if (!range_list_or_error)
 return range_list_or_error.takeError();
@@ -971,7 +984,7 @@
 DWARFUnit::FindRnglistFromIndex(uint32_t index) {
   if (llvm::Optional offset = GetRnglistOffset(index))
 return FindRnglistFromOffset(*offset);
-  if (m_rnglist_table)
+  if (GetRnglist())
 return llvm::createStringError(errc::invalid_argument,
"invalid range list table index %d", index);
 


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -235,15 +235,7 @@
   /// Return a rangelist's offset based on an index. The index designates
   /// an entry in the rangelist table's offset array and is supplied by
   /// DW_FORM_rnglistx.
-  llvm::Optional GetRnglistOffset(uint32_t Index) const {
-if (!m_rnglist_table)
-  return llvm::None;
-if (llvm::Optional off = m_rnglist_table->getOffsetEntry(
-m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(),
-Index))
-  return *off + m_ranges_base;
-return llvm::None;
-  }
+  llvm::Optional GetRnglistOffset(uint32_t Index);
 
   llvm::Optional GetLoclistOffset(uint32_t Index) {
 if (!m_loclist_table_header)
@@ -291,6 +283,8 @@
 return &m_die_array[0];
   }
 
+  const llvm::Optional &GetRnglist();
+
   SymbolFileDWARF &m_dwarf;
   std::shared_ptr m_dwo;
   DWARFUnitHeader m_header;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -495,6 +495,19 @@
 ranges_base, toString(table_or_error.takeError()).c_str());
 }
 
+const llvm::Optional &DWARFUnit::GetRnglist() {
+  return m_rnglist_table;
+}
+
+llvm::Optional DWARFUnit::GetRnglistOffset(uint32_t Index) {
+  if (!GetRnglist())
+return llvm::None;
+  if (llvm::Optional off = GetRnglist()->getOffsetEntry(
+  m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(), Index))
+return *off + m_ranges_base;
+  return llvm::None;
+}
+
 void DWARFUnit::

[Lldb-commits] [PATCH] D99653: [nfc] [lldb] 1/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

[Jan Kratochvil via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:498-500
+llvm::Optional DWARFUnit::GetRnglist() {
+  return m_rnglist_table;
+}

clayborg wrote:
> Return "const llvm::Optional &" to avoid making 
> a copy.
That was horrible, thanks for catching it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99653

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

[Lldb-commits] [PATCH] D98289: [lldb] 2/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 334692.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98289

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
@@ -0,0 +1,138 @@
+# DW_AT_ranges can use DW_FORM_sec_offset (instead of DW_FORM_rnglistx).
+# In such case DW_AT_rnglists_base does not need to be present.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
+# RUN: %lldb %t -o "image lookup -v -s lookup_rnglists" \
+# RUN:   -o exit | FileCheck %s
+
+# Failure was the block range 1..2 was not printed plus:
+# error: DW_AT_range-DW_FORM_sec_offset.s.tmp {0x003f}: DIE has DW_AT_ranges(0xc) attribute, but range extraction failed (missing or invalid range list table), please file a bug and attach the file at the start of this error message
+
+# CHECK-LABEL: image lookup -v -s lookup_rnglists
+# CHECK:  Function: id = {0x0029}, name = "rnglists", range = [0x-0x0003)
+# CHECK:Blocks: id = {0x0029}, range = [0x-0x0003)
+# CHECK-NEXT:   id = {0x003f}, range = [0x0001-0x0002)
+
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj \
+# RUN:   --defsym RNGLISTX=0 %s > %t-rnglistx
+# RUN: %lldb %t-rnglistx -o "image lookup -v -s lookup_rnglists" \
+# RUN:   -o exit 2>&1 | FileCheck --check-prefix=RNGLISTX %s
+
+# RNGLISTX-LABEL: image lookup -v -s lookup_rnglists
+# RNGLISTX: error: DW_AT_range-DW_FORM_sec_offset.s.tmp-rnglistx : DW_FORM_rnglistx cannot be used without DW_AT_rnglists_base
+
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj \
+# RUN:   --defsym RNGLISTX=0 --defsym RNGLISTBASE=0 %s > %t-rnglistbase
+# RUN: %lldb %t-rnglistbase -o "image lookup -v -s lookup_rnglists" \
+# RUN:   -o exit 2>&1 | FileCheck --check-prefix=RNGLISTBASE %s
+
+# RNGLISTBASE-LABEL: image lookup -v -s lookup_rnglists
+# RNGLISTBASE: error: DW_AT_range-DW_FORM_sec_offset.s.tmp-rnglistbase {0x0043}: DIE has DW_AT_ranges(0x0) attribute, but range extraction failed (invalid range list table index 0), please file a bug and attach the file at the start of this error message
+
+.text
+rnglists:
+nop
+.Lblock1_begin:
+lookup_rnglists:
+nop
+.Lblock1_end:
+nop
+.Lrnglists_end:
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   37  # DW_AT_producer
+.byte   8   # DW_FORM_string
+.byte   17  # DW_AT_low_pc
+.byte   27  # DW_FORM_addrx
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
+.byte   115 # DW_AT_addr_base
+.byte   23  # DW_FORM_sec_offset
+.ifdef RNGLISTBASE
+.byte   0x74# DW_AT_rnglists_base
+.byte   23  # DW_FORM_sec_offset
+.endif
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   2   # Abbreviation Code
+.byte   46  # DW_TAG_subprogram
+.byte   1   # DW_CHILDREN_yes
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   5   # Abbreviation Code
+.byte   11  # DW_TAG_lexical_block
+.byte   0   # DW_CHILDREN_no
+.byte   85  # DW_AT_ranges
+.ifndef RNGLISTX
+.byte   0x17# DW_FORM_sec_offset
+.else
+.byte   0x23# DW_FORM_rnglistx
+.endif
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   0   # EOM(3)
+
+.section.debug_info,"",@progbits
+.Lcu_begin0:
+.long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+.short  5   # DW

[Lldb-commits] [PATCH] D98289: [lldb] 2/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:489
 const llvm::Optional &DWARFUnit::GetRnglist() {
+  if (GetVersion() >= 5 && !m_rnglist_table_done) {
+m_rnglist_table_done = true;

`if (GetVersion() < 5) return llvm::None;` is no longer possible with the 
returned reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98289

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

[Lldb-commits] [PATCH] D96460: [LLDB] Arm64/Linux Add MTE and Pointer Authentication registers

[David Spickett via Phabricator via lldb-commits]

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

@omjavaid I get some new warnings when compiling this on x86:

  [732/1117] Building CXX object too.../RegisterContextDarwin_arm64.cpp.o
  In file included from 
/work/open_source/lldb-cross-compile/llvm-project/lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp:70:
  
/work/open_source/lldb-cross-compile/llvm-project/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:794:35:
 warning: ‘g_register_infos_mte’ defined but not used [-Wunused-variable]
794 | static lldb_private::RegisterInfo g_register_infos_mte[] = {
|   ^~~~
  
/work/open_source/lldb-cross-compile/llvm-project/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:791:35:
 warning: ‘g_register_infos_pauth’ defined but not used [-Wunused-variable]
791 | static lldb_private::RegisterInfo g_register_infos_pauth[] = {
|   ^~

Makes some sense that they're not used but presumably we could mark them as 
such? In case it's g++ not seeing the existing macros properly, I'm using:

  g++-9 (Ubuntu 9.3.0-23ubuntu1~16.04) 9.3.0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96460

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

[Lldb-commits] [lldb] 802c5ce - [lldb] Un-XFAIL TestAutoInstallMainExecutable on Windows

[Stella Stamenova via lldb-commits]

Author: Stella Stamenova
Date: 2021-04-01T08:46:23-07:00
New Revision: 802c5ce364a21c54c1568c8791b1d5f36c11829e

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

LOG: [lldb] Un-XFAIL TestAutoInstallMainExecutable on Windows

Added: 


Modified: 

lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py

Removed: 




diff  --git 
a/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
 
b/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
index 410e1f108c16..9993768df06f 100644
--- 
a/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
+++ 
b/lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
@@ -16,7 +16,6 @@ class TestAutoInstallMainExecutable(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfRemote
-@expectedFailureAll(oslist=["windows"]) # process modules not loaded
 def test_target_auto_install_main_executable(self):
 if lldbgdbserverutils.get_lldb_server_exe() is None:
   self.skipTest("lldb-server not found")



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

[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 334725.
mgorny added a comment.

Add some tests.

@labath, I'd use some help now. When the stop reason includes wrong PID, the 
GDB exchange seems to enter a dead loop:

  b-remote.async>  <   5> send packet: $c#63
  b-remote.async>  <  25> read packet: $S02thread:p404.10210;#cc
  intern-state <  16> send packet: $qfThreadInfo#bb
  intern-state <  26> read packet: $mp400.10200,p400.10204#e7
  intern-state <  16> send packet: $qsThreadInfo#c8
  intern-state <   5> read packet: $l#6c

I think this is because most of the code doesn't really care about 
`SetThreadStopInfo()`'s return value, and I'm not really convinced that's the 
best way of reporting an error. Could you suggest how to handle 'wrong PID' 
error here?


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

https://reviews.llvm.org/D99603

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -335,7 +335,7 @@
 
   size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string &value);
 
-  size_t UpdateThreadIDsFromStopReplyThreadsValue(std::string &value);
+  size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
 
   bool HandleNotifyPacket(StringExtractorGDBRemote &packet);
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1487,22 +1487,22 @@
   m_thread_pcs.clear();
 }
 
-size_t
-ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string &value) {
+size_t ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(
+llvm::StringRef value) {
   m_thread_ids.clear();
-  size_t comma_pos;
-  lldb::tid_t tid;
-  while ((comma_pos = value.find(',')) != std::string::npos) {
-value[comma_pos] = '\0';
-// thread in big endian hex
-tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-if (tid != LLDB_INVALID_THREAD_ID)
-  m_thread_ids.push_back(tid);
-value.erase(0, comma_pos + 1);
-  }
-  tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-  if (tid != LLDB_INVALID_THREAD_ID)
-m_thread_ids.push_back(tid);
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
+  StringExtractorGDBRemote thread_ids{value};
+
+  do {
+auto pid_tid = thread_ids.GetPidTid(pid);
+if (pid_tid && pid_tid->first == pid) {
+  lldb::tid_t tid = pid_tid->second;
+  if (tid != LLDB_INVALID_THREAD_ID &&
+  tid != StringExtractorGDBRemote::AllProcesses)
+m_thread_ids.push_back(tid);
+}
+  } while (thread_ids.GetChar() == ',');
+
   return m_thread_ids.size();
 }
 
@@ -1519,7 +1519,7 @@
 value.erase(0, comma_pos + 1);
   }
   pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
-  if (pc != LLDB_INVALID_THREAD_ID)
+  if (pc != LLDB_INVALID_ADDRESS)
 m_thread_pcs.push_back(pc);
   return m_thread_pcs.size();
 }
@@ -2141,6 +2141,7 @@
 }
 
 StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
   stop_packet.SetFilePos(0);
   const char stop_type = stop_packet.GetChar();
   switch (stop_type) {
@@ -2155,14 +2156,12 @@
 if (stop_id == 0) {
   // Our first stop, make sure we have a process ID, and also make sure we
   // know about our registers
-  if (GetID() == LLDB_INVALID_PROCESS_ID) {
-lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
-if (pid != LLDB_INVALID_PROCESS_ID)
-  SetID(pid);
-  }
+  if (GetID() == LLDB_INVALID_PROCESS_ID && pid != LLDB_INVALID_PROCESS_ID)
+SetID(pid);
   BuildDynamicRegisterInfo(true);
 }
 // Stop with signal and thread info
+lldb::pid_t stop_pid = LLDB_INVALID_PROCESS_ID;
 lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
 const uint8_t signo = stop_packet.GetHexU8();
 llvm::StringRef key;
@@ -2191,24 +2190,18 @@
 value.getAsInteger(16, x);
 exc_data.push_back(x);
   } else if (key.compare("thread") == 0) {
-// thread in big endian hex
-if (value.getAsInteger(16, tid))
+// thread-id
+StringExtractorGDBRemote thread_id{value};
+auto pid_tid = thread_id.GetPidTid(pid);
+if (pid_tid) {
+  stop_pid = pid_tid->first;
+  tid = pid_tid->second;
+} else
   tid = LLDB_INVALID_THREAD_ID;
   } e

[Lldb-commits] [PATCH] D99744: [lldb] Update test.rst with a paragraph about pdb

[Dave Lee via Phabricator via lldb-commits]

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

Debugging tests sometimes involves debugging the Python source. This adds a 
paragraph to
the "Debugging Test Failures" section about using `pdb`, and also describes how 
to run
lldb commands from pdb.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99744

Files:
  lldb/docs/resources/test.rst


Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -373,7 +373,20 @@
 ---
 
 On non-Windows platforms, you can use the ``-d`` option to ``dotest.py`` which
-will cause the script to wait for a while until a debugger is attached.
+will cause the script to print out the pid of the test and wait for a while
+until a debugger is attached. Then run ``lldb -p `` to attach.
+
+To instead debug a test's python source, edit the test and insert
+``import pdb; pdb.set_trace()`` at the point you want to start debugging. In
+addition to pdb's debugging facilities, lldb commands can be executed with the
+help of an lldb alias. For example ``lldb bt`` and ``lldb v some_var``. Add
+this line to your ``~/.pdbrc``:
+
+::
+
+alias lldb self.dbg.HandleCommand("%*")
+
+::
 
 Debugging Test Failures on Windows
 ``


Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -373,7 +373,20 @@
 ---
 
 On non-Windows platforms, you can use the ``-d`` option to ``dotest.py`` which
-will cause the script to wait for a while until a debugger is attached.
+will cause the script to print out the pid of the test and wait for a while
+until a debugger is attached. Then run ``lldb -p `` to attach.
+
+To instead debug a test's python source, edit the test and insert
+``import pdb; pdb.set_trace()`` at the point you want to start debugging. In
+addition to pdb's debugging facilities, lldb commands can be executed with the
+help of an lldb alias. For example ``lldb bt`` and ``lldb v some_var``. Add
+this line to your ``~/.pdbrc``:
+
+::
+
+alias lldb self.dbg.HandleCommand("%*")
+
+::
 
 Debugging Test Failures on Windows
 ``
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D99744: [lldb] Update test.rst with a paragraph about pdb

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Thanks, this is super useful. LGTM modulo the inline comment.




Comment at: lldb/docs/resources/test.rst:387
+
+alias lldb self.dbg.HandleCommand("%*")
+

This should be indented to show up as code. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99744

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

[Lldb-commits] [PATCH] D98761: Fix "image lookup --address" Summary results for inline functions.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Anyone else have any issue with this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98761

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

[Lldb-commits] [PATCH] D99744: [lldb] Update test.rst with a paragraph about pdb

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/docs/resources/test.rst:387
+
+alias lldb self.dbg.HandleCommand("%*")
+

JDevlieghere wrote:
> This should be indented to show up as code. 
thanks for catching it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99744

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

[Lldb-commits] [PATCH] D99744: [lldb] Update test.rst with a paragraph about pdb

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 334733.
kastiglione added a comment.

Indent source block


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99744

Files:
  lldb/docs/resources/test.rst


Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -373,7 +373,20 @@
 ---
 
 On non-Windows platforms, you can use the ``-d`` option to ``dotest.py`` which
-will cause the script to wait for a while until a debugger is attached.
+will cause the script to print out the pid of the test and wait for a while
+until a debugger is attached. Then run ``lldb -p `` to attach.
+
+To instead debug a test's python source, edit the test and insert
+``import pdb; pdb.set_trace()`` at the point you want to start debugging. In
+addition to pdb's debugging facilities, lldb commands can be executed with the
+help of an lldb alias. For example ``lldb bt`` and ``lldb v some_var``. Add
+this line to your ``~/.pdbrc``:
+
+::
+
+   alias lldb self.dbg.HandleCommand("%*")
+
+::
 
 Debugging Test Failures on Windows
 ``


Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -373,7 +373,20 @@
 ---
 
 On non-Windows platforms, you can use the ``-d`` option to ``dotest.py`` which
-will cause the script to wait for a while until a debugger is attached.
+will cause the script to print out the pid of the test and wait for a while
+until a debugger is attached. Then run ``lldb -p `` to attach.
+
+To instead debug a test's python source, edit the test and insert
+``import pdb; pdb.set_trace()`` at the point you want to start debugging. In
+addition to pdb's debugging facilities, lldb commands can be executed with the
+help of an lldb alias. For example ``lldb bt`` and ``lldb v some_var``. Add
+this line to your ``~/.pdbrc``:
+
+::
+
+   alias lldb self.dbg.HandleCommand("%*")
+
+::
 
 Debugging Test Failures on Windows
 ``
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D99744: [lldb] Update test.rst with a paragraph about pdb

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 334734.
kastiglione added a comment.

s/lldb alias/pdb alias


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99744

Files:
  lldb/docs/resources/test.rst


Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -373,7 +373,20 @@
 ---
 
 On non-Windows platforms, you can use the ``-d`` option to ``dotest.py`` which
-will cause the script to wait for a while until a debugger is attached.
+will cause the script to print out the pid of the test and wait for a while
+until a debugger is attached. Then run ``lldb -p `` to attach.
+
+To instead debug a test's python source, edit the test and insert
+``import pdb; pdb.set_trace()`` at the point you want to start debugging. In
+addition to pdb's debugging facilities, lldb commands can be executed with the
+help of a pdb alias. For example ``lldb bt`` and ``lldb v some_var``. Add this
+line to your ``~/.pdbrc``:
+
+::
+
+   alias lldb self.dbg.HandleCommand("%*")
+
+::
 
 Debugging Test Failures on Windows
 ``


Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -373,7 +373,20 @@
 ---
 
 On non-Windows platforms, you can use the ``-d`` option to ``dotest.py`` which
-will cause the script to wait for a while until a debugger is attached.
+will cause the script to print out the pid of the test and wait for a while
+until a debugger is attached. Then run ``lldb -p `` to attach.
+
+To instead debug a test's python source, edit the test and insert
+``import pdb; pdb.set_trace()`` at the point you want to start debugging. In
+addition to pdb's debugging facilities, lldb commands can be executed with the
+help of a pdb alias. For example ``lldb bt`` and ``lldb v some_var``. Add this
+line to your ``~/.pdbrc``:
+
+::
+
+   alias lldb self.dbg.HandleCommand("%*")
+
+::
 
 Debugging Test Failures on Windows
 ``
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 0c653d4 - [lldb] Update test.rst with a paragraph about pdb

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2021-04-01T09:53:07-07:00
New Revision: 0c653d4c3d1426267337576ab202bb594144111c

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

LOG: [lldb] Update test.rst with a paragraph about pdb

Debugging tests sometimes involves debugging the Python source. This adds a 
paragraph to
the "Debugging Test Failures" section about using `pdb`, and also describes how 
to run
lldb commands from pdb.

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

Added: 


Modified: 
lldb/docs/resources/test.rst

Removed: 




diff  --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst
index e066f8e209a14..2c08ddde28dc4 100644
--- a/lldb/docs/resources/test.rst
+++ b/lldb/docs/resources/test.rst
@@ -373,7 +373,20 @@ Debugging Test Failures
 ---
 
 On non-Windows platforms, you can use the ``-d`` option to ``dotest.py`` which
-will cause the script to wait for a while until a debugger is attached.
+will cause the script to print out the pid of the test and wait for a while
+until a debugger is attached. Then run ``lldb -p `` to attach.
+
+To instead debug a test's python source, edit the test and insert
+``import pdb; pdb.set_trace()`` at the point you want to start debugging. In
+addition to pdb's debugging facilities, lldb commands can be executed with the
+help of a pdb alias. For example ``lldb bt`` and ``lldb v some_var``. Add this
+line to your ``~/.pdbrc``:
+
+::
+
+   alias lldb self.dbg.HandleCommand("%*")
+
+::
 
 Debugging Test Failures on Windows
 ``



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

[Lldb-commits] [PATCH] D99744: [lldb] Update test.rst with a paragraph about pdb

[Dave Lee via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0c653d4c3d14: [lldb] Update test.rst with a paragraph about 
pdb (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99744

Files:
  lldb/docs/resources/test.rst


Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -373,7 +373,20 @@
 ---
 
 On non-Windows platforms, you can use the ``-d`` option to ``dotest.py`` which
-will cause the script to wait for a while until a debugger is attached.
+will cause the script to print out the pid of the test and wait for a while
+until a debugger is attached. Then run ``lldb -p `` to attach.
+
+To instead debug a test's python source, edit the test and insert
+``import pdb; pdb.set_trace()`` at the point you want to start debugging. In
+addition to pdb's debugging facilities, lldb commands can be executed with the
+help of a pdb alias. For example ``lldb bt`` and ``lldb v some_var``. Add this
+line to your ``~/.pdbrc``:
+
+::
+
+   alias lldb self.dbg.HandleCommand("%*")
+
+::
 
 Debugging Test Failures on Windows
 ``


Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -373,7 +373,20 @@
 ---
 
 On non-Windows platforms, you can use the ``-d`` option to ``dotest.py`` which
-will cause the script to wait for a while until a debugger is attached.
+will cause the script to print out the pid of the test and wait for a while
+until a debugger is attached. Then run ``lldb -p `` to attach.
+
+To instead debug a test's python source, edit the test and insert
+``import pdb; pdb.set_trace()`` at the point you want to start debugging. In
+addition to pdb's debugging facilities, lldb commands can be executed with the
+help of a pdb alias. For example ``lldb bt`` and ``lldb v some_var``. Add this
+line to your ``~/.pdbrc``:
+
+::
+
+   alias lldb self.dbg.HandleCommand("%*")
+
+::
 
 Debugging Test Failures on Windows
 ``
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98761: Fix "image lookup --address" Summary results for inline functions.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D98761#2664375 , @clayborg wrote:

> Anyone else have any issue with this patch?

sorry, I didn't see the accepted part!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98761

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

[Lldb-commits] [PATCH] D99746: [lldb/test] Respect --apple-sdk path when querying SDK info

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added a reviewer: JDevlieghere.
vsk requested review of this revision.
Herald added a project: LLDB.

Respect --apple-sdk  if it's specified. If the SDK simply mounted from
some disk image, and not actually installed, this is the only way to use it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99746

Files:
  lldb/packages/Python/lldbsuite/test/lldbutil.py


Index: lldb/packages/Python/lldbsuite/test/lldbutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -21,6 +21,7 @@
 # LLDB modules
 import lldb
 from . import lldbtest_config
+from . import configuration
 
 # How often failed simulator process launches are retried.
 SIMULATOR_RETRY = 3
@@ -62,6 +63,11 @@
 # 
 
 def get_xcode_sdk(os, env):
+# Respect --apple-sdk  if it's specified. If the SDK simply mounted
+# from some disk image, and not actually installed, this is the only way to
+# use it.
+if configuration.apple_sdk is not None:
+return configuration.apple_sdk
 if os == "ios":
 if env == "simulator":
 return "iphonesimulator"


Index: lldb/packages/Python/lldbsuite/test/lldbutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -21,6 +21,7 @@
 # LLDB modules
 import lldb
 from . import lldbtest_config
+from . import configuration
 
 # How often failed simulator process launches are retried.
 SIMULATOR_RETRY = 3
@@ -62,6 +63,11 @@
 # 
 
 def get_xcode_sdk(os, env):
+# Respect --apple-sdk  if it's specified. If the SDK simply mounted
+# from some disk image, and not actually installed, this is the only way to
+# use it.
+if configuration.apple_sdk is not None:
+return configuration.apple_sdk
 if os == "ios":
 if env == "simulator":
 return "iphonesimulator"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D99746: [lldb/test] Respect --apple-sdk path when querying SDK info

[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




Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:69
+# use it.
+if configuration.apple_sdk is not None:
+return configuration.apple_sdk

This will also exclude the empty string. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99746

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

[Lldb-commits] [PATCH] D99746: [lldb/test] Respect --apple-sdk path when querying SDK info

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:69
+# use it.
+if configuration.apple_sdk is not None:
+return configuration.apple_sdk

JDevlieghere wrote:
> This will also exclude the empty string. 
Thanks, I'll fix this before committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99746

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

[Lldb-commits] [lldb] 7d15fb5 - [lldb/test] Respect --apple-sdk path when querying SDK info

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2021-04-01T10:15:25-07:00
New Revision: 7d15fb5779452c5efbceb82ce0e1f8724a82d290

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

LOG: [lldb/test] Respect --apple-sdk path when querying SDK info

Respect --apple-sdk  if it's specified. If the SDK is simply
mounted from some disk image, and not actually installed, this is the
only way to use it.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbutil.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index d4fd7f4b1f654..06a84c2554000 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -21,6 +21,7 @@
 # LLDB modules
 import lldb
 from . import lldbtest_config
+from . import configuration
 
 # How often failed simulator process launches are retried.
 SIMULATOR_RETRY = 3
@@ -62,6 +63,11 @@ def mkdir_p(path):
 # 
 
 def get_xcode_sdk(os, env):
+# Respect --apple-sdk  if it's specified. If the SDK is simply
+# mounted from some disk image, and not actually installed, this is the
+# only way to use it.
+if configuration.apple_sdk:
+return configuration.apple_sdk
 if os == "ios":
 if env == "simulator":
 return "iphonesimulator"



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

[Lldb-commits] [PATCH] D99746: [lldb/test] Respect --apple-sdk path when querying SDK info

[Vedant Kumar via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d15fb577945: [lldb/test] Respect --apple-sdk path when 
querying SDK info (authored by vsk).

Changed prior to commit:
  https://reviews.llvm.org/D99746?vs=334737&id=334749#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99746

Files:
  lldb/packages/Python/lldbsuite/test/lldbutil.py


Index: lldb/packages/Python/lldbsuite/test/lldbutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -21,6 +21,7 @@
 # LLDB modules
 import lldb
 from . import lldbtest_config
+from . import configuration
 
 # How often failed simulator process launches are retried.
 SIMULATOR_RETRY = 3
@@ -62,6 +63,11 @@
 # 
 
 def get_xcode_sdk(os, env):
+# Respect --apple-sdk  if it's specified. If the SDK is simply
+# mounted from some disk image, and not actually installed, this is the
+# only way to use it.
+if configuration.apple_sdk:
+return configuration.apple_sdk
 if os == "ios":
 if env == "simulator":
 return "iphonesimulator"


Index: lldb/packages/Python/lldbsuite/test/lldbutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -21,6 +21,7 @@
 # LLDB modules
 import lldb
 from . import lldbtest_config
+from . import configuration
 
 # How often failed simulator process launches are retried.
 SIMULATOR_RETRY = 3
@@ -62,6 +63,11 @@
 # 
 
 def get_xcode_sdk(os, env):
+# Respect --apple-sdk  if it's specified. If the SDK is simply
+# mounted from some disk image, and not actually installed, this is the
+# only way to use it.
+if configuration.apple_sdk:
+return configuration.apple_sdk
 if os == "ios":
 if env == "simulator":
 return "iphonesimulator"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98289: [lldb] 2/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:489
 const llvm::Optional &DWARFUnit::GetRnglist() {
+  if (GetVersion() >= 5 && !m_rnglist_table_done) {
+m_rnglist_table_done = true;

jankratochvil wrote:
> `if (GetVersion() < 5) return llvm::None;` is no longer possible with the 
> returned reference.
Do we actually need "m_rnglist_table_done" here? Can't we just check if 
m_rnglist_table has a value since it is an optional?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:490
+  if (GetVersion() >= 5 && !m_rnglist_table_done) {
+m_rnglist_table_done = true;
+if (auto table_or_error =

Should we check m_ranges_base here to make sure it has a value?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:491-494
+if (auto table_or_error =
+ParseListTableHeader(
+m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(),
+m_ranges_base, DWARF32))

Will this function call return an error if "m_ranges_base" doesn't have a value?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:508-513
+  if (!m_ranges_base) {
+GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
+"%8.8x: DW_FORM_rnglistx cannot be used without DW_AT_rnglists_base",
+GetOffset());
+return llvm::None;
+  }

Do we need this check? We should probably be checking "m_ranges_base" in the 
DWARFUnit::GetRnglist() and report the error there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98289

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

[Lldb-commits] [lldb] 4d9039c - Add support for fetching signed values from tagged pointers.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-04-01T10:59:25-07:00
New Revision: 4d9039c8dc2d1f0be1b5ee486d5a83b1614b038a

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

LOG: Add support for fetching signed values from tagged pointers.

The ObjC runtime offers both signed & unsigned tagged pointer value
accessors to tagged pointer providers, but lldb's tagged pointer
code only implemented the unsigned one.  This patch adds an
emulation of the signed one.

The motivation for doing this is that NSNumbers use the signed
accessor (they are always signed) and we need to follow that in our
summary provider or we will get incorrect values for negative
NSNumbers.

The data-formatter-objc test file had NSNumber examples (along with lots of 
other
goodies) but the NSNumber values weren't tested.  So I also added
checks for those values to the test.

I also did a quick audit of the other types in that main.m file, and
it looks like pretty much all the other values are either intermediates
or are tested.

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

Added: 


Modified: 
lldb/source/Plugins/Language/ObjC/Cocoa.cpp

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

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

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

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

Removed: 




diff  --git a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp 
b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
index d871d3470e70b..fe647407800c4 100644
--- a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -351,7 +351,7 @@ static void NSNumber_FormatInt(ValueObject &valobj, Stream 
&stream, int value,
 }
 
 static void NSNumber_FormatLong(ValueObject &valobj, Stream &stream,
-uint64_t value, lldb::LanguageType lang) {
+int64_t value, lldb::LanguageType lang) {
   static ConstString g_TypeHint("NSNumber:long");
 
   std::string prefix, suffix;
@@ -456,9 +456,15 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
 return NSDecimalNumberSummaryProvider(valobj, stream, options);
 
   if (class_name == "NSNumber" || class_name == "__NSCFNumber") {
-uint64_t value = 0;
+int64_t value = 0;
 uint64_t i_bits = 0;
-if (descriptor->GetTaggedPointerInfo(&i_bits, &value)) {
+if (descriptor->GetTaggedPointerInfoSigned(&i_bits, &value)) {
+  // Check for "preserved" numbers.  We still don't support them yet.
+  if (i_bits & 0x8) {
+lldbassert(!static_cast("We should handle preserved numbers!"));
+return false;
+  }
+
   switch (i_bits) {
   case 0:
 NSNumber_FormatChar(valobj, stream, (char)value, 
options.GetLanguage());

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
index 9ef21c6e72081..1bea314f63fca 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -41,6 +41,12 @@ class ClassDescriptorV2 : public 
ObjCLanguageRuntime::ClassDescriptor {
 return false;
   }
 
+  bool GetTaggedPointerInfoSigned(uint64_t *info_bits = nullptr,
+  int64_t *value_bits = nullptr,
+  uint64_t *payload = nullptr) override {
+return false;
+  }
+
   uint64_t GetInstanceSize() override;
 
   ObjCLanguageRuntime::ObjCISA GetISA() override { return m_objc_class_ptr; }
@@ -253,7 +259,7 @@ class ClassDescriptorV2Tagged : public 
ObjCLanguageRuntime::ClassDescriptor {
 
   ClassDescriptorV2Tagged(
   ObjCLanguageRuntime::ClassDescriptorSP actual_class_sp,
-  uint64_t payload) {
+  uint64_t u_payload, int64_t s_payload) {
 if (!actual_class_sp) {
   m_valid = false;
   return;
@@ -264,9 +270,10 @@ class ClassDescriptorV2Tagged : public 
ObjCLanguageRuntime::ClassDescriptor {
   return;
 }
 m_valid = true;
-m_payload = payload;
+m_payload = u_payload;
 m_info_bits = (m_payload & 0x0FULL);
 m_value_bits = (m_payload & ~0x0FULL) >> 4;
+m_value_bits_signed = (s_payload & ~0x0FLL) >> 4;
   }
 
   ~ClassDescriptorV2Tagged() override = default;
@@ -308,6 +315,18 @@ class ClassDescriptorV2Tagged : public 
ObjCLanguageRuntime::ClassDescriptor {
 return true;
   }
 
+  bool GetTaggedPointerInfoSigned(uint64_t

[Lldb-commits] [PATCH] D99694: Add support for getting signed ObjC tagged pointer values

[Jim Ingham via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4d9039c8dc2d: Add support for fetching signed values from 
tagged pointers. (authored by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D99694?vs=334563&id=334770#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99694

Files:
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py
@@ -45,7 +45,6 @@
 '(Point) point = (v=7, h=12)', '(Point *) point_ptr = (v=7, h=12)',
 '(SEL) foo_selector = "foo_selector_impl"'
 ]
-
 self.expect("frame variable", substrs=expect_strings)
 
 if self.getArchitecture() in ['i386', 'x86_64']:
@@ -56,3 +55,28 @@
 '(HIRect) hi_rect = origin=(x = 3, y = 5) size=(width = 4, height = 6)',
 ]
 self.expect("frame variable", substrs=extra_string)
+
+# The original tests left out testing the NSNumber values, so do that here.
+# This set is all pointers, with summaries, so we only check the summary.
+var_list_pointer = [
+['NSNumber *', 'num1','(int)5'],
+['NSNumber *', 'num2','(float)3.14'],
+['NSNumber *', 'num3','(double)3.14'],
+['NSNumber *', 'num4','(int128_t)18446744073709551614'],
+['NSNumber *', 'num5','(char)65'],
+['NSNumber *', 'num6','(long)255'],
+['NSNumber *', 'num7','(long)200'],
+['NSNumber *', 'num8_Y',  'YES'],
+['NSNumber *', 'num8_N',  'NO'],
+['NSNumber *', 'num9','(short)-31616'],
+['NSNumber *', 'num_at1', '(int)12'],
+['NSNumber *', 'num_at2', '(int)-12'],
+['NSNumber *', 'num_at3', '(double)12.5'],
+['NSNumber *', 'num_at4', '(double)-12.5'],
+['NSDecimalNumber *', 'decimal_number', '123456 x 10^-10'],
+['NSDecimalNumber *', 'decimal_number_neg', '-123456 x 10^10']
+]
+for type, var_path, summary in var_list_pointer:
+self.expect_var_path(var_path, summary, None, type)
+
+
Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -87,10 +87,20 @@
 
 virtual bool IsValid() = 0;
 
+/// There are two routines in the ObjC runtime that tagged pointer clients
+/// can call to get the value from their tagged pointer, one that retrieves
+/// it as an unsigned value and one a signed value.  These two
+/// GetTaggedPointerInfo methods mirror those two ObjC runtime calls.
+/// @{
 virtual bool GetTaggedPointerInfo(uint64_t *info_bits = nullptr,
   uint64_t *value_bits = nullptr,
   uint64_t *payload = nullptr) = 0;
 
+virtual bool GetTaggedPointerInfoSigned(uint64_t *info_bits = nullptr,
+int64_t *value_bits = nullptr,
+uint64_t *payload = nullptr) = 0;
+/// @}
+ 
 virtual uint64_t GetInstanceSize() = 0;
 
 // use to implement version-specific additional constraints on pointers
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2457,7 +2457,6 @@
 AppleObjCRuntimeV2::TaggedPointerVendorRuntimeAssisted::GetClassDescriptor(
 lldb::addr_t ptr) {
   ClassDescriptorSP actual_class_descriptor_sp;
-  uint64_t data_payload;
   uint64_t unobfuscated = (ptr) ^ m_runtime.GetTaggedPointerObfuscator();
 
   if (!IsPossibleTaggedPointer(unobfuscated))
@@ -2485,12 +2484,15 @@
 m_cache[slot] = actual_class_descriptor_sp;
 

[Lldb-commits] [PATCH] D99694: Add support for getting signed ObjC tagged pointer values

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:2494
+   m_objc_debug_taggedpointer_payload_rshift);
+  return ClassDescriptorSP(new ClassDescriptorV2Tagged(
+  actual_class_descriptor_sp, data_payload, data_payload_signed));

note we can use `make_shared`.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:2585
 
-  return ClassDescriptorSP(
-  new ClassDescriptorV2Tagged(actual_class_descriptor_sp, data_payload));
+  return ClassDescriptorSP(new ClassDescriptorV2Tagged(
+  actual_class_descriptor_sp, data_payload, data_payload_signed));

We can use `make_shared` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99694

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

[Lldb-commits] [lldb] 18dbe0f - [lldb] Prevent that LLDB randomly crashes in CommandLineParser::addOption by initializing LLVM's command line parser

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-04-01T20:17:54+02:00
New Revision: 18dbe0f954a75f25bd57bba95dfa83cc5e2aa497

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

LOG: [lldb] Prevent that LLDB randomly crashes in CommandLineParser::addOption 
by initializing LLVM's command line parser

Since quite a while Apple's LLDB fork (that contains the Swift debugging
support) is randomly crashing in `CommandLineParser::addOption` with an error
such as `CommandLine Error: Option 'h' registered more than once!`

The backtrace of the crashing thread is shown below. There are also usually many
other threads also performing similar clang::FrontendActions which are all
trying to generate (usually outdated) Clang modules which are used by Swift for
various reasons.

```
[  6] LLDB`CommandLineParser::addOption(llvm::cl::Option*, 
llvm::cl::SubCommand*) + 856
[  7] LLDB`CommandLineParser::addOption(llvm::cl::Option*, 
llvm::cl::SubCommand*) + 733
[  8] LLDB`CommandLineParser::addOption(llvm::cl::Option*, bool) + 184
[  9] LLDB`llvm::cl::ParseCommandLineOptions(...) [inlined] 
::CommandLineParser::ParseCommandLineOptions(... + 1279
[  9] LLDB`llvm::cl::ParseCommandLineOptions(...) + 497
[ 10] LLDB`setCommandLineOpts(clang::CodeGenOptions const&) + 416
[ 11] LLDB`EmitAssemblyHelper::EmitAssemblyWithNewPassManager(...) + 98
[ 12] LLDB`clang::EmitBackendOutput(...) + 4580
[ 13] LLDB`PCHContainerGenerator::HandleTranslationUnit(clang::ASTContext&) + 
871
[ 14] LLDB`clang::MultiplexConsumer::HandleTranslationUnit(clang::ASTContext&) 
+ 43
[ 15] LLDB`clang::ParseAST(clang::Sema&, bool, bool) + 579
[ 16] LLDB`clang::FrontendAction::Execute() + 74
[ 17] LLDB`clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 1808
```

The underlying reason for the crash is that the CommandLine code in LLVM isn't
thread-safe and will never be thread-safe with its current architecture. The way
LLVM's CommandLine logic works is that all parts of the LLVM can provide command
line arguments by defining `cl::opt` global variables and their constructors
(which are invoked during static initialisation) register the variable in LLVM's
CommandLineParser (which is also just a global variable). At some later point
after static initialization we actually try to parse command line arguments and
we ask the CommandLineParser to parse our `argv`.  The CommandLineParser then
lazily constructs it's internal parsing state in a non-thread-safe way (this is
where the crash happens), parses the provided command line and then goes back to
the respective `cl::opt` global variables and sets their values according to the
parse result.

As all of this is based on global state, this whole mechanism isn't thread-safe
so the only time to ever use it is when we know we only have one active thread
dealing with LLVM logic. That's why nearly all callers of
`llvm::cl::ParseCommandLineOptions` are at the top of the `main` function of the
some LLVM-based tool. One of the few exceptions to this rule is in the
`setCommandLineOpts` function in `BackendUtil.cpp` which is in our backtrace:

```
static void setCommandLineOpts(const CodeGenOptions &CodeGenOpts) {
  SmallVector BackendArgs;
  BackendArgs.push_back("clang"); // Fake program name.
  if (!CodeGenOpts.DebugPass.empty()) {
BackendArgs.push_back("-debug-pass");
BackendArgs.push_back(CodeGenOpts.DebugPass.c_str());
  }
  if (!CodeGenOpts.LimitFloatPrecision.empty()) {
BackendArgs.push_back("-limit-float-precision");
BackendArgs.push_back(CodeGenOpts.LimitFloatPrecision.c_str());
  }
  BackendArgs.push_back(nullptr);
  llvm::cl::ParseCommandLineOptions(BackendArgs.size() - 1,
BackendArgs.data());
}
```

This is trying to set `cl::opt` variables in the LLVM backend to their right
value as the passed via CodeGenOptions by invoking the CommandLine parser. As
this is just in some generic Clang CodeGen code (where we allow having multiple
threads) this is code is clearly wrong. If we're unlucky it either overwrites
the value of the global variables or it causes the CommandLine parser to crash.

So the next question is why is this only crashing in LLDB? The main reason seems
to be that easiest way to crash this code is to concurrently enter the initial
CommandLineParser construction where it tries to collect all the registered
`cl::opt` options and checks for sanity:

```
  // If it's a DefaultOption, check to make sure it isn't already there.
  if (O->isDefaultOption() &&
  SC->OptionsMap.find(O->ArgStr) != SC->OptionsMap.end())
return;

  // Add argument to the argument map!
  if (!SC->OptionsMap.insert(std::make_pair(O->ArgStr, O)).second) {
errs() << ProgramName << ": CommandLine Error: Option '" << O->ArgStr
   << "' registered more than once!\n";
HadErrors = true;
  }
```

[Lldb-commits] [PATCH] D99652: [lldb] Prevent that LLDB randomly crashes in CommandLineParser::addOption by initializing LLVM's command line parser

[Raphael Isemann via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG18dbe0f954a7: [lldb] Prevent that LLDB randomly crashes in 
CommandLineParser::addOption by… (authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99652

Files:
  lldb/source/API/SystemInitializerFull.cpp


Index: lldb/source/API/SystemInitializerFull.cpp
===
--- lldb/source/API/SystemInitializerFull.cpp
+++ lldb/source/API/SystemInitializerFull.cpp
@@ -17,6 +17,7 @@
 #include "lldb/Target/ProcessTrace.h"
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/TargetSelect.h"
 
 #pragma clang diagnostic push
@@ -64,6 +65,13 @@
   llvm::InitializeAllAsmPrinters();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllDisassemblers();
+  // Initialize the command line parser in LLVM. This usually isn't necessary
+  // as we aren't dealing with command line options here, but otherwise some
+  // other code in Clang/LLVM might be tempted to call this function from a
+  // different thread later on which won't work (as the function isn't
+  // thread-safe).
+  const char *arg0 = "lldb";
+  llvm::cl::ParseCommandLineOptions(1, &arg0);
 
 #define LLDB_PLUGIN(p) LLDB_PLUGIN_INITIALIZE(p);
 #include "Plugins/Plugins.def"


Index: lldb/source/API/SystemInitializerFull.cpp
===
--- lldb/source/API/SystemInitializerFull.cpp
+++ lldb/source/API/SystemInitializerFull.cpp
@@ -17,6 +17,7 @@
 #include "lldb/Target/ProcessTrace.h"
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/TargetSelect.h"
 
 #pragma clang diagnostic push
@@ -64,6 +65,13 @@
   llvm::InitializeAllAsmPrinters();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllDisassemblers();
+  // Initialize the command line parser in LLVM. This usually isn't necessary
+  // as we aren't dealing with command line options here, but otherwise some
+  // other code in Clang/LLVM might be tempted to call this function from a
+  // different thread later on which won't work (as the function isn't
+  // thread-safe).
+  const char *arg0 = "lldb";
+  llvm::cl::ParseCommandLineOptions(1, &arg0);
 
 #define LLDB_PLUGIN(p) LLDB_PLUGIN_INITIALIZE(p);
 #include "Plugins/Plugins.def"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Reid Kleckner via Phabricator via lldb-commits]

rnk added a comment.

In D99426#2653725 , @MaskRay wrote:

> This touches a lot of files. I am a bit worried that it would not be easy for 
> a contributor to know OF_TextWithCRLF is needed to make SystemZ happy.

Right, we need to separate text-ness for System/Z EBCDIC re-encoding from 
Windows CRLF translation. I think it's best to have a separate, positive CRLF 
bit, and to make that spelling longer than OF_Text. I think, in general, more 
problems are caused by extra unintended CRLF than by missing carriage returns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D99426#2656217 , @rnk wrote:

> In D99426#2653725 , @MaskRay wrote:
>
>> This touches a lot of files. I am a bit worried that it would not be easy 
>> for a contributor to know OF_TextWithCRLF is needed to make SystemZ happy.
>
> Right, we need to separate text-ness for System/Z EBCDIC re-encoding from 
> Windows CRLF translation. I think it's best to have a separate, positive CRLF 
> bit, and to make that spelling longer than OF_Text. I think, in general, more 
> problems are caused by extra unintended CRLF than by missing carriage returns.

OK.

> OF_CRLF which indicates that CRLF translation is used.
> OF_TextWithCRLF = OF_Text | OF_CRLF indicates that the file is text and uses 
> CRLF translation.

My confusion came from I do not know what "CRLF translation" in the description 
refers to. So it seems like EBCDIC->ASCII translation for CRLF? I thought it 
was related to Windows.

The current comment in the source code should be clarified too:

  /// The file should be opened with CRLF translation on platforms that
  /// make this distinction.
  OF_CRLF = 2,
  OF_TextWithCRLF = 3,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan updated this revision to Diff 333960.
abhina.sreeskantharajan edited the summary of this revision.
abhina.sreeskantharajan added a comment.

Rebase + I updated the comments in FileSystem.h to be a bit more descriptive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

Files:
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/modularize/ModuleAssistant.cpp
  clang-tools-extra/pp-trace/PPTrace.cpp
  clang/lib/ARCMigrate/PlistReporter.cpp
  clang/lib/Driver/Compilation.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/DependencyGraph.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/lib/Frontend/ModuleDependencyCollector.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/tools/driver/cc1as_main.cpp
  flang/lib/Frontend/CompilerInstance.cpp
  lld/COFF/DriverUtils.cpp
  lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
  lldb/include/lldb/Utility/ReproducerProvider.h
  lldb/source/Utility/GDBRemote.cpp
  lldb/source/Utility/ReproducerProvider.cpp
  lldb/tools/lldb-server/LLDBServerUtilities.cpp
  llvm/include/llvm/Analysis/DOTGraphTraitsPass.h
  llvm/include/llvm/Support/FileSystem.h
  llvm/lib/CodeGen/RegAllocPBQP.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/ProfileData/GCOV.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/FileCollector.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/TimeProfiler.cpp
  llvm/lib/Support/Timer.cpp
  llvm/lib/Support/Unix/Program.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/lib/Support/Windows/Program.inc
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llc/llc.cpp
  llvm/tools/lli/lli.cpp
  llvm/tools/llvm-cxxmap/llvm-cxxmap.cpp
  llvm/tools/llvm-dis/llvm-dis.cpp
  llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
  llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
  llvm/tools/llvm-link/llvm-link.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp
  llvm/tools/llvm-xray/xray-account.cpp
  llvm/tools/llvm-xray/xray-converter.cpp
  llvm/tools/llvm-xray/xray-extract.cpp
  llvm/tools/llvm-xray/xray-graph-diff.cpp
  llvm/tools/llvm-xray/xray-graph.cpp
  llvm/tools/opt/opt.cpp
  llvm/tools/verify-uselistorder/verify-uselistorder.cpp
  llvm/unittests/Support/Path.cpp
  polly/lib/Exchange/JSONExporter.cpp

Index: polly/lib/Exchange/JSONExporter.cpp
===
--- polly/lib/Exchange/JSONExporter.cpp
+++ polly/lib/Exchange/JSONExporter.cpp
@@ -178,7 +178,7 @@
 
   // Write to file.
   std::error_code EC;
-  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_Text);
+  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_TextWithCRLF);
 
   std::string FunctionName = S.getFunction().getName().str();
   errs() << "Writing JScop '" << S.getNameStr() << "' in function '"
Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1253,7 +1253,7 @@
   path::append(FilePathname, "test");
 
   {
-raw_fd_ostream File(FilePathname, EC, sys::fs::OF_Text);
+raw_fd_ostream File(FilePathname, EC, sys::fs::OF_TextWithCRLF);
 ASSERT_NO_ERROR(EC);
 File << '\n';
   }
Index: llvm/tools/verify-uselistorder/verify-uselistorder.cpp
===
--- llvm/tools/verify-uselistorder/verify-uselistorder.cpp
+++ llvm/tools/verify-uselistorder/verify-uselistorder.cpp
@@ -136,7 +136,7 @@
 bool TempFile::writeAssembly(const Module &M) const {
   LLVM_DEBUG(dbgs() << " - write assembly\n");
   std::error_code EC;
-  raw_fd_ostream OS(Filename, EC, sys::fs::OF_Text);
+  raw_fd_ostream OS(Filename, EC, sys::fs::OF_TextWithCRLF);
   if (EC) {
 errs() << "verify-uselistorder: error: " << EC.message() << "\n";
 return true;
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -700,8 +700,8 @@
   OutputFilename = "-";
 
 std::error_code EC;
-sys::fs::OpenFlags Flags = OutputAssembly ? sys::fs::OF_Text
-  : sys::fs::OF_None;
+sys::fs::OpenFlags Flags 

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan added a comment.

In D99426#2656582 , @MaskRay wrote:

> In D99426#2656217 , @rnk wrote:
>
>> In D99426#2653725 , @MaskRay wrote:
>>
>>> This touches a lot of files. I am a bit worried that it would not be easy 
>>> for a contributor to know OF_TextWithCRLF is needed to make SystemZ happy.
>>
>> Right, we need to separate text-ness for System/Z EBCDIC re-encoding from 
>> Windows CRLF translation. I think it's best to have a separate, positive 
>> CRLF bit, and to make that spelling longer than OF_Text. I think, in 
>> general, more problems are caused by extra unintended CRLF than by missing 
>> carriage returns.
>
> OK.
>
>> OF_CRLF which indicates that CRLF translation is used.
>> OF_TextWithCRLF = OF_Text | OF_CRLF indicates that the file is text and uses 
>> CRLF translation.
>
> My confusion came from I do not know what "CRLF translation" in the 
> description refers to. So it seems like EBCDIC->ASCII translation for CRLF? I 
> thought it was related to Windows.
>
> The current comment in the source code should be clarified too:
>
>   /// The file should be opened with CRLF translation on platforms that
>   /// make this distinction.
>   OF_CRLF = 2,
>   OF_TextWithCRLF = 3,

Sorry for causing confusion. CRLF in text files is a Windows-only problem, it 
doesn't affect z/OS. On z/OS we only need text files to be marked accurately as 
text. I updated the description and comments in FileSystem.h to be more clear.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan updated this revision to Diff 333965.
abhina.sreeskantharajan edited the summary of this revision.
abhina.sreeskantharajan added a comment.

mention OF_CRLF is Windows-only


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

Files:
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/modularize/ModuleAssistant.cpp
  clang-tools-extra/pp-trace/PPTrace.cpp
  clang/lib/ARCMigrate/PlistReporter.cpp
  clang/lib/Driver/Compilation.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/DependencyGraph.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/lib/Frontend/ModuleDependencyCollector.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/tools/driver/cc1as_main.cpp
  flang/lib/Frontend/CompilerInstance.cpp
  lld/COFF/DriverUtils.cpp
  lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
  lldb/include/lldb/Utility/ReproducerProvider.h
  lldb/source/Utility/GDBRemote.cpp
  lldb/source/Utility/ReproducerProvider.cpp
  lldb/tools/lldb-server/LLDBServerUtilities.cpp
  llvm/include/llvm/Analysis/DOTGraphTraitsPass.h
  llvm/include/llvm/Support/FileSystem.h
  llvm/lib/CodeGen/RegAllocPBQP.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/ProfileData/GCOV.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/FileCollector.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/TimeProfiler.cpp
  llvm/lib/Support/Timer.cpp
  llvm/lib/Support/Unix/Program.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/lib/Support/Windows/Program.inc
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llc/llc.cpp
  llvm/tools/lli/lli.cpp
  llvm/tools/llvm-cxxmap/llvm-cxxmap.cpp
  llvm/tools/llvm-dis/llvm-dis.cpp
  llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
  llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
  llvm/tools/llvm-link/llvm-link.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp
  llvm/tools/llvm-xray/xray-account.cpp
  llvm/tools/llvm-xray/xray-converter.cpp
  llvm/tools/llvm-xray/xray-extract.cpp
  llvm/tools/llvm-xray/xray-graph-diff.cpp
  llvm/tools/llvm-xray/xray-graph.cpp
  llvm/tools/opt/opt.cpp
  llvm/tools/verify-uselistorder/verify-uselistorder.cpp
  llvm/unittests/Support/Path.cpp
  polly/lib/Exchange/JSONExporter.cpp

Index: polly/lib/Exchange/JSONExporter.cpp
===
--- polly/lib/Exchange/JSONExporter.cpp
+++ polly/lib/Exchange/JSONExporter.cpp
@@ -178,7 +178,7 @@
 
   // Write to file.
   std::error_code EC;
-  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_Text);
+  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_TextWithCRLF);
 
   std::string FunctionName = S.getFunction().getName().str();
   errs() << "Writing JScop '" << S.getNameStr() << "' in function '"
Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1253,7 +1253,7 @@
   path::append(FilePathname, "test");
 
   {
-raw_fd_ostream File(FilePathname, EC, sys::fs::OF_Text);
+raw_fd_ostream File(FilePathname, EC, sys::fs::OF_TextWithCRLF);
 ASSERT_NO_ERROR(EC);
 File << '\n';
   }
Index: llvm/tools/verify-uselistorder/verify-uselistorder.cpp
===
--- llvm/tools/verify-uselistorder/verify-uselistorder.cpp
+++ llvm/tools/verify-uselistorder/verify-uselistorder.cpp
@@ -136,7 +136,7 @@
 bool TempFile::writeAssembly(const Module &M) const {
   LLVM_DEBUG(dbgs() << " - write assembly\n");
   std::error_code EC;
-  raw_fd_ostream OS(Filename, EC, sys::fs::OF_Text);
+  raw_fd_ostream OS(Filename, EC, sys::fs::OF_TextWithCRLF);
   if (EC) {
 errs() << "verify-uselistorder: error: " << EC.message() << "\n";
 return true;
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -700,8 +700,8 @@
   OutputFilename = "-";
 
 std::error_code EC;
-sys::fs::OpenFlags Flags = OutputAssembly ? sys::fs::OF_Text
-  : sys::fs::OF_None;
+sys::fs::OpenFlags Flags =
+OutputAssembly ? sys::fs::OF_TextWi

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan updated this revision to Diff 334110.
abhina.sreeskantharajan edited the summary of this revision.
abhina.sreeskantharajan added a comment.

Set OF_Text for llvm/tools/dsymutil/DwarfLinkerForBinary.cpp instead of 
OF_TextWithCRLF. This was also another file I recently changed to text from 
binary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

Files:
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/modularize/ModuleAssistant.cpp
  clang-tools-extra/pp-trace/PPTrace.cpp
  clang/lib/ARCMigrate/PlistReporter.cpp
  clang/lib/Driver/Compilation.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/DependencyGraph.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/lib/Frontend/ModuleDependencyCollector.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/tools/driver/cc1as_main.cpp
  flang/lib/Frontend/CompilerInstance.cpp
  lld/COFF/DriverUtils.cpp
  lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
  lldb/include/lldb/Utility/ReproducerProvider.h
  lldb/source/Utility/GDBRemote.cpp
  lldb/source/Utility/ReproducerProvider.cpp
  lldb/tools/lldb-server/LLDBServerUtilities.cpp
  llvm/include/llvm/Analysis/DOTGraphTraitsPass.h
  llvm/include/llvm/Support/FileSystem.h
  llvm/lib/CodeGen/RegAllocPBQP.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/ProfileData/GCOV.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/FileCollector.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/TimeProfiler.cpp
  llvm/lib/Support/Timer.cpp
  llvm/lib/Support/Unix/Program.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/lib/Support/Windows/Program.inc
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llc/llc.cpp
  llvm/tools/lli/lli.cpp
  llvm/tools/llvm-cxxmap/llvm-cxxmap.cpp
  llvm/tools/llvm-dis/llvm-dis.cpp
  llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
  llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
  llvm/tools/llvm-link/llvm-link.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp
  llvm/tools/llvm-xray/xray-account.cpp
  llvm/tools/llvm-xray/xray-converter.cpp
  llvm/tools/llvm-xray/xray-extract.cpp
  llvm/tools/llvm-xray/xray-graph-diff.cpp
  llvm/tools/llvm-xray/xray-graph.cpp
  llvm/tools/opt/opt.cpp
  llvm/tools/verify-uselistorder/verify-uselistorder.cpp
  llvm/unittests/Support/Path.cpp
  polly/lib/Exchange/JSONExporter.cpp

Index: polly/lib/Exchange/JSONExporter.cpp
===
--- polly/lib/Exchange/JSONExporter.cpp
+++ polly/lib/Exchange/JSONExporter.cpp
@@ -178,7 +178,7 @@
 
   // Write to file.
   std::error_code EC;
-  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_Text);
+  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_TextWithCRLF);
 
   std::string FunctionName = S.getFunction().getName().str();
   errs() << "Writing JScop '" << S.getNameStr() << "' in function '"
Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1253,7 +1253,7 @@
   path::append(FilePathname, "test");
 
   {
-raw_fd_ostream File(FilePathname, EC, sys::fs::OF_Text);
+raw_fd_ostream File(FilePathname, EC, sys::fs::OF_TextWithCRLF);
 ASSERT_NO_ERROR(EC);
 File << '\n';
   }
Index: llvm/tools/verify-uselistorder/verify-uselistorder.cpp
===
--- llvm/tools/verify-uselistorder/verify-uselistorder.cpp
+++ llvm/tools/verify-uselistorder/verify-uselistorder.cpp
@@ -136,7 +136,7 @@
 bool TempFile::writeAssembly(const Module &M) const {
   LLVM_DEBUG(dbgs() << " - write assembly\n");
   std::error_code EC;
-  raw_fd_ostream OS(Filename, EC, sys::fs::OF_Text);
+  raw_fd_ostream OS(Filename, EC, sys::fs::OF_TextWithCRLF);
   if (EC) {
 errs() << "verify-uselistorder: error: " << EC.message() << "\n";
 return true;
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -700,8 +700,8 @@
   OutputFilename = "-";
 
 std::error_code EC;
-sys::fs::OpenFlags Flags = OutputAssembly ? sys::fs::OF_Text
-  : sys::fs::OF_Non

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[John Ericson via Phabricator via lldb-commits]

Ericson2314 updated this revision to Diff 334152.
Ericson2314 added a comment.

include(GNUInstallDirs) in each project unconditionally

Before projects that might not be built standalone only included it when they
were, but then we got into a situation where no project included it, and
things broke. There module is designed so that it's inclusion is idempotent,
so we just always include it.

We split the `if` for separate projects however to ensure that any conditional
initialization code after declaring the project that might need these variables
had them. `CMAKE_INSTALL_DOCDIR` depends on the project name so I don't think
it's a good idea to include before the project declaration. That left splitting
the `if`s as the safest option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

Files:
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/lib/Headers/CMakeLists.txt
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/include/CMakeLists.txt
  compiler-rt/lib/dfsan/CMakeLists.txt
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  flang/tools/f18/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  libc/CMakeLists.txt
  libc/lib/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/include/CMakeLists.txt
  libcxx/src/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  lld/tools/lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/tools/intel-features/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddOCaml.cmake
  llvm/cmake/modules/AddSphinxTarget.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake
  llvm/docs/CMake.rst
  llvm/examples/Bye/CMakeLists.txt
  llvm/include/llvm/CMakeLists.txt
  llvm/tools/llvm-config/BuildVariables.inc.in
  llvm/tools/llvm-config/llvm-config.cpp
  llvm/tools/lto/CMakeLists.txt
  llvm/tools/opt-viewer/CMakeLists.txt
  llvm/tools/remarks-shlib/CMakeLists.txt
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  openmp/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/ve/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/multiplex/CMakeLists.txt
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/cmake/polly_macros.cmake
  polly/lib/External/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
 #===--===##
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
 string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -81,15 +83,15 @@
 install(EXPORT ParallelSTLTargets
 FILE ParallelSTLTargets.cmake
 NAMESPACE pstl::
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(FILES "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfig.cmake"
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(DIRECTORY include/
-DESTINATION include
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
 PATTERN "*.in" EXCLUDE)
 install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
 
 add_custom_target(install-pstl
   COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

I like the composability enabled by making `OF_Text` and `OF_CRLF` independent. 
 I wonder, though, if there's a chokepoint where we could/should assert if the 
caller uses `OF_CRLF` without `OF_Text`, which would be suspicious.

I'm not concerned by the number of files touched, since it's mostly a 
mechanical refactor.  If anyone's worried that the mechanical changes obscure 
the behavior change, this _could_ be broken into an NFC refactor patch for the 
renaming followed by a patch that implements the behavioral distinction.  But 
I'm not going to insist on that.




Comment at: llvm/include/llvm/Support/FileSystem.h:746
   /// The file should be opened in text mode on platforms that make this
   /// distinction.
   OF_Text = 1,

Don't be shy to give examples in the comments, as they can illuminate the 
motivation for the design.

```
... on platforms, like SystemZ, that distinguish between text and binary files.
```



Comment at: llvm/include/llvm/Support/FileSystem.h:757
+  /// OF_TextWithCRLF = OF_Text | OF_CRLF
+  OF_TextWithCRLF = 3,
+

Nit:  If it were me, I'd put the `OF_Text | OF_CRLF` directly in the code (in 
place of the `3`) instead of in the comment.



Comment at: llvm/lib/Support/Windows/Path.inc:1086
 
-  if (Flags & OF_Text)
+  if (Flags & OF_CRLF)
 CrtOpenFlags |= _O_TEXT;

I assume it would be weird to set `OF_CRLF` without also setting `OF_Text`, so 
this change seems right for correctness.  But maybe this Windows-only code 
would better express the intent if it were written:

```
if (Flags & (OF_CRLF | OF_Text))
```

Another idea would be to assert if `OF_CRLF` is set but `OF_Text` is not.

Or maybe it's fine as it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: clang/cmake/modules/AddClang.cmake:127
+  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})

For the initial change, Id leave this off.  `CMAKE_INSTALL_LIBDIR` sometimes 
already deals with the bit suffix, so you can end up with two instances of `32` 
or `64`.  I think that cleaning that up separately, while focusing on the 
details of cleaning up how to handle `LLVM_LIBDIR_SUFFIX` is the right thing to 
do.  The same applies to the rest of the patch.



Comment at: compiler-rt/cmake/base-config-ix.cmake:72
+  set(COMPILER_RT_INSTALL_PATH "" CACHE PATH
+"Prefix where built compiler-rt artifacts should be installed, comes 
before CMAKE_INSTALL_PREFIX.")
   option(COMPILER_RT_INCLUDE_TESTS "Generate and build compiler-rt unit 
tests." OFF)

Please don't change the descriptions of the options as part of the 
`GNUInstallDirs` handling.  The change to `COMPILER_RT_INSTALL_PATH` looks 
incorrect.  Can you explain this change please?



Comment at: libcxx/CMakeLists.txt:32
+
+include(GNUInstallDirs)
 

Does this need to come here?  Why not push this to after the if block 
completes?  The same applies through out the rest of the patch.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

@ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can altering the 
value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan added inline comments.



Comment at: llvm/lib/Support/Windows/Path.inc:1086
 
-  if (Flags & OF_Text)
+  if (Flags & OF_CRLF)
 CrtOpenFlags |= _O_TEXT;

amccarth wrote:
> I assume it would be weird to set `OF_CRLF` without also setting `OF_Text`, 
> so this change seems right for correctness.  But maybe this Windows-only code 
> would better express the intent if it were written:
> 
> ```
> if (Flags & (OF_CRLF | OF_Text))
> ```
> 
> Another idea would be to assert if `OF_CRLF` is set but `OF_Text` is not.
> 
> Or maybe it's fine as it is.
I chose to create OF_CRLF flag because I only want Windows to turn on text mode 
for OF_TextWithCRLF and not OF_Text. 
This solution would return true even if Flags was just OF_Text. 
```
if (Flags & (OF_CRLF | OF_Text))
```

I think adding an assertion here would be a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan updated this revision to Diff 334220.
abhina.sreeskantharajan added a comment.

Specified z/OS in FileSystem.h comments, added an assertion in Window's Path.inc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

Files:
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/modularize/ModuleAssistant.cpp
  clang-tools-extra/pp-trace/PPTrace.cpp
  clang/lib/ARCMigrate/PlistReporter.cpp
  clang/lib/Driver/Compilation.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/DependencyGraph.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/lib/Frontend/ModuleDependencyCollector.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/tools/driver/cc1as_main.cpp
  flang/lib/Frontend/CompilerInstance.cpp
  lld/COFF/DriverUtils.cpp
  lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
  lldb/include/lldb/Utility/ReproducerProvider.h
  lldb/source/Utility/GDBRemote.cpp
  lldb/source/Utility/ReproducerProvider.cpp
  lldb/tools/lldb-server/LLDBServerUtilities.cpp
  llvm/include/llvm/Analysis/DOTGraphTraitsPass.h
  llvm/include/llvm/Support/FileSystem.h
  llvm/lib/CodeGen/RegAllocPBQP.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/ProfileData/GCOV.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/FileCollector.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/TimeProfiler.cpp
  llvm/lib/Support/Timer.cpp
  llvm/lib/Support/Unix/Program.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/lib/Support/Windows/Program.inc
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llc/llc.cpp
  llvm/tools/lli/lli.cpp
  llvm/tools/llvm-cxxmap/llvm-cxxmap.cpp
  llvm/tools/llvm-dis/llvm-dis.cpp
  llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
  llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
  llvm/tools/llvm-link/llvm-link.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp
  llvm/tools/llvm-xray/xray-account.cpp
  llvm/tools/llvm-xray/xray-converter.cpp
  llvm/tools/llvm-xray/xray-extract.cpp
  llvm/tools/llvm-xray/xray-graph-diff.cpp
  llvm/tools/llvm-xray/xray-graph.cpp
  llvm/tools/opt/opt.cpp
  llvm/tools/verify-uselistorder/verify-uselistorder.cpp
  llvm/unittests/Support/Path.cpp
  polly/lib/Exchange/JSONExporter.cpp

Index: polly/lib/Exchange/JSONExporter.cpp
===
--- polly/lib/Exchange/JSONExporter.cpp
+++ polly/lib/Exchange/JSONExporter.cpp
@@ -178,7 +178,7 @@
 
   // Write to file.
   std::error_code EC;
-  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_Text);
+  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_TextWithCRLF);
 
   std::string FunctionName = S.getFunction().getName().str();
   errs() << "Writing JScop '" << S.getNameStr() << "' in function '"
Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1253,7 +1253,7 @@
   path::append(FilePathname, "test");
 
   {
-raw_fd_ostream File(FilePathname, EC, sys::fs::OF_Text);
+raw_fd_ostream File(FilePathname, EC, sys::fs::OF_TextWithCRLF);
 ASSERT_NO_ERROR(EC);
 File << '\n';
   }
Index: llvm/tools/verify-uselistorder/verify-uselistorder.cpp
===
--- llvm/tools/verify-uselistorder/verify-uselistorder.cpp
+++ llvm/tools/verify-uselistorder/verify-uselistorder.cpp
@@ -136,7 +136,7 @@
 bool TempFile::writeAssembly(const Module &M) const {
   LLVM_DEBUG(dbgs() << " - write assembly\n");
   std::error_code EC;
-  raw_fd_ostream OS(Filename, EC, sys::fs::OF_Text);
+  raw_fd_ostream OS(Filename, EC, sys::fs::OF_TextWithCRLF);
   if (EC) {
 errs() << "verify-uselistorder: error: " << EC.message() << "\n";
 return true;
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -700,8 +700,8 @@
   OutputFilename = "-";
 
 std::error_code EC;
-sys::fs::OpenFlags Flags = OutputAssembly ? sys::fs::OF_Text
-  : sys::fs::OF_None;
+sys::fs::OpenFlags Flags =
+OutputAssembly ? sys::fs::OF_TextWithCRLF : sys::fs::OF_None;
 Out.reset(new ToolOutputFil

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan marked 2 inline comments as done.
abhina.sreeskantharajan added inline comments.



Comment at: llvm/include/llvm/Support/FileSystem.h:757
+  /// OF_TextWithCRLF = OF_Text | OF_CRLF
+  OF_TextWithCRLF = 3,
+

amccarth wrote:
> Nit:  If it were me, I'd put the `OF_Text | OF_CRLF` directly in the code (in 
> place of the `3`) instead of in the comment.
Thanks, your solution will also prevent future errors if there is any 
renumbering


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[John Ericson via Phabricator via lldb-commits]

Ericson2314 added inline comments.



Comment at: clang/cmake/modules/AddClang.cmake:127
+  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})

compnerd wrote:
> For the initial change, Id leave this off.  `CMAKE_INSTALL_LIBDIR` sometimes 
> already deals with the bit suffix, so you can end up with two instances of 
> `32` or `64`.  I think that cleaning that up separately, while focusing on 
> the details of cleaning up how to handle `LLVM_LIBDIR_SUFFIX` is the right 
> thing to do.  The same applies to the rest of the patch.
https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/GNUInstallDirs.cmake#L257
 Hmm I see what you mean. So you are saying `s/${ 
CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}/${ CMAKE_INSTALL_LIBDIR}/` 
everywhere?



Comment at: compiler-rt/cmake/base-config-ix.cmake:72
+  set(COMPILER_RT_INSTALL_PATH "" CACHE PATH
+"Prefix where built compiler-rt artifacts should be installed, comes 
before CMAKE_INSTALL_PREFIX.")
   option(COMPILER_RT_INCLUDE_TESTS "Generate and build compiler-rt unit 
tests." OFF)

compnerd wrote:
> Please don't change the descriptions of the options as part of the 
> `GNUInstallDirs` handling.  The change to `COMPILER_RT_INSTALL_PATH` looks 
> incorrect.  Can you explain this change please?
I tried explain this https://reviews.llvm.org/D99484#2655885 and the original 
description about prefixes. The GNU install dir variables are allowed to be 
absolute paths (and not necessary with the installation prefix) (and I needed 
that for the NixOS patch), so always prepending `COMPILER_RT_INSTALL_PATH` as 
is done doesn't work if that is `CMAKE_INSTALL_PREFIX` by default.

If you do `git grep '_PREFIX ""'` and also `git grep GRPC_INSTALL_PATH` you 
will see that many similar variables also were already empty by default. I 
agree this thing is a bit ugly, but by that argument I am not doing a new hack 
for `GNUInstallDIrs` but rather applying an existing ideom consistently in all 
packages.



Comment at: libcxx/CMakeLists.txt:32
+
+include(GNUInstallDirs)
 

compnerd wrote:
> Does this need to come here?  Why not push this to after the if block 
> completes?  The same applies through out the rest of the patch.
It might be fine here. But I was worried that in some of these cases code 
included in those blocks might refer to the `GNUInstallDirs` variables.

Originally I had `GNUInstallDirs` only included in the conditional block after 
`project(...)`, but then the combined build test failed because nothing 
including `GnuInstallDirs` in that case. If there is an "entrypoint" CMakeLists 
boilerplate that combined builds should always use, I think the best thing 
would be to change it back and then additionally put `GNUInstallDirs` there.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

compnerd wrote:
> @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can altering the 
> value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
It is sometimes modified to be per target when multiple targets are being used 
at once. All things `CMAKE_INSTALL_*` are globally scoped so in general the 
combination builds are quite awkward.

(Having worked on Meson, I am really missing 
https://mesonbuild.com/Subprojects.html which is exactly what's needed to do 
this without these bespoke idioms that never work well enough . Alas...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: clang/cmake/modules/AddClang.cmake:127
+  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})

Ericson2314 wrote:
> compnerd wrote:
> > For the initial change, Id leave this off.  `CMAKE_INSTALL_LIBDIR` 
> > sometimes already deals with the bit suffix, so you can end up with two 
> > instances of `32` or `64`.  I think that cleaning that up separately, while 
> > focusing on the details of cleaning up how to handle `LLVM_LIBDIR_SUFFIX` 
> > is the right thing to do.  The same applies to the rest of the patch.
> https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/GNUInstallDirs.cmake#L257
>  Hmm I see what you mean. So you are saying `s/${ 
> CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}/${ CMAKE_INSTALL_LIBDIR}/` 
> everywhere?
Yes, that is what I was referring to.  I'm suggesting that you do *not* make 
that change instead.  That needs a much more involved change to clean up the 
use of `${LLVM_LIBDIR_SUFFIX}`.  I think that this change is already extremely 
large as is, and folding more into it is not going to help.



Comment at: compiler-rt/cmake/base-config-ix.cmake:72
+  set(COMPILER_RT_INSTALL_PATH "" CACHE PATH
+"Prefix where built compiler-rt artifacts should be installed, comes 
before CMAKE_INSTALL_PREFIX.")
   option(COMPILER_RT_INCLUDE_TESTS "Generate and build compiler-rt unit 
tests." OFF)

Ericson2314 wrote:
> compnerd wrote:
> > Please don't change the descriptions of the options as part of the 
> > `GNUInstallDirs` handling.  The change to `COMPILER_RT_INSTALL_PATH` looks 
> > incorrect.  Can you explain this change please?
> I tried explain this https://reviews.llvm.org/D99484#2655885 and the original 
> description about prefixes. The GNU install dir variables are allowed to be 
> absolute paths (and not necessary with the installation prefix) (and I needed 
> that for the NixOS patch), so always prepending `COMPILER_RT_INSTALL_PATH` as 
> is done doesn't work if that is `CMAKE_INSTALL_PREFIX` by default.
> 
> If you do `git grep '_PREFIX ""'` and also `git grep GRPC_INSTALL_PATH` you 
> will see that many similar variables also were already empty by default. I 
> agree this thing is a bit ugly, but by that argument I am not doing a new 
> hack for `GNUInstallDIrs` but rather applying an existing ideom consistently 
> in all packages.
Sure, but the *descriptions* of the options are needed to changed for changing 
the value.

That said, I'm not convinced that this change is okay.  It changes the way that 
compiler-rt can be built and used when building a toolchain image.  The 
handling of the install prefix in compiler-rt is significantly different from 
the other parts of LLVM, and so, I think that it may need to be done as a 
separate larger effort.



Comment at: libcxx/CMakeLists.txt:32
+
+include(GNUInstallDirs)
 

Ericson2314 wrote:
> compnerd wrote:
> > Does this need to come here?  Why not push this to after the if block 
> > completes?  The same applies through out the rest of the patch.
> It might be fine here. But I was worried that in some of these cases code 
> included in those blocks might refer to the `GNUInstallDirs` variables.
> 
> Originally I had `GNUInstallDirs` only included in the conditional block 
> after `project(...)`, but then the combined build test failed because nothing 
> including `GnuInstallDirs` in that case. If there is an "entrypoint" 
> CMakeLists boilerplate that combined builds should always use, I think the 
> best thing would be to change it back and then additionally put 
> `GNUInstallDirs` there.
Unified builds always use `llvm/CMakeLists.txt`.  However, both should continue 
to work, and so you will need to add this in the subprojects as well.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

Ericson2314 wrote:
> compnerd wrote:
> > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can altering 
> > the value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
> It is sometimes modified to be per target when multiple targets are being 
> used at once. All things `CMAKE_INSTALL_*` are globally scoped so in general 
> the combination builds are quite awkward.
> 
> (Having worked on Meson, I am really missing 
> https://mesonbuild.com/Subprojects.html which is exactly what's needed to do 
> this without these bespoke idioms that never work well enough . Alas...)
I don't think that bringing up other build systems is par

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[John Ericson via Phabricator via lldb-commits]

Ericson2314 added inline comments.



Comment at: clang/cmake/modules/AddClang.cmake:127
+  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})

compnerd wrote:
> Ericson2314 wrote:
> > compnerd wrote:
> > > For the initial change, Id leave this off.  `CMAKE_INSTALL_LIBDIR` 
> > > sometimes already deals with the bit suffix, so you can end up with two 
> > > instances of `32` or `64`.  I think that cleaning that up separately, 
> > > while focusing on the details of cleaning up how to handle 
> > > `LLVM_LIBDIR_SUFFIX` is the right thing to do.  The same applies to the 
> > > rest of the patch.
> > https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/GNUInstallDirs.cmake#L257
> >  Hmm I see what you mean. So you are saying `s/${ 
> > CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}/${ CMAKE_INSTALL_LIBDIR}/` 
> > everywhere?
> Yes, that is what I was referring to.  I'm suggesting that you do *not* make 
> that change instead.  That needs a much more involved change to clean up the 
> use of `${LLVM_LIBDIR_SUFFIX}`.  I think that this change is already 
> extremely large as is, and folding more into it is not going to help.
So you are saying II should back all of these out into 
`lib${LLVM_LIBDIR_SUFFIX}` as they were before, for now?

Yes I don't want to make this bigger either, and would rather be on the hook 
for follow-up work than have this one be too massive to get over the finish 
line.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
 get_compiler_rt_target(${arch} target)
-set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+set(${install_dir} 
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
   else()

ldionne wrote:
> lebedev.ri wrote:
> > This looks suspect
> Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` like 
> elsewhere.
See the non-line comment I responded to @lebidev.ri with. In sort if

```
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
```

is a relative path, then we end up with

```
${CMAKE_INSTALL_PREFIX}/${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
```

instead of 

```
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${target}
```

as we do with the other per-package prefixes. Also if `CMAKE_INSTALL_LIBDIR` is 
already an absolute path, then
```
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target}
```
is the same thing, and closer to the second than the first.



Comment at: compiler-rt/cmake/base-config-ix.cmake:72
+  set(COMPILER_RT_INSTALL_PATH "" CACHE PATH
+"Prefix where built compiler-rt artifacts should be installed, comes 
before CMAKE_INSTALL_PREFIX.")
   option(COMPILER_RT_INCLUDE_TESTS "Generate and build compiler-rt unit 
tests." OFF)

compnerd wrote:
> Ericson2314 wrote:
> > compnerd wrote:
> > > Please don't change the descriptions of the options as part of the 
> > > `GNUInstallDirs` handling.  The change to `COMPILER_RT_INSTALL_PATH` 
> > > looks incorrect.  Can you explain this change please?
> > I tried explain this https://reviews.llvm.org/D99484#2655885 and the 
> > original description about prefixes. The GNU install dir variables are 
> > allowed to be absolute paths (and not necessary with the installation 
> > prefix) (and I needed that for the NixOS patch), so always prepending 
> > `COMPILER_RT_INSTALL_PATH` as is done doesn't work if that is 
> > `CMAKE_INSTALL_PREFIX` by default.
> > 
> > If you do `git grep '_PREFIX ""'` and also `git grep GRPC_INSTALL_PATH` you 
> > will see that many similar variables also were already empty by default. I 
> > agree this thing is a bit ugly, but by that argument I am not doing a new 
> > hack for `GNUInstallDIrs` but rather applying an existing ideom 
> > consistently in all packages.
> Sure, but the *descriptions* of the options are needed to changed for 
> changing the value.
> 
> That said, I'm not convinced that this change is okay.  It changes the way 
> that compiler-rt can be built and used when building a toolchain image.  The 
> handling of the install prefix in compiler-rt is significantly different from 
> the other parts of LLVM, and so, I think that it may need to be done as a 
> separate larger effort.
So just skip everything compile-rt related for now?



Comment at: libcxx/CMakeLists.txt:32
+
+include(GNUInstallDirs)
 

compnerd wrote:
> Ericson2314 wrote:
> > compnerd wrote:
> > > Does this need to come here?  Why not push this to after the if block 
> > > completes?  The same applies through out the rest of the patch.
> > It might be fine here. But I was worried that in some of these cases code 
> > included in those blocks might refer to the `G

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Louis Dionne via Phabricator via lldb-commits]

ldionne added inline comments.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

Ericson2314 wrote:
> ldionne wrote:
> > compnerd wrote:
> > > Ericson2314 wrote:
> > > > compnerd wrote:
> > > > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can 
> > > > > altering the value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
> > > > It is sometimes modified to be per target when multiple targets are 
> > > > being used at once. All things `CMAKE_INSTALL_*` are globally scoped so 
> > > > in general the combination builds are quite awkward.
> > > > 
> > > > (Having worked on Meson, I am really missing 
> > > > https://mesonbuild.com/Subprojects.html which is exactly what's needed 
> > > > to do this without these bespoke idioms that never work well enough . 
> > > > Alas...)
> > > I don't think that bringing up other build systems is particularly 
> > > helpful.
> > > 
> > > I do expect it to be modified, and I suspect that this is used 
> > > specifically for builds that @ldionne supports.
> > Actually, I've never used it myself, but @phosek seems to be using it for 
> > the Runtimes build to output one set of headers for each target, as 
> > mentioned above.
> > 
> > It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when driving 
> > the libc++ build from the runtimes build would be more in-line with the 
> > CMake way of doing things (one configuration == one build), but I'm curious 
> > to hear what @phosek has to say about that.
> > It seems to me that tweaking CMAKE_INSTALL_PREFIX globally when driving the 
> > libc++ build from the runtimes build would be more in-line with the CMake 
> > way of doing things (one configuration == one buid)
> 
> You mean trying to mutate it during the libc++ CMake eval and then set it 
> back after? That would keep the intended meaning of  `CMAKE_INSTALL_PREFIX` 
> being the actual prefix, but that feels awfully fragile to me. Or do you mean 
> something else?
I keep forgetting that the runtimes build uses `add_subdirectory` to include 
each sub-project instead of driving a proper CMake build for each of those.

Basically, what I'm saying is that whatever place we decide to build for N 
multiple targets, we should perform N different CMake builds setting the 
appropriate `CMAKE_INSTALL_PREFIX`, one for each target. But that discussion 
should happen elsewhere, not on this review.

As far as this review is concerned, I do believe you want instead:

```
${CMAKE_INSTALL_INCLUDEDIR}${LIBCXX_INSTALL_HEADER_PREFIX}
```

(reversed the order of variables)

We should have @phosek confirm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a subscriber: phosek.
ldionne added a comment.

I am generally OK with the libcxx and libcxxabi changes.




Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
 get_compiler_rt_target(${arch} target)
-set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+set(${install_dir} 
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
   else()

lebedev.ri wrote:
> This looks suspect
Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` like 
elsewhere.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

compnerd wrote:
> Ericson2314 wrote:
> > compnerd wrote:
> > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can altering 
> > > the value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
> > It is sometimes modified to be per target when multiple targets are being 
> > used at once. All things `CMAKE_INSTALL_*` are globally scoped so in 
> > general the combination builds are quite awkward.
> > 
> > (Having worked on Meson, I am really missing 
> > https://mesonbuild.com/Subprojects.html which is exactly what's needed to 
> > do this without these bespoke idioms that never work well enough . Alas...)
> I don't think that bringing up other build systems is particularly helpful.
> 
> I do expect it to be modified, and I suspect that this is used specifically 
> for builds that @ldionne supports.
Actually, I've never used it myself, but @phosek seems to be using it for the 
Runtimes build to output one set of headers for each target, as mentioned above.

It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when driving the 
libc++ build from the runtimes build would be more in-line with the CMake way 
of doing things (one configuration == one build), but I'm curious to hear what 
@phosek has to say about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Petr Hosek via Phabricator via lldb-commits]

phosek added inline comments.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

ldionne wrote:
> Ericson2314 wrote:
> > ldionne wrote:
> > > compnerd wrote:
> > > > Ericson2314 wrote:
> > > > > compnerd wrote:
> > > > > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can 
> > > > > > altering the value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
> > > > > It is sometimes modified to be per target when multiple targets are 
> > > > > being used at once. All things `CMAKE_INSTALL_*` are globally scoped 
> > > > > so in general the combination builds are quite awkward.
> > > > > 
> > > > > (Having worked on Meson, I am really missing 
> > > > > https://mesonbuild.com/Subprojects.html which is exactly what's 
> > > > > needed to do this without these bespoke idioms that never work well 
> > > > > enough . Alas...)
> > > > I don't think that bringing up other build systems is particularly 
> > > > helpful.
> > > > 
> > > > I do expect it to be modified, and I suspect that this is used 
> > > > specifically for builds that @ldionne supports.
> > > Actually, I've never used it myself, but @phosek seems to be using it for 
> > > the Runtimes build to output one set of headers for each target, as 
> > > mentioned above.
> > > 
> > > It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when driving 
> > > the libc++ build from the runtimes build would be more in-line with the 
> > > CMake way of doing things (one configuration == one build), but I'm 
> > > curious to hear what @phosek has to say about that.
> > > It seems to me that tweaking CMAKE_INSTALL_PREFIX globally when driving 
> > > the libc++ build from the runtimes build would be more in-line with the 
> > > CMake way of doing things (one configuration == one buid)
> > 
> > You mean trying to mutate it during the libc++ CMake eval and then set it 
> > back after? That would keep the intended meaning of  `CMAKE_INSTALL_PREFIX` 
> > being the actual prefix, but that feels awfully fragile to me. Or do you 
> > mean something else?
> I keep forgetting that the runtimes build uses `add_subdirectory` to include 
> each sub-project instead of driving a proper CMake build for each of those.
> 
> Basically, what I'm saying is that whatever place we decide to build for N 
> multiple targets, we should perform N different CMake builds setting the 
> appropriate `CMAKE_INSTALL_PREFIX`, one for each target. But that discussion 
> should happen elsewhere, not on this review.
> 
> As far as this review is concerned, I do believe you want instead:
> 
> ```
> ${CMAKE_INSTALL_INCLUDEDIR}${LIBCXX_INSTALL_HEADER_PREFIX}
> ```
> 
> (reversed the order of variables)
> 
> We should have @phosek confirm.
@ldionne `CMAKE_INSTALL_PREFIX` cannot be used for this purpose. When using the 
multiarch layout with the runtimes build, we want to place build artifacts in 
`${BUILD_DIR}/{include,lib}//...` and install them to 
`${CMAKE_INSTALL_PREFIX}/{include,lib}//...`. There are two issues:
1. `CMAKE_INSTALL_PREFIX` only comes into play during install step, but we want 
the build layout to match the installation layout so we need a different 
variable to control the output directory.
2. We already use `CMAKE_INSTALL_PREFIX` for the toolchain itself and there's 
no way to use it hierarchically, that is you cannot do something like 
`${SUPER_CMAKE_INSTALL_PREFIX}/{include,lib}/${CMAKE_INSTALL_PREFIX}/...` which 
is why we need a separate variable to control the installation directory.

Regarding `LIBCXX_INSTALL_HEADER_PREFIX`, I haven't found any current uses of 
that variable. I introduced it in D59168 which was an earlier attempt at having 
per-target headers, but D89013 is strictly better for the reasons I described 
in https://reviews.llvm.org/D89013#2662578 (no duplicate headers) so I think we 
can just remove this variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Petr Hosek via Phabricator via lldb-commits]

phosek added inline comments.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

phosek wrote:
> ldionne wrote:
> > Ericson2314 wrote:
> > > ldionne wrote:
> > > > compnerd wrote:
> > > > > Ericson2314 wrote:
> > > > > > compnerd wrote:
> > > > > > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can 
> > > > > > > altering the value of `CMAKE_INSTALL_INCLUDEDIR` serve the 
> > > > > > > purpose?
> > > > > > It is sometimes modified to be per target when multiple targets are 
> > > > > > being used at once. All things `CMAKE_INSTALL_*` are globally 
> > > > > > scoped so in general the combination builds are quite awkward.
> > > > > > 
> > > > > > (Having worked on Meson, I am really missing 
> > > > > > https://mesonbuild.com/Subprojects.html which is exactly what's 
> > > > > > needed to do this without these bespoke idioms that never work well 
> > > > > > enough . Alas...)
> > > > > I don't think that bringing up other build systems is particularly 
> > > > > helpful.
> > > > > 
> > > > > I do expect it to be modified, and I suspect that this is used 
> > > > > specifically for builds that @ldionne supports.
> > > > Actually, I've never used it myself, but @phosek seems to be using it 
> > > > for the Runtimes build to output one set of headers for each target, as 
> > > > mentioned above.
> > > > 
> > > > It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when 
> > > > driving the libc++ build from the runtimes build would be more in-line 
> > > > with the CMake way of doing things (one configuration == one build), 
> > > > but I'm curious to hear what @phosek has to say about that.
> > > > It seems to me that tweaking CMAKE_INSTALL_PREFIX globally when driving 
> > > > the libc++ build from the runtimes build would be more in-line with the 
> > > > CMake way of doing things (one configuration == one buid)
> > > 
> > > You mean trying to mutate it during the libc++ CMake eval and then set it 
> > > back after? That would keep the intended meaning of  
> > > `CMAKE_INSTALL_PREFIX` being the actual prefix, but that feels awfully 
> > > fragile to me. Or do you mean something else?
> > I keep forgetting that the runtimes build uses `add_subdirectory` to 
> > include each sub-project instead of driving a proper CMake build for each 
> > of those.
> > 
> > Basically, what I'm saying is that whatever place we decide to build for N 
> > multiple targets, we should perform N different CMake builds setting the 
> > appropriate `CMAKE_INSTALL_PREFIX`, one for each target. But that 
> > discussion should happen elsewhere, not on this review.
> > 
> > As far as this review is concerned, I do believe you want instead:
> > 
> > ```
> > ${CMAKE_INSTALL_INCLUDEDIR}${LIBCXX_INSTALL_HEADER_PREFIX}
> > ```
> > 
> > (reversed the order of variables)
> > 
> > We should have @phosek confirm.
> @ldionne `CMAKE_INSTALL_PREFIX` cannot be used for this purpose. When using 
> the multiarch layout with the runtimes build, we want to place build 
> artifacts in `${BUILD_DIR}/{include,lib}//...` and install them to 
> `${CMAKE_INSTALL_PREFIX}/{include,lib}//...`. There are two issues:
> 1. `CMAKE_INSTALL_PREFIX` only comes into play during install step, but we 
> want the build layout to match the installation layout so we need a different 
> variable to control the output directory.
> 2. We already use `CMAKE_INSTALL_PREFIX` for the toolchain itself and there's 
> no way to use it hierarchically, that is you cannot do something like 
> `${SUPER_CMAKE_INSTALL_PREFIX}/{include,lib}/${CMAKE_INSTALL_PREFIX}/...` 
> which is why we need a separate variable to control the installation 
> directory.
> 
> Regarding `LIBCXX_INSTALL_HEADER_PREFIX`, I haven't found any current uses of 
> that variable. I introduced it in D59168 which was an earlier attempt at 
> having per-target headers, but D89013 is strictly better for the reasons I 
> described in https://reviews.llvm.org/D89013#2662578 (no duplicate headers) 
> so I think we can just remove this variable.
I have sent D99697 for review which removes these variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[John Ericson via Phabricator via lldb-commits]

Ericson2314 marked an inline comment as not done.
Ericson2314 added inline comments.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
 get_compiler_rt_target(${arch} target)
-set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+set(${install_dir} 
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
   else()

phosek wrote:
> Ericson2314 wrote:
> > ldionne wrote:
> > > lebedev.ri wrote:
> > > > This looks suspect
> > > Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` like 
> > > elsewhere.
> > See the non-line comment I responded to @lebidev.ri with. In sort if
> > 
> > ```
> > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> > ```
> > 
> > is a relative path, then we end up with
> > 
> > ```
> > ${CMAKE_INSTALL_PREFIX}/${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> > ```
> > 
> > instead of 
> > 
> > ```
> > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${target}
> > ```
> > 
> > as we do with the other per-package prefixes. Also if 
> > `CMAKE_INSTALL_LIBDIR` is already an absolute path, then
> > ```
> > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target}
> > ```
> > is the same thing, and closer to the second than the first.
> I'm not sure if that's desirable. I'm not sure if we still need 
> `COMPILER_RT_INSTALL_PATH`. That variable is only used by 
> `clang/runtime/CMakeLists.txt` which predates `runtimes/CMakeLists.txt` and 
> was AFAIK only ever used by Apple. I think we should consider removing 
> `COMPILER_RT_INSTALL_PATH`. We'll need to check if 
> `clang/runtime/CMakeLists.txt` is still being used or not.
With D99697 now all approved (thanks again!) it looks like this is the next 
step? Would be very nice if this could be removed!



Comment at: polly/cmake/CMakeLists.txt:82-96
+set(POLLY_INSTALL_PREFIX "")
 set(POLLY_CONFIG_LLVM_CMAKE_DIR 
"${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_CMAKE_DIR 
"${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_LIBRARY_DIRS 
"${POLLY_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+set(POLLY_CONFIG_CMAKE_DIR 
"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
+set(POLLY_CONFIG_LIBRARY_DIRS 
"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_FULL_LIBDIR}${LLVM_LIBDIR_SUFFIX}")
 if (POLLY_BUNDLED_ISL)
   set(POLLY_CONFIG_INCLUDE_DIRS
+"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_FULL_LIBDIR}"

`POLLY_INSTALL_PREFIX`, like `COMPILER_RT_INSTALL_PATH`, I changed to be empty 
by default. If the `COMPILER_RT_INSTALL_PATH` can be removed, maybe 
`POLLY_INSTALL_PREFIX` can also be removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[John Ericson via Phabricator via lldb-commits]

Ericson2314 updated this revision to Diff 334576.
Ericson2314 added a comment.

Put on top of D99697 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

Files:
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/lib/Headers/CMakeLists.txt
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/include/CMakeLists.txt
  compiler-rt/lib/dfsan/CMakeLists.txt
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  flang/tools/f18/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  libc/CMakeLists.txt
  libc/lib/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/include/CMakeLists.txt
  libcxx/src/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  lld/tools/lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/tools/intel-features/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddOCaml.cmake
  llvm/cmake/modules/AddSphinxTarget.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake
  llvm/docs/CMake.rst
  llvm/examples/Bye/CMakeLists.txt
  llvm/include/llvm/CMakeLists.txt
  llvm/tools/llvm-config/BuildVariables.inc.in
  llvm/tools/llvm-config/llvm-config.cpp
  llvm/tools/lto/CMakeLists.txt
  llvm/tools/opt-viewer/CMakeLists.txt
  llvm/tools/remarks-shlib/CMakeLists.txt
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  openmp/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/ve/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/multiplex/CMakeLists.txt
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/cmake/polly_macros.cmake
  polly/lib/External/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
 #===--===##
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
 string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -81,15 +83,15 @@
 install(EXPORT ParallelSTLTargets
 FILE ParallelSTLTargets.cmake
 NAMESPACE pstl::
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(FILES "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfig.cmake"
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(DIRECTORY include/
-DESTINATION include
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
 PATTERN "*.in" EXCLUDE)
 install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
 
 add_custom_target(install-pstl
   COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -275,7 +275,7 @@
 install(DIRECTORY
   ${ISL_SOURCE_DIR}/include/
   ${ISL_BINARY_DIR}/include/
-  DESTINATION include/polly
+  DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/polly
   FILES_MATCHING
   PATTERN "*.h"
   PATTERN "CMakeFiles" EXCLUDE
Index: polly/cmake/polly_macros.cmake
===
--- polly/cmake/polly_macros.cmake
+++ polly/cmake/polly_macros.cmake
@@ -44,8 +44,8 @@

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[John Ericson via Phabricator via lldb-commits]

Ericson2314 updated this revision to Diff 334713.
Ericson2314 added a comment.

Rebase on newer version of previous diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

Files:
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/lib/Headers/CMakeLists.txt
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/include/CMakeLists.txt
  compiler-rt/lib/dfsan/CMakeLists.txt
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  flang/tools/f18/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  libc/CMakeLists.txt
  libc/lib/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/include/CMakeLists.txt
  libcxx/src/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  lld/tools/lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/tools/intel-features/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddOCaml.cmake
  llvm/cmake/modules/AddSphinxTarget.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake
  llvm/docs/CMake.rst
  llvm/examples/Bye/CMakeLists.txt
  llvm/include/llvm/CMakeLists.txt
  llvm/tools/llvm-config/BuildVariables.inc.in
  llvm/tools/llvm-config/llvm-config.cpp
  llvm/tools/lto/CMakeLists.txt
  llvm/tools/opt-viewer/CMakeLists.txt
  llvm/tools/remarks-shlib/CMakeLists.txt
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  openmp/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/ve/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/multiplex/CMakeLists.txt
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/cmake/polly_macros.cmake
  polly/lib/External/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
 #===--===##
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
 string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -81,15 +83,15 @@
 install(EXPORT ParallelSTLTargets
 FILE ParallelSTLTargets.cmake
 NAMESPACE pstl::
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(FILES "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfig.cmake"
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(DIRECTORY include/
-DESTINATION include
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
 PATTERN "*.in" EXCLUDE)
 install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
 
 add_custom_target(install-pstl
   COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -275,7 +275,7 @@
 install(DIRECTORY
   ${ISL_SOURCE_DIR}/include/
   ${ISL_BINARY_DIR}/include/
-  DESTINATION include/polly
+  DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/polly
   FILES_MATCHING
   PATTERN "*.h"
   PATTERN "CMakeFiles" EXCLUDE
Index: polly/cmake/polly_macros.cmake
===
--- polly/cmake/polly_macros.cmake
+++ polly/cmake/polly_macros.cmake
@@ -44,8 +44,8 @@
   if (NOT LLV

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Petr Hosek via Phabricator via lldb-commits]

phosek added inline comments.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
 get_compiler_rt_target(${arch} target)
-set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+set(${install_dir} 
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
   else()

Ericson2314 wrote:
> ldionne wrote:
> > lebedev.ri wrote:
> > > This looks suspect
> > Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` like 
> > elsewhere.
> See the non-line comment I responded to @lebidev.ri with. In sort if
> 
> ```
> ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> ```
> 
> is a relative path, then we end up with
> 
> ```
> ${CMAKE_INSTALL_PREFIX}/${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> ```
> 
> instead of 
> 
> ```
> ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${target}
> ```
> 
> as we do with the other per-package prefixes. Also if `CMAKE_INSTALL_LIBDIR` 
> is already an absolute path, then
> ```
> ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target}
> ```
> is the same thing, and closer to the second than the first.
I'm not sure if that's desirable. I'm not sure if we still need 
`COMPILER_RT_INSTALL_PATH`. That variable is only used by 
`clang/runtime/CMakeLists.txt` which predates `runtimes/CMakeLists.txt` and was 
AFAIK only ever used by Apple. I think we should consider removing 
`COMPILER_RT_INSTALL_PATH`. We'll need to check if 
`clang/runtime/CMakeLists.txt` is still being used or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[John Ericson via Phabricator via lldb-commits]

Ericson2314 added inline comments.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
 get_compiler_rt_target(${arch} target)
-set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+set(${install_dir} 
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
   else()

Ericson2314 wrote:
> phosek wrote:
> > Ericson2314 wrote:
> > > ldionne wrote:
> > > > lebedev.ri wrote:
> > > > > This looks suspect
> > > > Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` 
> > > > like elsewhere.
> > > See the non-line comment I responded to @lebidev.ri with. In sort if
> > > 
> > > ```
> > > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> > > ```
> > > 
> > > is a relative path, then we end up with
> > > 
> > > ```
> > > ${CMAKE_INSTALL_PREFIX}/${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> > > ```
> > > 
> > > instead of 
> > > 
> > > ```
> > > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${target}
> > > ```
> > > 
> > > as we do with the other per-package prefixes. Also if 
> > > `CMAKE_INSTALL_LIBDIR` is already an absolute path, then
> > > ```
> > > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target}
> > > ```
> > > is the same thing, and closer to the second than the first.
> > I'm not sure if that's desirable. I'm not sure if we still need 
> > `COMPILER_RT_INSTALL_PATH`. That variable is only used by 
> > `clang/runtime/CMakeLists.txt` which predates `runtimes/CMakeLists.txt` and 
> > was AFAIK only ever used by Apple. I think we should consider removing 
> > `COMPILER_RT_INSTALL_PATH`. We'll need to check if 
> > `clang/runtime/CMakeLists.txt` is still being used or not.
> With D99697 now all approved (thanks again!) it looks like this is the next 
> step? Would be very nice if this could be removed!
I went ahead and opened https://reviews.llvm.org/D99755 with @phosek's 
tentative proposal; I figured it would be easier for others to discuss in a 
dedicated patch than there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[John Ericson via Phabricator via lldb-commits]

Ericson2314 updated this revision to Diff 334765.
Ericson2314 added a comment.

Rebase now that previous diff landed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

Files:
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/lib/Headers/CMakeLists.txt
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/include/CMakeLists.txt
  compiler-rt/lib/dfsan/CMakeLists.txt
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  flang/tools/f18/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  libc/CMakeLists.txt
  libc/lib/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/include/CMakeLists.txt
  libcxx/src/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  lld/tools/lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/tools/intel-features/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddOCaml.cmake
  llvm/cmake/modules/AddSphinxTarget.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake
  llvm/docs/CMake.rst
  llvm/examples/Bye/CMakeLists.txt
  llvm/include/llvm/CMakeLists.txt
  llvm/tools/llvm-config/BuildVariables.inc.in
  llvm/tools/llvm-config/llvm-config.cpp
  llvm/tools/lto/CMakeLists.txt
  llvm/tools/opt-viewer/CMakeLists.txt
  llvm/tools/remarks-shlib/CMakeLists.txt
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  openmp/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/ve/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/multiplex/CMakeLists.txt
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/cmake/polly_macros.cmake
  polly/lib/External/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
 #===--===##
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
 string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -81,15 +83,15 @@
 install(EXPORT ParallelSTLTargets
 FILE ParallelSTLTargets.cmake
 NAMESPACE pstl::
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(FILES "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfig.cmake"
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(DIRECTORY include/
-DESTINATION include
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
 PATTERN "*.in" EXCLUDE)
 install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
 
 add_custom_target(install-pstl
   COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -275,7 +275,7 @@
 install(DIRECTORY
   ${ISL_SOURCE_DIR}/include/
   ${ISL_BINARY_DIR}/include/
-  DESTINATION include/polly
+  DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/polly
   FILES_MATCHING
   PATTERN "*.h"
   PATTERN "CMakeFiles" EXCLUDE
Index: polly/cmake/polly_macros.cmake
===
--- polly/cmake/polly_macros.cmake
+++ polly/cmake/polly_macros.cmake
@@ -44,8 +44,8 @@
   if (NOT LLVM_IN

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: clang/cmake/modules/AddClang.cmake:127
+  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})

Ericson2314 wrote:
> compnerd wrote:
> > Ericson2314 wrote:
> > > compnerd wrote:
> > > > For the initial change, Id leave this off.  `CMAKE_INSTALL_LIBDIR` 
> > > > sometimes already deals with the bit suffix, so you can end up with two 
> > > > instances of `32` or `64`.  I think that cleaning that up separately, 
> > > > while focusing on the details of cleaning up how to handle 
> > > > `LLVM_LIBDIR_SUFFIX` is the right thing to do.  The same applies to the 
> > > > rest of the patch.
> > > https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/GNUInstallDirs.cmake#L257
> > >  Hmm I see what you mean. So you are saying `s/${ 
> > > CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}/${ CMAKE_INSTALL_LIBDIR}/` 
> > > everywhere?
> > Yes, that is what I was referring to.  I'm suggesting that you do *not* 
> > make that change instead.  That needs a much more involved change to clean 
> > up the use of `${LLVM_LIBDIR_SUFFIX}`.  I think that this change is already 
> > extremely large as is, and folding more into it is not going to help.
> So you are saying II should back all of these out into 
> `lib${LLVM_LIBDIR_SUFFIX}` as they were before, for now?
> 
> Yes I don't want to make this bigger either, and would rather be on the hook 
> for follow-up work than have this one be too massive to get over the finish 
> line.
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [lldb] 2d73392 - Fix "image lookup --address" Summary results for inline functions.

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2021-04-01T11:36:26-07:00
New Revision: 2d733923b8d32230d5af777dd1d84ae2c7fc968d

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

LOG: Fix "image lookup --address" Summary results for inline functions.

Inline callstacks were being incorrectly displayed in the results of "image 
lookup --address". The deepest frame wasn't displaying the line table line 
entry, it was always showing the inline information's call file and line on the 
previous frame. This is now fixed and has tests to make sure it doesn't regress.

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

Added: 
lldb/test/Shell/Commands/command-image-lookup.yaml

Modified: 
lldb/include/lldb/Symbol/SymbolContext.h
lldb/source/Symbol/SymbolContext.cpp
lldb/source/Target/Trace.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/SymbolContext.h 
b/lldb/include/lldb/Symbol/SymbolContext.h
index c513dbb447f8a..1cf26a6959b7f 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -150,8 +150,8 @@ class SymbolContext {
   bool DumpStopContext(Stream *s, ExecutionContextScope *exe_scope,
const Address &so_addr, bool show_fullpaths,
bool show_module, bool show_inlined_frames,
-   bool show_function_arguments, bool show_function_name,
-   bool show_inline_callsite_line_info = true) const;
+   bool show_function_arguments,
+   bool show_function_name) const;
 
   /// Get the address range contained within a symbol context.
   ///

diff  --git a/lldb/source/Symbol/SymbolContext.cpp 
b/lldb/source/Symbol/SymbolContext.cpp
index 8cfea35180fa9..d20d6aa5a5d40 100644
--- a/lldb/source/Symbol/SymbolContext.cpp
+++ b/lldb/source/Symbol/SymbolContext.cpp
@@ -71,8 +71,7 @@ bool SymbolContext::DumpStopContext(Stream *s, 
ExecutionContextScope *exe_scope,
 const Address &addr, bool show_fullpaths,
 bool show_module, bool show_inlined_frames,
 bool show_function_arguments,
-bool show_function_name,
-bool show_inline_callsite_line_info) const 
{
+bool show_function_name) const {
   bool dumped_something = false;
   if (show_module && module_sp) {
 if (show_fullpaths)
@@ -128,13 +127,14 @@ bool SymbolContext::DumpStopContext(Stream *s, 
ExecutionContextScope *exe_scope,
   s->Printf(" + %" PRIu64, inlined_function_offset);
 }
   }
-  if (show_inline_callsite_line_info) {
-const Declaration &call_site = inlined_block_info->GetCallSite();
-if (call_site.IsValid()) {
-  s->PutCString(" at ");
-  call_site.DumpStopContext(s, show_fullpaths);
-}
-  } else if (line_entry.IsValid()) {
+  // "line_entry" will always be valid as GetParentOfInlinedScope(...) will
+  // fill it in correctly with the calling file and line. Previous code
+  // was extracting the calling file and line from inlined_block_info and
+  // using it right away which is not correct. On the first call to this
+  // function "line_entry" will contain the actual line table entry. On
+  // susequent calls "line_entry" will contain the calling file and line
+  // from the previous inline info.
+  if (line_entry.IsValid()) {
 s->PutCString(" at ");
 line_entry.DumpStopContext(s, show_fullpaths);
   }

diff  --git a/lldb/source/Target/Trace.cpp b/lldb/source/Target/Trace.cpp
index e862d17e73c51..c2de3dac48b28 100644
--- a/lldb/source/Target/Trace.cpp
+++ b/lldb/source/Target/Trace.cpp
@@ -141,8 +141,7 @@ static SymbolContext DumpSymbolContext(Stream &s, const 
SymbolContext &prev_sc,
 sc.DumpStopContext(&s, &target, address, /*show_fullpath*/ false,
/*show_module*/ true, /*show_inlined_frames*/ false,
/*show_function_arguments*/ true,
-   /*show_function_name*/ true,
-   /*show_inline_callsite_line_info*/ false);
+   /*show_function_name*/ true);
   s.Printf("\n");
   return sc;
 }

diff  --git a/lldb/test/Shell/Commands/command-image-lookup.yaml 
b/lldb/test/Shell/Commands/command-image-lookup.yaml
new file mode 100644
index 0..cf7be930517ee
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-image-lookup.yaml
@@ -0,0 +1,810 @@
+# RUN: yaml2obj %s -o %T/a.out
+# RUN: %lldb %T/a.out -o "image lookup --verbose --address 0x00013fa1" 
-o exit | FileCheck %s --check-prefix=NOINLINE
+# RUN: %lldb %T/a.out -o "i

[Lldb-commits] [PATCH] D98761: Fix "image lookup --address" Summary results for inline functions.

[Greg Clayton via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2d733923b8d3: Fix "image lookup --address" Summary 
results for inline functions. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98761

Files:
  lldb/include/lldb/Symbol/SymbolContext.h
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Target/Trace.cpp
  lldb/test/Shell/Commands/command-image-lookup.yaml

Index: lldb/test/Shell/Commands/command-image-lookup.yaml
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-image-lookup.yaml
@@ -0,0 +1,810 @@
+# RUN: yaml2obj %s -o %T/a.out
+# RUN: %lldb %T/a.out -o "image lookup --verbose --address 0x00013fa1" -o exit | FileCheck %s --check-prefix=NOINLINE
+# RUN: %lldb %T/a.out -o "image lookup --verbose --address 0x00013fa2" -o exit | FileCheck %s --check-prefix=INLINE_1
+# RUN: %lldb %T/a.out -o "image lookup --verbose --address 0x00013fa8" -o exit | FileCheck %s --check-prefix=INLINE_2
+
+#  NOINLINE: Summary: a.out`main + 33 at main.cpp:10
+# NOINLINE-NEXT: Module: file =
+
+#  INLINE_1: Summary: a.out`main + 34 [inlined] squares(int, int) at main.cpp:7:16
+# INLINE_1-NEXT:  a.out`main + 34 at main.cpp:11
+# INLINE_1-NEXT: Module: file =
+
+#  INLINE_2: Summary: a.out`main + 40 [inlined] square(int) at main.cpp:3:9
+# INLINE_2-NEXT: a.out`main + 40 [inlined] squares(int, int) + 6 at main.cpp:7
+# INLINE_2-NEXT: a.out`main + 34 at main.cpp:11
+# INLINE_2-NEXT: Module: file =
+
+
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x107
+  cpusubtype:  0x3
+  filetype:0xA
+  ncmds:   7
+  sizeofcmds:  1400
+  flags:   0x0
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_UUID
+cmdsize: 24
+uuid:E476BFB9-CC5C-34BC-B968-BF996B298060
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:1
+minos:   659200
+sdk: 721152
+ntools:  0
+  - cmd: LC_SYMTAB
+cmdsize: 24
+symoff:  4096
+nsyms:   4
+stroff:  4160
+strsize: 54
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  16384
+fileoff: 0
+filesize:0
+maxprot: 5
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x13F50
+size:100
+offset:  0x0
+align:   4
+reloff:  0x0
+nreloc:  0
+flags:   0x8400
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE070103000A00070078051B001800E476BFB9CC5C34BCB968BF996B29806032001800010F0A010B000200181004004010
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x13FB4
+size:72
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE070103000A00070078051B001800E476BFB9CC5C34BCB968BF996B29806032001800010F0A00
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __LINKEDIT
+vmaddr:  4294983680
+vmsize:  4096
+fileoff: 4096
+filesize:118
+maxprot: 1
+initprot:1
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 952
+segname: __DWARF
+vmaddr:  4294987776
+vmsize:  4096
+fileoff: 8192
+filesize:1530
+maxprot: 7
+initprot:3
+nsects:  11
+flags:   0
+Sections:
+  - sectname:__debug_line
+segname: __DWARF
+addr:0x15000
+size:130
+offset:  0x2000
+align:   0
+relo

[Lldb-commits] [PATCH] D99702: [lldb-vscode] Use LLVM's ScopeExit to ensure we always terminate the debugger

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

@wallace - minor inconvenience, but approving a patch in Phabricator without 
any textual comment does not result in an email to the list, making it look 
like a patch is being committed without approval - could you include some text 
in your Phab approvals, as it makes it a bit easier when doing 
post-commit/process review?

@JDevlieghere Is it worth having a C++ RAII object that calls initialize in the 
ctor and terminate in the dtor? (maybe even removing the init/terminate free 
functions, and making them only accessible through an RAII object if that's 
suitable) What do the other uses of init/terminate look like/could they benefit 
from such refactoring?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99702

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

[Lldb-commits] [PATCH] D99571: Update ProcessMachCore::DoLoadCore to handle binary hints with and without addresses

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Thanks for the feedback Shafik, updated the patch and landing in a sec.




Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:214
+  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+module_sp.reset(new Module(module_spec));
+  }

shafik wrote:
> We can use `make_shared` here instead.
Sounds good.



Comment at: 
lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py:142
+main_addr = main_sym.GetStartAddress()
+self.assertGreater(main_addr.GetLoadAddress(self.target), 
0x700)
+self.assertNotEqual(main_addr.GetLoadAddress(self.target), 
lldb.LLDB_INVALID_ADDRESS)

shafik wrote:
> `0x700` feels like it should be a variable with a name since we are 
> using in multiple places.
Good suggestion, done.



Comment at: 
lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp:90
+char buf[24];
+sprintf(buf, "0x%llx", address);
+ident += buf;

shafik wrote:
> `snprintf` we should also collect the return value and assert on the result.
Thanks, I switched to using a PRIx64 formatter to make it explicit how large 
the formatter's output will be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99571

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

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan added a comment.

@MaskRay is there still any confusion about the problem this patch is trying to 
solve and concerns about the renaming?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

  /// The file should be opened in text mode on platforms like z/OS that make
  /// this distinction.
  OF_Text = 1,
  F_Text = 1, // For compatibility
  
  /// The file should use a carriage linefeed '\r\n'.
  /// Only makes a difference on windows.
  OF_CRLF = 2,
  
  /// The file should be opened in text mode and use a carriage linefeed '\r\n'
  /// on platforms that make this distinction.
  OF_TextWithCRLF = OF_Text | OF_CRLF,

`OF_TextWithCRLF` needs to say what platforms make a difference.

Can you mention in the description for Windows and z/OS, how these flags make a 
difference.
It's still a bit unclear to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Michael Kruse via Phabricator via lldb-commits]

Meinersbur added inline comments.



Comment at: polly/cmake/CMakeLists.txt:82-96
+set(POLLY_INSTALL_PREFIX "")
 set(POLLY_CONFIG_LLVM_CMAKE_DIR 
"${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_CMAKE_DIR 
"${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_LIBRARY_DIRS 
"${POLLY_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+set(POLLY_CONFIG_CMAKE_DIR 
"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
+set(POLLY_CONFIG_LIBRARY_DIRS 
"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_FULL_LIBDIR}${LLVM_LIBDIR_SUFFIX}")
 if (POLLY_BUNDLED_ISL)
   set(POLLY_CONFIG_INCLUDE_DIRS
+"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_FULL_LIBDIR}"

Ericson2314 wrote:
> `POLLY_INSTALL_PREFIX`, like `COMPILER_RT_INSTALL_PATH`, I changed to be 
> empty by default. If the `COMPILER_RT_INSTALL_PATH` can be removed, maybe 
> `POLLY_INSTALL_PREFIX` can also be removed?
I don't have an overview atm on Polly's different paths, but could look into it 
if needed. Originally, this was derived from how Clang did it. If it works for 
COMPILER_RT_INSTALL_PATH, it should work for Polly as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan updated this revision to Diff 334792.
abhina.sreeskantharajan added a comment.

Update comment in FileSystem.h for OF_TextWithCRLF


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

Files:
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/modularize/ModuleAssistant.cpp
  clang-tools-extra/pp-trace/PPTrace.cpp
  clang/lib/ARCMigrate/PlistReporter.cpp
  clang/lib/Driver/Compilation.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/DependencyGraph.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/lib/Frontend/ModuleDependencyCollector.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/tools/driver/cc1as_main.cpp
  flang/lib/Frontend/CompilerInstance.cpp
  lld/COFF/DriverUtils.cpp
  lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
  lldb/include/lldb/Utility/ReproducerProvider.h
  lldb/source/Utility/GDBRemote.cpp
  lldb/source/Utility/ReproducerProvider.cpp
  lldb/tools/lldb-server/LLDBServerUtilities.cpp
  llvm/include/llvm/Analysis/DOTGraphTraitsPass.h
  llvm/include/llvm/Support/FileSystem.h
  llvm/lib/CodeGen/RegAllocPBQP.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/ProfileData/GCOV.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/FileCollector.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/TimeProfiler.cpp
  llvm/lib/Support/Timer.cpp
  llvm/lib/Support/Unix/Program.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/lib/Support/Windows/Program.inc
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llc/llc.cpp
  llvm/tools/lli/lli.cpp
  llvm/tools/llvm-cxxmap/llvm-cxxmap.cpp
  llvm/tools/llvm-dis/llvm-dis.cpp
  llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
  llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
  llvm/tools/llvm-link/llvm-link.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp
  llvm/tools/llvm-xray/xray-account.cpp
  llvm/tools/llvm-xray/xray-converter.cpp
  llvm/tools/llvm-xray/xray-extract.cpp
  llvm/tools/llvm-xray/xray-graph-diff.cpp
  llvm/tools/llvm-xray/xray-graph.cpp
  llvm/tools/opt/opt.cpp
  llvm/tools/verify-uselistorder/verify-uselistorder.cpp
  llvm/unittests/Support/Path.cpp
  polly/lib/Exchange/JSONExporter.cpp

Index: polly/lib/Exchange/JSONExporter.cpp
===
--- polly/lib/Exchange/JSONExporter.cpp
+++ polly/lib/Exchange/JSONExporter.cpp
@@ -178,7 +178,7 @@
 
   // Write to file.
   std::error_code EC;
-  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_Text);
+  ToolOutputFile F(FileName, EC, llvm::sys::fs::OF_TextWithCRLF);
 
   std::string FunctionName = S.getFunction().getName().str();
   errs() << "Writing JScop '" << S.getNameStr() << "' in function '"
Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1253,7 +1253,7 @@
   path::append(FilePathname, "test");
 
   {
-raw_fd_ostream File(FilePathname, EC, sys::fs::OF_Text);
+raw_fd_ostream File(FilePathname, EC, sys::fs::OF_TextWithCRLF);
 ASSERT_NO_ERROR(EC);
 File << '\n';
   }
Index: llvm/tools/verify-uselistorder/verify-uselistorder.cpp
===
--- llvm/tools/verify-uselistorder/verify-uselistorder.cpp
+++ llvm/tools/verify-uselistorder/verify-uselistorder.cpp
@@ -136,7 +136,7 @@
 bool TempFile::writeAssembly(const Module &M) const {
   LLVM_DEBUG(dbgs() << " - write assembly\n");
   std::error_code EC;
-  raw_fd_ostream OS(Filename, EC, sys::fs::OF_Text);
+  raw_fd_ostream OS(Filename, EC, sys::fs::OF_TextWithCRLF);
   if (EC) {
 errs() << "verify-uselistorder: error: " << EC.message() << "\n";
 return true;
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -700,8 +700,8 @@
   OutputFilename = "-";
 
 std::error_code EC;
-sys::fs::OpenFlags Flags = OutputAssembly ? sys::fs::OF_Text
-  : sys::fs::OF_None;
+sys::fs::OpenFlags Flags =
+OutputAssembly ? sys::fs::OF_TextWithCRLF : sys::fs::OF_None;
 Out.reset(new ToolOutputFile(OutputFilename, EC, Flags));

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

[Abhina Sree via Phabricator via lldb-commits]

abhina.sreeskantharajan added a comment.

In D99426#2664841 , @MaskRay wrote:

>   /// The file should be opened in text mode on platforms like z/OS that make
>   /// this distinction.
>   OF_Text = 1,
>   F_Text = 1, // For compatibility
>   
>   /// The file should use a carriage linefeed '\r\n'.
>   /// Only makes a difference on windows.
>   OF_CRLF = 2,
>   
>   /// The file should be opened in text mode and use a carriage linefeed 
> '\r\n'
>   /// on platforms that make this distinction.
>   OF_TextWithCRLF = OF_Text | OF_CRLF,
>
> `OF_TextWithCRLF` needs to say what platforms make a difference.
>
> Can you mention in the description for Windows and z/OS, how these flags make 
> a difference, and how developers should use these flags for portability?
> It's still a bit unclear to me.
>
> e.g. when I need to use `OF_CRLR` instead of `OF_Text`?

So developers should use either the OF_Text or OF_TextWithCRLF for text files 
and OF_None for binary files. If the developer doesn't want carriage returns on 
Windows, they should use OF_Text, if they do want carriage returns on Windows, 
they should use OF_TextWithCRLF.

So this is the behaviour per platform with my patch:

z/OS:
OF_None: open in binary mode
OF_Text : open in text mode
OF_TextWithCRLF: open in text mode

Windows:
OF_None: open file with no carriage return
OF_Text: open file with no carriage return
OF_TextWithCRLF: open file with carriage return

This is the old behaviour for comparison:
z/OS:
OF_None: open in binary mode
OF_Text: open in text mode

Windows:
OF_None: open file with no carriage return
OF_Text: open file with carriage return


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99426

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

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: lldb/test/Shell/Subprocess/clone-follow-parent-wp.test:1
+# REQUIRES: native && (system-linux || system-netbsd) && (target-x86 || 
target-x86_64 || target-aarch64) && dbregs-set
+# RUN: %clang_host -g %p/Inputs/clone.c -o %t

labath wrote:
> Why only these three arches?
Right, I was supposed to replace this with something better later. It's 
supposed to limit the test to arches where we support watchpoints.


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

https://reviews.llvm.org/D98822

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

[Lldb-commits] [PATCH] D98289: [lldb] 2/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s:11
+# Failure was the block range 1..2 was not printed plus:
+# error: DW_AT_range-DW_FORM_sec_offset.s.tmp {0x003f}: DIE has 
DW_AT_ranges(0xc) attribute, but range extraction failed (missing or invalid 
range list table), please file a bug and attach the file at the start of this 
error message
+

I'm not sure if `%t` is guaranteed to produce a file name with that 
format/name. So maybe using `{{.*}}` for the file name (and `{{.*}}-rnglistx` 
below) would be a bit more stable



Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s:32
+# RNGLISTBASE-LABEL: image lookup -v -s lookup_rnglists
+# RNGLISTBASE: error: DW_AT_range-DW_FORM_sec_offset.s.tmp-rnglistbase 
{0x0043}: DIE has DW_AT_ranges(0x0) attribute, but range extraction failed 
(invalid range list table index 0), please file a bug and attach the file at 
the start of this error message
+

Perhaps this could be more informative about what makes the range list index of 
0 invalid? "index 0 out of range of range list table (with range list base 
0xXXX) with offset entry count of XX (valid indexes 0-(XX-1))" Maybe that's too 
verbose/not worth worrying about since this'll only be relevant to DWARF 
producers trying to debug their DWARFv5, maybe no one will ever see this 
message in practice. Just a thought.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98289

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

[Lldb-commits] [lldb] 78a1412 - Handle all standalone combinations of LC_NOTEs w/ & w/o addr & uuid

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2021-04-01T18:59:36-07:00
New Revision: 78a1412845b5552465505889b69b5b11d9b20d35

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

LOG: Handle all standalone combinations of LC_NOTEs w/ & w/o addr & uuid

Fill out ProcessMachCore::DoLoadCore to handle LC_NOTE hints with
a UUID or with a UUID+address, and load the binary at the specified
offset correctly.  Add tests for all four combinations.  Change
DynamicLoaderStatic to not re-set a Section's load address in the
Target if it's already been specified.

Differential Revision: https://reviews.llvm.org/D99571
rdar://51490545

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp

Removed: 




diff  --git a/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp 
b/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
index c1fceeb1482c8..8a5528f1e474e 100644
--- a/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
+++ b/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
@@ -10,6 +10,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 
 #include "DynamicLoaderStatic.h"
@@ -105,6 +106,15 @@ void DynamicLoaderStatic::LoadAllImagesAtFileAddresses() {
 // the file...
 SectionSP section_sp(section_list->GetSectionAtIndex(sect_idx));
 if (section_sp) {
+  // If this section already has a load address set in the target,
+  // don't re-set it to the file address.  Something may have
+  // set it to a more correct value already.
+  if (m_process->GetTarget()
+  .GetSectionLoadList()
+  .GetSectionLoadAddress(section_sp) !=
+  LLDB_INVALID_ADDRESS) {
+continue;
+  }
   if (m_process->GetTarget().SetSectionLoadAddress(
   section_sp, section_sp->GetFileAddress()))
 changed = true;

diff  --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp 
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 82a946f932069..44bdf0a6aba90 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -21,6 +21,7 @@
 #include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/DataBuffer.h"
@@ -36,6 +37,7 @@
 
 #include "Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.h"
 #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h"
+#include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
 
 #include 
@@ -188,6 +190,59 @@ bool ProcessMachCore::GetDynamicLoaderAddress(lldb::addr_t 
addr) {
   return false;
 }
 
+// We have a hint about a binary -- a UUID, possibly a load address.
+// Try to load a file with that UUID into lldb, and if we have a load
+// address, set it correctly.  Else assume that the binary was loaded
+// with no slide.
+static bool load_standalone_binary(UUID uuid, addr_t addr, Target &target) {
+  if (uuid.IsValid()) {
+ModuleSpec module_spec;
+module_spec.GetUUID() = uuid;
+
+// Look up UUID in global module cache before attempting
+// dsymForUUID-like action.
+ModuleSP module_sp;
+Status error = ModuleList::GetSharedModule(module_spec, module_sp, nullptr,
+   nullptr, nullptr);
+
+if (!module_sp.get()) {
+  // Force a a dsymForUUID lookup, if that tool is available.
+  if (!module_spec.GetSymbolFileSpec())
+Symbols::DownloadObjectAndSymbolFile(module_spec, true);
+
+  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+module_sp = std::make_shared(module_spec);
+  }
+}
+
+if (module_sp.get() && module_sp->GetObjectFile()) {
+  target.SetArchitecture(module_sp->GetObjectFile()->GetArchitecture());
+  target.GetImages().AppendIfNeeded(module_sp, false);
+
+  Address base_addr = module_sp->GetObjectFile()->GetBaseAddress();
+  addr_t slide = 0;
+  if (addr != LLDB_INVALID_ADDRESS && base_addr.IsValid()) {
+addr_t file_load_addr = base_addr.GetFileAddress();

[Lldb-commits] [PATCH] D99571: Update ProcessMachCore::DoLoadCore to handle binary hints with and without addresses

[Jason Molenda via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG78a1412845b5: Handle all standalone combinations of LC_NOTEs 
w/ & w/o addr & uuid (authored by jasonmolenda).

Changed prior to commit:
  https://reviews.llvm.org/D99571?vs=334090&id=334881#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99571

Files:
  lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
  lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp

Index: lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp
===
--- lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp
+++ lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp
@@ -1,9 +1,11 @@
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -79,9 +81,17 @@
 
 void add_lc_note_kern_ver_str_load_command(
 std::vector> &loadcmds, std::vector &payload,
-int payload_file_offset, std::string uuid) {
+int payload_file_offset, std::string uuid, uint64_t address) {
   std::string ident = "EFI UUID=";
   ident += uuid;
+
+  if (address != 0x) {
+ident += "; stext=";
+char buf[24];
+sprintf(buf, "0x%" PRIx64, address);
+ident += buf;
+  }
+
   std::vector loadcmd_data;
 
   add_uint32(loadcmd_data, LC_NOTE); // note_command.cmd
@@ -111,7 +121,7 @@
 
 void add_lc_note_main_bin_spec_load_command(
 std::vector> &loadcmds, std::vector &payload,
-int payload_file_offset, std::string uuidstr) {
+int payload_file_offset, std::string uuidstr, uint64_t address) {
   std::vector loadcmd_data;
 
   add_uint32(loadcmd_data, LC_NOTE); // note_command.cmd
@@ -136,8 +146,8 @@
 
   // Now write the "main bin spec" payload.
   add_uint32(payload, 1);  // version
-  add_uint32(payload, 3);  // type == 3 [ firmware, standalone,e tc ]
-  add_uint64(payload, UINT64_MAX); // load address unknown/unspecified
+  add_uint32(payload, 3);  // type == 3 [ firmware, standalone, etc ]
+  add_uint64(payload, address);// load address
   uuid_t uuid;
   uuid_parse(uuidstr.c_str(), uuid);
   for (int i = 0; i < sizeof(uuid_t); i++)
@@ -259,9 +269,12 @@
 }
 
 int main(int argc, char **argv) {
-  if (argc != 4) {
-fprintf(stderr, "usage: create-empty-corefile version-string|main-bin-spec "
-" \n");
+  if (argc != 5) {
+fprintf(stderr,
+"usage: create-empty-corefile version-string|main-bin-spec "
+"  \n");
+fprintf(stderr,
+"  is base 16, 0x means unknown\n");
 fprintf(
 stderr,
 "Create a Mach-O corefile with an either LC_NOTE 'kern ver str' or \n");
@@ -284,13 +297,22 @@
   // An array of corefile contents (page data, lc_note data, etc)
   std::vector payload;
 
+  errno = 0;
+  uint64_t address = strtoull(argv[4], NULL, 16);
+  if (errno != 0) {
+fprintf(stderr, "Unable to parse address %s as base 16", argv[4]);
+exit(1);
+  }
+
   // First add all the load commands / payload so we can figure out how large
   // the load commands will actually be.
   load_commands.push_back(x86_lc_thread_load_command());
   if (strcmp(argv[1], "version-string") == 0)
-add_lc_note_kern_ver_str_load_command(load_commands, payload, 0, uuid);
+add_lc_note_kern_ver_str_load_command(load_commands, payload, 0, uuid,
+  address);
   else
-add_lc_note_main_bin_spec_load_command(load_commands, payload, 0, uuid);
+add_lc_note_main_bin_spec_load_command(load_commands, payload, 0, uuid,
+   address);
   add_lc_segment(load_commands, payload, 0);
 
   int size_of_load_commands = 0;
@@ -308,11 +330,11 @@
   load_commands.push_back(x86_lc_thread_load_command());
 
   if (strcmp(argv[1], "version-string") == 0)
-add_lc_note_kern_ver_str_load_command(load_commands, payload,
-  header_and_load_cmd_room, uuid);
+add_lc_note_kern_ver_str_load_command(
+load_commands, payload, header_and_load_cmd_room, uuid, address);
   else
-add_lc_note_main_bin_spec_load_command(load_commands, payload,
-   header_and_load_cmd_room, uuid);
+add_lc_note_main_bin_spec_load_command(
+load_commands, payload, header_and_load_cmd_room, uuid, address);
 
   add_lc_segment(load_commands, payload, header_and_load_cmd_room);
 
Index: lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwa

[Lldb-commits] [lldb] cf51bf7 - [lldb] Account for objc_debug_class_getNameRaw returning NULL

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-04-01T19:58:28-07:00
New Revision: cf51bf77b070f63391bacada5e166bfec2616989

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

LOG: [lldb] Account for objc_debug_class_getNameRaw returning NULL

On macOS Catalina, calling objc_debug_class_getNameRaw on some of the
ISA pointers returns NULL, causing us to crash and unwind before reading
all the Objective-C classes. This does not happen on macOS Big Sur.
Account for that possibility and skip the class when that happens.

Added: 


Modified: 

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

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index 6b65ace397baa..debdfeaf909a7 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -200,6 +200,7 @@ __lldb_apple_objc_v2_get_dynamic_class_info2(void 
*gdb_objc_realized_classes_ptr
 
 uint32_t count = 0;
 Class* realized_class_list = objc_copyRealizedClassList(&count);
+DEBUG_PRINTF ("count = %u\n", count);
 
 uint32_t idx = 0;
 for (uint32_t i=0; i<=count; ++i)
@@ -208,6 +209,8 @@ __lldb_apple_objc_v2_get_dynamic_class_info2(void 
*gdb_objc_realized_classes_ptr
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
+if (name_ptr == NULL)
+continue;
 const char *s = name_ptr;
 uint32_t h = 5381;
 for (unsigned char c = *s; c; c = *++s)



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