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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It's nice to see this code getting some use. I was starting to think we should delete it...



================
Comment at: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py:30-36
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+        self.runCmd("b main")
+        self.runCmd("b " + str(line_number('main.cpp', '// Break 1')))
+
+        self.runCmd("r")
----------------
`lldbutil.run_to_name_breakpoint`


================
Comment at: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py:50
+            patterns=[
+                "rand", # We expect to see a reference to the rand function
+                        # within the last instructions
----------------
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


================
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:
> 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.)


================
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
+////
+////===----------------------------------------------------------------------===//
+//
----------------
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?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77107