[PATCH v2 0/7] Final fixes before KS 2.0

[PATCH v2 0/7] Final fixes before KS 2.0

[Yordan Karadzhov (VMware)]

v2 changes:
 - Still showing all CPU plots from the new trace file when
   appending [PATCH kernel-shark: Preserve open graphs when
   appending data].
 - Setting "seq.buffer" to NULL after calling trace_seq_destroy()
   in [PATCH kernel-shark: Fix the checking if "trace_seq" was destroyed]
 - [PATCH kernel-shark: No slash at the end of KS_PLUGIN_INSTALL_PREFIX]
   is new.
 

Yordan Karadzhov (VMware) (7):
  kernel-shark: Preserve markers when appending data
  kernel-shark: Preserve open graphs when appending data
  kernel-shark: Clear before loading new session
  kernel-shark: Better handling of plugins when appending data file
  kernel-shark: Do draw the combo point of the mark
  kernel-shark: Fix the checking if "trace_seq" was destroyed
  kernel-shark: No slash at the end of KS_PLUGIN_INSTALL_PREFIX

 CMakeLists.txt          |  2 +-
 src/KsDualMarker.hpp    |  6 ++++++
 src/KsGLWidget.cpp      | 40 ++++++++++++++++++++++++++--------------
 src/KsGLWidget.hpp      |  4 +++-
 src/KsMainWindow.cpp    | 34 +++++++++++++++++++++++++++++-----
 src/KsPlotTools.cpp     |  3 +++
 src/KsSession.cpp       |  1 +
 src/KsTraceGraph.cpp    | 10 +++++++---
 src/KsTraceGraph.hpp    |  2 +-
 src/KsUtils.cpp         |  4 ++--
 src/libkshark-tepdata.c |  4 +++-
 11 files changed, 82 insertions(+), 28 deletions(-)

-- 
2.27.0

[PATCH v2 1/7] kernel-shark: Preserve markers when appending data

[Yordan Karadzhov (VMware)]

When an entry is selected in the KernelShark GUI (using marker A and
marker B) we only keep track the index of this entry inside the array
of entries loaded at the moment of selecting. However, then a data
file is appended, the new entries are merged to this array and the
array is sorted. As a result the index of the marker can/will point
to completely different entry.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/KsDualMarker.hpp |  6 ++++++
 src/KsMainWindow.cpp | 24 ++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/KsDualMarker.hpp b/src/KsDualMarker.hpp
index 0dcaf93..39c0ce2 100644
--- a/src/KsDualMarker.hpp
+++ b/src/KsDualMarker.hpp
@@ -154,6 +154,12 @@ public:
 
 	void updateLabels();
 
+	/** Get the index inside the data array marker A points to. */
+	ssize_t markerAPos() {return markerA()._isSet ? markerA()._pos : -1;}
+
+	/** Get the index inside the data array marker B points to. */
+	ssize_t markerBPos() {return markerB()._isSet ? markerB()._pos : -1;}
+
 signals:
 	/**
 	 * This signal is emitted when the Table View has to switch the color
diff --git a/src/KsMainWindow.cpp b/src/KsMainWindow.cpp
index d0a434a..fa893ce 100644
--- a/src/KsMainWindow.cpp
+++ b/src/KsMainWindow.cpp
@@ -571,8 +571,15 @@ void KsMainWindow::markEntry(ssize_t row, DualMarkerState st)
 /** Select given kshark_entry with a given maker. */
 void KsMainWindow::markEntry(const kshark_entry *e, DualMarkerState st)
 {
-	ssize_t row = kshark_find_entry_by_time(e->ts, _data.rows(),
-						0, _data.size() - 1);
+	ssize_t row;
+
+	if (!e) {
+		_mState.getMarker(st).reset();
+		return;
+	}
+
+	row = kshark_find_entry_by_time(e->ts, _data.rows(),
+					0, _data.size() - 1);
 
 	markEntry(row, st);
 }
