[Lldb-commits] [PATCH] D85241: [intel-pt] Disable/Enable tracing to guarantee the trace is correct

2020-10-23 Thread walter erquinigo 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 rG48d8af982516: [intel-pt] Disable/Enable tracing to guarantee 
the trace is correct (authored by wallace).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85241

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


Index: lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
+++ lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
@@ -17,6 +17,7 @@
 #include "ProcessorTrace.h"
 #include "lldb/Host/linux/Support.h"
 
+#include 
 #include 
 
 using namespace lldb;
@@ -273,6 +274,23 @@
 #ifndef PERF_ATTR_SIZE_VER5
   llvm_unreachable("perf event not supported");
 #else
+  // Disable the perf event to force a flush out of the CPU's internal buffer.
+  // Besides, we can guarantee that the CPU won't override any data as we are
+  // reading the buffer.
+  //
+  // The Intel documentation says:
+  //
+  // Packets are first buffered internally and then written out asynchronously.
+  // To collect packet output for postprocessing, a collector needs first to
+  // ensure that all packet data has been flushed from internal buffers.
+  // Software can ensure this by stopping packet generation by clearing
+  // IA32_RTIT_CTL.TraceEn (see “Disabling Packet Generation” in
+  // Section 35.2.7.2).
+  //
+  // This is achieved by the PERF_EVENT_IOC_DISABLE ioctl request, as mentioned
+  // in the man page of perf_event_open.
+  ioctl(*m_fd, PERF_EVENT_IOC_DISABLE);
+
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
   Status error;
   uint64_t head = m_mmap_meta->aux_head;
@@ -293,6 +311,9 @@
 
   ReadCyclicBuffer(buffer, GetAuxBuffer(), static_cast(head), offset);
   LLDB_LOG(log, "ReadCyclic BUffer Done");
+
+  // Reenable tracing now we have read the buffer
+  ioctl(*m_fd, PERF_EVENT_IOC_ENABLE);
   return error;
 #endif
 }


Index: lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
+++ lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
@@ -17,6 +17,7 @@
 #include "ProcessorTrace.h"
 #include "lldb/Host/linux/Support.h"
 
+#include 
 #include 
 
 using namespace lldb;
@@ -273,6 +274,23 @@
 #ifndef PERF_ATTR_SIZE_VER5
   llvm_unreachable("perf event not supported");
 #else
+  // Disable the perf event to force a flush out of the CPU's internal buffer.
+  // Besides, we can guarantee that the CPU won't override any data as we are
+  // reading the buffer.
+  //
+  // The Intel documentation says:
+  //
+  // Packets are first buffered internally and then written out asynchronously.
+  // To collect packet output for postprocessing, a collector needs first to
+  // ensure that all packet data has been flushed from internal buffers.
+  // Software can ensure this by stopping packet generation by clearing
+  // IA32_RTIT_CTL.TraceEn (see “Disabling Packet Generation” in
+  // Section 35.2.7.2).
+  //
+  // This is achieved by the PERF_EVENT_IOC_DISABLE ioctl request, as mentioned
+  // in the man page of perf_event_open.
+  ioctl(*m_fd, PERF_EVENT_IOC_DISABLE);
+
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
   Status error;
   uint64_t head = m_mmap_meta->aux_head;
@@ -293,6 +311,9 @@
 
   ReadCyclicBuffer(buffer, GetAuxBuffer(), static_cast(head), offset);
   LLDB_LOG(log, "ReadCyclic BUffer Done");
+
+  // Reenable tracing now we have read the buffer
+  ioctl(*m_fd, PERF_EVENT_IOC_ENABLE);
   return error;
 #endif
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D85241: [intel-pt] Disable/Enable tracing to guarantee the trace is correct

2020-10-21 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Sounds reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85241

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


[Lldb-commits] [PATCH] D85241: [intel-pt] Disable/Enable tracing to guarantee the trace is correct

2020-08-04 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 283009.
wallace added a comment.

remove an empty line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85241

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


