Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added parsing support for the QNX platform Coredump's thread ids format and fixed VSCODE .devcontainer compilation failure #1398

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Added parsing support for the QNX platform Coredump's thread ids format and fixed VSCODE .devcontainer compilation failure #1398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8eda10f
Add parsing for QNX coredumps thread's id format
khvysofq May 3, 2023
f2a41f1
fix: fix docker version that build in vscode devcontainer
khvysofq May 3, 2023
41ecdf4
improv: the thread iss should be start at 0 in the default scenario
khvysofq May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
"ms-vscode.powershell",
"jmrog.vscode-nuget-package-manager"
],
"postCreateCommand": "",
"build": {
"dockerfile": "Dockerfile"
}
"image":"mcr.microsoft.com/vscode/devcontainers/dotnet:6.0-focal",
"remoteUser": "root"
// "postCreateCommand": "",
// "build": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please just delete the commented out code

// "dockerfile": "Dockerfile"
// }
}

// Built with ❤ by [Pipeline Foundation](https://pipeline.foundation)
18 changes: 14 additions & 4 deletions src/MIDebugEngine/Engine.Impl/DebuggedThread.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ internal class ThreadCache
private List<DebuggedThread> _deadThreads;
private List<DebuggedThread> _newThreads;
private Dictionary<string, List<int>> _threadGroups;
private static uint s_targetId = uint.MaxValue;
private static uint s_targetId = 0; // Thread ids should be start at 0 in the default scenario
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you talk about why you changed this part? Unfortunately there are no comments to say for sure, but I think the idea of starting with uint.MaxValue was to select something that was highly unlikely to clash with actual thread ids in the case that the thread id couldn't be parsed.

This code was added in commit 12632f3280da94787c009c8a97accbe797ac6994, but the author didn't add any more details, and I am sure the author no longer remembers.

private const string c_defaultGroupId = "i1"; // gdb's default group id, also used for any process without group ids

private List<DebuggedThread> DeadThreads
Expand Down Expand Up @@ -212,7 +212,7 @@ internal async Task ThreadCreatedEvent(int id, string groupId)
Results results = await _debugger.MICommandFactory.ThreadInfo(tid);
if (results.ResultClass != ResultClass.done)
{
// This can happen on some versions of gdb where thread-info is not supported while running, so only assert if we're also not running.
// This can happen on some versions of gdb where thread-info is not supported while running, so only assert if we're also not running.
if (this._debugger.ProcessState != ProcessState.Running)
{
Debug.Fail("Thread info not successful");
Expand Down Expand Up @@ -368,9 +368,19 @@ private bool TryGetTidFromTargetId(string targetId, out uint tid)
// In gdb coredumps the thread name is in the form:" LWP <thread-id>"
return true;
}
else if (targetId.StartsWith("pid ", StringComparison.OrdinalIgnoreCase))
{
// In QNX gdb coredumps the thread name is in the form:"pid <pid> tid <pid>", eg "pid 4194373 tid 308"
int tid_pos = targetId.IndexOf("tid ", StringComparison.Ordinal);
int len = targetId.Length - (tid_pos + 4);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to put the rest of this block in if (tid_pos >= 0)

if (len > 0 && System.UInt32.TryParse(targetId.Substring(tid_pos + 4, len), out tid) && tid != 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Use a constant for tid and use '<const_name>.Length` instead of '4' so it is obvious what the '4' means.

Example:

const string tidPrefix = "tid ";
int tid_pos = targetId.IndexOf(tidPrefix, ...)
int len = targetId.Lenth - (tid_pos + tidPrefix.Length);
if (... && UInt32.TryParse(targetId.Substring(tid_pos + tidPrefix.Length...)

{
return true;
}
}
else
{
tid = --s_targetId;
tid = ++s_targetId;
return true;
}

Expand Down Expand Up @@ -417,7 +427,7 @@ private async Task<ThreadContext> CollectThreadsInfo(int cxtThreadId)
{
var tlist = threadsinfo.Find<ValueListValue>("threads");

// update our thread list
// update our thread list
lock (_threadList)
{
foreach (var thread in _threadList)
Expand Down