@@ -1341,7 +1348,20 @@ void KsMainWindow::loadDataFile(const QString& fileName)
 /** Append trace data for file. */
 void KsMainWindow::appendDataFile(const QString& fileName)
 {
+	kshark_entry *eMarkA(nullptr), *eMarkB(nullptr);
+	int rowA = _mState.markerAPos();
+	int rowB = _mState.markerBPos();
+
+	if (rowA >= 0)
+		eMarkA = _data.rows()[rowA];
+
+	if (rowB >= 0)
+		eMarkB = _data.rows()[rowB];
+
 	_load(fileName, true);
+
+	markEntry(eMarkA, DualMarkerState::A);
+	markEntry(eMarkB, DualMarkerState::B);
 }
 
 void KsMainWindow::_error(const QString &mesg,
-- 
2.27.0

[PATCH v2 2/7] kernel-shark: Preserve open graphs when appending data

[Yordan Karadzhov (VMware)]

If the user has already made some selection of graphs to be displayed
by the GUI, those particular graphs are likely to be important for him,
so it is better to keep this configuration after the data file is
appended. The opposite can be annoying for the user.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/KsGLWidget.cpp   | 40 ++++++++++++++++++++++++++--------------
 src/KsGLWidget.hpp   |  4 +++-
 src/KsMainWindow.cpp | 10 +++++++---
 src/KsTraceGraph.cpp | 10 +++++++---
 src/KsTraceGraph.hpp |  2 +-
 src/KsUtils.cpp      |  1 +
 6 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/src/KsGLWidget.cpp b/src/KsGLWidget.cpp
index 9f8e43b..8aab7b3 100644
--- a/src/KsGLWidget.cpp
+++ b/src/KsGLWidget.cpp
@@ -409,25 +409,12 @@ void KsGLWidget::keyReleaseEvent(QKeyEvent *event)
  */
 #define KS_MAX_START_PLOTS 16
 
-/**
- * @brief Load and show trace data.
- *
- * @param data: Input location for the KsDataStore object.
- *	  KsDataStore::loadDataFile() must be called first.
- */
-void KsGLWidget::loadData(KsDataStore *data)
+void KsGLWidget::_defaultPlots(kshark_context *kshark_ctx)
 {
-	kshark_context *kshark_ctx(nullptr);
 	QVector<int> streamIds, plotVec;
 	uint64_t tMin, tMax;
 	int nCPUs, nBins;
 
-	if (!kshark_instance(&kshark_ctx) || !kshark_ctx->n_streams)
-		return;
-
-	loadColors();
-
-	_data = data;
 	_model.reset();
 	_streamPlots.clear();
 
@@ -463,6 +450,31 @@ void KsGLWidget::loadData(KsDataStore *data)
 	tMin = _data->rows()[0]->ts;
 	tMax = _data->rows()[_data->size() - 1]->ts;
 	ksmodel_set_bining(_model.histo(), nBins, tMin, tMax);
+}
+
+/**
+ * @brief Load and show trace data.
+ *
+ * @param data: Input location for the KsDataStore object.
+ *		KsDataStore::loadDataFile() must be called first.
+ * @param resetPlots: If true, all existing graphs are closed
+ *		      and a default configuration of graphs is displayed
+ *		      (all CPU plots). If false, the current set of graphs
+ *		      is preserved.
+ */
+void KsGLWidget::loadData(KsDataStore *data, bool resetPlots)
+{
+	kshark_context *kshark_ctx(nullptr);
+
+	if (!kshark_instance(&kshark_ctx) || !kshark_ctx->n_streams)
+		return;
+
+	loadColors();
+
+	_data = data;
+	if (resetPlots)
+		_defaultPlots(kshark_ctx);
+
 	_model.fill(_data);
 }
 
diff --git a/src/KsGLWidget.hpp b/src/KsGLWidget.hpp
index 629ae37..6a72a35 100644
--- a/src/KsGLWidget.hpp
+++ b/src/KsGLWidget.hpp
@@ -101,7 +101,7 @@ public:
 
 	void keyReleaseEvent(QKeyEvent *event);
 
-	void loadData(KsDataStore *data);
+	void loadData(KsDataStore *data, bool resetPlots);
 
 	void loadColors();
 
@@ -326,6 +326,8 @@ private:
 	int _getLastCPU(struct kshark_trace_histo *histo,
 			int bin, int sd, int pid);
 
+	void _defaultPlots(kshark_context *kshark_ctx);
+
 	void _deselect();
 
 	int _bin0Offset() {return _labelSize + 2 * _hMargin;}
diff --git a/src/KsMainWindow.cpp b/src/KsMainWindow.cpp
index fa893ce..da1c0af 100644
--- a/src/KsMainWindow.cpp
+++ b/src/KsMainWindow.cpp
@@ -1283,7 +1283,8 @@ void KsMainWindow::_load(const QString& fileName, bool append)
 	QApplication::processEvents();
 
 	_view.reset();
-	_graph.reset();
+	if (!append)
+		_graph.reset();
 
 	auto lamLoadJob = [&, this] () {
 		QVector<kshark_dpi *> v;
@@ -1333,7 +1334,10 @@ void KsMainWindow::_load(const QString& fileName, bool append)
 	_view.loadData(&_data);
 	pb.setValue(175);
 
-	_graph.loadData(&_data);
+	_graph.loadData(&_data, !append);
+	if (append)
+		_graph.cpuReDraw(sd, KsUtils::getCPUList(sd));
+
 	pb.setValue(195);
 }
 
@@ -1454,7 +1458,7 @@ void KsMainWindow::loadSession(const QString &fileName)
 	_view.loadData(&_data);
 	pb.setValue(155);
 
-	_graph.loadData(&_data);
+	_graph.loadData(&_data, true);
 	_filterSyncCBoxUpdate(kshark_ctx);
 	pb.setValue(175);
 
diff --git a/src/KsTraceGraph.cpp b/src/KsTraceGraph.cpp
index 1e976df..65e5a79 100644
--- a/src/KsTraceGraph.cpp
+++ b/src/KsTraceGraph.cpp
@@ -148,12 +148,16 @@ KsTraceGraph::KsTraceGraph(QWidget *parent)
  * @brief Load and show trace data.
  *
  * @param data: Input location for the KsDataStore object.
- *	  KsDataStore::loadDataFile() must be called first.
+ *	  	KsDataStore::loadDataFile() must be called first.
+ * @param resetPlots: If true, all existing graphs are closed
+ *		      and a default configuration of graphs is displayed
+ *		      (all CPU plots). If false, the current set of graphs
+ *		      is preserved.
  */
-void KsTraceGraph::loadData(KsDataStore *data)
+void KsTraceGraph::loadData(KsDataStore *data, bool resetPlots)
 {
 	_data = data;
-	_glWindow.loadData(data);
+	_glWindow.loadData(data, resetPlots);
 	updateGeom();
 }
 
diff --git a/src/KsTraceGraph.hpp b/src/KsTraceGraph.hpp
index 6e83f21..b1132e0 100644
--- a/src/KsTraceGraph.hpp
+++ b/src/KsTraceGraph.hpp
@@ -45,7 +45,7 @@ class KsTraceGraph : public KsWidgetsLib::KsDataWidget
 public:
 	explicit KsTraceGraph(QWidget *parent = nullptr);
 
-	void loadData(KsDataStore *data);
+	void loadData(KsDataStore *data, bool resetPlots);
 
 	void setMarkerSM(KsDualMarkerSM *m);
 
diff --git a/src/KsUtils.cpp b/src/KsUtils.cpp
index ec53267..757f49c 100644
--- a/src/KsUtils.cpp
+++ b/src/KsUtils.cpp
@@ -785,6 +785,7 @@ int KsDataStore::appendDataFile(const QString &file, int64_t offset)
 	_rows = mergedRows;
 
 	registerCPUCollections();
+	emit updateWidgets(this);
 
 	return sd;
 }
