[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 421093.
wallace added a comment.

nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123281

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp
  lldb/source/Plugins/Trace/common/TraceSessionSaver.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -215,3 +215,65 @@
   RefreshLiveProcessState();
   return m_stop_id;
 }
+
+llvm::Expected
+Trace::GetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind) {
+  auto NotFoundError = [&]() {
+return createStringError(
+inconvertibleErrorCode(),
+formatv("The thread with tid={0} doesn't have the tracing data {1}",
+tid, kind));
+  };
+
+  auto it = m_postmortem_thread_data.find(tid);
+  if (it == m_postmortem_thread_data.end())
+return NotFoundError();
+
+  std::unordered_map _kind_to_file_spec_map =
+  it->second;
+  auto it2 = data_kind_to_file_spec_map.find(kind.str());
+  if (it2 == data_kind_to_file_spec_map.end())
+return NotFoundError();
+  return it2->second;
+}
+
+void Trace::SetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind,
+FileSpec file_spec) {
+  m_postmortem_thread_data[tid][kind.str()] = file_spec;
+}
+
+llvm::Error Trace::OnLiveThreadBinaryDataRead(
+lldb::tid_t tid, llvm::StringRef kind,
+std::function data)> callback) {
+  Expected> data = GetLiveThreadBinaryData(tid, kind);
+  if (!data)
+return data.takeError();
+  return callback(*data);
+}
+
+llvm::Error Trace::OnPostMortemThreadBinaryDataRead(
+lldb::tid_t tid, llvm::StringRef kind,
+std::function data)> callback) {
+  Expected file = GetPostMortemThreadDataFile(tid, kind);
+  if (!file)
+return file.takeError();
+  ErrorOr> trace_or_error =
+  MemoryBuffer::getFile(file->GetPath());
+  if (std::error_code err = trace_or_error.getError())
+return errorCodeToError(err);
+
+  MemoryBuffer  = **trace_or_error;
+  ArrayRef array_ref(
+  reinterpret_cast(data.getBufferStart()),
+  data.getBufferSize());
+  return callback(array_ref);
+}
+
+llvm::Error Trace::OnThreadBinaryDataRead(
+lldb::tid_t tid, llvm::StringRef kind,
+std::function data)> callback) {
+  if (m_live_process)
+return OnLiveThreadBinaryDataRead(tid, kind, callback);
+  else
+return OnPostMortemThreadBinaryDataRead(tid, kind, callback);
+}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
@@ -48,15 +48,8 @@
 return json_intel_pt_trace.takeError();
 
   llvm::Expected json_session_description =
-  TraceSessionSaver::BuildProcessesSection(
-  *live_process,
-  [&](lldb::tid_t tid)
-  -> llvm::Expected>> {
-if (!trace_ipt.IsTraced(tid))
-  return None;
-return trace_ipt.GetLiveThreadBuffer(tid);
-  },
-  directory);
+  TraceSessionSaver::BuildProcessesSection(*live_process,
+   "threadTraceBuffer", directory);
 
   if (!json_session_description)
 return json_session_description.takeError();
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -137,8 +137,10 @@
 StructuredData::ObjectSP configuration =
 StructuredData::ObjectSP()) override;
 
-  /// Get the thread buffer content for a live thread
-  llvm::Expected> GetLiveThreadBuffer(lldb::tid_t tid);
+  /// See \a Trace::OnThreadBinaryDataRead().
+  llvm::Error OnThreadBufferRead(
+  lldb::tid_t tid,
+  std::function callback)> callback);
 
   llvm::Expected GetCPUInfo();
 
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -78,10 +78,12 @@
 const pt_cpu _info,
 const std::vector _threads)
 : m_cpu_info(cpu_info) {
-  for 

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: jj10306, zrthxn.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Some parts of the code have to distinguish between live and postmortem threads
to figure out how to get some data, e.g. thread trace buffers. This makes the
code less generic and more error prone. An example of that is that we have
two different decoders: LiveThreadDecoder and PostMortemThreadDecoder. They
exist because getting the trace bufer is different for each case.

The problem doesn't stop there. Soon we'll have even more kinds of data, like
the context switch trace, whose fetching will be different for live and post-
mortem processes.

As a way to fix this, I'm creating a common API for accessing thread data,
which is able to figure out how to handle the postmortem and live cases on
behalf of the caller. As a result of that, I was able to eliminate the two
decoders and unify them into a simpler one. Not only that, our TraceSave
functionality only worked for live threads, but now it can also work for
postmortem processes, which might be useful now, but it might in the future.

This common API is OnThreadBinaryDataRead. More information in the inline 
documentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123281

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp
  lldb/source/Plugins/Trace/common/TraceSessionSaver.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -215,3 +215,67 @@
   RefreshLiveProcessState();
   return m_stop_id;
 }
+
+llvm::Expected
+Trace::GetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind) {
+  auto NotFoundError = [&]() {
+return createStringError(
+inconvertibleErrorCode(),
+formatv("The thread with tid={0} doesn't have the tracing data {1}",
+tid, kind));
+  };
+
+  auto it = m_postmortem_thread_data.find(tid);
+  if (it == m_postmortem_thread_data.end())
+return NotFoundError();
+
+  std::unordered_map _kind_to_file_spec_map =
+  it->second;
+  auto it2 = data_kind_to_file_spec_map.find(kind.str());
+  if (it2 == data_kind_to_file_spec_map.end())
+return NotFoundError();
+  return it2->second;
+}
+
+void Trace::SetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind,
+FileSpec file_spec) {
+  m_postmortem_thread_data[tid][kind.str()] = file_spec;
+}
+
+llvm::Error Trace::OnLiveThreadBinaryDataRead(
+lldb::tid_t tid, llvm::StringRef kind,
+std::function data)> callback) {
+  Expected> data = GetLiveThreadBinaryData(tid, kind);
+  if (!data)
+return data.takeError();
+  return callback(*data);
+}
+
+llvm::Error Trace::OnPostMortemThreadBinaryDataRead(
+lldb::tid_t tid, llvm::StringRef kind,
+std::function data)> callback) {
+  Expected file = GetPostMortemThreadDataFile(tid, kind);
+  if (!file)
+return file.takeError();
+  ErrorOr> trace_or_error =
+  MemoryBuffer::getFile(file->GetPath());
+  if (std::error_code err = trace_or_error.getError())
+return errorCodeToError(err);
+
+  MemoryBuffer  = **trace_or_error;
+  MutableArrayRef trace_data(
+  // The libipt library does not modify the trace buffer, hence the
+  // following cast is safe.
+  reinterpret_cast(const_cast(trace.getBufferStart())),
+  trace.getBufferSize());
+  return callback(trace_data);
+}
+
+llvm::Error Trace::OnThreadBinaryDataRead(
+lldb::tid_t tid, llvm::StringRef kind,
+std::function data)> callback) {
+  if (m_live_process)
+return OnLiveThreadBinaryDataRead(tid, kind, callback);
+  else
+return OnPostMortemThreadBinaryDataRead(tid, kind, callback);
+}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
@@ -48,15 +48,8 @@
 return json_intel_pt_trace.takeError();
 
   llvm::Expected json_session_description =
-  TraceSessionSaver::BuildProcessesSection(
-  *live_process,
-  [&](lldb::tid_t tid)
-  -> llvm::Expected>> {
-if (!trace_ipt.IsTraced(tid))
-  return None;
-return trace_ipt.GetLiveThreadBuffer(tid);
-  },
-  directory);
+  

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

[Fangrui Song via lldb-commits]

Author: Fangrui Song
Date: 2022-04-06T20:19:07-07:00
New Revision: 2aca33baf15926afe2520a06b1427a9894226fd2

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

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

(The upgrade of the ppc64le bot and D121257 have fixed compiler-rt failures. 
Tested by nemanjai.)

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

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

