[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D127922#3595530 , @labath wrote:

> I think we should talk about the threadsafe flag. The reason that the 
> non-threadsafe mode is safe now is because we're using unbuffered streams, 
> where each stream operation will map to a single write syscall (which will 
> then be serialized by the OS somehow). And since we pre-format the log 
> messages into a temporary buffer, we shouldn't even get overlapping messages 
> in the output. In fact, I would say that, before this, the threadsafe mode 
> was mostly useless.
>
> All of that goes out the window once you start doing arbitrary logic in place 
> of log writing. As they're implemented now, neither BufferedLogHandler nor 
> RotatingLogHandler are thread-safe. (Making the rotating handler thread safe 
> (without locks) should be relatively easy -- assuming we're willing to accept 
> races between printing messages, and new messages coming in -- but I don't 
> think that the Buffered handler can be made safe without locking at least the 
> flushing part). An it's not just that the messages may be slightly wonky. I 
> think it wouldn't be hard to crash the process by spewing log messages from 
> multiple threads (and we're pretty good at spewing :P).
>
> So, what I'd recommend is to drop the thread-safe flag, and make the locking 
> strategy the responsibility of the individual log handler. When writing to an 
> unbuffered stream, we wouldn't need any locks at all. The circular handler 
> could either lock into that or, if you're into that sort of thing, use 
> atomics to make enqueueing lock-free. The buffered handler could either use a 
> buffered ostream, wrapped in a mutex, or do something similar to the circular 
> one, while locking only the flushing code.

Yup that makes sense to me. I'll put up a patch for that tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127922

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

[Lldb-commits] [PATCH] D127234: [lldb] Add setting to override PE/COFF ABI by module name

[David Spickett via Phabricator via lldb-commits]

DavidSpickett accepted this revision.
DavidSpickett 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/D127234/new/

https://reviews.llvm.org/D127234

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

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

[David Spickett via Phabricator via lldb-commits]

DavidSpickett accepted this revision.
DavidSpickett 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/D127436/new/

https://reviews.llvm.org/D127436

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[David Spickett via Phabricator via lldb-commits]

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

LGTM. I like the ifs the way I suggested but up to you, either way it's not the 
easiest logic to parse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128268#3601104 , @DavidSpickett 
wrote:

> LGTM. I like the ifs the way I suggested but up to you, either way it's not 
> the easiest logic to parse.

Oh, sorry, I forgot about that comment. Yes, that does indeed look even nicer, 
I'll try to add that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo updated this revision to Diff 438951.
mstorsjo added a comment.

Refactored the code to add the `both_windows` variable as suggested (which 
turned out much nicer, thanks!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,61 @@
+## Check that i386 executables can be opened in both ABI modes.
+## I386 executables are special in the sense that the PECOFF object
+## file plugin returns two architectures for them, i386 and i686, which
+## causes more elaborate comparisons to be run against the Platform plugin's
+## architecture list. Check that this is accepted despite potential mismatches
+## in the environment part of the triple.
+
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+# CHECK-LABEL: image list --triple --basename
+# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4480
+  ImageBase:   268435456
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.debug_info
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  16384
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -979,7 +979,16 @@
 
   const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
   const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-  if (lhs_triple_vendor != rhs_triple_vendor) {
+
+  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
+  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
+
+  bool both_windows = lhs_triple.isOSWindows() && rhs_triple.isOSWindows();
+
+  // On Windows, the vendor field doesn't have any practical effect, but
+  // it is often set to either "pc" or "w64".
+  if ((lhs_triple_vendor != rhs_triple_vendor) &&
+  (exact_match || !both_windows)) {
 const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
 const bool lhs_vendor_specified = TripleVendorWasSpecified();
 // Both architectures had the vendor specified, so if they aren't equal
@@ -993,8 +1002,6 @@
   return false;
   }
 
-  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
-  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
   const llvm::Triple::EnvironmentType lhs_triple_env =
   lhs_triple.getEnvironment();
   const llvm::Triple::EnvironmentType rhs_triple_env =
@@ -1032,6 +1039,9 @@
   return true;
   }
 
+  if (!exact_match && both_windows)
+return true; // The Windows environments (MSVC vs GNU) are compatible
+
   return IsCompatibleEnvironment(lhs_triple_env, rhs_triple_env);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 requested changes to this revision.
jj10306 added a comment.
This revision now requires changes to proceed.

looks great overall, just a couple minor comments!




Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:46
 /// state and granularity.
 class TraceInstructionDumper {
 public:

nit: wdyt ab renaming this to `TraceInstructionsDumper` since now we have the 
`TraceInstructionWriter` which is responsible for displaying a single 
instruction? With how things are currently named it's slightly confusing at 
first glance because there names sound like they are both doing the same thing 
(something related to a single instruction).



Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:75
+/// Indicate a user-level info message. It's not part of the actual trace.
+virtual void InfoMessage(llvm::StringRef text) = 0;
+

since the JSON subclass doesn't need to implement this, consider providing a 
default stubbed out implementation at the super class level so that subclasses 
aren't necessarily required to implement it?



Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:120-122
+  /// Create an instruction entry for the current position without symbol
+  /// information.
+  InstructionEntry CreateBasicInstructionEntry();

nit: to be consistent with the `--raw` flag of the `thread trace dump 
instructions` command which says:
```
   -r ( --raw )
Dump only instruction address without disassembly nor symbol 
information.
```



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2210
 size_t m_continue;
+llvm::Optional m_output_file;
 TraceInstructionDumperOptions m_dumper_options;

Why is this stored on the command object and not the 
`TraceinstructionDumperOptions`?
Can you explain the separation of responsibility of the dumper options versus 
the options here?



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:121-122
 bool TraceCursorIntelPT::GoToId(user_id_t id) {
   if (m_decoded_thread_sp->GetInstructionsCount() <= id)
 return false;
   m_pos = id;

nit: maybe use the new `HasId` method here?



Comment at: lldb/source/Target/TraceInstructionDumper.cpp:90-91
 
-void TraceInstructionDumper::DumpInstructionSymbolContext(
-const Optional &prev_insn,
-const InstructionSymbolInfo &insn) {
-  if (prev_insn && IsSameInstructionSymbolContext(*prev_insn, insn))
-return;
+class TraceInstructionWriterCLI
+: public TraceInstructionDumper::TraceInstructionWriter {
+public:

Move declarations to header?



Comment at: lldb/source/Target/TraceInstructionDumper.cpp:193
+  : m_s(s), m_options(options),
+m_j(m_s.AsRawOstream(), options.pretty_print_json ? 2 : 0) {
+m_j.arrayBegin();





Comment at: lldb/source/Target/TraceInstructionDumper.cpp:219-224
+m_j.attribute("module", GetModuleName(insn));
+m_j.attribute("symbol",
+  insn.symbol_info->sc.GetFunctionName().AsCString());
+m_j.attribute("mnemonic", insn.symbol_info->instruction->GetMnemonic(
+  &insn.symbol_info->exe_ctx));
+

the values being passed to the `attribute` method are c strings. Unsure how 
this method handles a nullptr, but consider checking if these are nullptr 
before calling attribute if necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128316

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

[Lldb-commits] [lldb] 9846a1f - [lldb] Remove an outdated comment. NFC.

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2022-06-22T16:11:59+03:00
New Revision: 9846a1f2d4723a8c060370638bbd52360638d0fc

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

LOG: [lldb] Remove an outdated comment. NFC.

This comment became outdated in 053eb35651906e693906fad6c695fce11415ade7
(but was moved along); that commit moved the code and the comment
to a separate function, with a separate local variable
`num_of_bytes_read`. On error, the possibly garbage value is never
copied back to the caller's reference, thus the comment is no longer
relevant (and slightly confusing as is).

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

Added: 


Modified: 
lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp 
b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
index 555c19adcfa9a..59f5f6dfe7716 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -281,10 +281,6 @@ Status ProcessDebugger::ReadMemory(lldb::addr_t vm_addr, 
void *buf, size_t size,
   SIZE_T num_of_bytes_read = 0;
   if (!::ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
buf, size, &num_of_bytes_read)) {
-// Reading from the process can fail for a number of reasons - set the
-// error code and make sure that the number of bytes read is set back to 0
-// because in some scenarios the value of bytes_read returned from the API
-// is garbage.
 error.SetError(GetLastError(), eErrorTypeWin32);
 LLDB_LOG(log, "reading failed with error: {0}", error);
   } else {



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

[Lldb-commits] [PATCH] D128226: [lldb] Remove an outdated comment. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9846a1f2d472: [lldb] Remove an outdated comment. NFC. 
(authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128226

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp


Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -281,10 +281,6 @@
   SIZE_T num_of_bytes_read = 0;
   if (!::ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
buf, size, &num_of_bytes_read)) {
-// Reading from the process can fail for a number of reasons - set the
-// error code and make sure that the number of bytes read is set back to 0
-// because in some scenarios the value of bytes_read returned from the API
-// is garbage.
 error.SetError(GetLastError(), eErrorTypeWin32);
 LLDB_LOG(log, "reading failed with error: {0}", error);
   } else {


Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -281,10 +281,6 @@
   SIZE_T num_of_bytes_read = 0;
   if (!::ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
buf, size, &num_of_bytes_read)) {
-// Reading from the process can fail for a number of reasons - set the
-// error code and make sure that the number of bytes read is set back to 0
-// because in some scenarios the value of bytes_read returned from the API
-// is garbage.
 error.SetError(GetLastError(), eErrorTypeWin32);
 LLDB_LOG(log, "reading failed with error: {0}", error);
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 8a64dd5 - [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2022-06-22T17:16:05+03:00
New Revision: 8a64dd5b06146f073a4a326f0e24fa18e571b281

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

LOG: [lldb] Fix reading i686-windows executables with GNU environment

25c8a061c5739677d2fc0af29a8cc9520207b923 / D127048 added an option
for setting the ABI to GNU.

When an object file is loaded, there's only minimal verification
done for the architecture spec set for it, if the object file only
provides one.

However, for i386 object files, the PECOFF object file plugin
provides two architectures, i386-pc-windows and i686-pc-windows.
This picks a totally different codepath in
TargetList::CreateTargetInternal, where it's treated as a fat
binary. This goes through more verifications to see if the
architectures provided by the object file matches what the
platform plugin supports.

The PlatformWindows() constructor explicitly adds the
"i386-pc-windows" and "i686-pc-windows" architectures (even when
running on other architectures), which allows this "fat binary
verification" to succeed for the i386 object files that provide
two architectures.

However, after that commit, if the object file is advertised with
the different environment (either when lldb is built in a mingw
environment, or if that setting is set), the fat binary validation
won't accept the file any longer.

Update ArchSpec::IsEqualTo with more logic for the Windows use
cases; mismatching vendors is not an issue (they don't have any
practical effect on Windows), and GNU and MSVC environments are
compatible to the point that PlatformWindows can handle object
files for both environments/ABIs.

As a separate path forward, one could also consider to stop returning
two architecture specs from ObjectFilePECOFF::GetModuleSpecifications
for i386 files.

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

Added: 
lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml

Modified: 
lldb/source/Utility/ArchSpec.cpp

Removed: 




diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index c6feacfc7c1e6..a99aed82bc886 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -979,7 +979,16 @@ bool ArchSpec::IsEqualTo(const ArchSpec &rhs, bool 
exact_match) const {
 
   const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
   const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-  if (lhs_triple_vendor != rhs_triple_vendor) {
+
+  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
+  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
+
+  bool both_windows = lhs_triple.isOSWindows() && rhs_triple.isOSWindows();
+
+  // On Windows, the vendor field doesn't have any practical effect, but
+  // it is often set to either "pc" or "w64".
+  if ((lhs_triple_vendor != rhs_triple_vendor) &&
+  (exact_match || !both_windows)) {
 const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
 const bool lhs_vendor_specified = TripleVendorWasSpecified();
 // Both architectures had the vendor specified, so if they aren't equal
@@ -993,8 +1002,6 @@ bool ArchSpec::IsEqualTo(const ArchSpec &rhs, bool 
exact_match) const {
   return false;
   }
 
-  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
-  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
   const llvm::Triple::EnvironmentType lhs_triple_env =
   lhs_triple.getEnvironment();
   const llvm::Triple::EnvironmentType rhs_triple_env =
@@ -1032,6 +1039,9 @@ bool ArchSpec::IsEqualTo(const ArchSpec &rhs, bool 
exact_match) const {
   return true;
   }
 
+  if (!exact_match && both_windows)
+return true; // The Windows environments (MSVC vs GNU) are compatible
+
   return IsCompatibleEnvironment(lhs_triple_env, rhs_triple_env);
 }
 

diff  --git a/lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml 
b/lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
new file mode 100644
index 0..ca2bac38027fa
--- /dev/null
+++ b/lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,61 @@
+## Check that i386 executables can be opened in both ABI modes.
+## I386 executables are special in the sense that the PECOFF object
+## file plugin returns two architectures for them, i386 and i686, which
+## causes more elaborate comparisons to be run against the Platform plugin's
+## architecture list. Check that this is accepted despite potential mismatches
+## in the environment part of the triple.
+
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t

[Lldb-commits] [lldb] 2bae956 - [lldb] Resolve exe location for `target create`

[Martin Storsjö via lldb-commits]

Author: Alvin Wong
Date: 2022-06-22T17:16:05+03:00
New Revision: 2bae9560575362ffd756f193efa1de2d5c2f4cfd

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

LOG: [lldb] Resolve exe location for `target create`

This fixes an issue that, when you start lldb or use `target create`
with a program name which is on $PATH, or not specify the .exe suffix of
a program in the working directory on Windows, you get a confusing
error, for example:

(lldb) target create notepad
error: 'C:\WINDOWS\SYSTEM32\notepad.exe' doesn't contain any 'host'
platform architectures: i686, x86_64, i386, i386

Fixes https://github.com/mstorsjo/llvm-mingw/issues/265

Reviewed By: DavidSpickett

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

Added: 
lldb/test/Shell/Commands/command-target-create-resolve-exe.test

Modified: 
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Target/TargetList.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 7789698c1d678..23ebbdb64d028 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -299,12 +299,6 @@ class CommandObjectTargetCreate : public 
CommandObjectParsed {
 
   const char *file_path = command.GetArgumentAtIndex(0);
   LLDB_SCOPED_TIMERF("(lldb) target create '%s'", file_path);
-  FileSpec file_spec;
-
-  if (file_path) {
-file_spec.SetFile(file_path, FileSpec::Style::native);
-FileSystem::Instance().Resolve(file_spec);
-  }
 
   bool must_set_platform_path = false;
 
@@ -333,6 +327,18 @@ class CommandObjectTargetCreate : public 
CommandObjectParsed {
 
   PlatformSP platform_sp = target_sp->GetPlatform();
 
+  FileSpec file_spec;
+  if (file_path) {
+file_spec.SetFile(file_path, FileSpec::Style::native);
+FileSystem::Instance().Resolve(file_spec);
+
+// Try to resolve the exe based on PATH and/or platform-specific
+// suffixes, but only if using the host platform.
+if (platform_sp && platform_sp->IsHost() &&
+!FileSystem::Instance().Exists(file_spec))
+  FileSystem::Instance().ResolveExecutableLocation(file_spec);
+  }
+
   if (remote_file) {
 if (platform_sp) {
   // I have a remote file.. two possible cases

diff  --git a/lldb/source/Target/TargetList.cpp 
b/lldb/source/Target/TargetList.cpp
index 72b50811b8745..214e98ee91edb 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -121,6 +121,14 @@ Status TargetList::CreateTargetInternal(
   if (!user_exe_path.empty()) {
 ModuleSpec module_spec(FileSpec(user_exe_path, FileSpec::Style::native));
 FileSystem::Instance().Resolve(module_spec.GetFileSpec());
+
+// Try to resolve the exe based on PATH and/or platform-specific suffixes,
+// but only if using the host platform.
+if (platform_sp->IsHost() &&
+!FileSystem::Instance().Exists(module_spec.GetFileSpec()))
+  FileSystem::Instance().ResolveExecutableLocation(
+  module_spec.GetFileSpec());
+
 // Resolve the executable in case we are given a path to a application
 // bundle like a .app bundle on MacOSX.
 Host::ResolveExecutableInBundle(module_spec.GetFileSpec());

diff  --git a/lldb/test/Shell/Commands/command-target-create-resolve-exe.test 
b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
new file mode 100644
index 0..30263217ede1d
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -0,0 +1,28 @@
+# REQUIRES: system-windows
+
+## This checks that when starting lldb (or using `target create`) with a
+## program name which is on $PATH, or not specify the .exe suffix of a program
+## in the working directory on Windows, lldb can still detect the target
+## architecture correctly instead of producing an error.
+
+# RUN: mkdir -p "%t.dir"
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir/testmain.exe
+
+## Test with full path to exe
+# RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
+
+## Test with exe on path, with .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain.exe -b | FileCheck %s
+
+## Test with exe on path, without .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain -b | FileCheck %s
+
+## Test in cwd, with .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s
+
+## Test in cwd, without .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain -b | FileCheck %s
+
+# CHECK-LABEL: target create
+# CHECK-NEXT: Current executable set to '{{.*[/\\]}}testmain.exe'
+# CHECK-NOT: Error



___
lldb-commits mailing list
lldb-commits@l

[Lldb-commits] [lldb] 4d12378 - [lldb][windows] Fix crash on getting nested exception

[Martin Storsjö via lldb-commits]

Author: Alvin Wong
Date: 2022-06-22T17:16:06+03:00
New Revision: 4d123783957e547009e55346bf3a8ae43a88fa14

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

LOG: [lldb][windows] Fix crash on getting nested exception

LLDB tries to follow `EXCEPTION_RECORD::ExceptionRecord` to follow the
nested exception chain. In practice this code just causes Access
Violation whenever there is a nested exception. Since there does not
appear to be any code in LLDB that is actually using the nested
exceptions, this change just removes the crashing code and adds a
comment for future reference.

Fixes https://github.com/mstorsjo/llvm-mingw/issues/292

Reviewed By: DavidSpickett

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

Added: 


Modified: 
lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h 
b/lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
index d1692a6926b2..4499698369f5 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
+++ b/lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
@@ -25,11 +25,17 @@ namespace lldb_private {
 class ExceptionRecord {
 public:
   ExceptionRecord(const EXCEPTION_RECORD &record, lldb::tid_t thread_id) {
+// Notes about the `record.ExceptionRecord` field:
+// In the past, some code tried to parse the nested exception with it, but
+// in practice, that code just causes Access Violation. I suspect
+// `ExceptionRecord` here actually points to the address space of the
+// debuggee process. However, I did not manage to find any official or
+// unofficial reference that clarifies this point. If anyone would like to
+// reimplement this, please also keep in mind to check how this behaves 
when
+// debugging a WOW64 process. I suspect you may have to use the explicit
+// `EXCEPTION_RECORD32` and `EXCEPTION_RECORD64` structs.
 m_code = record.ExceptionCode;
 m_continuable = (record.ExceptionFlags == 0);
-if (record.ExceptionRecord)
-  m_next_exception.reset(
-  new ExceptionRecord(*record.ExceptionRecord, thread_id));
 m_exception_addr = reinterpret_cast(record.ExceptionAddress);
 m_thread_id = thread_id;
 m_arguments.assign(record.ExceptionInformation,
@@ -39,27 +45,16 @@ class ExceptionRecord {
   // MINIDUMP_EXCEPTIONs are almost identical to EXCEPTION_RECORDs.
   ExceptionRecord(const MINIDUMP_EXCEPTION &record, lldb::tid_t thread_id)
   : m_code(record.ExceptionCode), m_continuable(record.ExceptionFlags == 
0),
-m_next_exception(nullptr),
 m_exception_addr(static_cast(record.ExceptionAddress)),
 m_thread_id(thread_id),
 m_arguments(record.ExceptionInformation,
-record.ExceptionInformation + record.NumberParameters) {
-// Set up link to nested exception.
-if (record.ExceptionRecord) {
-  m_next_exception.reset(new ExceptionRecord(
-  *reinterpret_cast(record.ExceptionRecord),
-  thread_id));
-}
-  }
+record.ExceptionInformation + record.NumberParameters) {}
 
   virtual ~ExceptionRecord() {}
 
   DWORD
   GetExceptionCode() const { return m_code; }
   bool IsContinuable() const { return m_continuable; }
-  const ExceptionRecord *GetNextException() const {
-return m_next_exception.get();
-  }
   lldb::addr_t GetExceptionAddress() const { return m_exception_addr; }
 
   lldb::tid_t GetThreadID() const { return m_thread_id; }
@@ -69,7 +64,6 @@ class ExceptionRecord {
 private:
   DWORD m_code;
   bool m_continuable;
-  std::shared_ptr m_next_exception;
   lldb::addr_t m_exception_addr;
   lldb::tid_t m_thread_id;
   std::vector m_arguments;



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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a64dd5b0614: [lldb] Fix reading i686-windows executables 
with GNU environment (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,61 @@
+## Check that i386 executables can be opened in both ABI modes.
+## I386 executables are special in the sense that the PECOFF object
+## file plugin returns two architectures for them, i386 and i686, which
+## causes more elaborate comparisons to be run against the Platform plugin's
+## architecture list. Check that this is accepted despite potential mismatches
+## in the environment part of the triple.
+
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+# CHECK-LABEL: image list --triple --basename
+# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4480
+  ImageBase:   268435456
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.debug_info
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  16384
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -979,7 +979,16 @@
 
   const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
   const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-  if (lhs_triple_vendor != rhs_triple_vendor) {
+
+  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
+  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
+
+  bool both_windows = lhs_triple.isOSWindows() && rhs_triple.isOSWindows();
+
+  // On Windows, the vendor field doesn't have any practical effect, but
+  // it is often set to either "pc" or "w64".
+  if ((lhs_triple_vendor != rhs_triple_vendor) &&
+  (exact_match || !both_windows)) {
 const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
 const bool lhs_vendor_specified = TripleVendorWasSpecified();
 // Both architectures had the vendor specified, so if they aren't equal
@@ -993,8 +1002,6 @@
   return false;
   }
 
-  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
-  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
   const llvm::Triple::EnvironmentType lhs_triple_env =
   lhs_triple.getEnvironment();
   const llvm::Triple::EnvironmentType rhs_triple_env =
@@ -1032,6 +1039,9 @@
   return true;
   }
 
+  if (!exact_match && both_windows)
+return true; // The Windows environments (MSVC vs GNU) are compatible
+
   return IsCompatibleEnvironment(lhs_triple_env, rhs_triple_env);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3c86789 - [lldb] Add setting to override PE/COFF ABI by module name

[Martin Storsjö via lldb-commits]

Author: Alvin Wong
Date: 2022-06-22T17:16:06+03:00
New Revision: 3c867898c7be7ed2b5d119a2478a836a0c85f19b

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

LOG: [lldb] Add setting to override PE/COFF ABI by module name

The setting `plugin.object-file.pe-coff.module-abi` is a string-to-enum
map that allows specifying an ABI to a module name. For example:

ucrtbase.dll=msvc
libstdc++-6.dll=gnu

This allows for debugging a process which mixes both modules built using
the MSVC ABI and modules built using the MinGW ABI.

Depends on D127048

Reviewed By: DavidSpickett

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/OptionValueDictionary.h
lldb/source/Interpreter/OptionValueDictionary.cpp
lldb/source/Interpreter/Property.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/OptionValueDictionary.h 
b/lldb/include/lldb/Interpreter/OptionValueDictionary.h
index f96cbc9fe9e75..4c6bcaecd4c78 100644
--- a/lldb/include/lldb/Interpreter/OptionValueDictionary.h
+++ b/lldb/include/lldb/Interpreter/OptionValueDictionary.h
@@ -12,6 +12,7 @@
 #include 
 
 #include "lldb/Interpreter/OptionValue.h"
+#include "lldb/lldb-private-types.h"
 
 namespace lldb_private {
 
@@ -19,8 +20,10 @@ class OptionValueDictionary
 : public Cloneable {
 public:
   OptionValueDictionary(uint32_t type_mask = UINT32_MAX,
+OptionEnumValues enum_values = OptionEnumValues(),
 bool raw_value_dump = true)
-  : m_type_mask(type_mask), m_raw_value_dump(raw_value_dump) {}
+  : m_type_mask(type_mask), m_enum_values(enum_values),
+m_raw_value_dump(raw_value_dump) {}
 
   ~OptionValueDictionary() override = default;
 
@@ -75,6 +78,7 @@ class OptionValueDictionary
 protected:
   typedef std::map collection;
   uint32_t m_type_mask;
+  OptionEnumValues m_enum_values;
   collection m_values;
   bool m_raw_value_dump;
 };

diff  --git a/lldb/source/Interpreter/OptionValueDictionary.cpp 
b/lldb/source/Interpreter/OptionValueDictionary.cpp
index 26fed4a987e15..6baafc9213e1a 100644
--- a/lldb/source/Interpreter/OptionValueDictionary.cpp
+++ b/lldb/source/Interpreter/OptionValueDictionary.cpp
@@ -8,11 +8,12 @@
 
 #include "lldb/Interpreter/OptionValueDictionary.h"
 
-#include "llvm/ADT/StringRef.h"
 #include "lldb/DataFormatters/FormatManager.h"
+#include "lldb/Interpreter/OptionValueEnumeration.h"
 #include "lldb/Interpreter/OptionValueString.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/State.h"
+#include "llvm/ADT/StringRef.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -161,16 +162,26 @@ Status OptionValueDictionary::SetArgs(const Args &args,
 return error;
   }
 
-  lldb::OptionValueSP value_sp(CreateValueFromCStringForTypeMask(
-  value.str().c_str(), m_type_mask, error));
-  if (value_sp) {
+  if (m_type_mask == 1u << eTypeEnum) {
+auto enum_value =
+std::make_shared(m_enum_values, 0);
+error = enum_value->SetValueFromString(value);
 if (error.Fail())
   return error;
 m_value_was_set = true;
-SetValueForKey(ConstString(key), value_sp, true);
+SetValueForKey(ConstString(key), enum_value, true);
   } else {
-error.SetErrorString("dictionaries that can contain multiple types "
- "must subclass OptionValueArray");
+lldb::OptionValueSP value_sp(CreateValueFromCStringForTypeMask(
+value.str().c_str(), m_type_mask, error));
+if (value_sp) {
+  if (error.Fail())
+return error;
+  m_value_was_set = true;
+  SetValueForKey(ConstString(key), value_sp, true);
+} else {
+  error.SetErrorString("dictionaries that can contain multiple types "
+   "must subclass OptionValueArray");
+}
   }
 }
 break;

diff  --git a/lldb/source/Interpreter/Property.cpp 
b/lldb/source/Interpreter/Property.cpp
index fe3a8a31394b5..681596224d31a 100644
--- a/lldb/source/Interpreter/Property.cpp
+++ b/lldb/source/Interpreter/Property.cpp
@@ -68,9 +68,10 @@ Property::Property(const PropertyDefinition &definition)
   }
   case OptionValue::eTypeDictionary:
 // "definition.default_uint_value" is always a OptionValue::Type
-m_value_sp =
-std::make_shared(OptionValue::ConvertTypeToMask(
-(OptionValue::Type)definition.default_uint_value));
+m_value_sp = std::make_shared(
+OptionValue::ConvertTypeToMask(
+ 

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2bae95605753: [lldb] Resolve exe location for `target 
create` (authored by alvinhochun, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Target/TargetList.cpp
  lldb/test/Shell/Commands/command-target-create-resolve-exe.test


Index: lldb/test/Shell/Commands/command-target-create-resolve-exe.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -0,0 +1,28 @@
+# REQUIRES: system-windows
+
+## This checks that when starting lldb (or using `target create`) with a
+## program name which is on $PATH, or not specify the .exe suffix of a program
+## in the working directory on Windows, lldb can still detect the target
+## architecture correctly instead of producing an error.
+
+# RUN: mkdir -p "%t.dir"
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir/testmain.exe
+
+## Test with full path to exe
+# RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
+
+## Test with exe on path, with .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain.exe -b | FileCheck %s
+
+## Test with exe on path, without .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain -b | FileCheck %s
+
+## Test in cwd, with .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s
+
+## Test in cwd, without .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain -b | FileCheck %s
+
+# CHECK-LABEL: target create
+# CHECK-NEXT: Current executable set to '{{.*[/\\]}}testmain.exe'
+# CHECK-NOT: Error
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -121,6 +121,14 @@
   if (!user_exe_path.empty()) {
 ModuleSpec module_spec(FileSpec(user_exe_path, FileSpec::Style::native));
 FileSystem::Instance().Resolve(module_spec.GetFileSpec());
+
+// Try to resolve the exe based on PATH and/or platform-specific suffixes,
+// but only if using the host platform.
+if (platform_sp->IsHost() &&
+!FileSystem::Instance().Exists(module_spec.GetFileSpec()))
+  FileSystem::Instance().ResolveExecutableLocation(
+  module_spec.GetFileSpec());
+
 // Resolve the executable in case we are given a path to a application
 // bundle like a .app bundle on MacOSX.
 Host::ResolveExecutableInBundle(module_spec.GetFileSpec());
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -299,12 +299,6 @@
 
   const char *file_path = command.GetArgumentAtIndex(0);
   LLDB_SCOPED_TIMERF("(lldb) target create '%s'", file_path);
-  FileSpec file_spec;
-
-  if (file_path) {
-file_spec.SetFile(file_path, FileSpec::Style::native);
-FileSystem::Instance().Resolve(file_spec);
-  }
 
   bool must_set_platform_path = false;
 
@@ -333,6 +327,18 @@
 
   PlatformSP platform_sp = target_sp->GetPlatform();
 
+  FileSpec file_spec;
+  if (file_path) {
+file_spec.SetFile(file_path, FileSpec::Style::native);
+FileSystem::Instance().Resolve(file_spec);
+
+// Try to resolve the exe based on PATH and/or platform-specific
+// suffixes, but only if using the host platform.
+if (platform_sp && platform_sp->IsHost() &&
+!FileSystem::Instance().Exists(file_spec))
+  FileSystem::Instance().ResolveExecutableLocation(file_spec);
+  }
+
   if (remote_file) {
 if (platform_sp) {
   // I have a remote file.. two possible cases


Index: lldb/test/Shell/Commands/command-target-create-resolve-exe.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -0,0 +1,28 @@
+# REQUIRES: system-windows
+
+## This checks that when starting lldb (or using `target create`) with a
+## program name which is on $PATH, or not specify the .exe suffix of a program
+## in the working directory on Windows, lldb can still detect the target
+## architecture correctly instead of producing an error.
+
+# RUN: mkdir -p "%t.dir"
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir/testmain.exe
+
+## Test with full path to exe
+# RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
+
+## Test with exe on path, with .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain.exe -b | FileCheck %s
+
+## Test with exe on path, without .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain -b | FileCheck %s
+
+## Test in cwd, with .exe suffix
+# RUN: cd "%t.dir" && %lldb testmai

[Lldb-commits] [PATCH] D128201: [lldb][windows] Fix crash on getting nested exception

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4d123783957e: [lldb][windows] Fix crash on getting nested 
exception (authored by alvinhochun, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128201

Files:
  lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h


Index: lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
===
--- lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
+++ lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
@@ -25,11 +25,17 @@
 class ExceptionRecord {
 public:
   ExceptionRecord(const EXCEPTION_RECORD &record, lldb::tid_t thread_id) {
+// Notes about the `record.ExceptionRecord` field:
+// In the past, some code tried to parse the nested exception with it, but
+// in practice, that code just causes Access Violation. I suspect
+// `ExceptionRecord` here actually points to the address space of the
+// debuggee process. However, I did not manage to find any official or
+// unofficial reference that clarifies this point. If anyone would like to
+// reimplement this, please also keep in mind to check how this behaves 
when
+// debugging a WOW64 process. I suspect you may have to use the explicit
+// `EXCEPTION_RECORD32` and `EXCEPTION_RECORD64` structs.
 m_code = record.ExceptionCode;
 m_continuable = (record.ExceptionFlags == 0);
-if (record.ExceptionRecord)
-  m_next_exception.reset(
-  new ExceptionRecord(*record.ExceptionRecord, thread_id));
 m_exception_addr = reinterpret_cast(record.ExceptionAddress);
 m_thread_id = thread_id;
 m_arguments.assign(record.ExceptionInformation,
@@ -39,27 +45,16 @@
   // MINIDUMP_EXCEPTIONs are almost identical to EXCEPTION_RECORDs.
   ExceptionRecord(const MINIDUMP_EXCEPTION &record, lldb::tid_t thread_id)
   : m_code(record.ExceptionCode), m_continuable(record.ExceptionFlags == 
0),
-m_next_exception(nullptr),
 m_exception_addr(static_cast(record.ExceptionAddress)),
 m_thread_id(thread_id),
 m_arguments(record.ExceptionInformation,
-record.ExceptionInformation + record.NumberParameters) {
-// Set up link to nested exception.
-if (record.ExceptionRecord) {
-  m_next_exception.reset(new ExceptionRecord(
-  *reinterpret_cast(record.ExceptionRecord),
-  thread_id));
-}
-  }
+record.ExceptionInformation + record.NumberParameters) {}
 
   virtual ~ExceptionRecord() {}
 
   DWORD
   GetExceptionCode() const { return m_code; }
   bool IsContinuable() const { return m_continuable; }
-  const ExceptionRecord *GetNextException() const {
-return m_next_exception.get();
-  }
   lldb::addr_t GetExceptionAddress() const { return m_exception_addr; }
 
   lldb::tid_t GetThreadID() const { return m_thread_id; }
@@ -69,7 +64,6 @@
 private:
   DWORD m_code;
   bool m_continuable;
-  std::shared_ptr m_next_exception;
   lldb::addr_t m_exception_addr;
   lldb::tid_t m_thread_id;
   std::vector m_arguments;


Index: lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
===
--- lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
+++ lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
@@ -25,11 +25,17 @@
 class ExceptionRecord {
 public:
   ExceptionRecord(const EXCEPTION_RECORD &record, lldb::tid_t thread_id) {
+// Notes about the `record.ExceptionRecord` field:
+// In the past, some code tried to parse the nested exception with it, but
+// in practice, that code just causes Access Violation. I suspect
+// `ExceptionRecord` here actually points to the address space of the
+// debuggee process. However, I did not manage to find any official or
+// unofficial reference that clarifies this point. If anyone would like to
+// reimplement this, please also keep in mind to check how this behaves when
+// debugging a WOW64 process. I suspect you may have to use the explicit
+// `EXCEPTION_RECORD32` and `EXCEPTION_RECORD64` structs.
 m_code = record.ExceptionCode;
 m_continuable = (record.ExceptionFlags == 0);
-if (record.ExceptionRecord)
-  m_next_exception.reset(
-  new ExceptionRecord(*record.ExceptionRecord, thread_id));
 m_exception_addr = reinterpret_cast(record.ExceptionAddress);
 m_thread_id = thread_id;
 m_arguments.assign(record.ExceptionInformation,
@@ -39,27 +45,16 @@
   // MINIDUMP_EXCEPTIONs are almost identical to EXCEPTION_RECORDs.
   ExceptionRecord(const MINIDUMP_EXCEPTION &record, lldb::tid_t thread_id)
   : m_code(record.ExceptionCode), m_continuable(record.ExceptionFlags == 0),
-m_next_exception(nullptr),
 m_exception_addr(static_cast(record.ExceptionAd

[Lldb-commits] [PATCH] D127234: [lldb] Add setting to override PE/COFF ABI by module name

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c867898c7be: [lldb] Add setting to override PE/COFF ABI by 
module name (authored by alvinhochun, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127234

Files:
  lldb/include/lldb/Interpreter/OptionValueDictionary.h
  lldb/source/Interpreter/OptionValueDictionary.cpp
  lldb/source/Interpreter/Property.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
@@ -1,4 +1,8 @@
 # RUN: yaml2obj %s -o %t
+# RUN: yaml2obj %s -o %t.debug
+# RUN: mkdir -p %t.dir
+# RUN: yaml2obj %s -o %t.dir/UPPER_CASE
+# RUN: yaml2obj %s -o %t.dir/UPPER_CASE.debug
 
 ## Default ABI is msvc:
 # RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
@@ -10,6 +14,57 @@
 # RUN:   -f %t -o "image list --triple --basename" -o exit | \
 # RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
 
+## Default ABI is msvc, module override is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu, module override is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is msvc, module override is gnu (with .debug suffix):
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=gnu" \
+# RUN:   -f %t.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp.debug %s
+
+## Default ABI is gnu, module override is msvc (with .debug suffix):
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=msvc" \
+# RUN:   -f %t.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp.debug %s
+
+## Check that case-sensitive match is chosen before lower-case:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case=msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi UPPER_CASE=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE %s
+
+## Check that lower-case match with .debug suffix is chosen before case-sensitive match without .debug suffix:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi UPPER_CASE=msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case.debug=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE.debug %s
+
+## Check that case-sensitive match without .debug suffix is chosen before lower-case match without .debug suffix:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case.debug=msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi UPPER_CASE.debug=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE.debug %s
+
+## Check that lower-case match without .debug suffix works:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case.debug=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE.debug %s
+
 # CHECK-LABEL: image list --triple --basename
 # CHECK-NEXT: x86_64-pc-windows-[[ABI]] [[FILENAME]]
 
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFPrope

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova updated this revision to Diff 439021.
cassanova added a comment.

Updated ASCII header to work with 80-column limit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128292

Files:
  lldb/tools/lldb-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
  
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp


Index: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
===
--- /dev/null
+++ 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,47 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-===//
+
+#include 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace lldb;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  // Convert the data into a null-terminated string
+  std::string str((char *)data, size);
+
+  // Create a debugger and a dummy target
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.GetDummyTarget();
+
+  // Create a command interpreter for the current debugger
+  // A return object is needed to run the command interpreter
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter ci = debugger.GetCommandInterpreter();
+
+  // Use the fuzzer generated input as input for the command interpreter
+  if (ci.IsValid()) {
+ci.HandleCommand(str.c_str(), ro, false);
+  }
+
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+  SBModule::GarbageCollectAllocatedModules();
+
+  return 0;
+}
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
@@ -0,0 +1,4 @@
+kw1="breakpoint set"
+kw2="target"
+kw3="run"
+kw4="frame info"
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
@@ -0,0 +1,22 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_fuzzer(lldb-commandinterpreter-fuzzer
+  EXCLUDE_FROM_ALL
+  lldb-commandinterpreter-fuzzer.cpp
+  )
+
+if(TARGET lldb-commandinterpreter-fuzzer)
+  target_include_directories(lldb-commandinterpreter-fuzzer PRIVATE ..)
+  target_link_libraries(lldb-commandinterpreter-fuzzer
+PRIVATE
+liblldb
+)
+
+  add_custom_target(fuzz-lldb-commandinterpreter
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND cd ${CMAKE_CURRENT_SOURCE_DIR} && 
$ 
-dict=${CMAKE_CURRENT_SOURCE_DIR}/inputdictionary.txt  -only_ascii=1
+USES_TERMINAL
+)
+endif()
Index: lldb/tools/lldb-fuzzer/CMakeLists.txt
===
--- lldb/tools/lldb-fuzzer/CMakeLists.txt
+++ lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -1,2 +1,3 @@
+add_subdirectory(lldb-commandinterpreter-fuzzer)
 add_subdirectory(lldb-target-fuzzer)
 add_subdirectory(utils)


Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,47 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-===//
+
+#include 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace lldb;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzer

[Lldb-commits] [lldb] 0bc7105 - [lldb] Tentative attempt to fix command-target-create-resolve-exe.test on buildbot

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2022-06-22T18:48:04+03:00
New Revision: 0bc7105cd1447c2d85716c3dcd2924595decc939

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

LOG: [lldb] Tentative attempt to fix command-target-create-resolve-exe.test on 
buildbot

This test does succeed in my local test environment though, but
fails on the buildbot.

Added: 


Modified: 
lldb/test/Shell/Commands/command-target-create-resolve-exe.test

Removed: 




diff  --git a/lldb/test/Shell/Commands/command-target-create-resolve-exe.test 
b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
index 30263217ede1..1664a45fd32a 100644
--- a/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
+++ b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -12,10 +12,10 @@
 # RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
 
 ## Test with exe on path, with .exe suffix
-# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain.exe -b | FileCheck %s
+# RUN: env PATH="%t.dir;$PATH" %lldb testmain.exe -b | FileCheck %s
 
 ## Test with exe on path, without .exe suffix
-# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain -b | FileCheck %s
+# RUN: env PATH="%t.dir;$PATH" %lldb testmain -b | FileCheck %s
 
 ## Test in cwd, with .exe suffix
 # RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s



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

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I think we should allow this class to always exist and not conditionally 
compile it in or out. Then we add a new Host.h method to emit a log message in 
"lldb/Host/Host.h" and allow each host OS to emit a message. Maybe there is a 
default implementation that emits the message to stderr, and then we override 
this for in HostInfoMacOSX.h? Then each OS can use their OS logging API of 
choice.




Comment at: lldb/include/lldb/Utility/Log.h:99
 
+#if defined(__APPLE__)
+class OSLogLogHandler : public LogHandler {

remove #if so everyone can use this and we can enable this in any log.



Comment at: lldb/source/Utility/Log.cpp:35-39
+#if defined(__APPLE__)
+#include 
+static os_log_t g_os_log;
+static std::once_flag g_os_log_once;
+#endif

move this to HostInfoMacOSX.h in the Host::EmitLogMessage() as mentioned in the 
main feedback comment



Comment at: lldb/source/Utility/Log.cpp:399
+
+#if defined(__APPLE__)
+OSLogLogHandler::OSLogLogHandler() {

remove #if



Comment at: lldb/source/Utility/Log.cpp:400-404
+OSLogLogHandler::OSLogLogHandler() {
+  std::call_once(g_os_log_once, []() {
+g_os_log = os_log_create("com.apple.dt.lldb", "lldb");
+  });
+}

move this to HostInfoMacOSX.h to be used in 
HostInfoMacOSX::EmitLogMessage(StringRef)



Comment at: lldb/source/Utility/Log.cpp:407
+void OSLogLogHandler::Emit(llvm::StringRef message) {
+  os_log_with_type(g_os_log, OS_LOG_TYPE_DEFAULT, "%{public}s", 
message.data());
+}

Be careful as "message.data()" might cause more data to be emitted. We will 
need to use "message.str().c_str()" to ensure it is NULL terminated. Though we 
could require that the "message" come in as NULL terminated already, so maybe 
we don't need to, but we should document it, and possibly add a:

```
assert(message.data()[message.size()] == '\0');
```


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

https://reviews.llvm.org/D128321

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

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/lldb-private-enumerations.h:229
+  eLogHandlerCallback,
+  eLogHandlerRotating,
+#if defined(__APPLE__)

Maybe "eLogHandlerCircular"?



Comment at: lldb/include/lldb/lldb-private-enumerations.h:230
+  eLogHandlerRotating,
+#if defined(__APPLE__)
+  eLogHandlerOSLog,

remove #if. See inlined comments in https://reviews.llvm.org/D128321



Comment at: lldb/source/Commands/CommandObjectLog.cpp:37
+},
+#if defined(__APPLE__)
+{

remove #if (see https://reviews.llvm.org/D128321)



Comment at: lldb/source/Commands/CommandObjectLog.cpp:41
+"oslog",
+"Use the OSLog log handler",
+},





Comment at: lldb/source/Commands/CommandObjectLog.cpp:167
   buffer_size.Clear();
+  handler = eLogHandlerStream;
   log_options = 0;

Do we want to define a "eLogHandlerDefault" which points to "eLogHandlerStream"?



Comment at: lldb/source/Commands/Options.td:436-437
 Desc<"Set the log to be buffered, using the specified buffer size.">;
+  def log_handler : Option<"handler", "h">, Group<1>,
+EnumArg<"Value", "LogHandlerType()">, Desc<"Use a custom handler.">;
   def log_threadsafe : Option<"threadsafe", "t">, Group<1>,

Maybe "--type" or "--kind" would be better? Handler seems like an internal 
name. Maybe also mention what it will default to if not specified? Can the user 
see a list of the valid values or do we need to supply these in the description?



Comment at: lldb/source/Core/Debugger.cpp:1418
+return std::make_shared(buffer_size);
+#if defined(__APPLE__)
+  case eLogHandlerOSLog:

remove #if (see https://reviews.llvm.org/D128321)


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

https://reviews.llvm.org/D128323

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

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[Jim Ingham via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4298b1b8d137: Add a "-b" option to "process 
continue" to run to a set of breakpoints, (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

Files:
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/commands/process/continue_to_bkpt/Makefile
  lldb/test/API/commands/process/continue_to_bkpt/TestContinueToBkpts.py
  lldb/test/API/commands/process/continue_to_bkpt/main.c

Index: lldb/test/API/commands/process/continue_to_bkpt/main.c
===
--- /dev/null
+++ lldb/test/API/commands/process/continue_to_bkpt/main.c
@@ -0,0 +1,18 @@
+#include 
+
+int main (int argc, char const *argv[])
+{
+  int pass_me = argc + 10; // Stop here to get started.
+  printf("This is the zeroth stop\n");
+  printf("This is the first stop\n");
+  printf("This is the second stop\n");
+  printf("This is the third stop\n");
+  printf("This is the fourth stop\n");
+  printf("This is the fifth stop\n");
+  printf("This is the sixth stop\n");
+  printf("This is the seventh stop\n");
+  printf("This is the eighth stop\n");
+  printf("This is the nineth stop\n");
+
+  return 0;
+}
Index: lldb/test/API/commands/process/continue_to_bkpt/TestContinueToBkpts.py
===
--- /dev/null
+++ lldb/test/API/commands/process/continue_to_bkpt/TestContinueToBkpts.py
@@ -0,0 +1,132 @@
+"""
+Test the "process continue -b" option.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestContinueToBkpts(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(['pyapi'])
+def test_continue_to_breakpoints(self):
+"""Test that the continue to breakpoints feature works correctly."""
+self.build()
+self.do_test_continue_to_breakpoint()
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+self.main_source_spec = lldb.SBFileSpec("main.c")
+
+def continue_and_check(self, stop_list, bkpt_to_hit, loc_to_hit = 0):
+"""Build up a command that will run a continue -b commands using the breakpoints on stop_list, and
+   ensure that we hit bkpt_to_hit.
+   If loc_to_hit is not 0, also verify that we hit that location."""
+command = "process continue"
+for elem in stop_list:
+command += " -b {0}".format(elem)
+self.expect(command)
+self.assertEqual(self.thread.stop_reason, lldb.eStopReasonBreakpoint, "Hit a breakpoint")
+self.assertEqual(self.thread.GetStopReasonDataAtIndex(0), bkpt_to_hit, "Hit the right breakpoint")
+if loc_to_hit != 0:
+self.assertEqual(self.thread.GetStopReasonDataAtIndex(1), loc_to_hit, "Hit the right location")
+for bkpt_id in self.bkpt_list:
+bkpt = self.target.FindBreakpointByID(bkpt_id)
+self.assertTrue(bkpt.IsValid(), "Breakpoint id's round trip")
+if bkpt.MatchesName("disabled"):
+self.assertFalse(bkpt.IsEnabled(), "Disabled breakpoints stay disabled: {0}".format(bkpt.GetID()))
+else:
+self.assertTrue(bkpt.IsEnabled(), "Enabled breakpoints stay enabled: {0}".format(bkpt.GetID()))
+# Also do our multiple location one:
+bkpt = self.target.FindBreakpointByID(self.multiple_loc_id)
+self.assertTrue(bkpt.IsValid(), "Breakpoint with locations round trip")
+for i in range(1,3):
+loc = bkpt.FindLocationByID(i)
+self.assertTrue(loc.IsValid(), "Locations round trip")
+if i == 2:
+self.assertTrue(loc.IsEnabled(), "Locations that were enabled stay enabled")
+else:
+self.assertFalse(loc.IsEnabled(), "Locations that were disabled stay disabled")
+
+def do_test_continue_to_breakpoint(self):
+"""Test the continue to breakpoint feature."""
+(self.target, process, self.thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+   "Stop here to get started", self.main_source_spec)
+
+# Now set up all our breakpoints:
+bkpt_pattern = "This is the {0} stop"
+bkpt_elements = ["zeroth", "first", "second", "third", "fourth", "fifth", "sixth", "seventh", "eighth", "nineth"]
+disabled_bkpts = ["first", "eigth"]
+bkpts_for_MyBKPT = ["first", "sixth", "nineth"]
+self.bkpt_list = []
+for elem in bkpt_elements:
+bkpt = self.target.BreakpointCreateBySourceRegex(bkpt_pattern.format(elem), self.main_source_spec)
+

[Lldb-commits] [lldb] 4298b1b - Add a "-b" option to "process continue" to run to a set of breakpoints,

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-06-22T09:55:30-07:00
New Revision: 4298b1b8d13715963cc1a917bc122208a29710fb

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

LOG: Add a "-b" option to "process continue" to run to a set of breakpoints,
temporarily ignoring the others.

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

Added: 
lldb/test/API/commands/process/continue_to_bkpt/Makefile
lldb/test/API/commands/process/continue_to_bkpt/TestContinueToBkpts.py
lldb/test/API/commands/process/continue_to_bkpt/main.c

Modified: 
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index f5961d9b1c00d..def0e00af0876 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -8,9 +8,12 @@
 
 #include "CommandObjectProcess.h"
 #include "CommandObjectTrace.h"
+#include "CommandObjectBreakpoint.h"
 #include "CommandOptionsProcessLaunch.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Breakpoint/BreakpointIDList.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
+#include "lldb/Breakpoint/BreakpointName.h"
 #include "lldb/Breakpoint/BreakpointSite.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
@@ -29,6 +32,8 @@
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/State.h"
 
+#include "llvm/ADT/ScopeExit.h"
+
 #include 
 
 using namespace lldb;
@@ -516,7 +521,7 @@ class CommandObjectProcessContinue : public 
CommandObjectParsed {
 ~CommandOptions() override = default;
 
 Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
-  ExecutionContext *execution_context) override {
+  ExecutionContext *exe_ctx) override {
   Status error;
   const int short_option = m_getopt_table[option_idx].val;
   switch (short_option) {
@@ -526,7 +531,10 @@ class CommandObjectProcessContinue : public 
CommandObjectParsed {
   "invalid value for ignore option: \"%s\", should be a number.",
   option_arg.str().c_str());
 break;
-
+  case 'b':
+m_run_to_bkpt_args.AppendArgument(option_arg);
+m_any_bkpts_specified = true;
+  break;
   default:
 llvm_unreachable("Unimplemented option");
   }
@@ -535,15 +543,20 @@ class CommandObjectProcessContinue : public 
CommandObjectParsed {
 
 void OptionParsingStarting(ExecutionContext *execution_context) override {
   m_ignore = 0;
+  m_run_to_bkpt_args.Clear();
+  m_any_bkpts_specified = false;
 }
 
 llvm::ArrayRef GetDefinitions() override {
   return llvm::makeArrayRef(g_process_continue_options);
 }
 
-uint32_t m_ignore;
+uint32_t m_ignore = 0;
+Args m_run_to_bkpt_args;
+bool m_any_bkpts_specified = false;
   };
 
+
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 Process *process = m_exe_ctx.GetProcessPtr();
 bool synchronous_execution = m_interpreter.GetSynchronous();
@@ -579,6 +592,127 @@ class CommandObjectProcessContinue : public 
CommandObjectParsed {
   }
 }
   }
+  
+  Target *target = m_exe_ctx.GetTargetPtr();
+  BreakpointIDList run_to_bkpt_ids;
+  CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
+  m_options.m_run_to_bkpt_args, target, result, &run_to_bkpt_ids, 
+  BreakpointName::Permissions::disablePerm);
+  if (!result.Succeeded()) {
+return false;
+  }
+  result.Clear();
+  if (m_options.m_any_bkpts_specified && run_to_bkpt_ids.GetSize() == 0) {
+result.AppendError("continue-to breakpoints did not specify any actual 
"
+   "breakpoints or locations");
+return false;
+  }
+
+  // First figure out which breakpoints & locations were specified by the
+  // user:
+  size_t num_run_to_bkpt_ids = run_to_bkpt_ids.GetSize();
+  std::vector bkpts_disabled;
+  std::vector locs_disabled;
+  if (num_run_to_bkpt_ids != 0) {
+// Go through the ID's specified, and separate the breakpoints from 
are 
+// the breakpoint.location specifications since the latter require
+// special handling.  We also figure out whether there's at least one
+// specifier in the set that is enabled.
+BreakpointList &bkpt_list = target->GetBreakpointList();
+std::unordered_set bkpts_seen;
+std::unordered_set bkpts_with_locs_seen;
+BreakpointIDList with_locs;
+bool any_enabled = false;
+  
+for (size_t idx = 0; idx < num_run_to_bkpt_ids; idx++) {
+  BreakpointID bkpt_id =

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 4 inline comments as done.
wallace added inline comments.



Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:46
 /// state and granularity.
 class TraceInstructionDumper {
 public:

jj10306 wrote:
> nit: wdyt ab renaming this to `TraceInstructionsDumper` since now we have the 
> `TraceInstructionWriter` which is responsible for displaying a single 
> instruction? With how things are currently named it's slightly confusing at 
> first glance because there names sound like they are both doing the same 
> thing (something related to a single instruction).
I'm renaming TraceInstructionWriter to OutputWriter, just because that one is 
an internal class



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2210
 size_t m_continue;
+llvm::Optional m_output_file;
 TraceInstructionDumperOptions m_dumper_options;

jj10306 wrote:
> Why is this stored on the command object and not the 
> `TraceinstructionDumperOptions`?
> Can you explain the separation of responsibility of the dumper options versus 
> the options here?
basically just because this field is not used in the dumper at all. The dumper 
requires a Stream object, which is configured by the caller, either a file 
stream or a standard output stream (or maybe a python string stream). So in 
this case, because this field is only used by this command handler to create 
the actual Stream to pass to the dumper, I'm restricting this variable to here.



Comment at: lldb/source/Target/TraceInstructionDumper.cpp:90-91
 
-void TraceInstructionDumper::DumpInstructionSymbolContext(
-const Optional &prev_insn,
-const InstructionSymbolInfo &insn) {
-  if (prev_insn && IsSameInstructionSymbolContext(*prev_insn, insn))
-return;
+class TraceInstructionWriterCLI
+: public TraceInstructionDumper::TraceInstructionWriter {
+public:

jj10306 wrote:
> Move declarations to header?
in this case I'd rather not do that because I don't want anyone to see these 
classes. They are very restricted to the use case of the dumper and no one 
should try to use them for any reason. So I'm making them very private 
intentionally



Comment at: lldb/source/Target/TraceInstructionDumper.cpp:219-224
+m_j.attribute("module", GetModuleName(insn));
+m_j.attribute("symbol",
+  insn.symbol_info->sc.GetFunctionName().AsCString());
+m_j.attribute("mnemonic", insn.symbol_info->instruction->GetMnemonic(
+  &insn.symbol_info->exe_ctx));
+

jj10306 wrote:
> the values being passed to the `attribute` method are c strings. Unsure how 
> this method handles a nullptr, but consider checking if these are nullptr 
> before calling attribute if necessary?
good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128316

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

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

[walter erquinigo via Phabricator via lldb-commits]

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

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128316

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/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceTSC.py

Index: lldb/test/API/commands/trace/TestTraceTSC.py
===
--- lldb/test/API/commands/trace/TestTraceTSC.py
+++ lldb/test/API/commands/trace/TestTraceTSC.py
@@ -17,7 +17,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=\d+\] 0x00400511movl"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -58,7 +58,10 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=\d+\] 0x00400511movl"])
+
+self.expect("thread trace dump instructions --tsc -c 1 --pretty-json",
+patterns=['''"tsc": "\d+"'''])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -73,6 +76,9 @@
 self.expect("thread trace dump instructions --tsc -c 1",
 patterns=["0: \[tsc=unavailable\] 0x00400511movl"])
 
+self.expect("thread trace dump instructions --tsc -c 1 --json",
+patterns=['''"tsc":null'''])
+
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
 def testPSBPeriod(self):
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -13,11 +13,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+  substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+   "19520: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["67910: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 @testSBAPIAndCommands
@@ -26,11 +26,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+  substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+   "19520: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["67910: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 @testSBAPIAndCommands
@@ -39,11 +39,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing e

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

This broke the Windows LLDB bot:

https://lab.llvm.org/buildbot/#/builders/83/builds/20295/steps/7/logs/stdio


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

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

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D127436#3602224 , 
@stella.stamenova wrote:

> This broke the Windows LLDB bot:
>
> https://lab.llvm.org/buildbot/#/builders/83/builds/20295/steps/7/logs/stdio

Yep, noted. It worked for me in my MSVC build configuration - I'll investigate 
a bit more and either push another attempt at fixing it, or revert, in a little 
while.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

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

[Lldb-commits] [PATCH] D128069: [lldb] add SBSection.alignment to python bindings

[David M. Lary via Phabricator via lldb-commits]

dmlary added a comment.

> We should test any APIs we add in a python test IMHO.  Also testing that the 
> ".alignment" property works

Ok, I'll add tests for the added python function and property.

Now for the hard part, what's the recommended way to access elf section details 
within python without using the function added in this commit?  I do not think 
it is safe to hard code the expected alignment in the test case as the default 
alignment may vary across architectures.  Shelling out to `readelf` and parsing 
is possible, but feels clunky; is there an existing function to get this data?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128069

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

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 accepted this revision.
jj10306 added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Target/TraceInstructionDumper.cpp:193-194
+  OutputWriterJSON(Stream &s, const TraceInstructionDumperOptions &options)
+  : m_s(s), m_options(options), m_j(/*IndentSize=*/m_s.AsRawOstream(),
+options.pretty_print_json ? 2 : 0) {
+m_j.arrayBegin();

the inline comment should be next to the second parameter, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128316

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

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/source/Target/TraceInstructionDumper.cpp:193-194
+  OutputWriterJSON(Stream &s, const TraceInstructionDumperOptions &options)
+  : m_s(s), m_options(options), m_j(/*IndentSize=*/m_s.AsRawOstream(),
+options.pretty_print_json ? 2 : 0) {
+m_j.arrayBegin();

jj10306 wrote:
> the inline comment should be next to the second parameter, right?
silly me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128316

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

[Lldb-commits] [PATCH] D128361: [lldb] [test] Move part of fork tests to common helper

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

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, jingham.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D128361

Files:
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -12,25 +12,31 @@
 fork_capture = {1: "parent_pid", 2: "parent_tid",
 3: "child_pid", 4: "child_tid"}
 
-@add_test_categories(["fork"])
-def test_fork_multithreaded(self):
+def start_fork_test(self, args, variant="fork"):
 self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=["thread:new"]*2 + ["fork"])
-self.add_qSupported_packets(["multiprocess+", "fork-events+"])
+self.prep_debug_monitor_and_inferior(inferior_args=args)
+self.add_qSupported_packets(["multiprocess+",
+ "{}-events+".format(variant)])
 ret = self.expect_gdbremote_sequence()
-self.assertIn("fork-events+", ret["qSupported_response"])
+self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
 self.reset_test_sequence()
 
 # continue and expect fork
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format("fork"),
+{"direction": "send", "regex": self.fork_regex.format(variant),
  "capture": self.fork_capture},
 ], True)
 ret = self.expect_gdbremote_sequence()
-child_pid = ret["child_pid"]
 self.reset_test_sequence()
 
+return tuple(ret[x] for x in ("parent_pid", "parent_tid",
+  "child_pid", "child_tid"))
+
+@add_test_categories(["fork"])
+def test_fork_multithreaded(self):
+_, _, child_pid, _ = self.start_fork_test(["thread:new"]*2 + ["fork"])
+
 # detach the forked child
 self.test_sequence.add_log_lines([
 "read packet: $D;{}#00".format(child_pid),
@@ -40,26 +46,8 @@
 self.expect_gdbremote_sequence()
 
 def fork_and_detach_test(self, variant):
-self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=[variant])
-self.add_qSupported_packets(["multiprocess+",
- "{}-events+".format(variant)])
-ret = self.expect_gdbremote_sequence()
-self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
-self.reset_test_sequence()
-
-# continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
-ret = self.expect_gdbremote_sequence()
-parent_pid = ret["parent_pid"]
-parent_tid = ret["parent_tid"]
-child_pid = ret["child_pid"]
-child_tid = ret["child_tid"]
-self.reset_test_sequence()
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test([variant], variant))
 
 # detach the forked child
 self.test_sequence.add_log_lines([
@@ -106,26 +94,8 @@
 self.expect_gdbremote_sequence()
 
 def fork_and_follow_test(self, variant):
-self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=[variant])
-self.add_qSupported_packets(["multiprocess+",
- "{}-events+".format(variant)])
-ret = self.expect_gdbremote_sequence()
-self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
-self.reset_test_sequence()
-
-# continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
-ret = self.expect_gdbremote_sequence()
-parent_pid = ret["parent_pid"]
-parent_tid = ret["parent_tid"]
-child_pid = ret["child_pid"]
-child_tid = ret["child_tid"]
-self.reset_test_sequence()
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test([variant], variant))
 
 # switch to the forked child
 self.test_sequence.add_log_lines([
@@ -223,26 +193,8 @@
 
 @add_test_categories(["fork"])
 def test_detach_all(self):
-self.build()
-self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
-self.add_qSupported_packets(["multiprocess+",
- "fork-events+"])
- 

[Lldb-commits] [lldb] a1ee0b9 - [lldb] Second attempt at fixing command-target-create-resolve-exe.test on the buildbot

[Martin Storsjö via lldb-commits]

Author: Alvin Wong
Date: 2022-06-22T20:49:30+03:00
New Revision: a1ee0b947d46c9be1cc2ea8db21603bac84efb18

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

LOG: [lldb] Second attempt at fixing command-target-create-resolve-exe.test on 
the buildbot

Added: 


Modified: 
lldb/test/Shell/Commands/command-target-create-resolve-exe.test
lldb/test/Shell/Commands/lit.local.cfg

Removed: 




diff  --git a/lldb/test/Shell/Commands/command-target-create-resolve-exe.test 
b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
index 1664a45fd32a5..0ebbaf25e652d 100644
--- a/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
+++ b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -12,10 +12,10 @@
 # RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
 
 ## Test with exe on path, with .exe suffix
-# RUN: env PATH="%t.dir;$PATH" %lldb testmain.exe -b | FileCheck %s
+# RUN: env PATH="%t.dir%{pathsep}%{PATH}" %lldb testmain.exe -b | FileCheck %s
 
 ## Test with exe on path, without .exe suffix
-# RUN: env PATH="%t.dir;$PATH" %lldb testmain -b | FileCheck %s
+# RUN: env PATH="%t.dir%{pathsep}%{PATH}" %lldb testmain -b | FileCheck %s
 
 ## Test in cwd, with .exe suffix
 # RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s

diff  --git a/lldb/test/Shell/Commands/lit.local.cfg 
b/lldb/test/Shell/Commands/lit.local.cfg
index 60e87e4624e52..b83eee443fcca 100644
--- a/lldb/test/Shell/Commands/lit.local.cfg
+++ b/lldb/test/Shell/Commands/lit.local.cfg
@@ -1 +1,4 @@
 config.suffixes = ['.s', '.test', '.yaml']
+
+# This is needed by command-target-create-resolve-exe.test
+config.substitutions.append(('%{PATH}', config.environment['PATH']))



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

[Lldb-commits] [lldb] 130167e - [LLDB] Handle DIE with DW_AT_low_pc and empty ranges

[Alexander Yermolovich via lldb-commits]

Author: Alexander Yermolovich
Date: 2022-06-22T10:54:25-07:00
New Revision: 130167ed1effa36aa56c83c4293e732e5163d993

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

LOG: [LLDB] Handle DIE with DW_AT_low_pc and empty ranges

The case comes out of how BOLT handles transformation of
DW_AT_low_pc/DW_AT_high_pc into DW_AT_low_pc/DW_AT_high_pc
with latter being 0.

Reviewed By: clayborg

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

Added: 
lldb/test/Shell/Commands/Inputs/dwarf4-low-pc-ranges-inlining.s
lldb/test/Shell/Commands/Inputs/dwarf5-low-pc-ranges-inlining.s
lldb/test/Shell/Commands/dwarf4-low-pc-ranges-inlining.test
lldb/test/Shell/Commands/dwarf5-low-pc-ranges-inlining.test

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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index c0bf13e0281d3..7be67f83add3d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1279,6 +1279,8 @@ size_t SymbolFileDWARF::ParseBlocksRecursive(
 const size_t num_ranges = ranges.GetSize();
 for (size_t i = 0; i < num_ranges; ++i) {
   const DWARFRangeList::Entry &range = ranges.GetEntryRef(i);
+  if (range.GetByteSize() == 0)
+continue;
   const addr_t range_base = range.GetRangeBase();
   if (range_base >= subprogram_low_pc)
 block->AddRange(Block::Range(range_base - subprogram_low_pc,

diff  --git a/lldb/test/Shell/Commands/Inputs/dwarf4-low-pc-ranges-inlining.s 
b/lldb/test/Shell/Commands/Inputs/dwarf4-low-pc-ranges-inlining.s
new file mode 100644
index 0..458e26250b429
--- /dev/null
+++ b/lldb/test/Shell/Commands/Inputs/dwarf4-low-pc-ranges-inlining.s
@@ -0,0 +1,369 @@
+
+# Manually modified to have DW_AT_ranges point to end list.
+# int helper(int i) {
+#   return ++i;
+# }
+#
+# int main(int argc, char *argv[]) {
+#   return helper(argc);
+# }
+
+# Manually modified DW_TAG_inlined_subroutine to have DW_AT_low_pc with value 
0,
+# and DW_AT_ranges to point to end ranges list.
+
+   .text
+   .file   "main.cpp"
+   .section.text._Z6helperi,"ax",@progbits
+   .globl  _Z6helperi  # -- Begin function _Z6helperi
+   .p2align4, 0x90
+   .type   _Z6helperi,@function
+_Z6helperi: # @_Z6helperi
+.Lfunc_begin0:
+   .file   1 "/home/test" "main.cpp"
+   .loc1 1 0   # main.cpp:1:0
+   .cfi_startproc
+# %bb.0:# %entry
+   #DEBUG_VALUE: helper:i <- $edi
+# kill: def $edi killed $edi def $rdi
+   .loc1 2 10 prologue_end # main.cpp:2:10
+   leal1(%rdi), %eax
+.Ltmp0:
+   #DEBUG_VALUE: helper:i <- $eax
+   .loc1 2 3 is_stmt 0 # main.cpp:2:3
+   retq
+.Ltmp1:
+.Lfunc_end0:
+   .size   _Z6helperi, .Lfunc_end0-_Z6helperi
+   .cfi_endproc
+# -- End function
+   .section.text.main,"ax",@progbits
+   .globl  main# -- Begin function main
+   .p2align4, 0x90
+   .type   main,@function
+main:   # @main
+.Lfunc_begin1:
+   .loc1 5 0 is_stmt 1 # main.cpp:5:0
+   .cfi_startproc
+# %bb.0:# %entry
+   #DEBUG_VALUE: main:argc <- $edi
+   #DEBUG_VALUE: main:argv <- $rsi
+   #DEBUG_VALUE: helper:i <- $edi
+# kill: def $edi killed $edi def $rdi
+   .loc1 2 10 prologue_end # main.cpp:2:10
+   leal1(%rdi), %eax
+.Ltmp2:
+   #DEBUG_VALUE: helper:i <- $eax
+   .loc1 6 3   # main.cpp:6:3
+   retq
+.Ltmp3:
+.Lfunc_end1:
+   .size   main, .Lfunc_end1-main
+   .cfi_endproc
+# -- End function
+   .section.debug_loc,"",@progbits
+.Ldebug_loc0:
+   .quad   -1
+   .quad   .Lfunc_begin0   #   base address
+   .quad   .Lfunc_begin0-.Lfunc_begin0
+   .quad   .Ltmp0-.Lfunc_begin0
+   .short  1   # Loc expr size
+   .byte   85  # super-register DW_OP_reg5
+   .quad   .Ltmp0-.Lfunc_begin0
+   .quad   .Lfunc_end0-.Lfunc_begin0
+   .short  1   # Loc expr size
+   .byte   80  # super-register DW_OP_reg0
+   .quad   0
+   .quad   0
+.Ldebug_lo

[Lldb-commits] [PATCH] D127889: [LLDB] Handle DIE with DW_AT_low_pc and empty ranges

[Alexander Yermolovich via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG130167ed1eff: [LLDB] Handle DIE with DW_AT_low_pc and empty 
ranges (authored by ayermolo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127889

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/Commands/Inputs/dwarf4-low-pc-ranges-inlining.s
  lldb/test/Shell/Commands/Inputs/dwarf5-low-pc-ranges-inlining.s
  lldb/test/Shell/Commands/dwarf4-low-pc-ranges-inlining.test
  lldb/test/Shell/Commands/dwarf5-low-pc-ranges-inlining.test

Index: lldb/test/Shell/Commands/dwarf5-low-pc-ranges-inlining.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/dwarf5-low-pc-ranges-inlining.test
@@ -0,0 +1,13 @@
+# REQUIRES: system-linux
+
+# RUN: rm -rf %t.dir
+# RUN: mkdir %t.dir
+# RUN: cd %t.dir
+# RUN: llvm-mc -dwarf-version=5 -filetype=obj -triple x86_64-unknown-linux %p/Inputs/dwarf5-low-pc-ranges-inlining.s -o %t.dir/main5.o
+# RUN: %clang %t.dir/main5.o -o %t.dir/main5.exe
+# RUN: %lldb -b -o 'b main' %t.dir/main5.exe &> %t.dir/output.txt
+# RUN: cat %t.dir/output.txt | FileCheck %s
+
+# Test checks lldb handles a case of a DIE with DW_AT_low_pc with 0x0 address, and DW_AT_ranges that point to end of list.
+# CHECK-NOT: error: main5.exe
+# CHECK: Breakpoint 1: where
Index: lldb/test/Shell/Commands/dwarf4-low-pc-ranges-inlining.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/dwarf4-low-pc-ranges-inlining.test
@@ -0,0 +1,13 @@
+# REQUIRES: system-linux
+
+# RUN: rm -rf %t.dir
+# RUN: mkdir %t.dir
+# RUN: cd %t.dir
+# RUN: llvm-mc -dwarf-version=4 -filetype=obj -triple x86_64-unknown-linux %p/Inputs/dwarf4-low-pc-ranges-inlining.s -o %t.dir/main4.o
+# RUN: %clang %t.dir/main4.o -o %t.dir/main4.exe
+# RUN: %lldb -b -o 'b main' %t.dir/main4.exe &> %t.dir/output.txt
+# RUN: cat %t.dir/output.txt | FileCheck %s
+
+# Test checks lldb handles a case of a DIE with DW_AT_low_pc with 0x0 address, and DW_AT_ranges that point to end of list.
+# CHECK-NOT: error: main4.exe
+# CHECK: Breakpoint 1: where
Index: lldb/test/Shell/Commands/Inputs/dwarf5-low-pc-ranges-inlining.s
===
--- /dev/null
+++ lldb/test/Shell/Commands/Inputs/dwarf5-low-pc-ranges-inlining.s
@@ -0,0 +1,429 @@
+# Manually modified to have DW_AT_ranges point to end list.
+# int helper(int i) {
+#   return ++i;
+# }
+#
+# int main(int argc, char *argv[]) {
+#   return helper(argc);
+# }
+
+	.text
+	.file	"main.cpp"
+	.section	.text._Z6helperi,"ax",@progbits
+	.globl	_Z6helperi  # -- Begin function _Z6helperi
+	.p2align	4, 0x90
+	.type	_Z6helperi,@function
+_Z6helperi: # @_Z6helperi
+.Lfunc_begin0:
+	.file	0 "/home/test" "main.cpp" md5 0x3fc4870015f8bb98cd719b92f3dca96e
+	.loc	0 1 0   # main.cpp:1:0
+	.cfi_startproc
+# %bb.0:# %entry
+	#DEBUG_VALUE: helper:i <- $edi
+# kill: def $edi killed $edi def $rdi
+	.loc	0 2 10 prologue_end # main.cpp:2:10
+	leal	1(%rdi), %eax
+.Ltmp0:
+	#DEBUG_VALUE: helper:i <- $eax
+	.loc	0 2 3 is_stmt 0 # main.cpp:2:3
+	retq
+.Ltmp1:
+.Lfunc_end0:
+	.size	_Z6helperi, .Lfunc_end0-_Z6helperi
+	.cfi_endproc
+# -- End function
+	.section	.text.main,"ax",@progbits
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin1:
+	.loc	0 5 0 is_stmt 1 # main.cpp:5:0
+	.cfi_startproc
+# %bb.0:# %entry
+	#DEBUG_VALUE: main:argc <- $edi
+	#DEBUG_VALUE: main:argv <- $rsi
+	#DEBUG_VALUE: helper:i <- $edi
+# kill: def $edi killed $edi def $rdi
+	.loc	0 2 10 prologue_end # main.cpp:2:10
+	leal	1(%rdi), %eax
+.Ltmp2:
+	#DEBUG_VALUE: helper:i <- $eax
+	.loc	0 6 3   # main.cpp:6:3
+	retq
+.Ltmp3:
+.Lfunc_end1:
+	.size	main, .Lfunc_end1-main
+	.cfi_endproc
+# -- End function
+	.section	.debug_loclists,"",@progbits
+	.long	.Ldebug_list_header_end0-.Ldebug_list_header_start0 # Length
+.Ldebug_list_header_start0:
+	.short	5   # Version
+	.byte	8   # Address size
+	.byte	0   # Segment selector size
+	.long	2   # Offset entry count
+.Lloclists_table_base0:
+	.long	.Ldebug_loc0-.Lloclists_table_base0
+	.long	.Ldebug_loc1-.Lloclists_table_base0
+.Ldebug_loc0:
+	.byte	1   # DW_LLE_base_addressx
+	.byte	0   #   base address in

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Med Ismail Bennani via Phabricator via lldb-commits]

mib accepted this revision.
mib 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/D128292/new/

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D127436#3602224 , 
@stella.stamenova wrote:

> This broke the Windows LLDB bot:
>
> https://lab.llvm.org/buildbot/#/builders/83/builds/20295/steps/7/logs/stdio

The second fix attempt, in 
https://github.com/llvm/llvm-project/commit/a1ee0b947d46c9be1cc2ea8db21603bac84efb18,
 seems to have fixed this test now - however the latest test run seems to have 
some other failures: https://lab.llvm.org/buildbot/#/builders/83/builds/20304 
(Not sure if these are spurious failures or if they are other regressions that 
happened meanwhile)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

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

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

This just broke trunk. Look at this repro:

  #include 
  #include 
  
  void f3() {
int m;
m = 2; // thread 3 - line 6
  }
  
  void f2() {
int n;
n = 1; // thread 2 - line 11
std::thread t3(f3);
t3.join();
  }
  
  int main() { // main
std::thread t2(f2);
t2.join();
return 0;
  }

b main
b 6
b 11
r # stopped at main
c # stopped at 11
c # doesn't stop at 6! It goes to the end of the program


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

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

[Lldb-commits] [lldb] efbfde0 - [trace] Add an option to dump instructions in json and to a file

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-22T11:14:22-07:00
New Revision: efbfde0dd0f92d89767df53cbfb883ecf93ffa83

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

LOG: [trace] Add an option to dump instructions in json and to a file

In order to provide simple scripting support on top of instruction traces, a 
simple solution is to enhance the `dump instructions` command and allow 
printing in json and directly to a file. The format is verbose and not space 
efficient, but it's not supposed to be used for really large traces, in which 
case the TraceCursor API is the way to go.

- add a -j option for printing the dump in json
- add a -J option for pretty printing the json output
- add a -F option for specifying an output file
- add a -a option for dumping all the instructions available starting at the 
initial point configured with the other flags
- add tests for all cases
- refactored the instruction dumper and abstracted the actual "printing" logic. 
There are two writer implementations: CLI and JSON. This made the dumper itself 
much more readable and maintanable

sample output:

```
(lldb) thread trace dump instructions  -t -a --id 100 -J
[
  {
"id": 100,
"tsc": "43591204528448966"
"loadAddress": "0x407a91",
"module": "a.out",
"symbol": "void std::deque>::_M_push_back_aux(Foo&&)",
"mnemonic": "movq",
"source": "/usr/include/c++/8/bits/deque.tcc",
"line": 492,
"column": 30
  },
  ...
```

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

Added: 


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/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Target/TraceInstructionDumper.cpp
lldb/test/API/commands/trace/TestTraceDumpInstructions.py
lldb/test/API/commands/trace/TestTraceLoad.py
lldb/test/API/commands/trace/TestTraceTSC.py

Removed: 




diff  --git a/lldb/include/lldb/Target/TraceCursor.h 
b/lldb/include/lldb/Target/TraceCursor.h
index 606f6886964a7..79c4cd371e4f4 100644
--- a/lldb/include/lldb/Target/TraceCursor.h
+++ b/lldb/include/lldb/Target/TraceCursor.h
@@ -173,6 +173,11 @@ class TraceCursor {
   /// its position.
   virtual bool GoToId(lldb::user_id_t id) = 0;
 
+  /// \return
+  /// \b true if and only if there's an instruction item with the given \p
+  /// id.
+  virtual bool HasId(lldb::user_id_t id) const = 0;
+
   /// \return
   /// A unique identifier for the instruction or error this cursor is
   /// pointing to.

diff  --git a/lldb/include/lldb/Target/TraceInstructionDumper.h 
b/lldb/include/lldb/Target/TraceInstructionDumper.h
index 53a1a03ee1ce2..5afff20317198 100644
--- a/lldb/include/lldb/Target/TraceInstructionDumper.h
+++ b/lldb/include/lldb/Target/TraceInstructionDumper.h
@@ -15,17 +15,6 @@
 
 namespace lldb_private {
 
-/// Helper struct that holds symbol, disassembly and address information of an
-/// instruction.
-struct InstructionSymbolInfo {
-  SymbolContext sc;
-  Address address;
-  lldb::addr_t load_address;
-  lldb::DisassemblerSP disassembler;
-  lldb::InstructionSP instruction;
-  lldb_private::ExecutionContext exe_ctx;
-};
-
 /// Class that holds the configuration used by \a TraceInstructionDumper for
 /// traversing and dumping instructions.
 struct TraceInstructionDumperOptions {
@@ -36,6 +25,10 @@ struct TraceInstructionDumperOptions {
   /// Dump only instruction addresses without disassembly nor symbol
   /// information.
   bool raw = false;
+  /// Dump in json format.
+  bool json = false;
+  /// When dumping in JSON format, pretty print the output.
+  bool pretty_print_json = false;
   /// For each instruction, print the corresponding timestamp counter if
   /// available.
   bool show_tsc = false;
@@ -52,6 +45,42 @@ struct TraceInstructionDumperOptions {
 /// state and granularity.
 class TraceInstructionDumper {
 public:
+  /// Helper struct that holds symbol, disassembly and address information of 
an
+  /// instruction.
+  struct SymbolInfo {
+SymbolContext sc;
+Address address;
+lldb::DisassemblerSP disassembler;
+lldb::InstructionSP instruction;
+lldb_private::ExecutionContext exe_ctx;
+  };
+
+  /// Helper struct that holds all the information we know about an instruction
+  struct InstructionEntry {
+lldb::user_id_t id;
+lldb::addr_t load_address;
+llvm::Optional tsc;
+llvm::Optional error;
+llvm::Optional symbol_info;
+llvm::Optional prev_symbol_info;
+  };
+
+  /// Interface used to abstract away the format in which the instruction
+  /// information will be dumped.
+  class Output

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

[Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGefbfde0dd0f9: [trace] Add an option to dump instructions in 
json and to a file (authored by Walter Erquinigo ).

Changed prior to commit:
  https://reviews.llvm.org/D128316?vs=439072&id=439103#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128316

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/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceTSC.py

Index: lldb/test/API/commands/trace/TestTraceTSC.py
===
--- lldb/test/API/commands/trace/TestTraceTSC.py
+++ lldb/test/API/commands/trace/TestTraceTSC.py
@@ -17,7 +17,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=\d+\] 0x00400511movl"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -58,7 +58,10 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=\d+\] 0x00400511movl"])
+
+self.expect("thread trace dump instructions --tsc -c 1 --pretty-json",
+patterns=['''"tsc": "\d+"'''])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -73,6 +76,9 @@
 self.expect("thread trace dump instructions --tsc -c 1",
 patterns=["0: \[tsc=unavailable\] 0x00400511movl"])
 
+self.expect("thread trace dump instructions --tsc -c 1 --json",
+patterns=['''"tsc":null'''])
+
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
 def testPSBPeriod(self):
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -13,11 +13,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+  substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+   "19520: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["67910: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 @testSBAPIAndCommands
@@ -26,11 +26,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+  substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+   "19520: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["67910: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 @testSBAPIAndCommands
@@ -39,11 +39,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
 self.traceLo

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added subscribers: stella.stamenova, mstorsjo.
mstorsjo added a comment.

I think this might have broken a bunch of testcases on the 
lldb-x64-windows-ninja buildbot too, e.g. 
https://lab.llvm.org/buildbot/#/builders/83/builds/20304 (CC @stella.stamenova)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

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

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/source/Core/Debugger.cpp:1409-1411
+static std::shared_ptr
+CreateLogHandler(LogHandlerKind log_handler_kind, int fd, bool should_close,
+ size_t buffer_size) {

Many (or most) arguments passed to this function might not be used depending on 
the kind of LogHandler you're instantiating.  This is fine for now but if we 
add other handlers in the future that take different argument, it might become 
very confusing on which one to use.

May be we could use a parameter pack with some template specialization to make 
this more robust ?



Comment at: lldb/source/Core/Debugger.cpp:1444
+CreateLogHandler(log_handler_kind, GetOutputFile().GetDescriptor(),
+ !should_close, buffer_size);
   } else {

I find it confusing to negate `should_close` but to initialize it to `true`.

Since it’s not modified anywhere else, IMO it might be better to inline the 
value with a comment (`/*should_close=*/`), instead of using the variable.



Comment at: lldb/source/Core/Debugger.cpp:1466
+  CreateLogHandler(log_handler_kind, (*file)->GetDescriptor(),
+   !should_close, buffer_size);
   m_stream_handlers[log_file] = log_handler_sp;

Ditto


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

https://reviews.llvm.org/D128323

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

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/source/Commands/CommandObjectLog.cpp:167
   buffer_size.Clear();
+  handler = eLogHandlerStream;
   log_options = 0;

clayborg wrote:
> Do we want to define a "eLogHandlerDefault" which points to 
> "eLogHandlerStream"?
I was hoping we could use a type alias for this, that marks `eLogHandlerStream` 
as `eLogHandlerDefault` but I couldn't find of a nice C++ way to do it :/ 


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

https://reviews.llvm.org/D128323

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

[Lldb-commits] [PATCH] D128366: [lldb] Make Module::LookupInfo::Prune language-agnostic

[Alex Langford via Phabricator via lldb-commits]

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

Module::LookupInfo::Prune tries to prune results from lookups that don't
match the user-provided lookup information. If we're trying to match a
name with type eFunctionNameTypeFull, we want to match what the user
provided exactly. Currently, we try to see if what the user provided is
matches exactly assuming that a C++ name was provided. However, this is
specific to the C++ debugging path. LLDB supports more languages than
C++, so it is important that this codepath be language-agnostic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128366

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h

Index: lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
@@ -42,6 +42,10 @@
 
   static llvm::StringRef GetPluginNameStatic() { return "objcplusplus"; }
 
+  bool NamesAreEquivalentWithContext(
+  const ConstString &user_provided_name,
+  const ConstString &full_name_with_context) const override;
+
   // PluginInterface protocol
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 };
Index: lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
@@ -43,3 +43,18 @@
 return nullptr;
   }
 }
+
+bool ObjCPlusPlusLanguage::NamesAreEquivalentWithContext(
+const ConstString &user_provided_name,
+const ConstString &full_name_with_context) const {
+  if (const auto *cpp_lang = Language::FindPlugin(eLanguageTypeC_plus_plus)) {
+if (!cpp_lang->NamesAreEquivalentWithContext(user_provided_name,
+ full_name_with_context))
+  return false;
+  } else if (const auto *objc_lang = Language::FindPlugin(eLanguageTypeObjC)) {
+if (!objc_lang->NamesAreEquivalentWithContext(user_provided_name,
+  full_name_with_context))
+  return false;
+  }
+  return true;
+}
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -155,6 +155,10 @@
   return false;
   }
 
+  bool NamesAreEquivalentWithContext(
+  const ConstString &user_provided_name,
+  const ConstString &full_name_with_context) const override;
+
   // PluginInterface protocol
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 };
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -1147,6 +1147,19 @@
   return canReadValue && isZero;
 }
 
+bool ObjCLanguage::NamesAreEquivalentWithContext(
+const ConstString &user_provided_name,
+const ConstString &full_name_with_context) const {
+  ObjCLanguage::MethodName user_method(user_provided_name.GetCString(), false);
+  ObjCLanguage::MethodName full_method(full_name_with_context.GetCString(),
+   false);
+
+  if (!user_method.IsValid() || !full_method.IsValid())
+return true;
+  return user_method.GetClassName() == full_method.GetClassName() &&
+ user_method.GetSelector() == full_method.GetSelector();
+}
+
 bool ObjCLanguage::IsSourceFile(llvm::StringRef file_path) const {
   const auto suffixes = {".h", ".m", ".M"};
   for (auto suffix : suffixes) {
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -138,6 +138,10 @@
   ConstString FindBestAlternateFunctionMangledName(
   const Mangled mangled, const SymbolContext &sym_ctx) const override;
 
+  bool NamesAreEquivalentWithContext(
+  const ConstString &user_provided_name,
+  const ConstString &full_name_with_context) const override;
+
  

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova updated this revision to Diff 439111.
cassanova added a comment.

Updated CMakeLists file to save fuzzer artifacts (the files that the fuzzer 
writes when an input causes the program being fuzzed to fail) to a directory in 
the user's build directory, instead of saving them in the user's source 
directory. Also change fuzzer invocation to add a prefix to artifacts so that 
it is easier to identify them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128292

Files:
  lldb/tools/lldb-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
  
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp

Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,47 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-===//
+
+#include 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace lldb;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  // Convert the data into a null-terminated string
+  std::string str((char *)data, size);
+
+  // Create a debugger and a dummy target
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.GetDummyTarget();
+
+  // Create a command interpreter for the current debugger
+  // A return object is needed to run the command interpreter
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter ci = debugger.GetCommandInterpreter();
+
+  // Use the fuzzer generated input as input for the command interpreter
+  if (ci.IsValid()) {
+ci.HandleCommand(str.c_str(), ro, false);
+  }
+
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+  SBModule::GarbageCollectAllocatedModules();
+
+  return 0;
+}
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
@@ -0,0 +1,4 @@
+kw1="breakpoint set"
+kw2="target"
+kw3="run"
+kw4="frame info"
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
@@ -0,0 +1,28 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_fuzzer(lldb-commandinterpreter-fuzzer
+  EXCLUDE_FROM_ALL
+  lldb-commandinterpreter-fuzzer.cpp
+  )
+
+if(TARGET lldb-commandinterpreter-fuzzer)
+  target_include_directories(lldb-commandinterpreter-fuzzer PRIVATE ..)
+  target_link_libraries(lldb-commandinterpreter-fuzzer
+PRIVATE
+liblldb
+)
+
+  # This will create a directory specifially for the fuzzer's artifacts, go to that
+  # directory and run the fuzzer from there. When the fuzzer exits the input
+  # artifact that caused it to exit will be written to a directory within the
+  # build directory
+  add_custom_target(fuzz-lldb-commandinterpreter
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND mkdir -p ${CMAKE_BINARY_DIR}/commandinterpreter-fuzzer-artifacts &&
+cd ${CMAKE_BINARY_DIR}/commandinterpreter-fuzzer-artifacts
+&& $ -dict=${CMAKE_CURRENT_SOURCE_DIR}/inputdictionary.txt  -only_ascii=1 -artifact_prefix=commandinterpreter-
+USES_TERMINAL
+)
+endif()
Index: lldb/tools/lldb-fuzzer/CMakeLists.txt
===
--- lldb/tools/lldb-fuzzer/CMakeLists.txt
+++ lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -1,2 +1,3 @@
+add_subdirectory(lldb-commandinterpreter-fuzzer)
 add_subdirectory(lldb-target-fuzzer)
 add_subdirectory(utils)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] bae10a6 - Fix a bug with "process continue -b" when no breakpoints are

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-06-22T12:18:07-07:00
New Revision: bae10a6bbb1ed6ba8185884e3239fe3556602dab

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

LOG: Fix a bug with "process continue -b" when no breakpoints are
passed.  I was passing the empty list of breakponts to the
VerifyBreakpointList routine, but that treats empty as "choose
the default breakpoint" which we don't want here.

Added: 


Modified: 
lldb/source/Commands/CommandObjectProcess.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index def0e00af087..2f5f649636ae 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -595,9 +595,12 @@ class CommandObjectProcessContinue : public 
CommandObjectParsed {
   
   Target *target = m_exe_ctx.GetTargetPtr();
   BreakpointIDList run_to_bkpt_ids;
-  CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-  m_options.m_run_to_bkpt_args, target, result, &run_to_bkpt_ids, 
-  BreakpointName::Permissions::disablePerm);
+  // Don't pass an empty run_to_breakpoint list, as Verify will look for 
the
+  // default breakpoint.
+  if (m_options.m_run_to_bkpt_args.GetArgumentCount() > 0)
+CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
+m_options.m_run_to_bkpt_args, target, result, &run_to_bkpt_ids, 
+BreakpointName::Permissions::disablePerm);
   if (!result.Succeeded()) {
 return false;
   }



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

[Lldb-commits] [PATCH] D128377: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

Fix an off-by-one error in the utility function used to extract the dynamic 
class info. This resulted in a buffer overflow in the inferior which 
interrupted our utility function.


https://reviews.llvm.org/D128377

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


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -199,7 +199,7 @@
 DEBUG_PRINTF ("count = %u\n", count);
 
 uint32_t idx = 0;
-for (uint32_t i=0; i<=count; ++i)
+for (uint32_t i=0; iIndex: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -199,7 +199,7 @@
 DEBUG_PRINTF ("count = %u\n", count);
 
 uint32_t idx = 0;
-for (uint32_t i=0; i<=count; ++i)
+for (uint32_t i=0; i___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128378: [lldb] Return the correct utility function in AppleObjCRuntimeV2

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

A copy/paste error in GetClassInfoUtilityFunction resulted in the wrong
utility function being returned: copyRealizedClassList instead of
getRealizedClassList_trylock.


https://reviews.llvm.org/D128378

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


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1700,11 +1700,11 @@
   }
   case objc_getRealizedClassList_trylock: {
 if (!m_objc_getRealizedClassList_trylock_helper.utility_function)
-  m_objc_copyRealizedClassList_helper.utility_function =
+  m_objc_getRealizedClassList_trylock_helper.utility_function =
   GetClassInfoUtilityFunctionImpl(exe_ctx, helper,
   g_get_dynamic_class_info3_body,
   g_get_dynamic_class_info3_name);
-return m_objc_copyRealizedClassList_helper.utility_function.get();
+return m_objc_getRealizedClassList_trylock_helper.utility_function.get();
   }
   }
   llvm_unreachable("Unexpected helper");


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1700,11 +1700,11 @@
   }
   case objc_getRealizedClassList_trylock: {
 if (!m_objc_getRealizedClassList_trylock_helper.utility_function)
-  m_objc_copyRealizedClassList_helper.utility_function =
+  m_objc_getRealizedClassList_trylock_helper.utility_function =
   GetClassInfoUtilityFunctionImpl(exe_ctx, helper,
   g_get_dynamic_class_info3_body,
   g_get_dynamic_class_info3_name);
-return m_objc_copyRealizedClassList_helper.utility_function.get();
+return m_objc_getRealizedClassList_trylock_helper.utility_function.get();
   }
   }
   llvm_unreachable("Unexpected helper");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128377: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

[Adrian Prantl via Phabricator via lldb-commits]

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

I'm curious now why there is both `count` and `max_class_infos` and if the 
second is workaround for this bug? Anyway, this *looks* plausible!


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

https://reviews.llvm.org/D128377

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

[Lldb-commits] [PATCH] D128377: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D128377#3602856 , @aprantl wrote:

> I'm curious now why there is both `count` and `max_class_infos` and if the 
> second is workaround for this bug? Anyway, this *looks* plausible!

`count` is the actual value reported by the runtime. `max_class_infos` is a 
monotonically increasing number that allows us to detect that new classes have 
been instantiated. The latter is also used to allocate enough space. The 
guarantee from the runtime is that `count < max_class_infos`.


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

https://reviews.llvm.org/D128377

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

[Lldb-commits] [PATCH] D128378: [lldb] Return the correct utility function in AppleObjCRuntimeV2

[Adrian Prantl via Phabricator via lldb-commits]

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

Oh boy, these are scary. Should LLDB fail harder when the utility expression 
fails, so we can detect these earlier?


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

https://reviews.llvm.org/D128378

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

[Lldb-commits] [PATCH] D128378: [lldb] Return the correct utility function in AppleObjCRuntimeV2

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D128378#3602888 , @aprantl wrote:

> Oh boy, these are scary. Should LLDB fail harder when the utility expression 
> fails, so we can detect these earlier?

Fortunately we print an error, but we're also pretty resilient against some of 
this information missing. This bug was particularly tricky because it only 
causes a problem the second time that you execute the utility function, which 
only occurs when you have lazily instantiated classes (in Swift) which is why 
we didn't see this on llvm.org


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

https://reviews.llvm.org/D128378

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

[Lldb-commits] [PATCH] D128378: [lldb] Return the correct utility function in AppleObjCRuntimeV2

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf0d87dfe30e8: [lldb] Return the correct utility function in 
AppleObjCRuntimeV2 (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128378

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


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1700,11 +1700,11 @@
   }
   case objc_getRealizedClassList_trylock: {
 if (!m_objc_getRealizedClassList_trylock_helper.utility_function)
-  m_objc_copyRealizedClassList_helper.utility_function =
+  m_objc_getRealizedClassList_trylock_helper.utility_function =
   GetClassInfoUtilityFunctionImpl(exe_ctx, helper,
   g_get_dynamic_class_info3_body,
   g_get_dynamic_class_info3_name);
-return m_objc_copyRealizedClassList_helper.utility_function.get();
+return m_objc_getRealizedClassList_trylock_helper.utility_function.get();
   }
   }
   llvm_unreachable("Unexpected helper");


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1700,11 +1700,11 @@
   }
   case objc_getRealizedClassList_trylock: {
 if (!m_objc_getRealizedClassList_trylock_helper.utility_function)
-  m_objc_copyRealizedClassList_helper.utility_function =
+  m_objc_getRealizedClassList_trylock_helper.utility_function =
   GetClassInfoUtilityFunctionImpl(exe_ctx, helper,
   g_get_dynamic_class_info3_body,
   g_get_dynamic_class_info3_name);
-return m_objc_copyRealizedClassList_helper.utility_function.get();
+return m_objc_getRealizedClassList_trylock_helper.utility_function.get();
   }
   }
   llvm_unreachable("Unexpected helper");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] f0d87df - [lldb] Return the correct utility function in AppleObjCRuntimeV2

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-22T13:57:23-07:00
New Revision: f0d87dfe30e86d701034af9320b29d3e2e3a12b3

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

LOG: [lldb] Return the correct utility function in AppleObjCRuntimeV2

A copy/paste error in GetClassInfoUtilityFunction resulted in the wrong
utility function being returned: copyRealizedClassList instead of
getRealizedClassList_trylock.

Differential revision: https://reviews.llvm.org/D128378

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 4d21f1f3765f3..1b576f6870cc7 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1700,11 +1700,11 @@ 
AppleObjCRuntimeV2::DynamicClassInfoExtractor::GetClassInfoUtilityFunction(
   }
   case objc_getRealizedClassList_trylock: {
 if (!m_objc_getRealizedClassList_trylock_helper.utility_function)
-  m_objc_copyRealizedClassList_helper.utility_function =
+  m_objc_getRealizedClassList_trylock_helper.utility_function =
   GetClassInfoUtilityFunctionImpl(exe_ctx, helper,
   g_get_dynamic_class_info3_body,
   g_get_dynamic_class_info3_name);
-return m_objc_copyRealizedClassList_helper.utility_function.get();
+return m_objc_getRealizedClassList_trylock_helper.utility_function.get();
   }
   }
   llvm_unreachable("Unexpected helper");



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

[Lldb-commits] [lldb] d95c406 - [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-22T13:57:24-07:00
New Revision: d95c406c20ef8135fae74cc82406f498084749a0

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

LOG: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

Fix an off-by-one error in the utility function used to extract the
dynamic class info. This resulted in a buffer overflow in the inferior
which interrupted our utility function.

Differential revision: https://reviews.llvm.org/D128377

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 1b576f6870cc..f53b82ee33c8 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -199,7 +199,7 @@ __lldb_apple_objc_v2_get_dynamic_class_info2(void 
*gdb_objc_realized_classes_ptr
 DEBUG_PRINTF ("count = %u\n", count);
 
 uint32_t idx = 0;
-for (uint32_t i=0; i<=count; ++i)
+for (uint32_t i=0; ihttps://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128377: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd95c406c20ef: [lldb] Fix off-by-one error in the 
AppleObjCRuntimeV2 utility function (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128377

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


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -199,7 +199,7 @@
 DEBUG_PRINTF ("count = %u\n", count);
 
 uint32_t idx = 0;
-for (uint32_t i=0; i<=count; ++i)
+for (uint32_t i=0; iIndex: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -199,7 +199,7 @@
 DEBUG_PRINTF ("count = %u\n", count);
 
 uint32_t idx = 0;
-for (uint32_t i=0; i<=count; ++i)
+for (uint32_t i=0; i___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt:17
+
+  # This will create a directory specifially for the fuzzer's artifacts, go to 
that
+  # directory and run the fuzzer from there. When the fuzzer exits the input

typo



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt:23
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND mkdir -p ${CMAKE_BINARY_DIR}/commandinterpreter-fuzzer-artifacts &&
+cd ${CMAKE_BINARY_DIR}/commandinterpreter-fuzzer-artifacts

You might want to have a top-level `fuzzer-artifacts` that will have 
`commandinterpreter-fuzzer-artifacts` as a subdirectory


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova updated this revision to Diff 439158.
cassanova added a comment.

Added a subdirectory to the top-level build directory. This directory will hold 
directories for the artifacts of various fuzzers. Also corrected a typo in the 
command interpreter CMakeLists file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128292

Files:
  lldb/tools/lldb-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
  
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp

Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,47 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-===//
+
+#include 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace lldb;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  // Convert the data into a null-terminated string
+  std::string str((char *)data, size);
+
+  // Create a debugger and a dummy target
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.GetDummyTarget();
+
+  // Create a command interpreter for the current debugger
+  // A return object is needed to run the command interpreter
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter ci = debugger.GetCommandInterpreter();
+
+  // Use the fuzzer generated input as input for the command interpreter
+  if (ci.IsValid()) {
+ci.HandleCommand(str.c_str(), ro, false);
+  }
+
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+  SBModule::GarbageCollectAllocatedModules();
+
+  return 0;
+}
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
@@ -0,0 +1,4 @@
+kw1="breakpoint set"
+kw2="target"
+kw3="run"
+kw4="frame info"
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
@@ -0,0 +1,28 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_fuzzer(lldb-commandinterpreter-fuzzer
+  EXCLUDE_FROM_ALL
+  lldb-commandinterpreter-fuzzer.cpp
+  )
+
+if(TARGET lldb-commandinterpreter-fuzzer)
+  target_include_directories(lldb-commandinterpreter-fuzzer PRIVATE ..)
+  target_link_libraries(lldb-commandinterpreter-fuzzer
+PRIVATE
+liblldb
+)
+
+  # This will create a directory specifically for the fuzzer's artifacts, go to that
+  # directory and run the fuzzer from there. When the fuzzer exits the input
+  # artifact that caused it to exit will be written to a directory within the
+  # build directory
+  add_custom_target(fuzz-lldb-commandinterpreter
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND mkdir -p ${CMAKE_BINARY_DIR}/fuzzer-artifacts/commandinterpreter-artifacts &&
+cd ${CMAKE_BINARY_DIR}/fuzzer-artifacts/commandinterpreter-artifacts
+&& $ -dict=${CMAKE_CURRENT_SOURCE_DIR}/inputdictionary.txt  -only_ascii=1 -artifact_prefix=commandinterpreter-
+USES_TERMINAL
+)
+endif()
Index: lldb/tools/lldb-fuzzer/CMakeLists.txt
===
--- lldb/tools/lldb-fuzzer/CMakeLists.txt
+++ lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -1,2 +1,3 @@
+add_subdirectory(lldb-commandinterpreter-fuzzer)
 add_subdirectory(lldb-target-fuzzer)
 add_subdirectory(utils)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D128321#3602129 , @clayborg wrote:

> I think we should allow this class to always exist and not conditionally 
> compile it in or out. Then we add a new Host.h method to emit a log message 
> in "lldb/Host/Host.h" and allow each host OS to emit a message. Maybe there 
> is a default implementation that emits the message to stderr, and then we 
> override this for in HostInfoMacOSX.h? Then each OS can use their OS logging 
> API of choice.

Yeah, I considered that, but that would make Utility depend on Host which would 
(re)introduce the circular dependency between the two. The alternative would be 
to still make the class available unconditionally, but use target ifdefs to 
have different implementations.


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

https://reviews.llvm.org/D128321

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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Med Ismail Bennani via Phabricator via lldb-commits]

mib accepted this revision.
mib added a comment.

Ship it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG46be5faaf034: [lldb/Fuzzer] Add command interpreter fuzzer 
for LLDB (authored by cassanova).

Changed prior to commit:
  https://reviews.llvm.org/D128292?vs=439158&id=439163#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128292

Files:
  lldb/tools/lldb-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
  
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp

Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,47 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-===//
+
+#include 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace lldb;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  // Convert the data into a null-terminated string
+  std::string str((char *)data, size);
+
+  // Create a debugger and a dummy target
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.GetDummyTarget();
+
+  // Create a command interpreter for the current debugger
+  // A return object is needed to run the command interpreter
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter ci = debugger.GetCommandInterpreter();
+
+  // Use the fuzzer generated input as input for the command interpreter
+  if (ci.IsValid()) {
+ci.HandleCommand(str.c_str(), ro, false);
+  }
+
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+  SBModule::GarbageCollectAllocatedModules();
+
+  return 0;
+}
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
@@ -0,0 +1,4 @@
+kw1="breakpoint set"
+kw2="target"
+kw3="run"
+kw4="frame info"
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
@@ -0,0 +1,28 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_fuzzer(lldb-commandinterpreter-fuzzer
+  EXCLUDE_FROM_ALL
+  lldb-commandinterpreter-fuzzer.cpp
+  )
+
+if(TARGET lldb-commandinterpreter-fuzzer)
+  target_include_directories(lldb-commandinterpreter-fuzzer PRIVATE ..)
+  target_link_libraries(lldb-commandinterpreter-fuzzer
+PRIVATE
+liblldb
+)
+
+  # This will create a directory specifically for the fuzzer's artifacts, go to that
+  # directory and run the fuzzer from there. When the fuzzer exits the input
+  # artifact that caused it to exit will be written to a directory within the
+  # build directory
+  add_custom_target(fuzz-lldb-commandinterpreter
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND mkdir -p ${CMAKE_BINARY_DIR}/fuzzer-artifacts/commandinterpreter-artifacts &&
+cd ${CMAKE_BINARY_DIR}/fuzzer-artifacts/commandinterpreter-artifacts
+&& $ -dict=${CMAKE_CURRENT_SOURCE_DIR}/inputdictionary.txt  -only_ascii=1 -artifact_prefix=commandinterpreter-
+USES_TERMINAL
+)
+endif()
Index: lldb/tools/lldb-fuzzer/CMakeLists.txt
===
--- lldb/tools/lldb-fuzzer/CMakeLists.txt
+++ lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -1,2 +1,3 @@
+add_subdirectory(lldb-commandinterpreter-fuzzer)
 add_subdirectory(lldb-target-fuzzer)
 add_subdirectory(utils)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 46be5fa - [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via lldb-commits]

Author: Chelsea Cassanova
Date: 2022-06-22T17:42:55-04:00
New Revision: 46be5faaf03466c3751f8a2882bef5a217e15926

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

LOG: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

This adds a command interpreter fuzzer to LLDB's fuzzing library.
The input data from the fuzzer is used as input for the command
interpreter.
Input data for the fuzzer is guided by a dictionary of keywords used in
LLDB, such as "breakpoint", "target" and others.

Differential revision: https://reviews.llvm.org/D128292

Added: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt

lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp

Modified: 
lldb/tools/lldb-fuzzer/CMakeLists.txt

Removed: 




diff  --git a/lldb/tools/lldb-fuzzer/CMakeLists.txt 
b/lldb/tools/lldb-fuzzer/CMakeLists.txt
index 326c69a29dac1..867a41961c13c 100644
--- a/lldb/tools/lldb-fuzzer/CMakeLists.txt
+++ b/lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -1,2 +1,3 @@
+add_subdirectory(lldb-commandinterpreter-fuzzer)
 add_subdirectory(lldb-target-fuzzer)
 add_subdirectory(utils)

diff  --git 
a/lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt 
b/lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
new file mode 100644
index 0..5bfae5b574e95
--- /dev/null
+++ b/lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
@@ -0,0 +1,28 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_fuzzer(lldb-commandinterpreter-fuzzer
+  EXCLUDE_FROM_ALL
+  lldb-commandinterpreter-fuzzer.cpp
+  )
+
+if(TARGET lldb-commandinterpreter-fuzzer)
+  target_include_directories(lldb-commandinterpreter-fuzzer PRIVATE ..)
+  target_link_libraries(lldb-commandinterpreter-fuzzer
+PRIVATE
+liblldb
+)
+
+  # This will create a directory specifically for the fuzzer's artifacts, go 
to that
+  # directory and run the fuzzer from there. When the fuzzer exits the input
+  # artifact that caused it to exit will be written to a directory within the
+  # build directory
+  add_custom_target(fuzz-lldb-commandinterpreter
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND mkdir -p 
${CMAKE_BINARY_DIR}/fuzzer-artifacts/commandinterpreter-artifacts &&
+cd ${CMAKE_BINARY_DIR}/fuzzer-artifacts/commandinterpreter-artifacts
+&& $ 
-dict=${CMAKE_CURRENT_SOURCE_DIR}/inputdictionary.txt  -only_ascii=1 
-artifact_prefix=commandinterpreter-
+USES_TERMINAL
+)
+endif()

diff  --git 
a/lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt 
b/lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
new file mode 100644
index 0..ddd52a6d7806a
--- /dev/null
+++ b/lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
@@ -0,0 +1,4 @@
+kw1="breakpoint set"
+kw2="target"
+kw3="run"
+kw4="frame info"

diff  --git 
a/lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
 
b/lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
new file mode 100644
index 0..036954838b057
--- /dev/null
+++ 
b/lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,47 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-===//
+
+#include 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace lldb;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  // Convert the data into a null-terminated string
+  std::string str((char *)data, size);
+
+  // Create a debugger and a dummy target
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.GetDummyTarget();
+
+  // Create a command interpreter for the current debugger
+  // A return object is needed to run the command interpreter
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter ci = debugger.GetCommandInterpreter();
+
+  // Use the fuzzer generated input as input for the command interpreter
+  if (ci.IsValid()) {
+ci.Han

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 439164.
JDevlieghere marked 5 inline comments as done.
JDevlieghere added a comment.

Address remaining code review feedback


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

https://reviews.llvm.org/D128321

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -32,6 +32,12 @@
 #include 
 #endif
 
+#if defined(__APPLE__)
+#include 
+static os_log_t g_os_log;
+static std::once_flag g_os_log_once;
+#endif
+
 using namespace lldb_private;
 
 llvm::ManagedStatic Log::g_channel_map;
@@ -384,3 +390,24 @@
   }
   stream.flush();
 }
+
+SystemLogHandler::SystemLogHandler() {
+#if defined(__APPLE__)
+  std::call_once(g_os_log_once, []() {
+g_os_log = os_log_create("com.apple.dt.lldb", "lldb");
+  });
+#endif
+}
+
+void SystemLogHandler::Emit(llvm::StringRef message) {
+  std::string str(message);
+#if defined(__APPLE__)
+  if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
+os_log(g_os_log, "%{public}s", str.c_str());
+  } else {
+fprintf(stderr, "%s", str.c_str());
+  }
+#else
+  fprintf(stderr, "%s", str.c_str());
+#endif
+}
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -95,6 +95,12 @@
   size_t m_total_count = 0;
 };
 
+class SystemLogHandler : public LogHandler {
+public:
+  SystemLogHandler();
+  void Emit(llvm::StringRef message) override;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -32,6 +32,12 @@
 #include 
 #endif
 
+#if defined(__APPLE__)
+#include 
+static os_log_t g_os_log;
+static std::once_flag g_os_log_once;
+#endif
+
 using namespace lldb_private;
 
 llvm::ManagedStatic Log::g_channel_map;
@@ -384,3 +390,24 @@
   }
   stream.flush();
 }
+
+SystemLogHandler::SystemLogHandler() {
+#if defined(__APPLE__)
+  std::call_once(g_os_log_once, []() {
+g_os_log = os_log_create("com.apple.dt.lldb", "lldb");
+  });
+#endif
+}
+
+void SystemLogHandler::Emit(llvm::StringRef message) {
+  std::string str(message);
+#if defined(__APPLE__)
+  if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
+os_log(g_os_log, "%{public}s", str.c_str());
+  } else {
+fprintf(stderr, "%s", str.c_str());
+  }
+#else
+  fprintf(stderr, "%s", str.c_str());
+#endif
+}
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -95,6 +95,12 @@
   size_t m_total_count = 0;
 };
 
+class SystemLogHandler : public LogHandler {
+public:
+  SystemLogHandler();
+  void Emit(llvm::StringRef message) override;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 11 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Commands/Options.td:436-437
 Desc<"Set the log to be buffered, using the specified buffer size.">;
+  def log_handler : Option<"handler", "h">, Group<1>,
+EnumArg<"Value", "LogHandlerType()">, Desc<"Use a custom handler.">;
   def log_threadsafe : Option<"threadsafe", "t">, Group<1>,

clayborg wrote:
> Maybe "--type" or "--kind" would be better? Handler seems like an internal 
> name. Maybe also mention what it will default to if not specified? Can the 
> user see a list of the valid values or do we need to supply these in the 
> description?
I think the concept of a "log handler" is sufficiently well known that it 
should be easy for a a (power) user to understand. I'm concerned that "type" 
and "kind" are a tad too high level, and therefore could easily be confused 
with the log category and channel. For example, when someone asks to enable the 
"gdb packet log", is that a category, channel, type or kind? 



Comment at: lldb/source/Core/Debugger.cpp:1409-1411
+static std::shared_ptr
+CreateLogHandler(LogHandlerKind log_handler_kind, int fd, bool should_close,
+ size_t buffer_size) {

mib wrote:
> Many (or most) arguments passed to this function might not be used depending 
> on the kind of LogHandler you're instantiating.  This is fine for now but if 
> we add other handlers in the future that take different argument, it might 
> become very confusing on which one to use.
> 
> May be we could use a parameter pack with some template specialization to 
> make this more robust ?
What did you have in mind? We don't know a-priori what the type will be. You 
could template the function in the enum value, but you still wouldn't know 
which arguments to pass before dispatching to the right function. 


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

https://reviews.llvm.org/D128323

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

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 439170.
JDevlieghere marked 2 inline comments as done.

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

https://reviews.llvm.org/D128323

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/lldb-private-enumerations.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Debugger.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -1120,7 +1120,7 @@
   /*add_to_history*/ eLazyBoolNo, Result);
 
   if (!opts::Log.empty())
-Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, errs());
+Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, eLogHandlerStream, errs());
 
   if (opts::BreakpointSubcommand)
 return opts::breakpoint::evaluateBreakpoints(*Dbg);
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1406,11 +1406,27 @@
debugger_id, once);
 }
 
+static std::shared_ptr
+CreateLogHandler(LogHandlerKind log_handler_kind, int fd, bool should_close,
+ size_t buffer_size) {
+  switch (log_handler_kind) {
+  case eLogHandlerStream:
+return std::make_shared(fd, should_close, buffer_size);
+  case eLogHandlerCircular:
+return std::make_shared(buffer_size);
+  case eLogHandlerSystem:
+return std::make_shared();
+  case eLogHandlerCallback:
+return {};
+  }
+  return {};
+}
+
 bool Debugger::EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
- size_t buffer_size, llvm::raw_ostream &error_stream) {
-  const bool should_close = true;
+ size_t buffer_size, LogHandlerKind log_handler_kind,
+ llvm::raw_ostream &error_stream) {
 
   std::shared_ptr log_handler_sp;
   if (m_callback_handler_sp) {
@@ -1419,8 +1435,9 @@
 log_options |=
 LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
   } else if (log_file.empty()) {
-log_handler_sp = std::make_shared(
-GetOutputFile().GetDescriptor(), !should_close, buffer_size);
+log_handler_sp =
+CreateLogHandler(log_handler_kind, GetOutputFile().GetDescriptor(),
+ /*should_close=*/false, buffer_size);
   } else {
 auto pos = m_stream_handlers.find(log_file);
 if (pos != m_stream_handlers.end())
@@ -1440,8 +1457,9 @@
 return false;
   }
 
-  log_handler_sp = std::make_shared(
-  (*file)->GetDescriptor(), should_close, buffer_size);
+  log_handler_sp =
+  CreateLogHandler(log_handler_kind, (*file)->GetDescriptor(),
+   /*should_close=*/true, buffer_size);
   m_stream_handlers[log_file] = log_handler_sp;
 }
   }
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -433,6 +433,10 @@
 Desc<"Set the destination file to log to.">;
   def log_buffer_size : Option<"buffer", "b">, Group<1>, Arg<"UnsignedInteger">,
 Desc<"Set the log to be buffered, using the specified buffer size.">;
+  def log_handler : Option<"log-handler", "h">, Group<1>,
+EnumArg<"Value", "LogHandlerType()">, Desc<"Redirect the log output to a "
+"specific log handler. The default log handler (stream) writes to the "
+"debugger output stream or to a file if one is specified with -f.">;
   def log_threadsafe : Option<"threadsafe", "t">, Group<1>,
 Desc<"Enable thread safe logging to avoid interweaved log lines.">;
   def log_verbose : Option<"verbose", "v">, Group<1>,
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionValueEnumeration.h"
 #include "lldb/Interpreter/OptionValueUInt64.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/Args.h"
@@ -22,6 +23,28 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static constexpr OptionEnumValueElement g_log_handler_type[] = {
+{
+eLogHandlerStream,
+"stream",
+"Use the stream log handler",
+},
+{
+eLogHandlerCircular,
+"rotating",
+"Use the rotating log handler",
+},
+{
+eLogHandlerSystem,
+"system",
+"Use the system log handler",
+  

[Lldb-commits] [PATCH] D128386: [lldb] Make thread safety the responsibility of the log handlers

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, clayborg, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

As per Pavel's suggestion in D127922 , make 
thread safety the responsibility of the log handlers.


https://reviews.llvm.org/D128386

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/Log.cpp

Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -317,12 +317,7 @@
   auto handler_sp = GetHandler();
   if (!handler_sp)
 return;
-
-  Flags options = GetOptions();
-  if (options.Test(LLDB_LOG_OPTION_THREADSAFE))
-handler_sp->EmitThreadSafe(message);
-  else
-handler_sp->Emit(message);
+  handler_sp->Emit(message);
 }
 
 void Log::Format(llvm::StringRef file, llvm::StringRef function,
@@ -334,11 +329,6 @@
   WriteMessage(message.str());
 }
 
-void LogHandler::EmitThreadSafe(llvm::StringRef message) {
-  std::lock_guard guard(m_mutex);
-  Emit(message);
-}
-
 StreamLogHandler::StreamLogHandler(int fd, bool should_close,
size_t buffer_size)
 : m_stream(fd, should_close, false) {
@@ -346,11 +336,21 @@
 m_stream.SetBufferSize(buffer_size);
 }
 
-StreamLogHandler::~StreamLogHandler() {
+StreamLogHandler::~StreamLogHandler() { Flush(); }
+
+void StreamLogHandler::Flush() {
+  std::lock_guard guard(m_mutex);
   m_stream.flush();
 }
 
-void StreamLogHandler::Emit(llvm::StringRef message) { m_stream << message; }
+void StreamLogHandler::Emit(llvm::StringRef message) {
+  if (m_stream.GetBufferSize() > 0) {
+std::lock_guard guard(m_mutex);
+m_stream << message;
+  } else {
+m_stream << message;
+  }
+}
 
 CallbackLogHandler::CallbackLogHandler(lldb::LogOutputCallback callback,
void *baton)
@@ -364,6 +364,7 @@
 : m_messages(std::make_unique(size)), m_size(size) {}
 
 void RotatingLogHandler::Emit(llvm::StringRef message) {
+  std::lock_guard guard(m_mutex);
   ++m_total_count;
   const size_t index = m_next_index;
   m_next_index = NormalizeIndex(index + 1);
@@ -381,6 +382,7 @@
 }
 
 void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const {
+  std::lock_guard guard(m_mutex);
   const size_t start_idx = GetFirstMessageIndex();
   const size_t stop_idx = start_idx + GetNumMessages();
   for (size_t i = start_idx; i < stop_idx; ++i) {
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1448,8 +1448,7 @@
   assert(log_handler_sp);
 
   if (log_options == 0)
-log_options =
-LLDB_LOG_OPTION_PREPEND_THREAD_NAME | LLDB_LOG_OPTION_THREADSAFE;
+log_options = LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
 
   return Log::EnableLogChannel(log_handler_sp, log_options, channel, categories,
error_stream);
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -433,8 +433,6 @@
 Desc<"Set the destination file to log to.">;
   def log_buffer_size : Option<"buffer", "b">, Group<1>, Arg<"UnsignedInteger">,
 Desc<"Set the log to be buffered, using the specified buffer size.">;
-  def log_threadsafe : Option<"threadsafe", "t">, Group<1>,
-Desc<"Enable thread safe logging to avoid interweaved log lines.">;
   def log_verbose : Option<"verbose", "v">, Group<1>,
 Desc<"Enable verbose logging.">;
   def log_sequence : Option<"sequence", "s">, Group<1>,
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -94,9 +94,6 @@
 error =
 buffer_size.SetValueFromString(option_arg, eVarSetOperationAssign);
 break;
-  case 't':
-log_options |= LLDB_LOG_OPTION_THREADSAFE;
-break;
   case 'v':
 log_options |= LLDB_LOG_OPTION_VERBOSE;
 break;
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -33,7 +33,6 @@
 class raw_ostream;
 }
 // Logging Options
-#define LLDB_LOG_OPTION_THREADSAFE (1u << 0)
 #define LLDB_LOG_OPTION_VERBOSE (1u << 1)
 #define LLDB_LOG_OPTION_PREPEND_SEQUENCE (1u << 3)
 #define LLDB_LOG_OPTION_PREPEND_TIMESTAMP (1u << 4)
@@ -50,10 +49,6 @@
 public:
   virtual ~LogHandler() = default;
   virtual void Emit(llvm::StringRef message) = 0;
-  void EmitThreadSafe(llvm::StringRef message);
-
-private:
-  std::mutex m_mutex;
 };
 
 class StreamLogHan

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/include/lldb/Utility/Log.h:98
 
+class SystemLogHandler : public LogHandler {
+public:

Doxygen comment?



Comment at: lldb/source/Utility/Log.cpp:406
+  if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
+os_log(g_os_log, "%{public}s", str.c_str());
+  } else {

Can you add a FIXME for future reference that this ought to be moved into a 
macro into the header, so we can benefit from os_log's special compiler support 
and the only reason why we don't do this is that it's unsafe to include 
 in the LLVM headers?


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

https://reviews.llvm.org/D128321

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

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Utility/Log.cpp:406
+  if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
+os_log(g_os_log, "%{public}s", str.c_str());
+  } else {

aprantl wrote:
> Can you add a FIXME for future reference that this ought to be moved into a 
> macro into the header, so we can benefit from os_log's special compiler 
> support and the only reason why we don't do this is that it's unsafe to 
> include  in the LLVM headers?
I'd be happy to add the comment, but it wouldn't be entirely true though. The 
log handers are meant to sit underneath the more general logging infrastructure 
and as you pointed out, you need a constant string in order to benefit from the 
os_log optimization. Unlike for os_signposts, I think it's very unlikely that 
we'd ever want to bypass our whole logging system for that. 


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

https://reviews.llvm.org/D128321

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

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

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



Comment at: lldb/source/Utility/Log.cpp:406
+  if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
+os_log(g_os_log, "%{public}s", str.c_str());
+  } else {

JDevlieghere wrote:
> aprantl wrote:
> > Can you add a FIXME for future reference that this ought to be moved into a 
> > macro into the header, so we can benefit from os_log's special compiler 
> > support and the only reason why we don't do this is that it's unsafe to 
> > include  in the LLVM headers?
> I'd be happy to add the comment, but it wouldn't be entirely true though. The 
> log handers are meant to sit underneath the more general logging 
> infrastructure and as you pointed out, you need a constant string in order to 
> benefit from the os_log optimization. Unlike for os_signposts, I think it's 
> very unlikely that we'd ever want to bypass our whole logging system for 
> that. 
s/the comment/a comment/ -> describing what I said here :-) 


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

https://reviews.llvm.org/D128321

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

[Lldb-commits] [PATCH] D127500: [lldb] [llgs] Make `k` kill all processes, and fix multiple exits

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1050
+  lldb::pid_t pid = process->GetID();
+  m_mainloop.AddPendingCallback([&, pid](MainLoopBase &loop) {
+m_debugged_processes.erase(pid);

The universal reference capture is dangerous for callbacks that outlive the 
enclosing function. I think that `[this,pid]` would suffice.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1196
+  if (!bool(m_extensions_supported &
+NativeProcessProtocol::Extension::multiprocess))
+assert(!m_stdio_handle_up);

mgorny wrote:
> labath wrote:
> > I don't think this is right. I believe the important distinction is whether 
> > we have non-stop mode enabled or not, because in all-stop mode we are not 
> > able to send stdio while stopped, regardless of how many processes we're 
> > debugging.
> Well, the code exploded in all-stop mode as well because in order to kill 
> multiple processes, we effectively resume them all. I suppose there could be 
> a way around it (synchronously killing one process after another and waiting 
> for everything to happen) but I'm not convinced this is really worth the 
> added complexity.
I don't think this needs to be complex at all. What we need to basically do, is 
call StartSTDIOForwarding whenever the protocol allows us to send stdio, and 
StopSTDIOForwarding when we cannot. When we supported just a single process, 
the easiest way to achieve this was to hook into the started/stopped events of 
that process. Now that's no longer true, so maybe we just need to hook it up 
elsewhere.

I think starting could be done directly from the packet handlers 
(c/s/vCont/...). And those handlers already have to check for nonstop mode, so 
any deviations could be handled there:
```
if (all_stop) {
  StartSTDIOForwarding(); // Can forward from now until the stop-reply packet 
is send
  return Success;
} else {
  SendOKResponse();
  // Can we forward now? Or maybe it can be sent all the time?
}
```

Stopping could probably stay coupled with the sending of the stop-reply packet 
(i.e., handling of the process event), since it's the stop reply which 
determines whether the stdio packet can be sent.


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

https://reviews.llvm.org/D127500

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

[Lldb-commits] [PATCH] D127667: [lldb] [llgs] Implement the vKill packet

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Do we need the kill command to be asynchronous (return before the process 
actually dies) for anything. Could we just make it synchronous instead (call 
wait inside the process plugin, and return the wait result)?


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

https://reviews.llvm.org/D127667

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

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

In D127986#3600665 , @JDevlieghere 
wrote:

> @labath let me know if this is what you had in mind

Yes, that's pretty much it.


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [PATCH] D128324: [lldb] [llgs] Introduce a AppendThreadIDToResponse() helper

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:4162
+void GDBRemoteCommunicationServerLLGS::AppendThreadIDToResponse(
+StreamString &response, lldb::pid_t pid, lldb::tid_t tid) {
+  if (bool(m_extensions_supported &

This could technically be any Stream object, right?


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

https://reviews.llvm.org/D128324

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