[Lldb-commits] [PATCH] D77107: [intel-pt] Implement a basic test case

[walter erquinigo via Phabricator via lldb-commits]

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


================
Comment at: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py:50-54
+                "rand", # We expect to see a reference to the rand function
+                        # within the last instructions
+                hex(fun_start_adddress),  # We expect to have seen the first
+                                          # instruction of 'fun'
+                "at main.cpp:21" # We expect to see the exit condition of
----------------
labath wrote:
> labath wrote:
> > clayborg wrote:
> > > can we guarantee we will see any of these on a fully loaded machine running many tests simultaneously? Maybe we need to settle for the header of the output only to know that it tried to display something?
> > better avoid referencing functions from the system library... makes the test more hermetic
> What exactly is the case you're worried about? I'm not very familiar with how all this works, but I would think that the kernel trace buffer for this is application specific, and is automatically switched off when the os schedules a different process (anything else would be a security breach). If that is true, then we should have pretty good control over what goes into the buffer, and we can ensure that it is: (a) big enough; and/or (b) application does not execute too much code and overflows it (not calling rand would help us get a reasonable upper bound on that).
> 
> (Nonetheless it would be good to run some stress tests to verify this is stable.)
This is how it works: by default Intel PT has to be enabled on each logical CPU, where it traces everything, regardless of which thread if running. The kernel has the ability to switch Intel PT on and off on each logical CPU whenever there's a thread context switch, and this kind of filtering is what this LLDB plugin is using.

For some context, that kind of filtering is expensive because of the constant enabling/disabling of Intel PT and could incur in up to 5% total CPU cost according to Intel. Another drawback of this approach is that threads spawned by an already traced thread are not traced by default. The user has to enable filtered tracing explicitly for these threads.

A faster approach is to enable Intel PT on all CPUs without filtering. That leads to a ~2% total CPU cost according to some tests some colleagues and I ran. However, this needs having a secondary trace of context switches to be able to attribute Intel PT packets to individual threads. We are not following that approach in this plugin because of the added complexity. However, I plan to make this plugin flexible enough to be able to load Intel PT traces collected by other mechanisms which can do global tracing correctly.

Lastly, the PT Trace unit in the cpu writes PT packets on memory without interrupting the CPU itself nor the kernel. The only case in which packets couldn't be written is when the BUS is completely full. However, this is extremely rare and the CPU would retry later.

-------------------------
That being said, I see no reason why the trace wouldn't be collected at that point. Just in case I'll add a 0.1 ms wait time for the CPU to have enough time to send all the packets, which should be more than enough.


================
Comment at: lldb/test/API/tools/intel-features/intel-pt/test/main.cpp:2-8
+////
+//// 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
+////
+////===----------------------------------------------------------------------===//
+//
----------------
labath wrote:
> We're not putting license headers on tests.
> 
> (Do these get automatically created by some IDEs or something? Can they be configured not to do that?)
I just copy pasted it from another test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77107