date:20230830

[Lldb-commits] [PATCH] D159150: [lldb][NFCI] Replace bespoke iterator check with std::next

2023-08-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159150

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

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

2023-08-30 Thread Kito Cheng via Phabricator via lldb-commits

kito-cheng added inline comments.



Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1554
+// Turn them on by default now, since everyone seems to use them
+features_str += "+a,+m,";
   }

DavidSpickett wrote:
> ted wrote:
> > DavidSpickett wrote:
> > > You might want to take the lead from AArch64 here:
> > > ```
> > > // If any AArch64 variant, enable latest ISA with all extensions.
> > > ```
> > > If "+all" doesn't already work for riscv then you don't have to go and 
> > > make that work right now.
> > > 
> > > But in general we decided that much like llvm-objdump, we'll try to 
> > > disassemble any possible encoding. If the user happens to point the 
> > > disassembler at garbage that looks like a fancy extension on a cpu from 
> > > 20 years ago, that's on them.
> > While I like the "turn on the latest" philosophy in general, for RISC-V we 
> > don't want to do that. It's modular architecture means features can be 
> > turned on and off when a core is designed, so one core might have +d 
> > (floating point double), while an newer core might not have any floating 
> > point at all. I'm inclined to leave the features as they are now, with a 
> > and m turned on.
> Fair enough, I see that +all does not in fact work for risc-v llvm-objdump 
> which backs that up. I guess you'd have to pass through some kind of object 
> attributes to detect some of them.
RISC-V have ELF attribute* to record which arch used for this binary, so you 
may need to add something at 
`lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp` to read that and record 
something in ArchSpec like what `ObjectFileELF::ParseARMAttributes` do.

* 
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#tag_riscv_arch-5-ntbssubarch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159101

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

[Lldb-commits] [lldb] 394e52a - [lldb] NFC reflow comments in WatchpointLocations

2023-08-30 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2023-08-30T17:58:44-07:00
New Revision: 394e52a0bb576508e0fed59ab267e75cf1350fca

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

LOG: [lldb] NFC reflow comments in WatchpointLocations

Reading through this header, many of the comment
line breaks are a mess after various code reformats.

Added: 