-- 
2.27.0

[PATCH v2 3/7] kernel-shark: Clear before loading new session

[Yordan Karadzhov (VMware)]

Make sure that all already loaded Data streams are properly closed,
before loading a new configuration from session description file.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/KsSession.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/KsSession.cpp b/src/KsSession.cpp
index 1e19a5f..b9edb3a 100644
--- a/src/KsSession.cpp
+++ b/src/KsSession.cpp
@@ -128,6 +128,7 @@ void KsSession::loadDataStreams(kshark_context *kshark_ctx,
 
 	data->unregisterCPUCollections();
 
+	kshark_close_all(kshark_ctx);
 	dataSize = kshark_import_all_dstreams(kshark_ctx,
 					      _config,
 					      data->rows_r());
-- 
2.27.0

[PATCH v2 4/7] kernel-shark: Better handling of plugins when appending data file

[Yordan Karadzhov (VMware)]

It may sound like a good idea to reinitialize all plugins for all
existing Data streams after a new stream is appended, but it isn't.
Such reset will re-initialize all Data containers open by the
plugins, hence it requires to do a complete reload of all previously
loaded data. Currently we reset the plugins but do not reload the
data, which is a bug. Because, reloading the data can be slow on
large data sets, I prefer to fix the bug, by eliminating the reload
of the plugins.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/KsUtils.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/KsUtils.cpp b/src/KsUtils.cpp
index 757f49c..3db8951 100644
--- a/src/KsUtils.cpp
+++ b/src/KsUtils.cpp
@@ -677,8 +677,7 @@ int KsDataStore::_openDataFile(kshark_context *kshark_ctx,
 
 	if (kshark_is_tep(kshark_ctx->stream[sd])) {
 		kshark_tep_init_all_buffers(kshark_ctx, sd);
-		for (int i = 0; i < kshark_ctx->n_streams; ++i)
-			kshark_tep_handle_plugins(kshark_ctx, i);
+		kshark_tep_handle_plugins(kshark_ctx, sd);
 	}
 
 	return sd;
-- 
2.27.0

[PATCH v2 5/7] kernel-shark: Do draw the combo point of the mark

[Yordan Karadzhov (VMware)]

The lines of the code that initialize and plot the combo point
are missing in the original commit.

Fixing: fc14e40 (kernel-shark: Add combo point to Mark)
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/KsPlotTools.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/KsPlotTools.cpp b/src/KsPlotTools.cpp
index abef5f8..1d63a9b 100644
--- a/src/KsPlotTools.cpp
+++ b/src/KsPlotTools.cpp
@@ -680,6 +680,8 @@ Mark::Mark()
 	_cpu._size = 5.5f;
 	_task._color = Color(0, 255, 0);
 	_task._size = 5.5f;
+	_combo._color = Color(100, 150, 255);
+	_combo._size = 5.5f;
 }
 
 void Mark::_draw(const Color &col, float size) const
@@ -691,6 +693,7 @@ void Mark::_draw(const Color &col, float size) const
 
 	_cpu.draw();
 	_task.draw();
+	_combo.draw();
 }
 
 /**
-- 
2.27.0

[PATCH v2 6/7] kernel-shark: Fix the checking if "trace_seq" was destroyed

[Yordan Karadzhov (VMware)]

When closing a "tep" data stream we destroy the "trace_seq" object.
However, trace_seq_destroy() sets the buffer to "TRACE_SEQ_POISON"
which is different from NULL.

Because TRACE_SEQ_POISON is an internal definition of libtraceevent,
we have to set the buffer to NULL in order to indicate that it was
destroyed.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/libkshark-tepdata.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index bc5babb..acc554b 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -1615,8 +1615,10 @@ int kshark_tep_close_interface(struct kshark_data_stream *stream)
 	if (!tep_handle)
 		return -EFAULT;
 
-	if (seq.buffer)
+	if (seq.buffer) {
 		trace_seq_destroy(&seq);
+		seq.buffer = NULL;
+	}
 
 	if (tep_handle->advanced_event_filter) {
 		tep_filter_reset(tep_handle->advanced_event_filter);
-- 
2.27.0

[PATCH v2 7/7] kernel-shark: No slash at the end of KS_PLUGIN_INSTALL_PREFIX

[Yordan Karadzhov (VMware)]

This trailing slash is redundant.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 94023a8..cef6f05 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -111,7 +111,7 @@ if (NOT CMAKE_CXX_FLAGS_PACKAGE)
     set(CMAKE_CXX_FLAGS_PACKAGE "-O3")
 endif (NOT CMAKE_CXX_FLAGS_PACKAGE)
 
-set(KS_PLUGIN_INSTALL_PREFIX ${_LIBDIR}/${KS_APP_NAME}/plugins/)
+set(KS_PLUGIN_INSTALL_PREFIX ${_LIBDIR}/${KS_APP_NAME}/plugins)
 
 set(KS_ICON        KS_icon_shark.svg)
 set(KS_ICON_FIN    KS_icon_fin.svg)
-- 
2.27.0

Re: [PATCH v2 0/7] Final fixes before KS 2.0

[Steven Rostedt]

On Mon, 17 May 2021 17:21:33 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> v2 changes:
>  - Still showing all CPU plots from the new trace file when
>    appending [PATCH kernel-shark: Preserve open graphs when
>    appending data].
>  - Setting "seq.buffer" to NULL after calling trace_seq_destroy()
>    in [PATCH kernel-shark: Fix the checking if "trace_seq" was destroyed]
>  - [PATCH kernel-shark: No slash at the end of KS_PLUGIN_INSTALL_PREFIX]
>    is new.

Hi Yordan,

I was playing a bit with kernelshark, and found that if I load a file and
append one, exit, load them again, then click:

  File -> Sessions -> Restore Last Session

It crashes.

Looks to be something is freed and then reused, because when I ran it under
gdb, it crashed in allocation of memory (asprintf). That usually means that
something was freed twice, someplace else. Or freed and then used.

-- Steve

Re: [PATCH v2 0/7] Final fixes before KS 2.0

[Steven Rostedt]

On Mon, 17 May 2021 19:21:04 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 17 May 2021 17:21:33 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
> > v2 changes:
> >  - Still showing all CPU plots from the new trace file when
> >    appending [PATCH kernel-shark: Preserve open graphs when
> >    appending data].
> >  - Setting "seq.buffer" to NULL after calling trace_seq_destroy()
> >    in [PATCH kernel-shark: Fix the checking if "trace_seq" was destroyed]
> >  - [PATCH kernel-shark: No slash at the end of KS_PLUGIN_INSTALL_PREFIX]
> >    is new.  
> 
> Hi Yordan,
> 
> I was playing a bit with kernelshark, and found that if I load a file and
> append one, exit, load them again, then click:
> 
>   File -> Sessions -> Restore Last Session
> 
> It crashes.
> 
> Looks to be something is freed and then reused, because when I ran it under
> gdb, it crashed in allocation of memory (asprintf). That usually means that
> something was freed twice, someplace else. Or freed and then used.
> 

Running valgrind, reported this:

==6862== Invalid read of size 8
==6862==    at 0x494CA89: map_collection_back_request (libkshark-collection.c:474)

static int
map_collection_back_request(const struct kshark_entry_collection *col,
			    struct kshark_entry_request *req)
{
	size_t req_first, req_end;
	ssize_t col_index;
	int req_count;

	col_index = map_collection_request_init(col, req, false, &req_end);
	if (col_index == KS_EMPTY_BIN)
		return 0;

	/*
	 * Now loop over the intervals of the collection going backwards till
	 * the end of the inputted request and create a separate request for
	 * each of those interest.
	 */
	req_count = 1;
	while (col_index >= 0 && req_end <= col->break_points[col_index]) {

// col_index can be zero entering this loop.

		if (req_end >= col->resume_points[col_index]) {
			/*
			 * The last entry of the original request is inside
			 * the "col_index" collection interval. Close the
			 * collection request here and return.
			 */
			req->n = req->first - req_end + 1;
			break;
		}

		/*
		 * The last entry of the original request is outside of the
		 * "col_index" interval. Close the collection request at the
		 * end of this interval and move to the next one. Try to make
		 * another request there.
		 */
		req->n = req->first -
			 col->resume_points[col_index] + 1;

		--col_index;

// col_index is decremented (-1)

		if (req_end > col->break_points[col_index]) {

Reading a negative index in an array.

Which is where valgrind reported. But I don't think this is what caused the
crash.

-- Steve

Re: [PATCH v2 0/7] Final fixes before KS 2.0

[Yordan Karadzhov]

On 18.05.21 г. 2:21, Steven Rostedt wrote:
> On Mon, 17 May 2021 17:21:33 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> v2 changes:
>>   - Still showing all CPU plots from the new trace file when
>>     appending [PATCH kernel-shark: Preserve open graphs when
>>     appending data].
>>   - Setting "seq.buffer" to NULL after calling trace_seq_destroy()
>>     in [PATCH kernel-shark: Fix the checking if "trace_seq" was destroyed]
>>   - [PATCH kernel-shark: No slash at the end of KS_PLUGIN_INSTALL_PREFIX]
>>     is new.
> 
> Hi Yordan,
> 
> I was playing a bit with kernelshark, and found that if I load a file and
> append one, exit, load them again, then click:
> 
>    File -> Sessions -> Restore Last Session
> 
> It crashes.

Unfortunately I am not able to reproduce the crash. maybe it has 
something to do with the particular data files you use.

> 
> Looks to be something is freed and then reused, because when I ran it under
> gdb, it crashed in allocation of memory (asprintf). That usually means that
> something was freed twice, someplace else. Or freed and then used.

Is it possible to send me a backtrace of the stack?

Thanks!
Yordan

> 
> -- Steve
> 

Re: [PATCH v2 0/7] Final fixes before KS 2.0

[Steven Rostedt]

[-- Attachment #1: Type: text/plain, Size: 7312 bytes --]

On Tue, 18 May 2021 10:30:42 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> On 18.05.21 г. 2:21, Steven Rostedt wrote:
> > On Mon, 17 May 2021 17:21:33 +0300
> > "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> >   
> >> v2 changes:
> >>   - Still showing all CPU plots from the new trace file when
> >>     appending [PATCH kernel-shark: Preserve open graphs when
> >>     appending data].
> >>   - Setting "seq.buffer" to NULL after calling trace_seq_destroy()
> >>     in [PATCH kernel-shark: Fix the checking if "trace_seq" was destroyed]
> >>   - [PATCH kernel-shark: No slash at the end of KS_PLUGIN_INSTALL_PREFIX]
> >>     is new.  
> > 
> > Hi Yordan,
> > 
> > I was playing a bit with kernelshark, and found that if I load a file and
> > append one, exit, load them again, then click:
> > 
> >    File -> Sessions -> Restore Last Session
> > 
> > It crashes.  
> 
> Unfortunately I am not able to reproduce the crash. maybe it has 
> something to do with the particular data files you use.

BTW, sometimes I need to do it twice. That is, I hit "Restore Last Session"
twice.

As this is a memory corruption issue, it will behave differently on
different machines. Also, I do get the message:

  "Usage of trace_seq after it was destroyed"

> 
> > 
> > Looks to be something is freed and then reused, because when I ran it under
> > gdb, it crashed in allocation of memory (asprintf). That usually means that
> > something was freed twice, someplace else. Or freed and then used.  
> 
> Is it possible to send me a backtrace of the stack?

Here's the backtrace from gdb:

(gdb) bt
#0  0x00007ffff63cb02a in __strlen_sse2 () from /lib64/libc.so.6
#1  0x00007ffff63994f8 in __vfprintf_internal () from /lib64/libc.so.6
#2  0x00007ffff63aa015 in __vasprintf_internal () from /lib64/libc.so.6
#3  0x00007ffff63844fa in asprintf () from /lib64/libc.so.6
#4  0x00007ffff7ec88f9 in tepdata_get_latency (entry=<optimized out>, 
    stream=0x7fff70ff1320)
    at /work/git-local/kernel-shark.git/src/libkshark-tepdata.c:805
#5  tepdata_get_latency (stream=0x7fff70ff1320, entry=<optimized out>)
    at /work/git-local/kernel-shark.git/src/libkshark-tepdata.c:776
#6  0x00007ffff7f59a40 in KsViewModel::getValueStr (this=0x7fffffffc6e8, 
    column=<optimized out>, row=0)
    at /work/git-local/kernel-shark.git/src/KsModels.cpp:358
#7  0x00007ffff7f59aa7 in KsViewModel::getValue (this=<optimized out>, 
    column=<optimized out>, row=<optimized out>)
    at /work/git-local/kernel-shark.git/src/KsModels.cpp:377