Added: 


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

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

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 78f584f18bacd..931eecd9c9681 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -245,7 +245,7 @@ set(PPC_LINUX_DEFAULT_IEEELONGDOUBLE OFF CACHE BOOL
 set(CLANG_SPAWN_CC1 OFF CACHE BOOL
 "Whether clang should use a new process for the CC1 invocation")
 
-option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on Linux" OFF)
+option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on linux-gnu" ON)
 
 # TODO: verify the values against LangStandards.def?
 set(CLANG_DEFAULT_STD_C "" CACHE STRING

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

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

diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
 
b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
index 19aad2ab1ec32..cef500f0e7754 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
@@ -2,6 +2,7 @@
 from lldbsuite.test import decorators
 
 decor = [decorators.skipUnlessHasCallSiteInfo,
+ decorators.skipIf(archs=['arm'],oslist=["linux"]),
  decorators.skipIf(dwarf_version=['<', '4']),
  

[Lldb-commits] [PATCH] D123269: debugserver would never write modified xmm/ymm/zmm register values into the inferior

[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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123269

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

[Lldb-commits] [PATCH] D123269: debugserver would never write modified xmm/ymm/zmm register values into the inferior

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 421044.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123269

Files:
  lldb/test/Shell/Register/x86-64-write.test
  lldb/test/Shell/Register/x86-64-ymm-write.test
  lldb/test/Shell/Register/x86-64-zmm-write.test
  lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp


Index: lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -2632,7 +2632,8 @@
>value.uint8, 16);
 memcpy(_state.context.fpu.avx.__fpu_ymmh0 + (reg - fpu_ymm0),
(>value.uint8) + 16, 16);
-return true;
+success = true;
+break;
   case fpu_k0:
   case fpu_k1:
   case fpu_k2:
@@ -2643,7 +2644,8 @@
   case fpu_k7:
 memcpy(_state.context.fpu.avx512f.__fpu_k0 + (reg - fpu_k0),
>value.uint8, 8);
-return true;
+success = true;
+break;
   case fpu_zmm0:
   case fpu_zmm1:
   case fpu_zmm2:
@@ -2666,7 +2668,8 @@
>value.uint8 + 16, 16);
 memcpy(_state.context.fpu.avx512f.__fpu_zmmh0 + (reg - fpu_zmm0),
>value.uint8 + 32, 32);
-return true;
+success = true;
+break;
   case fpu_zmm16:
   case fpu_zmm17:
   case fpu_zmm18:
@@ -2685,7 +2688,8 @@
   case fpu_zmm31:
 memcpy(_state.context.fpu.avx512f.__fpu_zmm16 + (reg - fpu_zmm16),
>value.uint8, 64);
-return true;
+success = true;
+break;
   }
   break;
 
Index: lldb/test/Shell/Register/x86-64-zmm-write.test
===
--- lldb/test/Shell/Register/x86-64-zmm-write.test
+++ lldb/test/Shell/Register/x86-64-zmm-write.test
@@ -1,4 +1,9 @@
-# XFAIL: system-darwin
+# xfail with system debugserver until the fix for
+# https://reviews.llvm.org/D123269 in
+# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+# has made it into released tools.
+# XFAIL: system-debugserver
+
 # XFAIL: system-freebsd
 # XFAIL: system-linux
 # XFAIL: system-netbsd
Index: lldb/test/Shell/Register/x86-64-ymm-write.test
===
--- lldb/test/Shell/Register/x86-64-ymm-write.test
+++ lldb/test/Shell/Register/x86-64-ymm-write.test
@@ -1,4 +1,9 @@
-# XFAIL: system-darwin
+# xfail with system debugserver until the fix for
+# https://reviews.llvm.org/D123269 in
+# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+# has made it into released tools.
+# XFAIL: system-debugserver
+
 # XFAIL: system-windows
 # REQUIRES: native && target-x86_64 && native-cpu-avx
 # RUN: %clangxx_host %p/Inputs/x86-ymm-write.cpp -o %t
Index: lldb/test/Shell/Register/x86-64-write.test
===
--- lldb/test/Shell/Register/x86-64-write.test
+++ lldb/test/Shell/Register/x86-64-write.test
@@ -1,4 +1,9 @@
-# XFAIL: system-darwin
+# xfail with system debugserver until the fix for
+# https://reviews.llvm.org/D123269 in
+# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+# has made it into released tools.
+# XFAIL: system-debugserver
+
 # XFAIL: system-windows
 # REQUIRES: native && target-x86_64
 # RUN: %clangxx_host %p/Inputs/x86-64-write.cpp -o %t


Index: lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -2632,7 +2632,8 @@
>value.uint8, 16);
 memcpy(_state.context.fpu.avx.__fpu_ymmh0 + (reg - fpu_ymm0),
(>value.uint8) + 16, 16);
-return true;
+success = true;
+break;
   case fpu_k0:
   case fpu_k1:
   case fpu_k2:
@@ -2643,7 +2644,8 @@
   case fpu_k7:
 memcpy(_state.context.fpu.avx512f.__fpu_k0 + (reg - fpu_k0),
>value.uint8, 8);
-return true;
+success = true;
+break;
   case fpu_zmm0:
   case fpu_zmm1:
   case fpu_zmm2:
@@ -2666,7 +2668,8 @@
>value.uint8 + 16, 16);
 memcpy(_state.context.fpu.avx512f.__fpu_zmmh0 + (reg - fpu_zmm0),
>value.uint8 + 32, 32);
-return true;
+success = true;
+break;
   case fpu_zmm16:
   case fpu_zmm17:
   case fpu_zmm18:
@@ -2685,7 +2688,8 @@
   case fpu_zmm31:
 memcpy(_state.context.fpu.avx512f.__fpu_zmm16 + (reg - fpu_zmm16),
>value.uint8, 64);
-return true;
+success = true;
+break;
   }
   break;

[Lldb-commits] [PATCH] D123269: debugserver would never write modified xmm/ymm/zmm register values into the inferior

[Jason Molenda via Phabricator via lldb-commits]

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

A small mistake in the method that accepts new xmm/ymm/zmm register values 
caused them to not be thread_set_state'd so the changes never took effect.

Update the three shell tests that test this functionality so they are xfail'ed 
with the system debugserver (which won't have this fix yet), but do pass with 
in-tree debugserver now on x86_64 darwin systems.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123269

Files:
  lldb/test/Shell/Register/x86-64-write.test
  lldb/test/Shell/Register/x86-64-ymm-write.test
  lldb/test/Shell/Register/x86-64-zmm-write.test
  lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp


Index: lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -2632,7 +2632,8 @@
>value.uint8, 16);
 memcpy(_state.context.fpu.avx.__fpu_ymmh0 + (reg - fpu_ymm0),
(>value.uint8) + 16, 16);
-return true;
+success = true;
+break;
   case fpu_k0:
   case fpu_k1:
   case fpu_k2:
@@ -2643,7 +2644,8 @@
   case fpu_k7:
 memcpy(_state.context.fpu.avx512f.__fpu_k0 + (reg - fpu_k0),
>value.uint8, 8);
-return true;
+success = true;
+break;
   case fpu_zmm0:
   case fpu_zmm1:
   case fpu_zmm2:
@@ -2666,7 +2668,8 @@
>value.uint8 + 16, 16);
 memcpy(_state.context.fpu.avx512f.__fpu_zmmh0 + (reg - fpu_zmm0),
>value.uint8 + 32, 32);
-return true;
+success = true;
+break;
   case fpu_zmm16:
   case fpu_zmm17:
   case fpu_zmm18:
@@ -2685,7 +2688,8 @@
   case fpu_zmm31:
 memcpy(_state.context.fpu.avx512f.__fpu_zmm16 + (reg - fpu_zmm16),
>value.uint8, 64);
-return true;
+success = true;
+break;
   }
   break;
 
Index: lldb/test/Shell/Register/x86-64-zmm-write.test
===
--- lldb/test/Shell/Register/x86-64-zmm-write.test
+++ lldb/test/Shell/Register/x86-64-zmm-write.test
@@ -1,4 +1,4 @@
-# XFAIL: system-darwin
+# XFAIL: system-debugserver
 # XFAIL: system-freebsd
 # XFAIL: system-linux
 # XFAIL: system-netbsd
Index: lldb/test/Shell/Register/x86-64-ymm-write.test
===
--- lldb/test/Shell/Register/x86-64-ymm-write.test
+++ lldb/test/Shell/Register/x86-64-ymm-write.test
@@ -1,4 +1,4 @@
-# XFAIL: system-darwin
+# XFAIL: system-debugserver
 # XFAIL: system-windows
 # REQUIRES: native && target-x86_64 && native-cpu-avx
 # RUN: %clangxx_host %p/Inputs/x86-ymm-write.cpp -o %t
Index: lldb/test/Shell/Register/x86-64-write.test
===
--- lldb/test/Shell/Register/x86-64-write.test
+++ lldb/test/Shell/Register/x86-64-write.test
@@ -1,4 +1,4 @@
-# XFAIL: system-darwin
+# XFAIL: system-debugserver
 # XFAIL: system-windows
 # REQUIRES: native && target-x86_64
 # RUN: %clangxx_host %p/Inputs/x86-64-write.cpp -o %t


Index: lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -2632,7 +2632,8 @@
>value.uint8, 16);
 memcpy(_state.context.fpu.avx.__fpu_ymmh0 + (reg - fpu_ymm0),
(>value.uint8) + 16, 16);
-return true;
+success = true;
+break;
   case fpu_k0:
   case fpu_k1:
   case fpu_k2:
@@ -2643,7 +2644,8 @@
   case fpu_k7:
 memcpy(_state.context.fpu.avx512f.__fpu_k0 + (reg - fpu_k0),
>value.uint8, 8);
-return true;
+success = true;
+break;
   case fpu_zmm0:
   case fpu_zmm1:
   case fpu_zmm2:
@@ -2666,7 +2668,8 @@
>value.uint8 + 16, 16);
 memcpy(_state.context.fpu.avx512f.__fpu_zmmh0 + (reg - fpu_zmm0),
>value.uint8 + 32, 32);
-return true;
+success = true;
+break;
   case fpu_zmm16:
   case fpu_zmm17:
   case fpu_zmm18:
@@ -2685,7 +2688,8 @@
   case fpu_zmm31:
 memcpy(_state.context.fpu.avx512f.__fpu_zmm16 + (reg - fpu_zmm16),
>value.uint8, 64);
-return true;
+success = true;
+break;
   }
   break;
 
Index: lldb/test/Shell/Register/x86-64-zmm-write.test

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D123020#3426867 , @JDevlieghere 
wrote:

> In D123020#3426839 , @llunak wrote:
>
>> In D123020#3426252 , @labath wrote:
>>
 BTW, does it make sense to get even things like this reviewed, or is it ok 
 if I push these directly if I'm reasonably certain I know what I'm doing? 
 I feel like I'm spamming you by now.