Modified: 
lldb/include/lldb/Breakpoint/WatchpointOptions.h

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/WatchpointOptions.h 
b/lldb/include/lldb/Breakpoint/WatchpointOptions.h
index 369120e985342b..cedd1b186d659a 100644
--- a/lldb/include/lldb/Breakpoint/WatchpointOptions.h
+++ b/lldb/include/lldb/Breakpoint/WatchpointOptions.h
@@ -59,24 +59,20 @@ class WatchpointOptions {
   // synchronous callbacks: 1) They should NOT resume the target themselves.
   // Just return false if you want the target to restart. 2) Watchpoints with
   // synchronous callbacks can't have conditions (or rather, they can have
-  // them, but they
-  //won't do anything.  Ditto with ignore counts, etc...  You are supposed
-  //to control that all through the
-  //callback.
+  // them, but they won't do anything.  Ditto with ignore counts, etc...
+  // You are supposed to control that all through the callback.
   // Asynchronous callbacks get run as part of the "ShouldStop" logic in the
   // thread plan.  The logic there is:
   //   a) If the watchpoint is thread specific and not for this thread, 
continue
-  //   w/o running the callback.
+  //  w/o running the callback.
   //   b) If the ignore count says we shouldn't stop, then ditto.
   //   c) If the condition says we shouldn't stop, then ditto.
   //   d) Otherwise, the callback will get run, and if it returns true we will
-  //   stop, and if false we won't.
+  //  stop, and if false we won't.
   //  The asynchronous callback can run the target itself, but at present that
-  //  should be the last action the
-  //  callback does.  We will relax this condition at some point, but it will
-  //  take a bit of plumbing to get
+  //  should be the last action the callback does.  We will relax this
+  //  condition at some point, but it will take a bit of plumbing to get
   //  that to work.
-  //
 
   /// Adds a callback to the watchpoint option set.
   ///
@@ -87,8 +83,8 @@ class WatchpointOptions {
   ///A baton which will get passed back to the callback when it is invoked.
   ///
   /// \param[in] synchronous
-  ///Whether this is a synchronous or asynchronous callback.  See 
discussion
-  ///above.
+  ///Whether this is a synchronous or asynchronous callback.
+  ///See discussion above.
   void SetCallback(WatchpointHitCallback callback,
const lldb::BatonSP _sp, bool synchronous = false);
 
@@ -102,11 +98,9 @@ class WatchpointOptions {
   ///
   /// \param[in] context
   ///The context in which the callback is to be invoked.  This includes the
-  ///stop event, the
-  ///execution context of the stop (since you might hit the same watchpoint
-  ///on multiple threads) and
-  ///whether we are currently executing synchronous or asynchronous
-  ///callbacks.
+  ///stop event, the execution context of the stop (since you might hit
+  ///the same watchpoint on multiple threads) and whether we are currently
+  ///executing synchronous or asynchronous callbacks.
   ///
   /// \param[in] watch_id
   ///The watchpoint ID that owns this option set.
@@ -129,23 +123,24 @@ class WatchpointOptions {
   /// The baton.
   Baton *GetBaton();
 
-  /// Fetch  a const version of the baton from the callback.
+  /// Fetch a const version of the baton from the callback.
   ///
   /// \return
   /// The baton.
   const Baton *GetBaton() const;
 
   /// Return the current thread spec for this option. This will return nullptr
-  /// if the no thread specifications have been set for this Option yet.
+  /// if the no thread specifications have been set for this WatchpointOptions
+  /// yet.
+  ///
   /// \return
   /// The thread specification pointer for this option, or nullptr if none
-  /// has
-  /// been set yet.
+  /// has been set yet.
   const ThreadSpec *GetThreadSpecNoCreate() const;
 
-  /// Returns a pointer to the ThreadSpec for this option, creating it. if it
-  /// hasn't been created already.   This API is used for setting the
-  /// ThreadSpec items for this option.
+  /// Returns a pointer to the ThreadSpec for this option, creating it if it
+  /// hasn't been created already. This API is used for setting the
+  /// ThreadSpec items for this WatchpointOptions.
   ThreadSpec *GetThreadSpec();
 
   void SetThreadID(lldb::tid_t thread_id);
@@ -184,11 +179,7 @@ class WatchpointOptions {

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

- Fix inconsistencies and use `use_color` everywhere


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

https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -96,7 +96,8 @@
 Disassembler *ThreadPlanAssemblyTracer::GetDisassembler() {
   if (!m_disassembler_sp)
 m_disassembler_sp = Disassembler::FindPlugin(
-m_process.GetTarget().GetArchitecture(), nullptr, nullptr);
+m_process.GetTarget().GetArchitecture(), nullptr,
+m_process.GetTarget().GetDebugger().GetUseColor(), nullptr);
   return m_disassembler_sp.get();
 }
 
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -70,7 +70,7 @@
 
 const bool prefer_file_cache = true;
 DisassemblerSP disasm_sp(Disassembler::DisassembleBytes(
-m_arch, nullptr, nullptr, range.GetBaseAddress(), opcode_data,
+m_arch, nullptr, nullptr, false, range.GetBaseAddress(), opcode_data,
 opcode_size, 9, prefer_file_cache));
 
 Log *log = GetLog(LLDBLog::Unwind);
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -22,8 +22,8 @@
 
 class DisassemblerLLVMC : public lldb_private::Disassembler {
 public:
-  DisassemblerLLVMC(const lldb_private::ArchSpec ,
-const char *flavor /* = NULL */);
+  DisassemblerLLVMC(const lldb_private::ArchSpec , const char *flavor,
+bool use_color = false);
 
   ~DisassemblerLLVMC() override;
 
@@ -35,7 +35,8 @@
   static llvm::StringRef GetPluginNameStatic() { return "llvm-mc"; }
 
   static lldb::DisassemblerSP CreateInstance(const lldb_private::ArchSpec ,
- const char *flavor);
+ const char *flavor,
+ bool use_color);
 
   size_t DecodeInstructions(const lldb_private::Address _addr,
 const lldb_private::DataExtractor ,
@@ -72,6 +73,7 @@
   InstructionLLVMC *m_inst;
   std::mutex m_mutex;
   bool m_data_from_file;
+  bool m_use_color;
   // Save the AArch64 ADRP instruction word and address it was at,
   // in case the next instruction is an ADD to the same register;
   // this is a pc-relative address calculation and we need both
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -54,7 +54,7 @@
 public:
   static std::unique_ptr
   Create(const char *triple, const char *cpu, const char *features_str,
- unsigned flavor, DisassemblerLLVMC );
+ unsigned flavor, bool use_color, DisassemblerLLVMC );
 
   ~MCDisasmInstance() = default;
 
@@ -1227,7 +1227,7 @@
 std::unique_ptr
 DisassemblerLLVMC::MCDisasmInstance::Create(const char *triple, const char *cpu,
 const char *features_str,
-unsigned flavor,
+unsigned flavor, bool use_color,
 DisassemblerLLVMC ) {
   using Instance = std::unique_ptr;
 
@@ -1291,6 +1291,7 @@
 return Instance();
 
   instr_printer_up->setPrintBranchImmAsAddress(true);
+  instr_printer_up->setUseColor(use_color);
 
   // Not all targets may have registered createMCInstrAnalysis().
   std::unique_ptr instr_analysis_up(
@@ -1344,6 +1345,8 @@
   llvm::raw_string_ostream inst_stream(inst_string);
   llvm::raw_string_ostream comments_stream(comments_string);
 
+

[Lldb-commits] [PATCH] D159237: [lldb][NFCI] Remove unneeded ConstString conversions

2023-08-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159237

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

[Lldb-commits] [PATCH] D158833: [lldb] Move ScriptInterpreter Interfaces to subdirectory (NFC)

2023-08-30 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

In D158833#4626023 , @mib wrote:

> In D158833#4625775 , @bulbazord 
> wrote:
>
>> Re-organizing the paths seems okay to me, especially since this is going to 
>> grow further. I think the header guards are going to need some adjustment 
>> though.
>
> TBH, I didn't even know they were guidelines wrt header guards 
>
> For long file names, I've been using this pattern in the past of separating 
> the words by `_` in the header guards, which also makes it more readable.
>
> I don't think that causes any harm but if you feel strongly about it I can 
> change it.

I don't think it's a huge deal personally. If you look hard enough, LLDB (and 
probably LLVM) are full of tiny mistakes in the header guard from moving files 
around.

For reference, the LLVM coding standards talks about it here: 
https://llvm.org/docs/CodingStandards.html#header-guard


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158833

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

[Lldb-commits] [PATCH] D159142: [lldb] Add support for recognizing swift ast sections in object files

2023-08-30 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

It seems like there is consensus for actually implementing this functionality 
but that there were some questions about the actual string. I hope everyone 
finds my reasoning understandable. If nobody has a strong objection to the 
actual string, I will land this tomorrow morning or afternoon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159142

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

[Lldb-commits] [PATCH] D159237: [lldb][NFCI] Remove unneeded ConstString conversions

2023-08-30 Thread Alex Langford via Phabricator via lldb-commits

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

ConstString can be implicitly converted into a llvm::StringRef. This is
very useful in many places, but it also hides places where we are
creating a ConstString only to use it as a StringRef for the entire
lifespan of the ConstString object.

I locally removed the implicit conversion and found some of the places we
were doing this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159237

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -76,7 +76,7 @@
 class ThreadOptionValueProperties
 : public Cloneable {
 public:
-  ThreadOptionValueProperties(ConstString name) : Cloneable(name) {}
+  ThreadOptionValueProperties(llvm::StringRef name) : Cloneable(name) {}
 
   const Property *
   GetPropertyAtIndex(size_t idx,
@@ -100,8 +100,7 @@
 
 ThreadProperties::ThreadProperties(bool is_global) : Properties() {
   if (is_global) {
-m_collection_sp =
-std::make_shared(ConstString("thread"));
+m_collection_sp = std::make_shared("thread");
 m_collection_sp->Initialize(g_thread_properties);
   } else
 m_collection_sp =
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -4062,7 +4062,7 @@
 class TargetOptionValueProperties
 : public Cloneable {
 public:
-  TargetOptionValueProperties(ConstString name) : Cloneable(name) {}
+  TargetOptionValueProperties(llvm::StringRef name) : Cloneable(name) {}
 
   const Property *
   GetPropertyAtIndex(size_t idx,
@@ -4098,7 +4098,7 @@
OptionValueProperties> {
 public:
   TargetExperimentalOptionValueProperties()
-  : Cloneable(ConstString(Properties::GetExperimentalSettingsName())) {}
+  : Cloneable(Properties::GetExperimentalSettingsName()) {}
 };
 
 TargetExperimentalProperties::TargetExperimentalProperties()
@@ -4152,8 +4152,7 @@
 "errors if the setting is not present.",
 true, m_experimental_properties_up->GetValueProperties());
   } else {
-m_collection_sp =
-std::make_shared(ConstString("target"));
+m_collection_sp = std::make_shared("target");
 m_collection_sp->Initialize(g_target_properties);
 m_experimental_properties_up =
 std::make_unique();
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -89,7 +89,7 @@
 class ProcessOptionValueProperties
 : public Cloneable {
 public:
-  ProcessOptionValueProperties(ConstString name) : Cloneable(name) {}
+  ProcessOptionValueProperties(llvm::StringRef name) : Cloneable(name) {}
 
   const Property *
   GetPropertyAtIndex(size_t idx,
@@ -146,8 +146,7 @@
OptionValueProperties> {
 public:
   ProcessExperimentalOptionValueProperties()
-  : Cloneable(
-ConstString(Properties::GetExperimentalSettingsName())) {}
+  : Cloneable(Properties::GetExperimentalSettingsName()) {}
 };
 
 ProcessExperimentalProperties::ProcessExperimentalProperties()
@@ -162,8 +161,7 @@
 {
   if (process == nullptr) {
 // Global process properties, set them up one time
-m_collection_sp =
-std::make_shared(ConstString("process"));
+m_collection_sp = std::make_shared("process");
 m_collection_sp->Initialize(g_process_properties);
 m_collection_sp->AppendProperty(
 "thread", "Settings specific to threads.", true,
Index: lldb/source/Target/Platform.cpp

[Lldb-commits] [PATCH] D159234: [llvm-objdump] Enable assembly highlighting in llvm-objdump

2023-08-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: MaskRay, jhenderson.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Enable assembly highlighting in llvm-objdump.


https://reviews.llvm.org/D159234

Files:
  llvm/include/llvm/Support/FormattedStream.h
  llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
  llvm/tools/llvm-objdump/llvm-objdump.cpp


Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -901,6 +901,7 @@
   if (!InstPrinter)
 reportError(Obj.getFileName(),
 "no instruction printer for target " + TripleName);
+  InstPrinter->setUseColor(!NoColor);
   InstPrinter->setPrintImmHex(PrintImmHex);
   InstPrinter->setPrintBranchImmAsAddress(true);
   InstPrinter->setSymbolizeOperands(SymbolizeOperands);
Index: llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
===
--- /dev/null
+++ llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
@@ -0,0 +1,25 @@
+// RUN: llvm-mc -triple arm64-apple-macosx %s -filetype=obj -o %t
+// RUN: llvm-objdump --color --disassemble %t | FileCheck %s 
--check-prefix=COLOR
+// RUN: llvm-objdump --no-color --disassemble %t | FileCheck %s 
--check-prefix=NOCOLOR
+
+subsp, sp, #16
+strw0, [sp, #12]
+ldrw8, [sp, #12]
+ldrw9, [sp, #12]
+mulw0, w8, w9
+addsp, sp, #16
+
+
+// COLOR: sub  [0;36msp, [0;36msp, [0;31m#0x10
+// COLOR: str  [0;36mw0, [[0;36msp, [0;31m#0xc]
+// COLOR: ldr  [0;36mw8, [[0;36msp, [0;31m#0xc]
+// COLOR: ldr  [0;36mw9, [[0;36msp, [0;31m#0xc]
+// COLOR: mul  [0;36mw0, [0;36mw8, [0;36mw9
+// COLOR: add  [0;36msp, [0;36msp, [0;31m#0x10
+
+// NOCOLOR: subsp, sp, #0x10
+// NOCOLOR: strw0, [sp, #0xc]
+// NOCOLOR: ldrw8, [sp, #0xc]
+// NOCOLOR: ldrw9, [sp, #0xc]
+// NOCOLOR: mulw0, w8, w9
+// NOCOLOR: addsp, sp, #0x10
Index: llvm/include/llvm/Support/FormattedStream.h
===
--- llvm/include/llvm/Support/FormattedStream.h
+++ llvm/include/llvm/Support/FormattedStream.h
@@ -145,11 +145,6 @@
 return *this;
   }
 
-  raw_ostream (enum Colors Color, bool Bold, bool BG) override {
-TheStream->changeColor(Color, Bold, BG);
-return *this;
-  }
-
   bool is_displayed() const override {
 return TheStream->is_displayed();
   }


Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -901,6 +901,7 @@
   if (!InstPrinter)
 reportError(Obj.getFileName(),
 "no instruction printer for target " + TripleName);
+  InstPrinter->setUseColor(!NoColor);
   InstPrinter->setPrintImmHex(PrintImmHex);
   InstPrinter->setPrintBranchImmAsAddress(true);
   InstPrinter->setSymbolizeOperands(SymbolizeOperands);
Index: llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
===
--- /dev/null
+++ llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
@@ -0,0 +1,25 @@
+// RUN: llvm-mc -triple arm64-apple-macosx %s -filetype=obj -o %t
+// RUN: llvm-objdump --color --disassemble %t | FileCheck %s --check-prefix=COLOR
+// RUN: llvm-objdump --no-color --disassemble %t | FileCheck %s --check-prefix=NOCOLOR
+
+sub	sp, sp, #16
+str	w0, [sp, #12]
+ldr	w8, [sp, #12]
+ldr	w9, [sp, #12]
+mul	w0, w8, w9
+add	sp, sp, #16
+
+
+// COLOR: sub	[0;36msp, [0;36msp, [0;31m#0x10
+// COLOR: str	[0;36mw0, [[0;36msp, [0;31m#0xc]
+// COLOR: ldr	[0;36mw8, [[0;36msp, [0;31m#0xc]
+// COLOR: ldr	[0;36mw9, [[0;36msp, [0;31m#0xc]
+// COLOR: mul	[0;36mw0, [0;36mw8, [0;36mw9
+// COLOR: add	[0;36msp, [0;36msp, [0;31m#0x10
+
+// NOCOLOR: sub	sp, sp, #0x10
+// NOCOLOR: str	w0, [sp, #0xc]
+// NOCOLOR: ldr	w8, [sp, #0xc]
+// NOCOLOR: ldr	w9, [sp, #0xc]
+// NOCOLOR: mul	w0, w8, w9
+// NOCOLOR: add	sp, sp, #0x10
Index: llvm/include/llvm/Support/FormattedStream.h
===
--- llvm/include/llvm/Support/FormattedStream.h
+++ llvm/include/llvm/Support/FormattedStream.h
@@ -145,11 +145,6 @@
 return *this;
   }
 
-  raw_ostream (enum Colors Color, bool Bold, bool BG) override {
-TheStream->changeColor(Color, Bold, BG);
-return *this;
-  }
-
   bool is_displayed() const override {
 return TheStream->is_displayed();
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3621f83 - Don't enable stdin/out with a no-output test program

2023-08-30 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2023-08-30T15:12:31-07:00
New Revision: 3621f83804809fa50892ee81d9b3854ec2e5074f

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

LOG: Don't enable stdin/out with a no-output test program

A followon to  https://reviews.llvm.org/D158237 ,
where this text can print stdout text when run
under address-sanitizer, and the test harness
does not expect any output, resulting in a test
failure on a sanitizer CI bot.

Added: 


Modified: 
lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp

Removed: 




diff  --git a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp 
b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
index 1f53b6d9d9944e..66e92415911afb 100644
--- a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -24,8 +24,9 @@ using namespace llvm;
 TEST_F(TestBase, LaunchModePreservesEnvironment) {
   putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE"));
 
-  auto ClientOr = TestClient::launch(getLogFileName(),
- {getInferiorPath("environment_check")});
+  auto ClientOr = TestClient::launchCustom(
+  getLogFileName(),
+  /* disable_stdio */ true, {}, {getInferiorPath("environment_check")});
   ASSERT_THAT_EXPECTED(ClientOr, Succeeded());
   auto  = **ClientOr;
 
@@ -56,8 +57,9 @@ TEST_F(TestBase, DS_TEST(DebugserverEnv)) {
 }
 
 TEST_F(TestBase, LLGS_TEST(vAttachRichError)) {
-  auto ClientOr = TestClient::launch(getLogFileName(),
- {getInferiorPath("environment_check")});
+  auto ClientOr = TestClient::launchCustom(
+  getLogFileName(),
+  /* disable_stdio */ true, {}, {getInferiorPath("environment_check")});
   ASSERT_THAT_EXPECTED(ClientOr, Succeeded());
   auto  = **ClientOr;
 



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

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

2023-08-30 Thread Ted Woodward via Phabricator via lldb-commits

ted added a comment.

In D159101#4627705 , @DavidSpickett 
wrote:

> I have some vague idea that maybe we could put a hack in the test suite to 
> use the qemu-user platform instead of lldb-server. To at least give it a go, 
> but I haven't tried it myself yet.

Downstream I've got a hack to the qemu-user platform that allows it to be 
automatically selected for riscv32/64-unknown-linux. I didn't think it would be 
appropriate upstream. So I can launch lldb with a riscv binary and do a run, 
and it launches qemu-riscv32/64. I'm planning on running the test suite on 
upstream + this patch, with that hack, and seeing what happens.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159101

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

[Lldb-commits] [PATCH] D159142: [lldb] Add support for recognizing swift ast sections in object files

2023-08-30 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/source/Core/Section.cpp:153
+  case eSectionTypeSwiftModules:
+return "swift-modules";
   }

compnerd wrote:
> kastiglione wrote:
> > I wonder if this should be "swiftmodules". I have never seen it spelled 
> > with a hyphen.
> Is it actually more than one module?  If not, `swift-module` would make sense 
> given the DWARF cases - it is a "Swift Module" converted to lower kebab case.
I added a hyphen to match the previous section types, but I'm not tied to the 
naming scheme.
I don't know if there is more than one module, but the enum case is plural so I 
made the string plural too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159142

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

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-30 Thread Jason Molenda via Phabricator via lldb-commits

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

LGTM this is just mechanical passing around of the user's setting though the 
layers.




Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h:76
   bool m_data_from_file;
+  bool m_use_colors;
   // Save the AArch64 ADRP instruction word and address it was at,

Most of this patch uses `use_color`, but in this class there's a couple use of 
`colors`. I don't think it was an intentional difference, might as well make 
them all the same.  In llvm the MCInstPrinter has a setUseColor but 
llvm::raw_string_ostream has a enable_colors, maybe that's where the plural 
came in.


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

https://reviews.llvm.org/D159164

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

[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

2023-08-30 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

Sorry, I got distracted, but after the fact I also agree this is reasonable.

We might actually be able to support externally adding C++ based synthetic 
providers, though I think the best way to do that is to add a way to make an 
SBTypeSynthetic from the appropriate C++ class, and then you could make that 
and add it in the lldb init function in the C++ library that adds the provider.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158010

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

[Lldb-commits] [lldb] 79d5d9a - [lldb] Allow synthetic providers in C++ and fix linking problems

2023-08-30 Thread walter erquinigo via lldb-commits


Author: walter erquinigo
Date: 2023-08-30T14:14:28-04:00
New Revision: 79d5d9a0824ddcd5493a451e5009dd6393646e51

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

LOG: [lldb] Allow synthetic providers in C++ and fix linking problems

- Allow the definition of synthetic formatters in C++ even when LLDB is built 
without python scripting support.
- Fix linking problems with the CXXSyntheticChildren

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/TypeSynthetic.h
lldb/source/Commands/CommandObjectType.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/DataFormatters/TypeSynthetic.cpp

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/TypeSynthetic.h 
b/lldb/include/lldb/DataFormatters/TypeSynthetic.h
index 890a6eb4f44876..41be9b7efda8fd 100644
--- a/lldb/include/lldb/DataFormatters/TypeSynthetic.h
+++ b/lldb/include/lldb/DataFormatters/TypeSynthetic.h
@@ -228,9 +228,9 @@ class SyntheticChildren {
 uint32_t m_flags = lldb::eTypeOptionCascade;
   };
 
-  SyntheticChildren(const Flags ) : m_flags(flags) {}
+  SyntheticChildren(const Flags );
 
-  virtual ~SyntheticChildren() = default;
+  virtual ~SyntheticChildren();
 
   bool Cascades() const { return m_flags.GetCascades(); }
 
@@ -239,8 +239,8 @@ class SyntheticChildren {
   bool SkipsReferences() const { return m_flags.GetSkipReferences(); }
 
   bool NonCacheable() const { return m_flags.GetNonCacheable(); }
-  
-  bool WantsDereference() const { return 
m_flags.GetFrontEndWantsDereference();} 
+
+  bool WantsDereference() const { return 
m_flags.GetFrontEndWantsDereference();}
 
   void SetCascades(bool value) { m_flags.SetCascades(value); }
 
@@ -361,9 +361,9 @@ class CXXSyntheticChildren : public SyntheticChildren {
 lldb::ValueObjectSP)>
   CreateFrontEndCallback;
   CXXSyntheticChildren(const SyntheticChildren::Flags ,
-   const char *description, CreateFrontEndCallback 
callback)
-  : SyntheticChildren(flags), m_create_callback(std::move(callback)),
-m_description(description ? description : "") {}
+   const char *description, CreateFrontEndCallback 
callback);
+
+  virtual ~CXXSyntheticChildren();
 
   bool IsScripted() override { return false; }
 

diff  --git a/lldb/source/Commands/CommandObjectType.cpp 
b/lldb/source/Commands/CommandObjectType.cpp
index e6dd63a6cb2138..2969f82f95882e 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/lldb/source/Commands/CommandObjectType.cpp
@@ -2171,8 +2171,6 @@ class CommandObjectTypeFilterList
"Show a list of current filters.") {}
 };
 
-#if LLDB_ENABLE_PYTHON
-
 // CommandObjectTypeSynthList
 
 class CommandObjectTypeSynthList
@@ -2184,8 +2182,6 @@ class CommandObjectTypeSynthList
 "Show a list of current synthetic providers.") {}
 };
 
-#endif
-
 // CommandObjectTypeFilterDelete
 
 class CommandObjectTypeFilterDelete : public CommandObjectTypeFormatterDelete {
@@ -2197,8 +2193,6 @@ class CommandObjectTypeFilterDelete : public 
CommandObjectTypeFormatterDelete {
   ~CommandObjectTypeFilterDelete() override = default;
 };
 
-#if LLDB_ENABLE_PYTHON
-
 // CommandObjectTypeSynthDelete
 
 class CommandObjectTypeSynthDelete : public CommandObjectTypeFormatterDelete {
@@ -2210,7 +2204,6 @@ class CommandObjectTypeSynthDelete : public 
CommandObjectTypeFormatterDelete {
   ~CommandObjectTypeSynthDelete() override = default;
 };
 
-#endif
 
 // CommandObjectTypeFilterClear
 
@@ -,7 +2215,6 @@ class CommandObjectTypeFilterClear : public 
CommandObjectTypeFormatterClear {
 "Delete all existing filter.") {}
 };
 
-#if LLDB_ENABLE_PYTHON
 // CommandObjectTypeSynthClear
 
 class CommandObjectTypeSynthClear : public CommandObjectTypeFormatterClear {
@@ -2393,7 +2385,6 @@ bool CommandObjectTypeSynthAdd::AddSynth(ConstString 
type_name,
   return true;
 }
 
-#endif
 #define LLDB_OPTIONS_type_filter_add
 #include "CommandOptions.inc"
 
@@ -2941,8 +2932,6 @@ class CommandObjectTypeFormat : public 
CommandObjectMultiword {
   ~CommandObjectTypeFormat() override = default;
 };
 
-#if LLDB_ENABLE_PYTHON
-
 class CommandObjectTypeSynth : public CommandObjectMultiword {
 public:
   CommandObjectTypeSynth(CommandInterpreter )
@@ -2970,8 +2959,6 @@ class CommandObjectTypeSynth : public 
CommandObjectMultiword {
   ~CommandObjectTypeSynth() override = default;
 };
 
-#endif
-
 class CommandObjectTypeFilter : public CommandObjectMultiword {
 public:
   CommandObjectTypeFilter(CommandInterpreter )
@@ -3056,10 +3043,8 @@ CommandObjectType::CommandObjectType(CommandInterpreter 
)

[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

2023-08-30 Thread walter erquinigo via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG79d5d9a0824d: [lldb] Allow synthetic providers in C++ and 
fix linking problems (authored by wallace).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158010

Files:
  lldb/include/lldb/DataFormatters/TypeSynthetic.h
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/DataFormatters/TypeSynthetic.cpp

Index: lldb/source/DataFormatters/TypeSynthetic.cpp
===
--- lldb/source/DataFormatters/TypeSynthetic.cpp
+++ lldb/source/DataFormatters/TypeSynthetic.cpp
@@ -84,6 +84,27 @@
   return std::string(sstr.GetString());
 }
 
+SyntheticChildren::SyntheticChildren(const Flags ) : m_flags(flags) {}
+
+SyntheticChildren::~SyntheticChildren() = default;
+
+CXXSyntheticChildren::CXXSyntheticChildren(
+const SyntheticChildren::Flags , const char *description,
+CreateFrontEndCallback callback)
+: SyntheticChildren(flags), m_create_callback(std::move(callback)),
+  m_description(description ? description : "") {}
+
+CXXSyntheticChildren::~CXXSyntheticChildren() = default;
+
+bool SyntheticChildren::IsScripted() { return false; }
+
+std::string SyntheticChildren::GetDescription() { return ""; }
+
+SyntheticChildrenFrontEnd::AutoPointer
+SyntheticChildren::GetFrontEnd(ValueObject ) {
+  return nullptr;
+}
+
 std::string CXXSyntheticChildren::GetDescription() {
   StreamString sstr;
   sstr.Printf("%s%s%s %s", Cascades() ? "" : " (not cascading)",
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -218,10 +218,8 @@
 SetValueFormat(DataVisualization::GetFormat(*this, eNoDynamicValues));
 SetSummaryFormat(
 DataVisualization::GetSummaryFormat(*this, GetDynamicValueType()));
-#if LLDB_ENABLE_PYTHON
 SetSyntheticChildren(
 DataVisualization::GetSyntheticChildren(*this, GetDynamicValueType()));
-#endif
   }
 
   return any_change;
@@ -1153,7 +1151,7 @@
 Stream , ValueObjectRepresentationStyle val_obj_display,
 Format custom_format, PrintableRepresentationSpecialCases special,
 bool do_dump_error) {
-
+
   // If the ValueObject has an error, we might end up dumping the type, which
   // is useful, but if we don't even have a type, then don't examine the object
   // further as that's not meaningful, only the error is.
@@ -2766,15 +2764,15 @@
 
 ValueObjectSP ValueObject::Cast(const CompilerType _type) {
   // Only allow casts if the original type is equal or larger than the cast
-  // type.  We don't know how to fetch more data for all the ConstResult types, 
+  // type.  We don't know how to fetch more data for all the ConstResult types,
   // so we can't guarantee this will work:
   Status error;
   CompilerType my_type = GetCompilerType();
 
-  ExecutionContextScope *exe_scope 
+  ExecutionContextScope *exe_scope
   = ExecutionContext(GetExecutionContextRef())
   .GetBestExecutionContextScope();
-  if (compiler_type.GetByteSize(exe_scope) 
+  if (compiler_type.GetByteSize(exe_scope)
   <= GetCompilerType().GetByteSize(exe_scope)) {
 return DoCast(compiler_type);
   }
Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2171,8 +2171,6 @@
"Show a list of current filters.") {}
 };
 
-#if LLDB_ENABLE_PYTHON
-
 // CommandObjectTypeSynthList
 
 class CommandObjectTypeSynthList
@@ -2184,8 +2182,6 @@
 "Show a list of current synthetic providers.") {}
 };
 
-#endif
-
 // CommandObjectTypeFilterDelete
 
 class CommandObjectTypeFilterDelete : public CommandObjectTypeFormatterDelete {
@@ -2197,8 +2193,6 @@
   ~CommandObjectTypeFilterDelete() override = default;
 };
 
-#if LLDB_ENABLE_PYTHON
-
 // CommandObjectTypeSynthDelete
 
 class CommandObjectTypeSynthDelete : public CommandObjectTypeFormatterDelete {
@@ -2210,7 +2204,6 @@
   ~CommandObjectTypeSynthDelete() override = default;
 };
 
-#endif
 
 // CommandObjectTypeFilterClear
 
@@ -,7 +2215,6 @@
 "Delete all existing filter.") {}
 };
 
-#if LLDB_ENABLE_PYTHON
 // CommandObjectTypeSynthClear
 
 class CommandObjectTypeSynthClear : public CommandObjectTypeFormatterClear {
@@ -2393,7 +2385,6 @@
   return true;
 }
 
-#endif
 #define LLDB_OPTIONS_type_filter_add
 #include "CommandOptions.inc"
 
@@ -2941,8 +2932,6 @@
   ~CommandObjectTypeFormat() override = default;
 };
 
-#if LLDB_ENABLE_PYTHON
-
 class CommandObjectTypeSynth : public CommandObjectMultiword {
 public:
   CommandObjectTypeSynth(CommandInterpreter )
@@ -2970,8 +2959,6 @@

[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

2023-08-30 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D158583#4627644 , @DavidSpickett 
wrote:

> The added context helps document what was already there so that's a nice 
> improvement.
>
> Something I'm not clear on mechanically. The original r_debug has eAdd set. 
> Then the second r_debug is loaded, which also has eAdd set. What is the state 
> of r_map at that point? I wonder if it could be somehow different between the 
> 2 copies, with each having a subset of the list.

Once we stop at the breakpoint the second time, second r_debug has eConsistent 
set when the original has eAdd, but we only look at the original r_debug in the 
ld.so binary.

Actually the _only_ thing the "_r_debug" in the main executable has set is the 
"r_state" when we stop at the second breakpoint, the r_map is set correctly in 
the ld.so version, but not in the a.out version. If you continue to run, the 
a.out version does eventually get updated. It just depends when the ld.so 
writes new things into the new r_debug struct.

I actually ran a test where I created a global variable with the same name but 
different type:

  char _r_debug[40] = "012345678901234567890123456789012345678"; // Same size 
as "r_debug" for safety

And then ran the program to the entry point and the "r_state" bytes in the 
above character array had been written to zero, but nothing else was touched. 
So I know this definitely isn't meant to happen.

The ld.so's "_r_debug.r_map" is correct and has a linked list of all the 
current libraries. But it really depends on when the _r_debug struct is written 
to again by ld.so.

> Also, why do we not have to wait for eConsistent on the second r_debug?

Because the first eAdd state indicates the ld.so is _about_ to load the shared 
libraries. So debuggers can't actually set breakpoints in them until all of the 
shared libraries are actually loaded.

> Is it that we do in fact wait for that, but we load the library list from the 
> first r_debug, then when the second one gets eConsistent, we load the rest 
> from that.

It is because when we receive the eConsistent the libraries are actually now 
loaded into memory and then we can load the libraries in LLDB and can actually 
resolve any breakpoints since the .text sections are now mapped and we can 
write breakpoints into their .text sections.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158583

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

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

First thing to note is that WriteRegister also behaves this way, but there it 
is more appropriate because it updates only part of the buffer before writing 
it out in its entirety. Useful to know where the pattern came from though.

You would need roughly the following per `WriteXYZ`:

  -  error = WriteTLS();
  +  error = WriteTLS(src);
  
  -Status NativeRegisterContextLinux_arm64::WriteTLS() {
  +Status NativeRegisterContextLinux_arm64::WriteTLS(const void* 
src/*=nullptr*/) {
  
  -  ioVec.iov_base = GetTLSBuffer();
  +  ioVec.iov_base = src ? const_cast(src) : GetTLSBuffer();
  
  -  Status WriteTLS();
  +  Status WriteTLS(const void* src=nullptr);

We can assume that the buffer is the same as the data to be written back if 
it's something static like TLS. For SVE/SME, we would have resized our buffer 
first, so it holds there too.

The added complexity isn't that much but I think it adds more thinking time for 
a developer than it potentially saves in copying time. I already feel like the 
separate `xyz_is_valid` is enough to think about and having a potential second 
data source just adds to that load.

From the header docs it seems I was right that this is used primarily for 
expression evaluation:

  // These two functions are used to implement "push" and "pop" of register
  // states.  They are used primarily for expression evaluation, where we need
  // to push a new state (storing the old one in data_sp) and then restoring
  // the original state by passing the data_sp we got from ReadAllRegisters to
  // WriteAllRegisterValues.

Which you would be doing a lot of in a formatter for example, but you'd get 
better savings implementing a more efficient packet format to do all that at 
once, I guess.

QSaveRegisterState / QRestoreRegisterState packets call it as part of 
expression evaluation, though in theory it's not always that. That's an lldb 
extension anyway so we're in control of it at least. In theory this could be 
used to restore state that is not just the previous state but I don't know how 
you'd trigger that from lldb.

The other use is `NativeProcessLinux::Syscall` which is sufficiently rare we 
can ignore that.

I did do a very rough benchmark where I printed the same expression 2000 times, 
so each one is doing a save/restore. Once with the code in this review right 
now, and again with this potential optimisation added to GPR/FPR/TLS (I'm on a 
Mountain Jade machine without SVE). Caveat shared machine, made up benchmark, 
etc. but all runs of both hovered between 16 and 17 seconds. Neither seemed to 
be consistently lower or higher than the other. Doesn't mean this isn't a 
speedup in isolation but if it is, it's dwarfed by the syscalls and packets 
sent back and forth.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

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

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

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

clang-format some missing bits.

Replace a missing memcpy in the FPR case. This only works because the buffer 
happens to
still contain the previous state. If there is some route to restore arbitray 
states,
this would be broken, but I don't know how that might happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -499,6 +499,30 @@
   return Status("Failed to write register value");
 }
 
+enum SavedRegistersKind : uint32_t {
+  GPR,
+  SVE, // Used for SVE and SSVE.
+  FPR, // When there is no SVE, or SVE in FPSIMD mode.
+  MTE,
+  TLS,
+};
+
+static uint8_t *AddSavedRegistersKind(uint8_t *dst, SavedRegistersKind kind) {
+  *(reinterpret_cast(dst)) = kind;
+  return dst + sizeof(uint32_t);
+}
+
+static uint8_t *AddSavedRegistersData(uint8_t *dst, void *src, size_t size) {
+  ::memcpy(dst, src, size);
+  return dst + size;
+}
+
+static uint8_t *AddSavedRegisters(uint8_t *dst, enum SavedRegistersKind kind,
+  void *src, size_t size) {
+  dst = AddSavedRegistersKind(dst, kind);
+  return AddSavedRegistersData(dst, src, size);
+}
+
 Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
 lldb::WritableDataBufferSP _sp) {
   // AArch64 register data must contain GPRs and either FPR or SVE registers.
@@ -512,33 +536,33 @@
   // corresponds to register sets enabled by current register context.
 
   Status error;
-  uint32_t reg_data_byte_size = GetGPRBufferSize();
+  uint32_t reg_data_byte_size = sizeof(SavedRegistersKind) + GetGPRBufferSize();
   error = ReadGPR();
   if (error.Fail())
 return error;
 
   // If SVE is enabled we need not copy FPR separately.
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-reg_data_byte_size += GetSVEBufferSize();
-// Also store the current SVE mode.
-reg_data_byte_size += sizeof(uint32_t);
+// Store mode and register data.
+reg_data_byte_size +=
+sizeof(SavedRegistersKind) + sizeof(m_sve_state) + GetSVEBufferSize();
 error = ReadAllSVE();
   } else {
-reg_data_byte_size += GetFPRSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetFPRSize();
 error = ReadFPR();
   }
   if (error.Fail())
 return error;
 
   if (GetRegisterInfo().IsMTEEnabled()) {
-reg_data_byte_size += GetMTEControlSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetMTEControlSize();
 error = ReadMTEControl();
 if (error.Fail())
   return error;
   }
 
   // tpidr is always present but tpidr2 depends on SME.
-  reg_data_byte_size += GetTLSBufferSize();
+  reg_data_byte_size += sizeof(SavedRegistersKind) + GetTLSBufferSize();
   error = ReadTLS();
   if (error.Fail())
 return error;
@@ -546,25 +570,26 @@
   data_sp.reset(new DataBufferHeap(reg_data_byte_size, 0));
   uint8_t *dst = data_sp->GetBytes();
 
-  ::memcpy(dst, GetGPRBuffer(), GetGPRBufferSize());
-  dst += GetGPRBufferSize();
+  dst = AddSavedRegisters(dst, SavedRegistersKind::GPR, GetGPRBuffer(),
+  GetGPRBufferSize());
 
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-*dst = static_cast(m_sve_state);
+dst = AddSavedRegistersKind(dst, SavedRegistersKind::SVE);
+*(reinterpret_cast(dst)) = m_sve_state;
 dst += sizeof(m_sve_state);
-::memcpy(dst, GetSVEBuffer(), GetSVEBufferSize());
-dst += GetSVEBufferSize();
+dst = AddSavedRegistersData(dst, GetSVEBuffer(), GetSVEBufferSize());
   } else {
-::memcpy(dst, GetFPRBuffer(), GetFPRSize());
-dst += GetFPRSize();
+dst = AddSavedRegisters(dst, SavedRegistersKind::FPR, GetFPRBuffer(),
+GetFPRSize());
   }
 
   if (GetRegisterInfo().IsMTEEnabled()) {
-::memcpy(dst, GetMTEControl(), GetMTEControlSize());
-dst += GetMTEControlSize();
+dst = AddSavedRegisters(dst, SavedRegistersKind::MTE, GetMTEControl(),
+GetMTEControlSize());
   }
 
-  ::memcpy(dst, GetTLSBuffer(), GetTLSBufferSize());
+  dst = AddSavedRegisters(dst, SavedRegistersKind::TLS, GetTLSBuffer(),
+  GetTLSBufferSize());
 
   return error;
 }
@@ -599,7 +624,8 @@
 return error;
   }
 
-  uint64_t reg_data_min_size = GetGPRBufferSize() + GetFPRSize();
+  uint64_t reg_data_min_size =
+  GetGPRBufferSize() + GetFPRSize() + 2 * (sizeof(SavedRegistersKind));
   if (data_sp->GetByteSize() < reg_data_min_size) {

[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

2023-08-30 Thread walter erquinigo via Phabricator via lldb-commits

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

For the sake of unblocking this diff, I'm accepting it because there's no clear 
actionable feedback at the moment.  @electriclilies, please follow up any 
feedback that is submitted post-landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158010

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

[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

2023-08-30 Thread Sheng-Yi Hung via Phabricator via lldb-commits

aokblast updated this revision to Diff 554724.
aokblast added a comment.

This change fix the indentation for the ObjectFileELF.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159076

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp


Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -937,7 +937,7 @@
 Address ObjectFileELF::GetBaseAddress() {
   if (GetType() == ObjectFile::eTypeObjectFile) {
 for (SectionHeaderCollIter I = std::next(m_section_headers.begin());
-I != m_section_headers.end(); ++I) {
+ I != m_section_headers.end(); ++I) {
   const ELFSectionHeaderInfo  = *I;
   if (header.sh_flags & SHF_ALLOC)
return Address(GetSectionList()->FindSectionByID(SectionIndex(I)), 0);


Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -937,7 +937,7 @@
 Address ObjectFileELF::GetBaseAddress() {
   if (GetType() == ObjectFile::eTypeObjectFile) {
 for (SectionHeaderCollIter I = std::next(m_section_headers.begin());
-	 I != m_section_headers.end(); ++I) {
+ I != m_section_headers.end(); ++I) {
   const ELFSectionHeaderInfo  = *I;
   if (header.sh_flags & SHF_ALLOC)
 	return Address(GetSectionList()->FindSectionByID(SectionIndex(I)), 0);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

2023-08-30 Thread Sheng-Yi Hung via Phabricator via lldb-commits

aokblast updated this revision to Diff 554722.
aokblast added a comment.

Add "real comment" for commented code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159076

Files:
  
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp


Index: 
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
===
--- 
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
+++ 
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
@@ -181,6 +181,7 @@
 
   if (header.getDataEncoding() == llvm::ELF::ELFDATA2MSB) {
 // TODO: swap byte order for big endian
+return false;
   }
 
   return true;
@@ -321,6 +322,7 @@
 
   bool this_is_kernel = is_kernel(memory_module_sp.get());
 
+  // TODO: figure out why UUID is not same in FreeBSD Kernel dump
   // If the kernel specify what UUID should be found, we should match it
   // if (m_uuid.IsValid() && m_uuid != memory_module_sp->GetUUID()) {
   // if (log) {
@@ -368,6 +370,7 @@
 s.Printf("Kernel UUID: %s\n", m_uuid.GetAsString().c_str());
 s.Printf("Load Address: 0x%" PRIx64 "\n", m_load_address);
 
+// TODO: figure out why UUID is not same in FreeBSD Kernel dump
 // Delete more than one kernel image that accidently add by user
 // ModuleList incorrect_kernels;
 // for (ModuleSP module_sp : target.GetImages().Modules()) {


Index: lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
===
--- lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
+++ lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
@@ -181,6 +181,7 @@
 
   if (header.getDataEncoding() == llvm::ELF::ELFDATA2MSB) {
 // TODO: swap byte order for big endian
+return false;
   }
 
   return true;
@@ -321,6 +322,7 @@
 
   bool this_is_kernel = is_kernel(memory_module_sp.get());
 
+  // TODO: figure out why UUID is not same in FreeBSD Kernel dump
   // If the kernel specify what UUID should be found, we should match it
   // if (m_uuid.IsValid() && m_uuid != memory_module_sp->GetUUID()) {
   // if (log) {
@@ -368,6 +370,7 @@
 s.Printf("Kernel UUID: %s\n", m_uuid.GetAsString().c_str());
 s.Printf("Load Address: 0x%" PRIx64 "\n", m_load_address);
 
+// TODO: figure out why UUID is not same in FreeBSD Kernel dump
 // Delete more than one kernel image that accidently add by user
 // ModuleList incorrect_kernels;
 // for (ModuleSP module_sp : target.GetImages().Modules()) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()

2023-08-30 Thread walter erquinigo via Phabricator via lldb-commits

wallace marked an inline comment as done.
wallace added a comment.

@aprantl , it turns out that there are no tests for this. I also don't know how 
easy it'd be to test this very specific feature, because it relies on the 
terminal not being affected by stuff like test runners.


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

https://reviews.llvm.org/D159031

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

[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()

2023-08-30 Thread walter erquinigo via Phabricator via lldb-commits

wallace updated this revision to Diff 554712.

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

https://reviews.llvm.org/D159031

Files:
  lldb/include/lldb/Core/IOHandler.h
  lldb/include/lldb/Host/Editline.h
  lldb/source/Core/IOHandler.cpp
  lldb/source/Expression/REPL.cpp


Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -528,17 +528,15 @@
   current_code.append(m_code.CopyList());
 
   IOHandlerEditline  = static_cast(io_handler);
-  const StringList *current_lines = editline.GetCurrentLines();
-  if (current_lines) {
-const uint32_t current_line_idx = editline.GetCurrentLineIndex();
-
-if (current_line_idx < current_lines->GetSize()) {
-  for (uint32_t i = 0; i < current_line_idx; ++i) {
-const char *line_cstr = current_lines->GetStringAtIndex(i);
-if (line_cstr) {
-  current_code.append("\n");
-  current_code.append(line_cstr);
-}
+  StringList current_lines = editline.GetCurrentLines();
+  const uint32_t current_line_idx = editline.GetCurrentLineIndex();
+
+  if (current_line_idx < current_lines.GetSize()) {
+for (uint32_t i = 0; i < current_line_idx; ++i) {
+  const char *line_cstr = current_lines.GetStringAtIndex(i);
+  if (line_cstr) {
+current_code.append("\n");
+current_code.append(line_cstr);
   }
 }
   }
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -508,6 +508,21 @@
   return m_curr_line_idx;
 }
 
+StringList IOHandlerEditline::GetCurrentLines() const {
+#if LLDB_ENABLE_LIBEDIT
+  if (m_editline_up)
+return m_editline_up->GetInputAsStringList();
+#endif
+  // When libedit is not used, the current lines can be gotten from
+  // `m_current_lines_ptr`, which is updated whenever a new line is processed.
+  // This doesn't happen when libedit is used, in which case
+  // `m_current_lines_ptr` is only updated when the full input is terminated.
+
+  if (m_current_lines_ptr)
+return *m_current_lines_ptr;
+  return StringList();
+}
+
 bool IOHandlerEditline::GetLines(StringList , bool ) {
   m_current_lines_ptr = 
 
Index: lldb/include/lldb/Host/Editline.h
===
--- lldb/include/lldb/Host/Editline.h
+++ lldb/include/lldb/Host/Editline.h
@@ -227,6 +227,9 @@
 
   void PrintAsync(Stream *stream, const char *s, size_t len);
 
+  /// Convert the current input lines into a UTF8 StringList
+  StringList GetInputAsStringList(int line_count = UINT32_MAX);
+
 private:
   /// Sets the lowest line number for multi-line editing sessions.  A value of
   /// zero suppresses
@@ -282,9 +285,6 @@
   /// Save the line currently being edited
   void SaveEditedLine();
 
-  /// Convert the current input lines into a UTF8 StringList
-  StringList GetInputAsStringList(int line_count = UINT32_MAX);
-
   /// Replaces the current multi-line session with the next entry from history.
   unsigned char RecallHistory(HistoryOperation op);
 
Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -403,7 +403,7 @@
 
   void SetInterruptExits(bool b) { m_interrupt_exits = b; }
 
-  const StringList *GetCurrentLines() const { return m_current_lines_ptr; }
+  StringList GetCurrentLines() const;
 
   uint32_t GetCurrentLineIndex() const;
 


Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -528,17 +528,15 @@
   current_code.append(m_code.CopyList());
 
   IOHandlerEditline  = static_cast(io_handler);
-  const StringList *current_lines = editline.GetCurrentLines();
-  if (current_lines) {
-const uint32_t current_line_idx = editline.GetCurrentLineIndex();
-
-if (current_line_idx < current_lines->GetSize()) {
-  for (uint32_t i = 0; i < current_line_idx; ++i) {
-const char *line_cstr = current_lines->GetStringAtIndex(i);
-if (line_cstr) {
-  current_code.append("\n");
-  current_code.append(line_cstr);
-}
+  StringList current_lines = editline.GetCurrentLines();
+  const uint32_t current_line_idx = editline.GetCurrentLineIndex();
+
+  if (current_line_idx < current_lines.GetSize()) {
+for (uint32_t i = 0; i < current_line_idx; ++i) {
+  const char *line_cstr = current_lines.GetStringAtIndex(i);
+  if (line_cstr) {
+current_code.append("\n");
+current_code.append(line_cstr);
   }
 }
   }
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++

[Lldb-commits] [PATCH] D156817: [lldb][windows] _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

2023-08-30 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a4b3fdb8232: [lldb][windows] _wsopen_s does not accept bits 
other than `_S_IREAD | _S_IWRITE` (authored by yshui, committed by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D156817?vs=546133=554679#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156817

Files:
  lldb/source/Host/windows/FileSystem.cpp


Index: lldb/source/Host/windows/FileSystem.cpp
===
--- lldb/source/Host/windows/FileSystem.cpp
+++ lldb/source/Host/windows/FileSystem.cpp
@@ -101,6 +101,8 @@
   std::wstring wpath;
   if (!llvm::ConvertUTF8toWide(path, wpath))
 return -1;
+  // All other bits are rejected by _wsopen_s
+  mode = mode & (_S_IREAD | _S_IWRITE);
   int result;
   ::_wsopen_s(, wpath.c_str(), flags, _SH_DENYNO, mode);
   return result;


Index: lldb/source/Host/windows/FileSystem.cpp
===
--- lldb/source/Host/windows/FileSystem.cpp
+++ lldb/source/Host/windows/FileSystem.cpp
@@ -101,6 +101,8 @@
   std::wstring wpath;
   if (!llvm::ConvertUTF8toWide(path, wpath))
 return -1;
+  // All other bits are rejected by _wsopen_s
+  mode = mode & (_S_IREAD | _S_IWRITE);
   int result;
   ::_wsopen_s(, wpath.c_str(), flags, _SH_DENYNO, mode);
   return result;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 9a4b3fd - [lldb][windows] _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

2023-08-30 Thread Martin Storsjö via lldb-commits


Author: Yuxuan Shui
Date: 2023-08-30T15:53:31+03:00
New Revision: 9a4b3fdb82327e808213070fd157be3c557b8b9d

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

LOG: [lldb][windows] _wsopen_s does not accept bits other than `_S_IREAD | 
_S_IWRITE`

When sending file from a Linux host to a Windows remote, Linux host will try to 
copy the source file's permission bits, which will contain `_S_I?GRP` and 
`_S_I?OTH` bits. Those bits are rejected by `_wsopen_s`, causing it to return 
EINVAL.

This patch masks out the rejected bits.

GitHub issue: #64313

Reviewed By: jasonmolenda, DavidSpickett

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

Added: 


Modified: 
lldb/source/Host/windows/FileSystem.cpp

Removed: 




diff  --git a/lldb/source/Host/windows/FileSystem.cpp 
b/lldb/source/Host/windows/FileSystem.cpp
index b919d9bcd9dd4a..4b0cd74b8013b0 100644
--- a/lldb/source/Host/windows/FileSystem.cpp
+++ b/lldb/source/Host/windows/FileSystem.cpp
@@ -101,6 +101,8 @@ int FileSystem::Open(const char *path, int flags, int mode) 
{
   std::wstring wpath;
   if (!llvm::ConvertUTF8toWide(path, wpath))
 return -1;
+  // All other bits are rejected by _wsopen_s
+  mode = mode & (_S_IREAD | _S_IWRITE);
   int result;
   ::_wsopen_s(, wpath.c_str(), flags, _SH_DENYNO, mode);
   return result;



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

[Lldb-commits] [PATCH] D157488: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

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

Remove HWCAP check from program file. The runner will check cppuinfo
and even if that was fooled, the prctl would fail at runtime if you
didn't have MTE.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157488

Files:
  lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
  
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
  lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c


Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
@@ -0,0 +1,14 @@
+#include 
+
+// This is its own function so that lldb can call it.
+int setup_mte() {
+  return prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | 
PR_MTE_TCF_SYNC,
+   0, 0, 0);
+}
+
+int main(int argc, char const *argv[]) {
+  if (setup_mte())
+return 1;
+
+  return 0; // Set a break point here.
+}
Index: 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
===
--- /dev/null
+++ 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly reads, writes and restores the MTE control register on
+AArch64 Linux.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MTECtrlRegisterTestCase(TestBase):
+@no_debug_info_test
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(["linux"]))
+def test_mte_ctrl_register(self):
+if not self.isAArch64MTE():
+self.skipTest("Target must support MTE.")
+
+self.build()
+self.line = line_number("main.c", "// Set a break point here.")
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1
+)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"process status",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."],
+)
+
+# Bit 0 = tagged addressing enabled
+# Bit 1 = synchronous faults
+# Bit 2 = asynchronous faults
+# We start enabled with synchronous faults.
+self.expect("register read mte_ctrl", substrs=["0x0003"])
+
+# Change to asynchronous faults.
+self.runCmd("register write mte_ctrl 5")
+self.expect("register read mte_ctrl", substrs=["0x0005"])
+
+# This would return to synchronous faults if we did not restore the
+# previous value.
+self.expect("expression setup_mte()", substrs=["= 0"])
+self.expect("register read mte_ctrl", substrs=["0x0005"])
\ No newline at end of file
Index: 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules


Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
@@ -0,0 +1,14 @@
+#include 
+
+// This is its own function so that lldb can call it.
+int setup_mte() {
+  return prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC,
+   0, 0, 0);
+}
+
+int main(int argc, char const *argv[]) {
+  if (setup_mte())
+return 1;
+
+  return 0; // Set a break point here.
+}
Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly reads, writes and restores the MTE control register on
+AArch64 Linux.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MTECtrlRegisterTestCase(TestBase):
+@no_debug_info_test
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(["linux"]))
+def test_mte_ctrl_register(self):
+if not self.isAArch64MTE():
+self.skipTest("Target must support MTE.")
+
+

[Lldb-commits] [PATCH] D157488: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

> This looks fine but instead of adding a new test cant we add MTE control 
> register tests to already available MTE tests?

We have these existing tests:
mte_core_file - clearly it can't go here, core files are read only.
mte_memory_region - memory regions have nothing to do with registers.
mte_tag_access - Could go here.
mte_tag_faults - Could go here.

You can call it `def test_mte_ctrl_is_restored` and that explains it if it 
fails among a set of somewhat unrelated tests, but it makes it harder to see at 
a glance if we've tested that register specifically. Doing it in it's own test 
file is clear what it's doing and it's next to the other register tests and the 
forthcoming SME tests.

I know I've gone either way on adding new test cases or tacking onto older ones 
in the past but in this case that's the way I'm leaning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157488

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

[Lldb-commits] [PATCH] D157488: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

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

Remove unused includes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157488

Files:
  lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
  
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
  lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c


Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
@@ -0,0 +1,18 @@
+#include 
+#include 
+#include 
+
+int setup_mte() {
+  return prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | 
PR_MTE_TCF_SYNC,
+   0, 0, 0);
+}
+
+int main(int argc, char const *argv[]) {
+  if (!(getauxval(AT_HWCAP2) & HWCAP2_MTE))
+return 1;
+
+  if (setup_mte())
+return 1;
+
+  return 0; // Set a break point here.
+}
Index: 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
===
--- /dev/null
+++ 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly reads, writes and restores the MTE control register on
+AArch64 Linux.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MTECtrlRegisterTestCase(TestBase):
+@no_debug_info_test
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(["linux"]))
+def test_mte_ctrl_register(self):
+if not self.isAArch64MTE():
+self.skipTest("Target must support MTE.")
+
+self.build()
+self.line = line_number("main.c", "// Set a break point here.")
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1
+)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"process status",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."],
+)
+
+# Bit 0 = tagged addressing enabled
+# Bit 1 = synchronous faults
+# Bit 2 = asynchronous faults
+# We start enabled with synchronous faults.
+self.expect("register read mte_ctrl", substrs=["0x0003"])
+
+# Change to asynchronous faults.
+self.runCmd("register write mte_ctrl 5")
+self.expect("register read mte_ctrl", substrs=["0x0005"])
+
+# This would return to synchronous faults if we did not restore the
+# previous value.
+self.expect("expression setup_mte()", substrs=["= 0"])
+self.expect("register read mte_ctrl", substrs=["0x0005"])
\ No newline at end of file
Index: 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules


Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
@@ -0,0 +1,18 @@
+#include 
+#include 
+#include 
+
+int setup_mte() {
+  return prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC,
+   0, 0, 0);
+}
+
+int main(int argc, char const *argv[]) {
+  if (!(getauxval(AT_HWCAP2) & HWCAP2_MTE))
+return 1;
+
+  if (setup_mte())
+return 1;
+
+  return 0; // Set a break point here.
+}
Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly reads, writes and restores the MTE control register on
+AArch64 Linux.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MTECtrlRegisterTestCase(TestBase):
+@no_debug_info_test
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(["linux"]))
+def test_mte_ctrl_register(self):
+if not self.isAArch64MTE():
+self.skipTest("Target must support MTE.")
+
+self.build()
+self.line = line_number("main.c", "// Set a break point

[Lldb-commits] [PATCH] D157000: [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

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



Comment at: 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c:43
 
+void write_simd_regs_expr() {
+#define WRITE_SIMD(NUM)
\

omjavaid wrote:
> There is subtle difference between write_simd_regs and write_simd_regs_expr. 
> 
> BTW, cant we avoid another function and just do something like
> 
> 
> ```
> #define WRITE_SIMD(NUM, I)
>   \
>   asm volatile("MOV v" #NUM ".d[0], %0\n\t"   
>   \
>"MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM + I))
> ```
> 
Done. I also looked into some kind of for loop to generate the WRITE_SIMD but 
it ends up more complex and approaching the length of just repeating it anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157000

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

[Lldb-commits] [PATCH] D157000: [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

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

Single function to set the simd values, that takes a base value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157000

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c


Index: 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
===
--- lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -1,10 +1,11 @@
 #include 
 #include 
 
-void write_simd_regs() {
+// base is added to each value. If base = 2, then v0 = 2, v1 = 3, etc.
+void write_simd_regs(unsigned base) {
 #define WRITE_SIMD(NUM)
\
   asm volatile("MOV v" #NUM ".d[0], %0\n\t"
\
-   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM))
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(base + NUM))
 
   WRITE_SIMD(0);
   WRITE_SIMD(1);
@@ -102,7 +103,7 @@
 #endif
   // else test plain SIMD access.
 
-  write_simd_regs();
+  write_simd_regs(0);
 
   return verify_simd_regs(); // Set a break point here.
 }
Index: 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
===
--- 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
+++ 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
@@ -1,6 +1,6 @@
 """
-Test that LLDB correctly reads and writes AArch64 SIMD registers in SVE,
-streaming SVE and normal SIMD modes.
+Test that LLDB correctly reads and writes and restores AArch64 SIMD registers
+in SVE, streaming SVE and normal SIMD modes.
 
 There are a few operating modes and we use different strategies for each:
 * Without SVE, in SIMD mode - read the SIMD regset.
@@ -48,6 +48,13 @@
 pad = " ".join(["0x00"] * 7)
 return "{{0x{:02x} {} 0x{:02x} {}}}".format(n, pad, n, pad)
 
+def check_simd_values(self, value_offset):
+# These are 128 bit registers, so getting them from the API as unsigned
+# values doesn't work. Check the command output instead.
+for i in range(32):
+self.expect("register read v{}".format(i),
+substrs=[self.make_simd_value(i+value_offset)])
+
 def sve_simd_registers_impl(self, mode):
 self.skip_if_needed(mode)
 
@@ -68,12 +75,9 @@
 substrs=["stop reason = breakpoint 1."],
 )
 
-# These are 128 bit registers, so getting them from the API as unsigned
-# values doesn't work. Check the command output instead.
-for i in range(32):
-self.expect(
-"register read v{}".format(i), 
substrs=[self.make_simd_value(i)]
-)
+self.check_simd_values(0)
+self.runCmd("expression write_simd_regs(1)")
+self.check_simd_values(0)
 
 # Write a new set of values. The kernel will move the program back to
 # non-streaming mode here.
@@ -83,10 +87,7 @@
 )
 
 # Should be visible within lldb.
-for i in range(32):
-self.expect(
-"register read v{}".format(i), substrs=[self.make_simd_value(i 
+ 1)]
-)
+self.check_simd_values(1)
 
 # The program should agree with lldb.
 self.expect("continue", substrs=["exited with status = 0"])


Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
===
--- lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -1,10 +1,11 @@
 #include 
 #include 
 
-void write_simd_regs() {
+// base is added to each value. If base = 2, then v0 = 2, v1 = 3, etc.
+void write_simd_regs(unsigned base) {
 #define WRITE_SIMD(NUM)\
   asm volatile("MOV v" #NUM ".d[0], %0\n\t"\
-   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM))
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(base + NUM))
 
   WRITE_SIMD(0);
   WRITE_SIMD(1);
@@ -102,7 +103,7 @@
 #endif
   // else test plain SIMD access.
 
-  write_simd_regs();
+  write_simd_regs(0);
 
   return verify_simd_regs(); // Set a break point here.
 }
Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
===
---

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

> I've done the following:

Great. Could you include that in the commit message? Seems like we get a lot of 
questions about how mature risc-v support is, so we can point to this as a 
milestone for that.

There is https://lldb.llvm.org/#platform-support but there's probably too many 
caveats to add it there yet.

I have some vague idea that maybe we could put a hack in the test suite to use 
the qemu-user platform instead of lldb-server. To at least give it a go, but I 
haven't tried it myself yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159101

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

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:145
+  return false;
+}
+

ted wrote:
> DavidSpickett wrote:
> > Looking at the comments in lldb/include/lldb/Target/ABI.h I'm not sure 
> > which of these should be implemented. I think this one is what most plugins 
> > provide.
> > 
> > One way to figure this out is to figure out what actually needs this. 
> > Return false from both and try a bunch of things to see if it fails, run an 
> > expression, step in and out etc.
> > 
> > I'd be more comfortable having one not implemented if we know how the other 
> > one gets used.
> The first one is used for calling functions via JIT. The second is used for 
> calling functions via the IR Interpreter. I didn't want to enable JIT, so I 
> took the Hexagon implementation (Hexagon doesn't support JIT in lldb, but can 
> call functions with the IR interpreter) and reworked it for RISC-V.
> 
> Here's a function call on riscv64:
> (lldb) re r pc
>   pc = 0x000106b2 factrv64`main + 28 at factorial.c:32:8  
> factrv64`main + 28 at factorial.c:32:8
> (lldb) p factorial(3)
> (int) 6
> 
Ok cool, as long as one of us knows what these do :)

The other way to kinda chaos engineer this is to take your host's target, make 
a function fail and see what parts of the test suite fail. Then that can tell 
you what to test manually for riscv's version of the same functions.



Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1554
+// Turn them on by default now, since everyone seems to use them
+features_str += "+a,+m,";
   }

ted wrote:
> DavidSpickett wrote:
> > You might want to take the lead from AArch64 here:
> > ```
> > // If any AArch64 variant, enable latest ISA with all extensions.
> > ```
> > If "+all" doesn't already work for riscv then you don't have to go and make 
> > that work right now.
> > 
> > But in general we decided that much like llvm-objdump, we'll try to 
> > disassemble any possible encoding. If the user happens to point the 
> > disassembler at garbage that looks like a fancy extension on a cpu from 20 
> > years ago, that's on them.
> While I like the "turn on the latest" philosophy in general, for RISC-V we 
> don't want to do that. It's modular architecture means features can be turned 
> on and off when a core is designed, so one core might have +d (floating point 
> double), while an newer core might not have any floating point at all. I'm 
> inclined to leave the features as they are now, with a and m turned on.
Fair enough, I see that +all does not in fact work for risc-v llvm-objdump 
which backs that up. I guess you'd have to pass through some kind of object 
attributes to detect some of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159101

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

[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

The added context helps document what was already there so that's a nice 
improvement.

Something I'm not clear on mechanically. The original r_debug has eAdd set. 
Then the second r_debug is loaded, which also has eAdd set. What is the state 
of r_map at that point? I wonder if it could be somehow different between the 2 
copies, with each having a subset of the list.

Also, why do we not have to wait for eConsistent on the second r_debug? Is it 
that we do in fact wait for that, but we load the library list from the first 
r_debug, then when the second one gets eConsistent, we load the rest from that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158583

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

[Lldb-commits] [PATCH] D158971: [lldb][NFC] Put disassembler test classes and methods in anonymous namespace

2023-08-30 Thread David Spickett via Phabricator via lldb-commits

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

> Are you referring to 
> https://llvm.org/docs/CodingStandards.html#anonymous-namespaces?

That's the one, please include that in the commit message. Other than that this 
LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158971

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

[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

2023-08-30 Thread Mark de Wever via Phabricator via lldb-commits

Mordante added a comment.

In D159127#4626311 , @kastiglione 
wrote:

> Thanks for doing this!
>
> Question to all: Should the summary string include the unit? lldb doesn't 
> always show the type, so it could help comprehension if the unit is included. 
> For example `60s` instead of `60`.

I was wondering about that too, but I wasn't sure whether that was wanted. If 
we do that what do you prefer for micro seconds us or µs. The latter uses 
Unicode. The C++ Standard allows both http://eel.is/c++draft/time.duration.io.




Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/chrono/TestDataFormatterLibcxxChrono.py:23
+# clean slate for the next test case.
+def cleanup():
+self.runCmd("type format clear", check=False)

aprantl wrote:
> You probably copied & pasted this — this is no longer needed since every test 
> function is now running in its own instance.
I indeed copy-pasted the vector code; it's the first time I look at these 
formatters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159127

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

[Lldb-commits] [PATCH] D159150: [lldb][NFCI] Replace bespoke iterator check with std::next

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

[Lldb-commits] [lldb] 394e52a - [lldb] NFC reflow comments in WatchpointLocations

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

[Lldb-commits] [PATCH] D159237: [lldb][NFCI] Remove unneeded ConstString conversions

[Lldb-commits] [PATCH] D158833: [lldb] Move ScriptInterpreter Interfaces to subdirectory (NFC)

[Lldb-commits] [PATCH] D159142: [lldb] Add support for recognizing swift ast sections in object files

[Lldb-commits] [PATCH] D159237: [lldb][NFCI] Remove unneeded ConstString conversions

[Lldb-commits] [PATCH] D159234: [llvm-objdump] Enable assembly highlighting in llvm-objdump

[Lldb-commits] [lldb] 3621f83 - Don't enable stdin/out with a no-output test program

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

[Lldb-commits] [PATCH] D159142: [lldb] Add support for recognizing swift ast sections in object files

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

[Lldb-commits] [lldb] 79d5d9a - [lldb] Allow synthetic providers in C++ and fix linking problems

[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()

[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()

[Lldb-commits] [PATCH] D156817: [lldb][windows] _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

[Lldb-commits] [lldb] 9a4b3fd - [lldb][windows] _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

[Lldb-commits] [PATCH] D157488: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

[Lldb-commits] [PATCH] D157488: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

[Lldb-commits] [PATCH] D157488: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

[Lldb-commits] [PATCH] D157000: [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

[Lldb-commits] [PATCH] D157000: [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

[Lldb-commits] [PATCH] D158971: [lldb][NFC] Put disassembler test classes and methods in anonymous namespace

[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

36 matches

Site Navigation

Mail list logo

Footer information