#8  0x00007ffff7f59b37 in KsViewModel::data (this=<optimized out>, index=..., 
    role=<optimized out>) at /work/git-local/kernel-shark.git/src/KsModels.cpp:312
#9  0x00007ffff6ac2139 in QSortFilterProxyModel::data(QModelIndex const&, int) const ()
   from /lib64/libQt5Core.so.5
#10 0x00007ffff799a185 in QStyledItemDelegate::initStyleOption(QStyleOptionViewItem*, QModelIndex const&) const () from /lib64/libQt5Widgets.so.5
#11 0x00007ffff79997ea in QStyledItemDelegate::sizeHint(QStyleOptionViewItem const&, QModelIndex const&) const () from /lib64/libQt5Widgets.so.5
#12 0x00007ffff79c72ef in QTableViewPrivate::widthHintForIndex(QModelIndex const&, int, QStyleOptionViewItem const&) const () from /lib64/libQt5Widgets.so.5
#13 0x00007ffff79c7510 in QTableView::sizeHintForColumn(int) const ()
   from /lib64/libQt5Widgets.so.5
--Type <RET> for more, q to quit, c to continue without paging--
#14 0x00007ffff7985142 in QHeaderViewPrivate::resizeSections(QHeaderView::ResizeMode, bool) () from /lib64/libQt5Widgets.so.5
#15 0x00007ffff7f80523 in KsTraceViewer::_resizeToContents (this=0x7fffffffc660)
    at /work/git-local/kernel-shark.git/src/KsTraceViewer.cpp:573