Index: lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
+++ lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
@@ -17,6 +17,7 @@
 #include "ProcessorTrace.h"
 #include "lldb/Host/linux/Support.h"
 
+#include 
 #include 
 
 using namespace lldb;
@@ -273,6 +274,23 @@
 #ifndef PERF_ATTR_SIZE_VER5
   llvm_unreachable("perf event not supported");
 #else
+  // Disable the perf event to force a flush out of the CPU's internal buffer.
+  // Besides, we can guarantee that the CPU won't override any data as we are
+  // reading the buffer.
+  //
+  // The Intel documentation says:
+  //
+  // Packets are first buffered internally and then written out asynchronously.
+  // To collect packet output for postprocessing, a collector needs first to
+  // ensure that all packet data has been flushed from internal buffers.
+  // Software can ensure this by stopping packet generation by clearing
+  // IA32_RTIT_CTL.TraceEn (see “Disabling Packet Generation” in
+  // Section 35.2.7.2).
+  //
+  // This is achieved by the PERF_EVENT_IOC_DISABLE ioctl request, as mentioned
+  // in the man page of perf_event_open.
+  ioctl(*m_fd, PERF_EVENT_IOC_DISABLE);
+
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
   Status error;
   uint64_t head = m_mmap_meta->aux_head;
@@ -293,6 +311,9 @@
 
   ReadCyclicBuffer(buffer, GetAuxBuffer(), static_cast(head), offset);
   LLDB_LOG(log, "ReadCyclic BUffer Done");
+
+  // Reenable tracing now we have read the buffer
+  ioctl(*m_fd, PERF_EVENT_IOC_ENABLE);
   return error;
 #endif
 }


Index: lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
+++ lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
@@ -17,6 +17,7 @@
 #include "ProcessorTrace.h"
 #include "lldb/Host/linux/Support.h"
 
+#include 
 #include 
 
 using namespace lldb;
@@ -273,6 +274,23 @@
 #ifndef PERF_ATTR_SIZE_VER5
   llvm_unreachable("perf event not supported");
 #else
+  // Disable the perf event to force a flush out of the CPU's internal buffer.
+  // Besides, we can guarantee that the CPU won't override any data as we are
+  // reading the buffer.
+  //
+  // The Intel documentation says:
+  //
+  // Packets are first buffered internally and then written out asynchronously.
+  // To collect packet output for postprocessing, a collector needs first to
+  // ensure that all packet data has been flushed from internal buffers.
+  // Software can ensure this by stopping packet generation by clearing
+  // IA32_RTIT_CTL.TraceEn (see “Disabling Packet Generation” in
+  // Section 35.2.7.2).
+  //
+  // This is achieved by the PERF_EVENT_IOC_DISABLE ioctl request, as mentioned
+  // in the man page of perf_event_open.
+  ioctl(*m_fd, PERF_EVENT_IOC_DISABLE);
+
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
   Status error;
   uint64_t head = m_mmap_meta->aux_head;
@@ -293,6 +311,9 @@
 
   ReadCyclicBuffer(buffer, GetAuxBuffer(), static_cast(head), offset);
   LLDB_LOG(log, "ReadCyclic BUffer Done");
+
+  // Reenable tracing now we have read the buffer
+  ioctl(*m_fd, PERF_EVENT_IOC_ENABLE);
   return error;
 #endif
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D85241: [intel-pt] Disable/Enable tracing to guarantee the trace is correct

2020-08-04 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
wallace added a reviewer: clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
wallace requested review of this revision.
Herald added a subscriber: JDevlieghere.

As mentioned in the comment inside the code, the Intel documentation
states that the internal CPU buffer is flushed out to RAM only when tracing is
disabled. Otherwise, the buffer on RAM might be stale.

This diff disables tracing when the trace buffer is going to be read. This is a
quite safe operation, as the reading is done when the inferior is paused at a
breakpoint, so we are not losing any packets because there's no code being
executed.

After the reading is finished, tracing is enabled back.

