https://github.com/JDevlieghere closed
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/JDevlieghere approved this pull request.
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
nmosier wrote:
@walter-erquinigo Could you merge this PR for me? I don't have commit access.
Thanks!
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi
https://github.com/walter-erquinigo approved this pull request.
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
nmosier wrote:
Sounds good. My changes are ready -- only that `testStartPerCpuSession` test is
failing now.
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailma
walter-erquinigo wrote:
Just let me know when your changes are ready and only that test fails.
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lld
walter-erquinigo wrote:
> Also, I noticed some error messages were different for some tests — I'm
> guessing they may have changed in newer versions of libipt.
That makes sense. In this case I'd just drop text comparisons for those tests.
Just knowing that an error was found is good enough.
>
nmosier wrote:
I went ahead and turned `TraceItemStorage` into a variant. I also deleted the
memory usage / bytes from the tests. Also, I noticed some error messages were
different for some tests — I'm guessing they may have changed in newer versions
of libipt.
Right now, only 1 test fails:
`
https://github.com/nmosier updated
https://github.com/llvm/llvm-project/pull/77252
>From 7b184bbc1ae24d548d8574d2620b642e3304423a Mon Sep 17 00:00:00 2001
From: Nicholas Mosier
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors
Fix #77251.
---
.../
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 036e48e2f5f890e1f9574cdb610e2336f12038a2
77aa29a286cbf06a9959790ec26eb025ab5e3a16 --
https://github.com/nmosier updated
https://github.com/llvm/llvm-project/pull/77252
>From 77aa29a286cbf06a9959790ec26eb025ab5e3a16 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors
Fix #77251.
---
.../
@@ -107,7 +107,15 @@ DecodedThread::CreateNewTraceItem(lldb::TraceItemKind
kind) {
(*m_last_tsc)->second.items_count++;
if (m_last_nanoseconds)
(*m_last_nanoseconds)->second.items_count++;
- return m_item_data.back();
+
+ TraceItemStorage &data = m_item_data.back()
https://github.com/nmosier edited
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -107,7 +107,15 @@ DecodedThread::CreateNewTraceItem(lldb::TraceItemKind
kind) {
(*m_last_tsc)->second.items_count++;
if (m_last_nanoseconds)
(*m_last_nanoseconds)->second.items_count++;
- return m_item_data.back();
+
+ TraceItemStorage &data = m_item_data.back()
walter-erquinigo wrote:
Nice! Could you just delete all the tests that check for number of bytes? I
think it's better to delete them because they might change in different systems.
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits maili
@@ -276,6 +276,9 @@ class DecodedThread : public
std::enable_shared_from_this {
/// The string message of this item if it's an error
std::string error;
+
+TraceItemStorage() {}
+~TraceItemStorage() {}
walter-erquinigo wrote:
do you still need
@@ -107,7 +107,15 @@ DecodedThread::CreateNewTraceItem(lldb::TraceItemKind
kind) {
(*m_last_tsc)->second.items_count++;
if (m_last_nanoseconds)
(*m_last_nanoseconds)->second.items_count++;
- return m_item_data.back();
+
+ TraceItemStorage &data = m_item_data.back()
nmosier wrote:
Thanks! That test passes now. Another test was crashing as well, but I fixed
that (it was due to `TraceItemStorage.error` not being initialized properly in
`DecodedThread::CreateNewTraceItem` when creating an error item.
49 out of 56 tests now pass, with the remaining tests bein
https://github.com/nmosier updated
https://github.com/llvm/llvm-project/pull/77252
>From eece351a68e014d7a46bd1e3512298dd5dda Mon Sep 17 00:00:00 2001
From: Nicholas Mosier
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors
Fix #77251.
---
lldb
walter-erquinigo wrote:
> because the prior call to `CreateProcess` on line 107 returned null.
If that call returned null, that means that CreateProcess wasn't able to find
the "trace" process plugin. Another thing to add, which should definitely work,
is to invoke
```
ProcessTrace::Initializ
nmosier wrote:
Thanks for the suggestions. Unfortunately, it's still crashing, now on the line
```c
process_sp->SetID(static_cast(pid));
```
because the prior call to `CreateProcess` on line 107 returned null.
https://github.com/llvm/llvm-project/pull/77252
__
walter-erquinigo wrote:
> At least one test fails: `./bin/lldb -o 'trace load -v
> /llvm/lldb/test/API/commands/trace/intelpt-trace/trace_2threads.json'`
> crashes with an assertion failure on TraceIntelPTBundleLoader.cpp:127
> (`*process_sp` is a null pointer dereference).
>
> Do you know wh
nmosier wrote:
At least one test fails: `./bin/lldb -o 'trace load -v
/llvm/lldb/test/API/commands/trace/intelpt-trace/trace_2threads.json'` crashes
with an assertion failure on TraceIntelPTBundleLoader.cpp:127 (`*process_sp` is
a null pointer dereference).
Do you know why `process_sp` would
walter-erquinigo wrote:
`third-party/llvm-project/lldb/test/API/commands/trace` has a set of pythong
tests.
You should be able to run them with
```
build lldb-dotest
lldb-dotest -p TestTrace
```
Also, the CMake variable `LLDB_INCLUDE_TESTS` must be enabled (e.g. `ON`) in
order to build `lldb
nmosier wrote:
> Btw, how are you testing them?
Not very rigorously, ha. Just something like
```
b main
r
process trace start
c
thread trace dump instructions
```
I was using a version with my original fixes to debug other code that was
crashing, and it worked fine for that.
Are there existing
https://github.com/walter-erquinigo commented:
These changes lgtm at this point. Btw, how are you testing them?
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mai
@@ -108,8 +108,8 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t
pid,
///*listener*/ nullptr, "trace",
///*crash_file*/ nullptr,
///*can_connect*/ false);
-
- process_sp->SetID(static_cast(pid));
+ //
+ // process_sp->SetID(static_cast(pid));
-
@@ -103,12 +103,10 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t
pid,
ParsedProcess parsed_process;
parsed_process.target_sp = target_sp;
- // This should instead try to directly create an instance of ProcessTrace.
- // ProcessSP process_sp = target_sp->Cr
@@ -85,6 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
return interpolate(next_range->nanos);
}
+DecodedThread::TraceItemStorage::TraceItemStorage(
+const TraceItemStorage &other) {
+ std::memcpy(this, &other, sizeof *this);
nmo
https://github.com/nmosier updated
https://github.com/llvm/llvm-project/pull/77252
>From ffe2dff5783e57dfbc40b1840a784a28ea18ef26 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors
Fix #77251.
---
lldb
https://github.com/nmosier updated
https://github.com/llvm/llvm-project/pull/77252
>From a4876e327f4e248ab4a48538f64507f60023d9d2 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors
Fix #77251.
---
lldb
@@ -85,6 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
return interpolate(next_range->nanos);
}
+DecodedThread::TraceItemStorage::TraceItemStorage(
+const TraceItemStorage &other) {
+ std::memcpy(this, &other, sizeof *this);
wal
@@ -108,8 +108,8 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t
pid,
///*listener*/ nullptr, "trace",
///*crash_file*/ nullptr,
///*can_connect*/ false);
-
- process_sp->SetID(static_cast(pid));
+ //
+ // process_sp->SetID(static_cast(pid));
-
@@ -103,12 +103,10 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t
pid,
ParsedProcess parsed_process;
parsed_process.target_sp = target_sp;
- // This should instead try to directly create an instance of ProcessTrace.
- // ProcessSP process_sp = target_sp->Cr
@@ -103,12 +103,10 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t
pid,
ParsedProcess parsed_process;
parsed_process.target_sp = target_sp;
- // This should instead try to directly create an instance of ProcessTrace.
- // ProcessSP process_sp = target_sp->Cr
https://github.com/nmosier updated
https://github.com/llvm/llvm-project/pull/77252
>From c99dbd7f915ab996a68c4d9eb0cee2edb3b33b84 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors
Fix #77251.
---
.../
@@ -85,6 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
return interpolate(next_range->nanos);
}
+DecodedThread::TraceItemStorage::TraceItemStorage(
+const TraceItemStorage &other) {
+ std::memcpy(this, &other, sizeof *this);
nmo
@@ -276,6 +276,10 @@ class DecodedThread : public
std::enable_shared_from_this {
/// The string message of this item if it's an error
std::string error;
+
+TraceItemStorage() {}
+~TraceItemStorage() {}
nmosier wrote:
`TraceItemStorage()` can'
@@ -276,6 +276,10 @@ class DecodedThread : public
std::enable_shared_from_this {
/// The string message of this item if it's an error
std::string error;
+
+TraceItemStorage() {}
+~TraceItemStorage() {}
JDevlieghere wrote:
Can this be `= defau
@@ -85,6 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
return interpolate(next_range->nanos);
}
+DecodedThread::TraceItemStorage::TraceItemStorage(
+const TraceItemStorage &other) {
+ std::memcpy(this, &other, sizeof *this);
JDe
@@ -103,12 +103,10 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t
pid,
ParsedProcess parsed_process;
parsed_process.target_sp = target_sp;
- // This should instead try to directly create an instance of ProcessTrace.
- // ProcessSP process_sp = target_sp->Cr
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Nicholas Mosier (nmosier)
Changes
Fix #77251.
---
Full diff: https://github.com/llvm/llvm-project/pull/77252.diff
6 Files Affected:
- (modified)
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp (+1-3)
- (modified)
https://github.com/nmosier created
https://github.com/llvm/llvm-project/pull/77252
Fix #77251.
>From c8b55528ce76a5d3ae1736a0f73499d96173d927 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors
Fix #772
43 matches
Mail list logo