#16 0x00007ffff7f80eb3 in KsTraceViewer::loadData (this=this@entry=0x7fffffffc660, data=
    0x7fffffffb508, data@entry=0x7fffffffc640)
    at /work/git-local/kernel-shark.git/src/KsTraceViewer.cpp:163
#17 0x00007ffff7f8ee6a in KsMainWindow::loadSession (this=0x7fffffffc5d0, fileName=...)
    at /work/git-local/kernel-shark.git/src/KsMainWindow.cpp:1434
#18 0x00007ffff7f8f165 in KsMainWindow::_restoreSession (this=0x7fffffffc5d0)
    at /work/git-local/kernel-shark.git/src/KsMainWindow.cpp:668
#19 0x00007ffff6b14386 in void doActivate<false>(QObject*, int, void**) ()
   from /lib64/libQt5Core.so.5
#20 0x00007ffff770a646 in QAction::triggered(bool) () from /lib64/libQt5Widgets.so.5
#21 0x00007ffff770cf31 in QAction::activate(QAction::ActionEvent) ()
   from /lib64/libQt5Widgets.so.5
#22 0x00007ffff788be9a in QMenuPrivate::activateCausedStack(QVector<QPointer<QWidget> > const&, QAction*, QAction::ActionEvent, bool) () from /lib64/libQt5Widgets.so.5
#23 0x00007ffff7893512 in QMenuPrivate::activateAction(QAction*, QAction::ActionEvent, bool) () from /lib64/libQt5Widgets.so.5
#24 0x00007ffff7751b1e in QWidget::event(QEvent*) () from /lib64/libQt5Widgets.so.5
#25 0x00007ffff7710ec3 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
   from /lib64/libQt5Widgets.so.5