>>>
>>> Generally, I would say yes. I'm not even sure that some of your other 
>>> patches should have been submitted without a pre-commit review (*all* llvm 
>>> patches are expected to be reviewed).
>>
>> I see. I haven't contributed anything for a while and git history shows a 
>> number of commits to do not refer to reviews.llvm.org, so I assumed simple 
>> patches were fine. I'll get even the simple ones reviewed if that's expected.
>
> FWIW the official policy is outlined here: 
> https://llvm.org/docs/CodeReview.html
>
>>> I would say that the entire ScopedTimeout class should be multiplier-based. 
>>> Instead of passing a fix value, the user would say "I know this packed is 
>>> slow, so I want to use X-times the usual timeout value". Then, we could 
>>> support valgrind (and various *SANs) by just setting the initial timeout 
>>> value to a reasonably high value, and the multipliers would take care of 
>>> the rest.
>>
>> For the Valgrind case it doesn't really matter what the timeout is, as long 
>> as it's long enough to not time out on normal calls. Even multiplying is 
>> just taking a guess at it, Valgrind doesn't slow things down linearly.
>>
>> In D123020#3426572 , @JDevlieghere 
>> wrote:
>>
>>> Is this really something we want to hardcode at all? It seems like this 
>>> should be a setting that is configured by the test harness.
>>
>> This is not about tests. I had a double-free abort, so I ran lldb normally 
>> in valgrind and it aborted attach because of the 6-second timeout in 
>> GDBRemoteCommunicationClient::QueryNoAckModeSupported(). That's too short 
>> for anything in Valgrind, and I thought this change would be useful in 
>> general, as otherwise lldb is presumably unusable for Valgrind runs.
>
> My suggestion was that this timeout should be configurable through a setting. 
> If it is, then you can increase it when running under Valgrind (or anywhere 
> else that slows down lldb, like the sanitizers). I wouldn't mind having a bit 
> of code that checks `llvm::sys::RunningOnValgrind()` and increases the 
> default timeouts so that you don't even have to change your setting. But what 
> I don't think is a good idea is to sprinkle checks for whether we're running 
> under Vanguard throughout the code.

The GDB remote packet timeout is customizable:

  (lldb) settings set plugin.process.gdb-remote.packet-timeout 1

So maybe we don't need this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [lldb] 815fa5b - [lldb] Remove duplicate "warning:"

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-04-06T15:05:27-07:00
New Revision: 815fa5bf44c459cf5e72b35da4311d6681952e1b

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

LOG: [lldb] Remove duplicate "warning:"