It's a bit hard to write a test for this now, as Greg Clayton and I will
refactor the PT support, but I tested manually by doing a script that automates
the following flow

  (lldb) b main
  Breakpoint 1: where = a.out`main + 15 at main.cpp:4:7, address = 
0x0040050f
  (lldb) r
  Process 3078226 launched: 
'/home/wallace/llvm-sand/build/Release/Linux-x86_64/llvm/lldb-test-build.noindex/tools/intel-features/intel-pt/simple-test/TestIntelPTSimpleBinary.test_basic_flow/a.out'
 (x86_64)
  Process 3078226 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.1
  frame #0: 0x0040050f a.out`main at main.cpp:4:7
  (lldb) processor-trace start
  (lldb) b 5
  Breakpoint 2: where = a.out`main + 22 at main.cpp:5:12, address = 
0x00400516
  (lldb) c
  Process 3078226 resuming
  Process 3078226 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 2.1
  frame #0: 0x00400516 a.out`main at main.cpp:5:12
  (lldb) processor-trace show-instr-log 
  thread #1: tid=3078226
  0x40050f <+15>: movl   $0x0, -0x8(%rbp)
  
  >>> Before, some runs of the script up to this point lead to empty traces
  
  (lldb) b 6
  Breakpoint 3: where = a.out`main + 42 at main.cpp:6:14, address = 
0x0040052a
  (lldb) c
  Process 3092991 resuming
  Process 3092991 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 3.1
  frame #0: 0x0040052a a.out`main at main.cpp:6:14
  (lldb) processor-trace show-instr-log 

  thread #1: tid=3092991
  0x40050f <+15>: movl   $0x0, -0x8(%rbp)
  0x400516 <+22>: movl   $0x0, -0xc(%rbp)
  0x40051d <+29>: cmpl   $0x2710, -0xc(%rbp)   ; imm = 0x2710 
  0x400524 <+36>: jge0x400546  ; <+70> at main.cpp
  0x400524 <+36>: jge0x400546  ; <+70> at main.cpp
  
  >>> The trace was re-enabled correctly and includes the instruction of the
  first reading.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85241

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


Index: lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
+++ lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
@@ -17,6 +17,7 @@
 #include "ProcessorTrace.h"
 #include "lldb/Host/linux/Support.h"
 
+#include 
 #include 
 
 using namespace lldb;
@@ -143,6 +144,7 @@
   }
   m_mmap_aux = std::unique_ptr(
   reinterpret_cast(mmap_aux), munmap_delete(bufsize));
+
   return error;
 #endif
 }
@@ -273,6 +275,23 @@
 #ifndef PERF_ATTR_SIZE_VER5
   llvm_unreachable("perf event not supported");
 #else
+  // Disable the perf event to force a flush out of the CPU's internal buffer.
+  // Besides, we can guarantee that the CPU won't override any data as we are
+  // reading the buffer.
+  //
+  // The Intel documentation says:
+  //
+  // Packets are first buffered internally and then written out asynchronously.
+  // To collect packet output for postprocessing, a collector needs first to
+  // ensure that all packet data has been flushed from internal buffers.
+  // Software can ensure this by stopping packet generation by clearing
+  // IA32_RTIT_CTL.TraceEn (see “Disabling Packet Generation” in
+  // Section 35.2.7.2).
+  //
+  // This is achieved by the PERF_EVENT_IOC_DISABLE ioctl request, as mentioned
+  // in the man page of perf_event_open.
+  ioctl(*m_fd, PERF_EVENT_IOC_DISABLE);
+
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
   Status error;
   uint64_t head = m_mmap_meta->aux_head;
@@ -293,6 +312,9 @@
 
   ReadCyclicBuffer(buffer, GetAuxBuffer(), static_cast(head), offset);
   LLDB_LOG(log, "ReadCyclic BUffer Done");
+
+  // Reenable tracing now we have read the buffer
+  ioctl(*m_fd, PERF_EVENT_IOC_ENABLE);
   return error;
 #endif
 }


Index: lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
+++