#26 0x00007ffff7717eeb in QApplication::notify(QObject*, QEvent*) ()
   from /lib64/libQt5Widgets.so.5
#27 0x00007ffff6ae4bd8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) ()
   from /lib64/libQt5Core.so.5
--Type <RET> for more, q to quit, c to continue without paging--
#28 0x00007ffff7716efa in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool, bool) ()
   from /lib64/libQt5Widgets.so.5
#29 0x00007ffff776a8e3 in QWidgetWindow::handleMouseEvent(QMouseEvent*) ()
   from /lib64/libQt5Widgets.so.5
#30 0x00007ffff776d6be in QWidgetWindow::event(QEvent*) ()
   from /lib64/libQt5Widgets.so.5
#31 0x00007ffff7710ec3 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
   from /lib64/libQt5Widgets.so.5
#32 0x00007ffff6ae4bd8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) ()
   from /lib64/libQt5Core.so.5
#33 0x00007ffff70b7143 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) () from /lib64/libQt5Gui.so.5
#34 0x00007ffff70988cc in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /lib64/libQt5Gui.so.5
#35 0x00007fffe5c5747e in xcbSourceDispatch(_GSource*, int (*)(void*), void*) ()
   from /lib64/libQt5XcbQpa.so.5
#36 0x00007ffff5450a9f in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
#37 0x00007ffff54a2a98 in g_main_context_iterate.constprop ()
   from /lib64/libglib-2.0.so.0