Remove "warning:" from the warning message itself. The default event
handler is already emitting the appropriate prefix.

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 32d3527ea10c6..b8104af5c1fa7 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2341,14 +2341,14 @@ void AppleObjCRuntimeV2::WarnIfNoClassesCached(
   Debugger (GetProcess()->GetTarget().GetDebugger());
   switch (reason) {
   case SharedCacheWarningReason::eNotEnoughClassesRead:
-Debugger::ReportWarning("warning: could not find Objective-C class data in 
"
+Debugger::ReportWarning("could not find Objective-C class data in "
 "the process. This may reduce the quality of type "
 "information available.\n",
 debugger.GetID(), _no_classes_cached_warning);
 break;
   case SharedCacheWarningReason::eExpressionExecutionFailure:
 Debugger::ReportWarning(
-"warning: could not execute support code to read "
+"could not execute support code to read "
 "Objective-C class data in the process. This may "
 "reduce the quality of type information available.\n",
 debugger.GetID(), _no_classes_cached_warning);
@@ -2373,7 +2373,7 @@ void AppleObjCRuntimeV2::WarnIfNoExpandedSharedCache() {
   std::string buffer;
   llvm::raw_string_ostream os(buffer);
 
-  os << "warning: libobjc.A.dylib is being read from process memory. This "
+  os << "libobjc.A.dylib is being read from process memory. This "
 "indicates that LLDB could not ";
   if (PlatformSP platform_sp = target.GetPlatform()) {
 if (platform_sp->IsHost()) {



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

[Lldb-commits] [PATCH] D123226: [lldb] use one shared ThreadPool and task groups

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Core/Debugger.cpp:1969
+llvm::ThreadPool ::GetThreadPool() {
+  static llvm::ThreadPool threadpool(llvm::optimal_concurrency());
+  return threadpool;

aganea wrote:
> clayborg wrote:
> > aganea wrote:
> > > Ideally this should be explicitly created on the stack & passed along on 
> > > the callstack or in a context, like MLIR does: 
> > > https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/IR/MLIRContext.h#L48
> > We need one instance of the thread pool so we can't create on the stack. 
> > 
> > Static is not great because of the C++ global destructor chain could try 
> > and use if "main" exits and we still have threads running. I would do 
> > something like the "g_debugger_list_ptr" where you create a static variable 
> > in Debugger.cpp:105 which is a pointer, and we initialize it inside of 
> > Debugger::Initialize() like we do for "g_debugger_list_ptr". Then the 
> > thread pool will not cause shutdown crashes when "main" exits before other 
> > threads.
> > 
> I meant having the `ThreadPool` in a context created by main() or the lib 
> caller, before getting here in library/plugin code, and passed along. LLD 
> does that too: 
> https://github.com/llvm/llvm-project/blob/main/lld/ELF/Driver.cpp#L94 - the 
> statics in `Debugger.cpp` seems like a workaround for that. In any case, it's 
> better than having the `static` here.
In LLDB, the lldb_private::Debugger has static functions that hand out things 
that are needed for the debugger to do its work, so I like the static function 
here. We don't allow any llvm or clang code to make it through our public API 
(in lldb::SB* classes), so we do need code inside the debugger to be able to 
access global resources and the debugger class is where this should live. 

We can also just have code like:
```
// NOTE: intentional leak to avoid issues with C++ destructor chain
static llvm::ThreadPool *g_thread_pool = nullptr;
static llvm::once_flag g_once_flag;
llvm::call_once(g_once_flag, []() {
  g_thread_pool = new llvm::ThreadPool(llvm::optimal_concurrency());
});
return *g_thread_pool;
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123226

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

[Lldb-commits] [PATCH] D123254: [lldb] Disable GCC's -Wstringop-truncation warning. NFC.

[Jonas Devlieghere via Phabricator via lldb-commits]

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

I saw this warning on the bots and was confused because I too came to the 
conclusion that we were using strncpy correctly. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123254

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

[Lldb-commits] [PATCH] D123226: [lldb] use one shared ThreadPool and task groups

[Alexandre Ganea via Phabricator via lldb-commits]

aganea added inline comments.



Comment at: lldb/source/Core/Debugger.cpp:1969
+llvm::ThreadPool ::GetThreadPool() {
+  static llvm::ThreadPool threadpool(llvm::optimal_concurrency());
+  return threadpool;

clayborg wrote:
> aganea wrote:
> > Ideally this should be explicitly created on the stack & passed along on 
> > the callstack or in a context, like MLIR does: 
> > https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/IR/MLIRContext.h#L48
> We need one instance of the thread pool so we can't create on the stack. 
> 
> Static is not great because of the C++ global destructor chain could try and 
> use if "main" exits and we still have threads running. I would do something 
> like the "g_debugger_list_ptr" where you create a static variable in 
> Debugger.cpp:105 which is a pointer, and we initialize it inside of 
> Debugger::Initialize() like we do for "g_debugger_list_ptr". Then the thread 
> pool will not cause shutdown crashes when "main" exits before other threads.
> 
I meant having the `ThreadPool` in a context created by main() or the lib 
caller, before getting here in library/plugin code, and passed along. LLD does 
that too: https://github.com/llvm/llvm-project/blob/main/lld/ELF/Driver.cpp#L94 
- the statics in `Debugger.cpp` seems like a workaround for that. In any case, 
it's better than having the `static` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123226

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

[Lldb-commits] [PATCH] D123226: [lldb] use one shared ThreadPool and task groups

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/source/Core/Debugger.cpp:1969
+llvm::ThreadPool ::GetThreadPool() {
+  static llvm::ThreadPool threadpool(llvm::optimal_concurrency());
+  return threadpool;

aganea wrote:
> Ideally this should be explicitly created on the stack & passed along on the 
> callstack or in a context, like MLIR does: 
> https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/IR/MLIRContext.h#L48
We need one instance of the thread pool so we can't create on the stack. 

Static is not great because of the C++ global destructor chain could try and 
use if "main" exits and we still have threads running. I would do something 
like the "g_debugger_list_ptr" where you create a static variable in 
Debugger.cpp:105 which is a pointer, and we initialize it inside of 
Debugger::Initialize() like we do for "g_debugger_list_ptr". Then the thread 
pool will not cause shutdown crashes when "main" exits before other threads.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123226

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

[Lldb-commits] [PATCH] D123254: [lldb] Disable GCC's -Wstringop-truncation warning. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, JDevlieghere.
Herald added subscribers: pengfei, mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This warning gives false positives about lldb's correct use of
strncpy to fill fixed length fields that don't need null termination,
in lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp, like this:

  In file included from /usr/include/string.h:495,
   from /usr/include/c++/9/cstring:42,
   from ../include/llvm/ADT/StringRef.h:19,
   from 
../tools/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:10:
  In function ‘char* strncpy(char*, const char*, size_t)’,
  inlined from ‘lldb::offset_t CreateAllImageInfosPayload(const ProcessSP&, 
lldb::offset_t, lldb_private::StreamString&, lldb::SaveCoreStyle)’ at 
../tools/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:6341:16:
  /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:34: warning: ‘char* 
__builtin_strncpy(char*, const char*, long unsigned int)’ specified bound 16 
equals destination size [-Wstringop-truncation]
106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos 
(__dest));
|  ^~~

The warning could be squelched locally with

  #pragma GCC diagnostic ignored "-Wstringop-truncation"

too, but Clang also interprets those GCC pragmas, and produces
a -Wunknown-warning-option warning instead. That could be remedied
by wrapping the pragma in an "#ifndef __clang__" - but that makes
things even more messy. Instead, just silence this warning entirely.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123254

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -183,6 +183,9 @@
 check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
 append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
+check_cxx_compiler_flag("-Wstringop-truncation" 
CXX_SUPPORTS_STRINGOP_TRUNCATION)
+append_if(CXX_SUPPORTS_STRINGOP_TRUNCATION "-Wno-stringop-truncation" 
CMAKE_CXX_FLAGS)
+
 # Disable Clang warnings
 check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
 append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -183,6 +183,9 @@
 check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
 append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
+check_cxx_compiler_flag("-Wstringop-truncation" CXX_SUPPORTS_STRINGOP_TRUNCATION)
+append_if(CXX_SUPPORTS_STRINGOP_TRUNCATION "-Wno-stringop-truncation" CMAKE_CXX_FLAGS)
+
 # Disable Clang warnings
 check_cxx_compiler_flag("-Wdeprecated-register" CXX_SUPPORTS_DEPRECATED_REGISTER)
 append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" CMAKE_CXX_FLAGS)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122859: [trace] Show ideas for the main interfaces for new HTR

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Target/TraceHTR.h:81
+// and not to a child.
+lldb::user_id_t GetLastInstructionId() { return m_last_insn_id; }
+

This seems like it should be stored as an optional value and if the optional 
value has no valid, it would need to be  computed lazily?



Comment at: lldb/include/lldb/Target/TraceHTR.h:83
+
+size_t GetChildrenCount() { return m_children; }
+

Should this function be able to lazily compute itself? Right now you would need 
to parse the entire stream of instructions for a HTRInMemoryBlock contents to 
be valid? Is that what we want?



Comment at: lldb/include/lldb/Target/TraceHTR.h:88
+// most 2^32 elements, which seems reasonable.
+size_t (size_t index) { return m_children_indices[index]; }
+

If you go with the GetChildrenCount() + GetChildAtIndex(...) workflow, then you 
can't lazily compute children. Do we want to go with an iterator kind of thing 
where you ask for the iterator for the first child and then ask for next until 
some ending?



Comment at: lldb/include/lldb/Target/TraceHTR.h:90
+
+llvm::Optional GetParent() { return m_parent_index; }
+

I would avoid size_t as it is uint32_t on 32 bit systems and uint64_t on 64 bit 
systems. Probably just pick uint64_t?



Comment at: lldb/include/lldb/Target/TraceHTR.h:113
+  // We need this accessor because each block doesn't know its own index
+  HTRInMemoryBlock (size_t index) { return m_blocks[index]; }
+

Bounds check "index"?



Comment at: lldb/include/lldb/Target/TraceHTR.h:187-188
+  union {
+TraceHTRInMemoryStorage *m_in_memory_storage;
+TraceHTROnDiskStore *m_on_disk_storage;
+  };

Should we just make a TraceHTR base class that has all of the needed methods 
and store a single pointer here? Then TraceHTRInMemoryStorage and 
TraceHTROnDiskStore would subclass it?



Comment at: lldb/include/lldb/Target/TraceHTR.h:191-192
+  union {
+HTRInMemoryBlock *m_in_memory_block;
+HTRInMemoryBlock *m_on_disk_block;
+  };

Should we just make a HTRBlock base class that has all of the needed methods 
and store a single pointer here? Then HTRInMemoryBlock and HTROnDiskBlock would 
subclass it?



Comment at: lldb/include/lldb/Target/TraceHTR.h:192
+HTRInMemoryBlock *m_in_memory_block;
+HTRInMemoryBlock *m_on_disk_block;
+  };





Comment at: lldb/include/lldb/Target/TraceHTR.h:308
+private:
+  std::map> m_mapping; // symbol id -> block 
index
+};


is the idea to somehow represent a lldb_private::Symbol from this index? What 
does a "symbol id" mean?



Comment at: lldb/include/lldb/Target/TraceHTR.h:352-355
+  // Only of them active at a given time. An abstract class might also come
+  // handy.
+  std::unique_ptr m_in_memory_storage;
+  std::unique_ptr m_in_disk_storage;

Yeah, I would make a base class here and just store one unique pointer here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122859

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

[Lldb-commits] [lldb] 6795f37 - [NFC][trace][intelpt] Remove unneeded import

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-04-06T13:34:11-07:00
New Revision: 6795f37c1481d702e60e17b9d8e0b98faab979ca

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

LOG: [NFC][trace][intelpt] Remove unneeded import

Remove an unnecessary import to silence a compiler warning.

Added: 


Modified: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h 
b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
index 34883d3cf3005..6bf39cad1e168 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
@@ -10,7 +10,6 @@
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPTSESSIONFILEPARSER_H
 
 #include "../common/TraceSessionFileParser.h"
-#include "TraceIntelPT.h"
 #include "TraceIntelPTJSONStructs.h"
 
 namespace lldb_private {



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

[Lldb-commits] [PATCH] D123226: [lldb] use one shared ThreadPool and task groups

[Alexandre Ganea via Phabricator via lldb-commits]

aganea added inline comments.



Comment at: lldb/source/Core/Debugger.cpp:1969
+llvm::ThreadPool ::GetThreadPool() {
+  static llvm::ThreadPool threadpool(llvm::optimal_concurrency());
+  return threadpool;

Ideally this should be explicitly created on the stack & passed along on the 
callstack or in a context, like MLIR does: 
https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/IR/MLIRContext.h#L48



Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:127
 
-  pool.async(finalize_fn, ::function_basenames);
-  pool.async(finalize_fn, ::function_fullnames);
-  pool.async(finalize_fn, ::function_methods);
-  pool.async(finalize_fn, ::function_selectors);
-  pool.async(finalize_fn, ::objc_class_selectors);
-  pool.async(finalize_fn, ::globals);
-  pool.async(finalize_fn, ::types);
-  pool.async(finalize_fn, ::namespaces);
-  pool.wait();
+  pool.async(group, finalize_fn, ::function_basenames);
+  pool.async(group, finalize_fn, ::function_fullnames);

Like suggested in D123225, wouldn't it be better if we had 
`group.async(finalize_fn, ::function_basenames)`? Same for the waits: 
`group.wait();`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123226

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

[Lldb-commits] [PATCH] D123205: [lldb] Silence GCC/glibc warnings about ignoring the return value of write(). NFC.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e38824221db: [lldb] Silence GCC/glibc warnings about 
ignoring the return value of write(). (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123205

Files:
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -893,7 +893,8 @@
 return result;
   }
 
-  write(fds[WRITE], data, size);
+  int r = write(fds[WRITE], data, size);
+  (void)r;
   // Close the write end of the pipe, so that the command interpreter will exit
   // when it consumes all the data.
   llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -893,7 +893,8 @@
 return result;
   }
 
-  write(fds[WRITE], data, size);
+  int r = write(fds[WRITE], data, size);
+  (void)r;
   // Close the write end of the pipe, so that the command interpreter will exit
   // when it consumes all the data.
   llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123203: [lldb] Silence GCC warnings about missing returns after fully covered switches. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGae2aa2d21b24: [lldb] Silence GCC warnings about missing 
returns after fully covered switches. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123203

Files:
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Core/DebuggerEvents.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp


Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -102,6 +102,7 @@
 case BitOffset:
   return 10;
 }
+llvm_unreachable("Fully covered switch above!");
   };
 
   auto createError = [&](const char *expected_value_message) {
Index: lldb/source/Core/DebuggerEvents.cpp
===
--- lldb/source/Core/DebuggerEvents.cpp
+++ lldb/source/Core/DebuggerEvents.cpp
@@ -52,6 +52,7 @@
   case Type::Error:
 return "error";
   }
+  llvm_unreachable("Fully covered switch above!");
 }
 
 void DiagnosticEventData::Dump(Stream *s) const {
Index: lldb/source/Breakpoint/Breakpoint.cpp
===
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -1026,6 +1026,7 @@
 case eBreakpointEventTypeThreadChanged: return "thread changed";
 case eBreakpointEventTypeAutoContinueChanged: return "autocontinue 
changed";
   };
+  llvm_unreachable("Fully covered switch above!");
 }
 
 Log *Breakpoint::BreakpointEventData::GetLogChannel() {


Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -102,6 +102,7 @@
 case BitOffset:
   return 10;
 }
+llvm_unreachable("Fully covered switch above!");
   };
 
   auto createError = [&](const char *expected_value_message) {
Index: lldb/source/Core/DebuggerEvents.cpp
===
--- lldb/source/Core/DebuggerEvents.cpp
+++ lldb/source/Core/DebuggerEvents.cpp
@@ -52,6 +52,7 @@
   case Type::Error:
 return "error";
   }
+  llvm_unreachable("Fully covered switch above!");
 }
 
 void DiagnosticEventData::Dump(Stream *s) const {
Index: lldb/source/Breakpoint/Breakpoint.cpp
===
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -1026,6 +1026,7 @@
 case eBreakpointEventTypeThreadChanged: return "thread changed";
 case eBreakpointEventTypeAutoContinueChanged: return "autocontinue changed";
   };
+  llvm_unreachable("Fully covered switch above!");
 }
 
 Log *Breakpoint::BreakpointEventData::GetLogChannel() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123202: [lldb] Fix detecting warning options for GCC

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe546bbfda0ab: [lldb] Fix detecting warning options for GCC 
(authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123202

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -168,22 +168,27 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# GCC silently accepts any -Wno- option, but warns about those options
+# being unrecognized only if the compilation triggers other warnings to be
+# printed. Therefore, check for whether the compiler supports options in the
+# form -W, and if supported, add the corresponding -Wno- option.
+
 # Disable GCC warnings
-check_cxx_compiler_flag("-Wno-deprecated-declarations" 
CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS 
"-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-declarations" 
CXX_SUPPORTS_DEPRECATED_DECLARATIONS)
+append_if(CXX_SUPPORTS_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-unknown-pragmas" CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS)
-append_if(CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wunknown-pragmas" CXX_SUPPORTS_UNKNOWN_PRAGMAS)
+append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-strict-aliasing" CXX_SUPPORTS_NO_STRICT_ALIASING)
-append_if(CXX_SUPPORTS_NO_STRICT_ALIASING "-Wno-strict-aliasing" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
+append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
 # Disable Clang warnings
-check_cxx_compiler_flag("-Wno-deprecated-register" 
CXX_SUPPORTS_NO_DEPRECATED_REGISTER)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
+append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-vla-extension" CXX_SUPPORTS_NO_VLA_EXTENSION)
-append_if(CXX_SUPPORTS_NO_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wvla-extension" CXX_SUPPORTS_VLA_EXTENSION)
+append_if(CXX_SUPPORTS_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
 
 # Disable MSVC warnings
 if( MSVC )


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -168,22 +168,27 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# GCC silently accepts any -Wno- option, but warns about those options
+# being unrecognized only if the compilation triggers other warnings to be
+# printed. Therefore, check for whether the compiler supports options in the
+# form -W, and if supported, add the corresponding -Wno- option.
+
 # Disable GCC warnings
-check_cxx_compiler_flag("-Wno-deprecated-declarations" CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-declarations" CXX_SUPPORTS_DEPRECATED_DECLARATIONS)
+append_if(CXX_SUPPORTS_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-unknown-pragmas" CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS)
-append_if(CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wunknown-pragmas" CXX_SUPPORTS_UNKNOWN_PRAGMAS)
+append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-strict-aliasing" CXX_SUPPORTS_NO_STRICT_ALIASING)
-append_if(CXX_SUPPORTS_NO_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
+append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
 # Disable Clang warnings
-check_cxx_compiler_flag("-Wno-deprecated-register" CXX_SUPPORTS_NO_DEPRECATED_REGISTER)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_REGISTER "-Wno-deprecated-register" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-register" CXX_SUPPORTS_DEPRECATED_REGISTER)
+append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-vla-extension" CXX_SUPPORTS_NO_VLA_EXTENSION)
-append_if(CXX_SUPPORTS_NO_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wvla-extension" CXX_SUPPORTS_VLA_EXTENSION)

[Lldb-commits] [lldb] ae2aa2d - [lldb] Silence GCC warnings about missing returns after fully covered switches. NFC.

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2022-04-06T22:50:07+03:00
New Revision: ae2aa2d21b24a912314e618d1ceb8e036449b0b1

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

LOG: [lldb] Silence GCC warnings about missing returns after fully covered 
switches. NFC.

This silences warnings like this:

lldb/source/Core/DebuggerEvents.cpp: In member function ‘llvm::StringRef 
lldb_private::DiagnosticEventData::GetPrefix() const’:
lldb/source/Core/DebuggerEvents.cpp:55:1: warning: control reaches end of 
non-void function [-Wreturn-type]
   55 | }

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

Added: 


Modified: 
lldb/source/Breakpoint/Breakpoint.cpp
lldb/source/Core/DebuggerEvents.cpp
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp

Removed: 




diff  --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index afadf81e32c8c..578cf14283d92 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -1026,6 +1026,7 @@ const char 
*Breakpoint::BreakpointEventTypeAsCString(BreakpointEventType type) {
 case eBreakpointEventTypeThreadChanged: return "thread changed";
 case eBreakpointEventTypeAutoContinueChanged: return "autocontinue 
changed";
   };
+  llvm_unreachable("Fully covered switch above!");
 }
 
 Log *Breakpoint::BreakpointEventData::GetLogChannel() {

diff  --git a/lldb/source/Core/DebuggerEvents.cpp 
b/lldb/source/Core/DebuggerEvents.cpp
index a433ec31738f4..fdf55f68ce8e7 100644
--- a/lldb/source/Core/DebuggerEvents.cpp
+++ b/lldb/source/Core/DebuggerEvents.cpp
@@ -52,6 +52,7 @@ llvm::StringRef DiagnosticEventData::GetPrefix() const {
   case Type::Error:
 return "error";
   }
+  llvm_unreachable("Fully covered switch above!");
 }
 
 void DiagnosticEventData::Dump(Stream *s) const {

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index 4882ab64f16ac..8c10828627971 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -102,6 +102,7 @@ static Expected ReadIntelPTConfigFile(const char 
*file,
 case BitOffset:
   return 10;
 }
+llvm_unreachable("Fully covered switch above!");
   };
 
   auto createError = [&](const char *expected_value_message) {



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

[Lldb-commits] [lldb] 6e38824 - [lldb] Silence GCC/glibc warnings about ignoring the return value of write(). NFC.

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2022-04-06T22:50:07+03:00
New Revision: 6e38824221db2e0d77d334d424331b3cf1ac279a

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

LOG: [lldb] Silence GCC/glibc warnings about ignoring the return value of 
write(). NFC.

This matches how another similar warning is silenced in
Host/posix/ProcessLauncherPosixFork.cpp.

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

Added: 


Modified: 
lldb/source/Core/Debugger.cpp

Removed: 




diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index cac5ae4dacd75..37e54a9abefaa 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -893,7 +893,8 @@ Status Debugger::SetInputString(const char *data) {
 return result;
   }
 
-  write(fds[WRITE], data, size);
+  int r = write(fds[WRITE], data, size);
+  (void)r;
   // Close the write end of the pipe, so that the command interpreter will exit
   // when it consumes all the data.
   llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);



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

[Lldb-commits] [lldb] e546bbf - [lldb] Fix detecting warning options for GCC

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2022-04-06T22:50:07+03:00
New Revision: e546bbfda0ab91cf78c096d8c035851cc7c3b9f3

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

LOG: [lldb] Fix detecting warning options for GCC

If testing for a warning option like -Wno- with GCC, GCC won't
print any diagnostic at all, leading to the options being accepted
incorrectly. However later, if compiling a file that actually prints
another warning, GCC will also print warnings about these -Wno-
options being unrecognized.

This avoids warning spam like this, for every lldb source file that
produces build warnings with GCC:

At global scope:
cc1plus: warning: unrecognized command line option ‘-Wno-vla-extension’
cc1plus: warning: unrecognized command line option 
‘-Wno-deprecated-register’

This matches how such warning options are detected and added in
llvm/cmake/modules/HandleLLVMOptions.cmake, e.g. like this:

check_cxx_compiler_flag("-Wclass-memaccess" 
CXX_SUPPORTS_CLASS_MEMACCESS_FLAG)
append_if(CXX_SUPPORTS_CLASS_MEMACCESS_FLAG "-Wno-class-memaccess" 
CMAKE_CXX_FLAGS)

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

Added: 


Modified: 
lldb/cmake/modules/LLDBConfig.cmake

Removed: 




diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index 69aaadf29ef64..25d1cc526ab03 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -168,22 +168,27 @@ else ()
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# GCC silently accepts any -Wno- option, but warns about those options
+# being unrecognized only if the compilation triggers other warnings to be
+# printed. Therefore, check for whether the compiler supports options in the
+# form -W, and if supported, add the corresponding -Wno- option.
+
 # Disable GCC warnings
-check_cxx_compiler_flag("-Wno-deprecated-declarations" 
CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS 
"-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-declarations" 
CXX_SUPPORTS_DEPRECATED_DECLARATIONS)
+append_if(CXX_SUPPORTS_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-unknown-pragmas" CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS)
-append_if(CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wunknown-pragmas" CXX_SUPPORTS_UNKNOWN_PRAGMAS)
+append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-strict-aliasing" CXX_SUPPORTS_NO_STRICT_ALIASING)
-append_if(CXX_SUPPORTS_NO_STRICT_ALIASING "-Wno-strict-aliasing" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
+append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
 # Disable Clang warnings
-check_cxx_compiler_flag("-Wno-deprecated-register" 
CXX_SUPPORTS_NO_DEPRECATED_REGISTER)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
+append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-vla-extension" CXX_SUPPORTS_NO_VLA_EXTENSION)
-append_if(CXX_SUPPORTS_NO_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wvla-extension" CXX_SUPPORTS_VLA_EXTENSION)
+append_if(CXX_SUPPORTS_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
 
 # Disable MSVC warnings
 if( MSVC )



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

[Lldb-commits] [lldb] 05b4bf2 - [trace][intelpt] Introduce instruction Ids

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-04-06T12:19:36-07:00
New Revision: 05b4bf2571244da2cef438e907036b35a0e1f99a

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

LOG: [trace][intelpt] Introduce instruction Ids

In order to support quick arbitrary access to instructions in the trace, we need
each instruction to have an id. It could be an index or any other value that the
trace plugin defines.

This will be useful for reverse debugging or for creating callstacks, as each
frame will need an instruction id associated with them.

I've updated the `thread trace dump instructions` command accordingly. It now
prints the instruction id instead of relative offset. I've also added a new --id
argument that allows starting the dump from an arbitrary position.

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

Added: 
lldb/test/API/commands/trace/TestTraceTSC.py

Modified: 
lldb/include/lldb/Target/TraceCursor.h
lldb/include/lldb/Target/TraceInstructionDumper.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/Options.td
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
lldb/source/Target/TraceInstructionDumper.cpp
lldb/test/API/commands/trace/TestTraceDumpInstructions.py
lldb/test/API/commands/trace/TestTraceStartStop.py

Removed: 
lldb/test/API/commands/trace/TestTraceTimestampCounters.py



diff  --git a/lldb/include/lldb/Target/TraceCursor.h 
b/lldb/include/lldb/Target/TraceCursor.h
index 2e3c4eb22b04e..710bb963a4aa9 100644
--- a/lldb/include/lldb/Target/TraceCursor.h
+++ b/lldb/include/lldb/Target/TraceCursor.h
@@ -74,9 +74,10 @@ class TraceCursor {
 public:
   /// Helper enum to indicate the reference point when invoking
   /// \a TraceCursor::Seek().
+  /// The following values are inspired by \a std::istream::seekg.
   enum class SeekType {
 /// The beginning of the trace, i.e the oldest item.
-Set = 0,
+Beginning = 0,
 /// The current position in the trace.
 Current,
 /// The end of the trace, i.e the most recent item.
@@ -133,6 +134,51 @@ class TraceCursor {
   /// \b true if the cursor effectively moved, \b false otherwise.
   virtual bool Next() = 0;
 
+  /// Instruction identifiers:
+  ///
+  /// When building complex higher level tools, fast random accesses in the
+  /// trace might be needed, for which each instruction requires a unique
+  /// identifier within its thread trace. For example, a tool might want to
+  /// repeatedly inspect random consecutive portions of a trace. This means 
that
+  /// it will need to first move quickly to the beginning of each section and
+  /// then start its iteration. Given that the number of instructions can be in
+  /// the order of hundreds of millions, fast random access is necessary.
+  ///
+  /// An example of such a tool could be an inspector of the call graph of a
+  /// trace, where each call is represented with its start and end 
instructions.
+  /// Inspecting all the instructions of a call requires moving to its first
+  /// instruction and then iterating until the last instruction, which 
following
+  /// the pattern explained above.
+  ///
+  /// Instead of using 0-based indices as identifiers, each Trace plug-in can
+  /// decide the nature of these identifiers and thus no assumptions can be 
made
+  /// regarding their ordering and sequentiality. The reason is that an
+  /// instruction might be encoded by the plug-in in a way that hides its 
actual
+  /// 0-based index in the trace, but it's still possible to efficiently find
+  /// it.
+  ///
+  /// Requirements:
+  /// - For a given thread, no two instructions have the same id.
+  /// - In terms of efficiency, moving the cursor to a given id should be as
+  ///   fast as possible, but not necessarily O(1). That's why the recommended
+  ///   way to traverse sequential instructions is to use the \a
+  ///   TraceCursor::Next() method and only use \a TraceCursor::GoToId(id)
+  ///   sparingly.
+
+  /// Make the cursor point to the item whose identifier is \p id.
+  ///
+  /// \return
+  /// \b true if the given identifier exists and the cursor effectively
+  /// moved. Otherwise, \b false is returned and the cursor doesn't change
+  /// its position.
+  virtual bool GoToId(lldb::user_id_t id) = 0;
+
+  /// \return
+  /// A unique identifier for the instruction or error this cursor is
+  /// pointing to.
+  virtual lldb::user_id_t GetId() const = 0;
+  /// \}
+
   /// Make the cursor point to an item in the trace based on an origin point 
and
  

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

[Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG05b4bf257124: [trace][intelpt] Introduce instruction Ids 
(authored by Walter Erquinigo wall...@fb.com).

Changed prior to commit:
  https://reviews.llvm.org/D122254?vs=418680=420969#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122254

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceStartStop.py
  lldb/test/API/commands/trace/TestTraceTSC.py
  lldb/test/API/commands/trace/TestTraceTimestampCounters.py

Index: lldb/test/API/commands/trace/TestTraceTSC.py
===
--- lldb/test/API/commands/trace/TestTraceTSC.py
+++ lldb/test/API/commands/trace/TestTraceTSC.py
@@ -19,7 +19,35 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["\[0\] \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+
+@testSBAPIAndCommands
+@skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+def testMultipleTscsPerThread(self):
+self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+self.expect("r")
+
+self.traceStartThread(enableTsc=True)
+
+# After each stop there'll be a new TSC
+self.expect("n")
+self.expect("n")
+self.expect("n")
+
+# We'll get the most recent instructions, with at least 3 different TSCs
+self.runCmd("thread trace dump instructions --tsc --raw")
+id_to_tsc = {}
+for line in self.res.GetOutput().splitlines():
+m = re.search("(.+): \[tsc=(.+)\].*", line)
+if m:
+id_to_tsc[int(m.group(1))] = m.group(2)
+self.assertEqual(len(id_to_tsc), 6)
+
+# We check that the values are right when dumping a specific id
+for id in range(0, 6):
+self.expect(f"thread trace dump instructions --tsc --id {id} -c 1",
+substrs=[f"{id}: [tsc={id_to_tsc[id]}]"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -32,7 +60,7 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["\[0\] \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -45,7 +73,7 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["\[0\] \[tsc=unavailable\] 0x00400511movl"])
+patterns=["0: \[tsc=unavailable\] 0x00400511movl"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===
--- lldb/test/API/commands/trace/TestTraceStartStop.py
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -114,29 +114,29 @@
 self.expect("thread trace dump instructions -f",
 patterns=[f'''thread #1: tid = .*
   a.out`main \+ 4 at main.cpp:2
-\[ 0\] {ADDRESS_REGEX}movl'''])
+0: {ADDRESS_REGEX}movl'''])
 
 # We can reconstruct the instructions up to the second line
 self.expect("n")
 self.expect("thread trace dump instructions -f",
 patterns=[f'''thread #1: tid = .*
   a.out`main \+ 4 at main.cpp:2
-\[ 0\] {ADDRESS_REGEX}movl .*
+0: {ADDRESS_REGEX}movl .*
   a.out`main \+ 11 at main.cpp:4
-\[ 1\] {ADDRESS_REGEX}movl .*
-\[ 2\] {ADDRESS_REGEX}jmp  .* ; <\+28> at main.cpp:4
-\[ 3\] {ADDRESS_REGEX}cmpl .*
-\[ 4\] {ADDRESS_REGEX}jle  .* ; <\+20> at main.cpp:5'''])
+1: {ADDRESS_REGEX}movl .*
+2: {ADDRESS_REGEX}jmp  .* ; <\+28> at main.cpp:4
+3: {ADDRESS_REGEX}cmpl .*
+4: {ADDRESS_REGEX}jle  .* ; <\+20> at main.cpp:5'''])
 
 self.expect("thread trace dump 

[Lldb-commits] [PATCH] D123203: [lldb] Silence GCC warnings about missing returns after fully covered switches. NFC.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123203

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

[Lldb-commits] [PATCH] D123202: [lldb] Fix detecting warning options for GCC

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Cool, I learned something new today. Thanks for adding that comment!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123202

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

[Lldb-commits] [PATCH] D122975: parallelize calling of Module::PreloadSymbols()

[Luboš Luňák via Phabricator via lldb-commits]

llunak updated this revision to Diff 420885.
llunak added a comment.

Rebased on ThreadPool groups (D123225 ) and 
adding such thread pool to LLDB (D123226 ).


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

https://reviews.llvm.org/D122975

Files:
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -62,6 +62,7 @@
 
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/Support/ThreadPool.h"
 
 #include 
 #include 
@@ -1623,6 +1624,18 @@
 void Target::ModulesDidLoad(ModuleList _list) {
   const size_t num_images = module_list.GetSize();
   if (m_valid && num_images) {
+if (GetPreloadSymbols()) {
+  // Try to preload symbols in parallel.
+  llvm::ThreadPool  = Debugger::GetThreadPool();
+  llvm::ThreadPool::TaskGroup group;
+  auto preload_symbols_fn = [&](size_t idx) {
+ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
+module_sp->PreloadSymbols();
+  };
+  for (size_t idx = 0; idx < num_images; ++idx)
+pool.async(group, preload_symbols_fn, idx);
+  pool.wait(group);
+}
 for (size_t idx = 0; idx < num_images; ++idx) {
   ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
   LoadScriptingResourceForModule(module_sp, this);
@@ -2170,11 +2183,6 @@
   });
 }
 
-// Preload symbols outside of any lock, so hopefully we can do this for
-// each library in parallel.
-if (GetPreloadSymbols())
-  module_sp->PreloadSymbols();
-
 llvm::SmallVector replaced_modules;
 for (ModuleSP _module_sp : old_modules) {
   if (m_images.GetIndexForModule(old_module_sp.get()) !=


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -62,6 +62,7 @@
 
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/Support/ThreadPool.h"
 
 #include 
 #include 
@@ -1623,6 +1624,18 @@
 void Target::ModulesDidLoad(ModuleList _list) {
   const size_t num_images = module_list.GetSize();
   if (m_valid && num_images) {
+if (GetPreloadSymbols()) {
+  // Try to preload symbols in parallel.
+  llvm::ThreadPool  = Debugger::GetThreadPool();
+  llvm::ThreadPool::TaskGroup group;
+  auto preload_symbols_fn = [&](size_t idx) {
+ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
+module_sp->PreloadSymbols();
+  };
+  for (size_t idx = 0; idx < num_images; ++idx)
+pool.async(group, preload_symbols_fn, idx);
+  pool.wait(group);
+}
 for (size_t idx = 0; idx < num_images; ++idx) {
   ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
   LoadScriptingResourceForModule(module_sp, this);
@@ -2170,11 +2183,6 @@
   });
 }
 
-// Preload symbols outside of any lock, so hopefully we can do this for
-// each library in parallel.
-if (GetPreloadSymbols())
-  module_sp->PreloadSymbols();
-
 llvm::SmallVector replaced_modules;
 for (ModuleSP _module_sp : old_modules) {
   if (m_images.GetIndexForModule(old_module_sp.get()) !=
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123226: [lldb] use one shared ThreadPool and task groups

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

You cannot access/find the global thread pool?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123226

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

[Lldb-commits] [PATCH] D123226: [lldb] use one shared ThreadPool and task groups

[Luboš Luňák via Phabricator via lldb-commits]

llunak created this revision.
llunak added reviewers: clayborg, labath.
llunak added a project: LLDB.
Herald added subscribers: JDevlieghere, arphaman.
Herald added a project: All.
llunak requested review of this revision.
Herald added a subscriber: lldb-commits.

As a preparation for parallelizing loading of symbols (D122975 
), it is necessary to use just one thread 
pool to avoid using a thread pool from inside a task of another thread pool.

I don't know if it's acceptable to have a static like this. If it isn't, then I 
don't know where to put it (I don't know how to access e.g. Debugger from 
ManualDWARFIndex).

The ThreadPool groups change is D123225  .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123226

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -13,6 +13,7 @@
 #include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
 #include "lldb/Core/DataFileCache.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Progress.h"
 #include "lldb/Symbol/ObjectFile.h"
@@ -94,7 +95,8 @@
 
   // Share one thread pool across operations to avoid the overhead of
   // recreating the threads.
-  llvm::ThreadPool pool(llvm::optimal_concurrency(units_to_index.size()));
+  llvm::ThreadPool  = Debugger::GetThreadPool();
+  llvm::ThreadPool::TaskGroup group;
 
   // Create a task runner that extracts dies for each DWARF unit in a
   // separate thread.
@@ -105,14 +107,14 @@
   // to wait until all units have been indexed in case a DIE in one
   // unit refers to another and the indexes accesses those DIEs.
   for (size_t i = 0; i < units_to_index.size(); ++i)
-pool.async(extract_fn, i);
-  pool.wait();
+pool.async(group, extract_fn, i);
+  pool.wait(group);
 
   // Now create a task runner that can index each DWARF unit in a
   // separate thread so we can index quickly.
   for (size_t i = 0; i < units_to_index.size(); ++i)
-pool.async(parser_fn, i);
-  pool.wait();
+pool.async(group, parser_fn, i);
+  pool.wait(group);
 
   auto finalize_fn = [this, , ](NameToDIE(IndexSet::*index)) {
 NameToDIE  = m_set.*index;
@@ -122,15 +124,15 @@
 progress.Increment();
   };
 
-  pool.async(finalize_fn, ::function_basenames);
-  pool.async(finalize_fn, ::function_fullnames);
-  pool.async(finalize_fn, ::function_methods);
-  pool.async(finalize_fn, ::function_selectors);
-  pool.async(finalize_fn, ::objc_class_selectors);
-  pool.async(finalize_fn, ::globals);
-  pool.async(finalize_fn, ::types);
-  pool.async(finalize_fn, ::namespaces);
-  pool.wait();
+  pool.async(group, finalize_fn, ::function_basenames);
+  pool.async(group, finalize_fn, ::function_fullnames);
+  pool.async(group, finalize_fn, ::function_methods);
+  pool.async(group, finalize_fn, ::function_selectors);
+  pool.async(group, finalize_fn, ::objc_class_selectors);
+  pool.async(group, finalize_fn, ::globals);
+  pool.async(group, finalize_fn, ::types);
+  pool.async(group, finalize_fn, ::namespaces);
+  pool.wait(group);
 
   SaveToCache();
 }
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -66,6 +66,7 @@
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -1963,3 +1964,8 @@
 
   return err;
 }
+
+llvm::ThreadPool ::GetThreadPool() {
+  static llvm::ThreadPool threadpool(llvm::optimal_concurrency());
+  return threadpool;
+}
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -49,6 +49,7 @@
 
 namespace llvm {
 class raw_ostream;
+class ThreadPool;
 }
 
 namespace lldb_private {
@@ -377,6 +378,9 @@
 return m_broadcaster_manager_sp;
   }
 
+  /// Shared thread poll. Use only with ThreadPool::TaskGroup.
+  static llvm::ThreadPool ();
+
   /// Report warning events.
   ///
   /// Progress events will be delivered to any debuggers that have listeners
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123203: [lldb] Silence GCC warnings about missing returns after fully covered switches. NFC.

[Pavel Labath via Phabricator via lldb-commits]

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

Yes, that's the way we usually deal with this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123203

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

[Lldb-commits] [PATCH] D123205: [lldb] Silence GCC/glibc warnings about ignoring the return value of write(). NFC.

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.

The gcc behavior is quite annoying here...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123205

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

[Lldb-commits] [PATCH] D123206: [lldb] Silence warnings about unused static variables in RegisterInfos_arm64.h

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

Move them to the only source file that included RegisterInfos_arm64.h
that actually used these variables.

This silences warnings like these:

  In file included from 
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp:42:
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:790:35: warning: 
‘g_register_infos_mte’ defined but not used [-Wunused-variable]
790 | static lldb_private::RegisterInfo g_register_infos_mte[] = {
|   ^~~~
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:787:35: warning: 
‘g_register_infos_pauth’ defined but not used [-Wunused-variable]
787 | static lldb_private::RegisterInfo g_register_infos_pauth[] = {
|   ^~


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123206

Files:
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h


Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -784,10 +784,5 @@
 {DEFINE_DBG(wcr, 15)}
 };
 // clang-format on
-static lldb_private::RegisterInfo g_register_infos_pauth[] = {
-DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
-
-static lldb_private::RegisterInfo g_register_infos_mte[] = {
-DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -72,6 +72,12 @@
 #include "RegisterInfos_arm64_sve.h"
 #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT
 
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,


Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -784,10 +784,5 @@
 {DEFINE_DBG(wcr, 15)}
 };
 // clang-format on
-static lldb_private::RegisterInfo g_register_infos_pauth[] = {
-DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
-
-static lldb_private::RegisterInfo g_register_infos_mte[] = {
-DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -72,6 +72,12 @@
 #include "RegisterInfos_arm64_sve.h"
 #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT
 
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123202: [lldb] Fix detecting warning options for GCC

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.

This is great. I've been bothered by this for quite a while, but I didn't 
realize the fix is that easy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123202

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

[Lldb-commits] [PATCH] D123205: [lldb] Silence GCC/glibc warnings about ignoring the return value of write(). NFC.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This matches how another similar warning is silenced in
Host/posix/ProcessLauncherPosixFork.cpp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123205

Files:
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -893,7 +893,8 @@
 return result;
   }
 
-  write(fds[WRITE], data, size);
+  int r = write(fds[WRITE], data, size);
+  (void)r;
   // Close the write end of the pipe, so that the command interpreter will exit
   // when it consumes all the data.
   llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -893,7 +893,8 @@
 return result;
   }
 
-  write(fds[WRITE], data, size);
+  int r = write(fds[WRITE], data, size);
+  (void)r;
   // Close the write end of the pipe, so that the command interpreter will exit
   // when it consumes all the data.
   llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123204: [lldb] Fix initialization of LazyBool/bool variables m_overwrite/m_overwrite_lazy. NFCI.

[Martin Storsjö via Phabricator via lldb-commits]

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

This silences a GCC warning after
1f7b58f2a50461493f083b2ed807b25e036286f6 
 / D122680 
:

lldb/source/Commands/CommandObjectCommands.cpp:1650:22: warning: enum constant 
in boolean context [-Wint-in-bool-context]
 1650 |   bool m_overwrite = eLazyBoolCalculate;

  |  ^~


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123204

Files:
  lldb/source/Commands/CommandObjectCommands.cpp


Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1480,7 +1480,7 @@
 std::string m_class_name;
 std::string m_funct_name;
 std::string m_short_help;
-LazyBool m_overwrite_lazy;
+LazyBool m_overwrite_lazy = eLazyBoolCalculate;
 ScriptedCommandSynchronicity m_synchronicity =
 eScriptedCommandSynchronicitySynchronous;
   };
@@ -1647,7 +1647,7 @@
   std::string m_cmd_name;
   CommandObjectMultiword *m_container = nullptr;
   std::string m_short_help;
-  bool m_overwrite = eLazyBoolCalculate;
+  bool m_overwrite = false;
   ScriptedCommandSynchronicity m_synchronicity =
   eScriptedCommandSynchronicitySynchronous;
 };


Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1480,7 +1480,7 @@
 std::string m_class_name;
 std::string m_funct_name;
 std::string m_short_help;
-LazyBool m_overwrite_lazy;
+LazyBool m_overwrite_lazy = eLazyBoolCalculate;
 ScriptedCommandSynchronicity m_synchronicity =
 eScriptedCommandSynchronicitySynchronous;
   };
@@ -1647,7 +1647,7 @@
   std::string m_cmd_name;
   CommandObjectMultiword *m_container = nullptr;
   std::string m_short_help;
-  bool m_overwrite = eLazyBoolCalculate;
+  bool m_overwrite = false;
   ScriptedCommandSynchronicity m_synchronicity =
   eScriptedCommandSynchronicitySynchronous;
 };
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123203: [lldb] Silence GCC warnings about missing returns after fully covered switches. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This silences warnings like this:

lldb/source/Core/DebuggerEvents.cpp: In member function ‘llvm::StringRef 
lldb_private::DiagnosticEventData::GetPrefix() const’:
lldb/source/Core/DebuggerEvents.cpp:55:1: warning: control reaches end of 
non-void function [-Wreturn-type]

  55 | }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123203

Files:
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Core/DebuggerEvents.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp


Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -102,6 +102,7 @@
 case BitOffset:
   return 10;
 }
+llvm_unreachable("Fully covered switch above!");
   };
 
   auto createError = [&](const char *expected_value_message) {
Index: lldb/source/Core/DebuggerEvents.cpp
===
--- lldb/source/Core/DebuggerEvents.cpp
+++ lldb/source/Core/DebuggerEvents.cpp
@@ -52,6 +52,7 @@
   case Type::Error:
 return "error";
   }
+  llvm_unreachable("Fully covered switch above!");
 }
 
 void DiagnosticEventData::Dump(Stream *s) const {
Index: lldb/source/Breakpoint/Breakpoint.cpp
===
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -1026,6 +1026,7 @@
 case eBreakpointEventTypeThreadChanged: return "thread changed";
 case eBreakpointEventTypeAutoContinueChanged: return "autocontinue 
changed";
   };
+  llvm_unreachable("Fully covered switch above!");
 }
 
 Log *Breakpoint::BreakpointEventData::GetLogChannel() {


Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -102,6 +102,7 @@
 case BitOffset:
   return 10;
 }
+llvm_unreachable("Fully covered switch above!");
   };
 
   auto createError = [&](const char *expected_value_message) {
Index: lldb/source/Core/DebuggerEvents.cpp
===
--- lldb/source/Core/DebuggerEvents.cpp
+++ lldb/source/Core/DebuggerEvents.cpp
@@ -52,6 +52,7 @@
   case Type::Error:
 return "error";
   }
+  llvm_unreachable("Fully covered switch above!");
 }
 
 void DiagnosticEventData::Dump(Stream *s) const {
Index: lldb/source/Breakpoint/Breakpoint.cpp
===
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -1026,6 +1026,7 @@
 case eBreakpointEventTypeThreadChanged: return "thread changed";
 case eBreakpointEventTypeAutoContinueChanged: return "autocontinue changed";
   };
+  llvm_unreachable("Fully covered switch above!");
 }
 
 Log *Breakpoint::BreakpointEventData::GetLogChannel() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123202: [lldb] Fix detecting warning options for GCC

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a subscriber: mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

If testing for a warning option like -Wno- with GCC, GCC won't
print any diagnostic at all, leading to the options being accepted
incorrectly. However later, if compiling a file that actually prints
another warning, GCC will also print warnings about these -Wno-
options being unrecognized.

This avoids warning spam like this, for every lldb source file that
produces build warnings with GCC:

  At global scope:
  cc1plus: warning: unrecognized command line option ‘-Wno-vla-extension’
  cc1plus: warning: unrecognized command line option ‘-Wno-deprecated-register’

This matches how such warning options are detected and added in
llvm/cmake/modules/HandleLLVMOptions.cmake, e.g. like this:

  check_cxx_compiler_flag("-Wclass-memaccess" CXX_SUPPORTS_CLASS_MEMACCESS_FLAG)
  append_if(CXX_SUPPORTS_CLASS_MEMACCESS_FLAG "-Wno-class-memaccess" 
CMAKE_CXX_FLAGS)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123202

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -168,22 +168,27 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# GCC silently accepts any -Wno- option, but warns about those options
+# being unrecognized only if the compilation triggers other warnings to be
+# printed. Therefore, check for whether the compiler supports options in the
+# form -W, and if supported, add the corresponding -Wno- option.
+
 # Disable GCC warnings
-check_cxx_compiler_flag("-Wno-deprecated-declarations" 
CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS 
"-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-declarations" 
CXX_SUPPORTS_DEPRECATED_DECLARATIONS)
+append_if(CXX_SUPPORTS_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-unknown-pragmas" CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS)
-append_if(CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wunknown-pragmas" CXX_SUPPORTS_UNKNOWN_PRAGMAS)
+append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-strict-aliasing" CXX_SUPPORTS_NO_STRICT_ALIASING)
-append_if(CXX_SUPPORTS_NO_STRICT_ALIASING "-Wno-strict-aliasing" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
+append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
 # Disable Clang warnings
-check_cxx_compiler_flag("-Wno-deprecated-register" 
CXX_SUPPORTS_NO_DEPRECATED_REGISTER)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
+append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-vla-extension" CXX_SUPPORTS_NO_VLA_EXTENSION)
-append_if(CXX_SUPPORTS_NO_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wvla-extension" CXX_SUPPORTS_VLA_EXTENSION)
+append_if(CXX_SUPPORTS_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
 
 # Disable MSVC warnings
 if( MSVC )


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -168,22 +168,27 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# GCC silently accepts any -Wno- option, but warns about those options
+# being unrecognized only if the compilation triggers other warnings to be
+# printed. Therefore, check for whether the compiler supports options in the
+# form -W, and if supported, add the corresponding -Wno- option.
+
 # Disable GCC warnings
-check_cxx_compiler_flag("-Wno-deprecated-declarations" CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-declarations" CXX_SUPPORTS_DEPRECATED_DECLARATIONS)
+append_if(CXX_SUPPORTS_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-unknown-pragmas" CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS)
-append_if(CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wunknown-pragmas" CXX_SUPPORTS_UNKNOWN_PRAGMAS)
+append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-strict-aliasing" CXX_SUPPORTS_NO_STRICT_ALIASING)

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

[David Spickett via Phabricator via lldb-commits]

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

Fix "expression"/"p" result by removing non-address bits in 
processElfCore::ReadMemory.
I missed that it overrides this. (it was nothing to do with the target 
addressable bits setting)

Add tests for that and process/target API use with the corefile.

I did try to avoid the mmap call to reduce the corefile further but couldn't 
find
any corefile filter that would still include the buffer without being > 300k.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122411

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/corefile
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c

Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
===
--- lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
@@ -13,6 +13,13 @@
   if (buf == MAP_FAILED)
 return 1;
 
+  // Some known values to go in the corefile, since we cannot
+  // write to corefile memory.
+  buf[0] = 'L';
+  buf[1] = 'L';
+  buf[2] = 'D';
+  buf[3] = 'B';
+
 #define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
 
   // Set top byte to something.
@@ -21,5 +28,11 @@
   // Address is now:
   // <8 bit top byte tag>
 
+  // Uncomment this line to crash and generate a corefile.
+  // Prints so we know what fixed address to look for in testing.
+  // printf("buf: %p\n", buf);
+  // printf("buf_with_non_address: %p\n", buf_with_non_address);
+  // *(char*)0 = 0;
+
   return 0; // Set break point at this line.
 }
Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
===
--- lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -163,6 +163,10 @@
 # Open log ignoring utf-8 decode errors
 with open(log_file, 'r', errors='ignore') as f:
 read_packet = "send packet: $x{:x}"
+
+# Since we allocated a 4k page that page will be aligned to 4k, which
+# also fits the 512 byte line size of the memory cache. So we can expect
+# to find a packet with its exact address.
 read_buf_packet = read_packet.format(buf)
 read_buf_with_non_address_packet = read_packet.format(buf_with_non_address)
 
@@ -180,3 +184,51 @@
 
 if not found_read_buf:
 self.fail("Did not find any reads of buf.")
+
+@skipIfLLVMTargetMissing("AArch64")
+def test_non_address_bit_memory_corefile(self):
+self.runCmd("target create --core corefile")
+
+self.expect("thread list", substrs=['stopped',
+'stop reason = signal SIGSEGV'])
+
+# No caching (the program/corefile are the cache) and no writing
+# to memory. So just check that tagged/untagged addresses read
+# the same location.
+
+# These are known addresses in the corefile, since we won't have symbols.
+buf = 0xa75a5000
+buf_with_non_address = 0xff0ba75a5000
+
+expected = ["4c 4c 44 42", "LLDB"]
+self.expect("memory read 0x{:x}".format(buf), substrs=expected)
+self.expect("memory read 0x{:x}".format(buf_with_non_address), substrs=expected)
+
+# This takes a more direct route to ReadMemory. As opposed to "memory read"
+# above that might fix the addresses up front for display reasons.
+self.expect("expression (char*)0x{:x}".format(buf), substrs=["LLDB"])
+self.expect("expression (char*)0x{:x}".format(buf_with_non_address), substrs=["LLDB"])
+
+def check_reads(addrs, method, num_bytes, expected):
+error = lldb.SBError()
+for addr in addrs:
+if num_bytes is None:
+got = method(addr, error)
+else:
+got = method(addr, num_bytes, error)
+
+self.assertTrue(error.Success())
+self.assertEqual(expected, got)
+
+addr_buf = lldb.SBAddress()
+addr_buf.SetLoadAddress(buf, self.target())
+addr_buf_with_non_address = lldb.SBAddress()
+addr_buf_with_non_address.SetLoadAddress(buf_with_non_address, self.target())
+check_reads([addr_buf, addr_buf_with_non_address], self.target().ReadMemory,
+4, b'LLDB')
+
+addrs = [buf, 

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

[David Spickett via Phabricator via lldb-commits]

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

Check that the "expression" command also treats pointers as equivalent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

Files:
  lldb/source/Target/Process.cpp
  lldb/source/Target/ProcessTrace.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/Makefile
  
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c

Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
@@ -0,0 +1,25 @@
+#include 
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+  size_t page_size = sysconf(_SC_PAGESIZE);
+  // Note that we allocate memory here because if we used
+  // stack or globals lldb might read it in the course of
+  // running to the breakpoint. Before the test can look
+  // for those reads.
+  char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE,
+   MAP_ANONYMOUS | MAP_SHARED, -1, 0);
+  if (buf == MAP_FAILED)
+return 1;
+
+#define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
+
+  // Set top byte to something.
+  char *buf_with_non_address = (char *)((size_t)buf | (size_t)0xff << 56);
+  sign_ptr(buf_with_non_address);
+  // Address is now:
+  // <8 bit top byte tag>
+
+  return 0; // Set break point at this line.
+}
Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -0,0 +1,182 @@
+"""
+Test that lldb removes non-address bits in situations where they would cause
+failures if not removed. Like when reading memory. Tests are done at command
+and API level because commands may remove non-address bits for display
+reasons which can make it seem like the operation as a whole works but at the
+API level it won't if we don't remove them there also.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxNonAddressBitMemoryAccessTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def setup_test(self):
+if not self.isAArch64PAuth():
+self.skipTest('Target must support pointer authentication.')
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c",
+line_number('main.c', '// Set break point at this line.'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+def check_cmd_read_write(self, write_to, read_from, data):
+self.runCmd("memory write {} {}".format(write_to, data))
+self.expect("memory read {}".format(read_from),
+substrs=[data])
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_non_address_bit_memory_read_write_cmds(self):
+self.setup_test()
+
+# Writes should be visible through either pointer
+self.check_cmd_read_write("buf", "buf", "01 02 03 04")
+self.check_cmd_read_write("buf_with_non_address", "buf_with_non_address", "02 03 04 05")
+self.check_cmd_read_write("buf", "buf_with_non_address", "03 04 05 06")
+self.check_cmd_read_write("buf_with_non_address", "buf", "04 05 06 07")
+
+# Printing either should get the same result
+self.expect("expression -f hex -- *(uint32_t*)buf", substrs=["0x07060504"])
+self.expect("expression -f hex -- *(uint32_t*)buf_with_non_address",
+substrs=["0x07060504"])
+
+def get_ptr_values(self):
+frame  = self.process().GetThreadAtIndex(0).GetFrameAtIndex(0)
+buf = frame.FindVariable("buf").GetValueAsUnsigned()
+buf_with_non_address = frame.FindVariable("buf_with_non_address").GetValueAsUnsigned()
+return buf, buf_with_non_address
+
+def check_api_read_write(self, write_to, read_from, data):
+error = lldb.SBError()
+written = self.process().WriteMemory(write_to, data, error)
+self.assertTrue(error.Success())
+self.assertEqual(len(data),