#38 0x00007ffff544de73 in g_main_context_iteration () from /lib64/libglib-2.0.so.0
#39 0x00007ffff6b316f3 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /lib64/libQt5Core.so.5
#40 0x00007ffff6ae357b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /lib64/libQt5Core.so.5
#41 0x00007ffff6aeb1b4 in QCoreApplication::exec() () from /lib64/libQt5Core.so.5
#42 0x0000000000402aa1 in main (argc=<optimized out>, argv=<optimized out>)
--Type <RET> for more, q to quit, c to continue without paging--
    at /work/git-local/kernel-shark.git/src/kernelshark.cpp:154

You may want to play with valgrind some more.

Attached is my .cache/kernelshark/ content.

-- Steve

[-- Attachment #2: kshark-cache.tar --]
[-- Type: application/x-tar, Size: 10240 bytes --]

Re: [PATCH v2 0/7] Final fixes before KS 2.0

[Yordan Karadzhov]

On 18.05.21 г. 15:46, Steven Rostedt wrote:
>> Unfortunately I am not able to reproduce the crash. maybe it has
>> something to do with the particular data files you use.
> BTW, sometimes I need to do it twice. That is, I hit "Restore Last Session"
> twice.
> 

I restored the session 20 times in a row without having a crash.


> As this is a memory corruption issue, it will behave differently on
> different machines. Also, I do get the message:
> 
>    "Usage of trace_seq after it was destroyed"
> 
>>> Looks to be something is freed and then reused, because when I ran it under
>>> gdb, it crashed in allocation of memory (asprintf). That usually means that
>>> something was freed twice, someplace else. Or freed and then used.
>> Is it possible to send me a backtrace of the stack?
> Here's the backtrace from gdb:
> 
> (gdb) bt
> #0  0x00007ffff63cb02a in __strlen_sse2 () from /lib64/libc.so.6
> #1  0x00007ffff63994f8 in __vfprintf_internal () from /lib64/libc.so.6
> #2  0x00007ffff63aa015 in __vasprintf_internal () from /lib64/libc.so.6
> #3  0x00007ffff63844fa in asprintf () from /lib64/libc.so.6
> #4  0x00007ffff7ec88f9 in tepdata_get_latency (entry=<optimized out>,

This backtrace is very suspicious. It is exactly the same crash I had 
before applying [PATCH v2 6/7] kernel-shark: Fix the checking if 
"trace_seq" was destroyed.

I guess because of some reason it still fails to detect that the 
trace_seq was destroyed and needs to be initialized again.

Thanks!
Yordan

Re: [PATCH v2 0/7] Final fixes before KS 2.0

[Steven Rostedt]

On Tue, 18 May 2021 15:58:55 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> On 18.05.21 г. 15:46, Steven Rostedt wrote:
> >> Unfortunately I am not able to reproduce the crash. maybe it has
> >> something to do with the particular data files you use.  
> > BTW, sometimes I need to do it twice. That is, I hit "Restore Last Session"
> > twice.
> >   
> 
> I restored the session 20 times in a row without having a crash.

Nevermind. I just realized that I was in the process of rebasing your
patches and testing each one when I noticed this bug.

After applying the rest of the patches, I'm not able to reproduce it. I
thought I was done with the rebase (doing too many things in parallel
isn't the best thing :-p).

OK, I think this is good to go (although the col_index looks like it can
still be referenced as a negative).

Sorry for the goose chase on the crash :-/

-- Steve