Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 325552

Summary: GDB 7.x getOsId() pattern match too restrictive (DSF)
Product: [Tools] CDT Reporter: John Dallaway <john>
Component: cdt-debug-dsf-gdbAssignee: Marc Khouzam <marc.khouzam>
Status: RESOLVED FIXED QA Contact: Marc Khouzam <marc.khouzam>
Severity: normal    
Priority: P3 CC: john.cortell, pawel.1.piech
Version: 7.0Flags: john.cortell: review+
Target Milestone: 7.0.2   
Hardware: PC   
OS: All   
Whiteboard:
Attachments:
Description Flags
MIThread-100917.patch
marc.khouzam: iplog+
Updated fix to make case-insensitive marc.khouzam: iplog-

Description John Dallaway CLA 2010-09-17 04:35:30 EDT 
Build Identifier: Eclipse 3.6.0 I20100608-0911 (with cdt_7_0 branch and CDT HEAD)

When debugging on remote hardware with an RTOS, the OS-specific thread ID returned by GDB can have a variety of formats. When debugging with GDB 6.x, CLIInfoThreadsInfo#parseThreadInfo() accommodates any alphanumeric character sequence as the thread ID. When debugging with GDB 7.x, the pattern matching in MIThread#parseOsId() is too restrictive so the OS-specific thread ID may not be seen in the Debug view.

Reproducible: Always
Comment 1 John Dallaway CLA 2010-09-17 04:41:16 EDT 
Created attachment 179091 [details]
MIThread-100917.patch

This patch addresses the reported issue by adding an extra pattern match which catches any alphanumeric OS-specific thread ID which does not match the existing patterns. It should be usable for both CDT HEAD and the cdt_7_0 branch.
Comment 2 Marc Khouzam CLA 2010-12-16 14:49:10 EST 
Created attachment 185359 [details]
Updated fix to make case-insensitive

Same patch as John's but I made it case insensitive in case Windows needs it.

Committed to HEAD.
Comment 3 Marc Khouzam CLA 2010-12-16 14:59:18 EST 
Committed to the 7_0 branch

John D can you confirm it works as you expect?

John C can you review?
Comment 6 John Dallaway CLA 2010-12-17 11:54:03 EST 
(In reply to comment #3)

> Committed to the 7_0 branch
> 
> John D can you confirm it works as you expect?

The final patch applied to the cdt_7_0 branch works well for me. Thank you.
Comment 7 John Cortell CLA 2011-01-20 17:05:47 EST 
The fix looks good to me, but these sort of utility methods really should have unit tests to protect against regressions as we add support for new ID formats. I've added MIThreadTests.java.

I do have one question, though. Shouldn't we be updating parseParentId(), too? At least for the possibility of "Thread" being in a different case. Also, is it not possible those IDs will be in the same format as the OS ones?
Comment 9 John Dallaway CLA 2011-01-21 04:13:41 EST 
(In reply to comment #7)

John, thank you for reviewing the patch.

> I do have one question, though. Shouldn't we be updating parseParentId(), too?
> At least for the possibility of "Thread" being in a different case. Also, is it
> not possible those IDs will be in the same format as the OS ones?

parseParentID() parses the same OS ID strings as parseOsId() but extracts the parent thread ID from those strings in cases where the parent thread ID is prefixed to the thread ID. It already accommodates lowercase "thread". I believe that the current implementation is OK.
Comment 10 John Cortell CLA 2011-01-21 11:10:16 EST 
(In reply to comment #9)
> parseParentID() parses the same OS ID strings as parseOsId() but extracts the
> parent thread ID from those strings in cases where the parent thread ID is
> prefixed to the thread ID. It already accommodates lowercase "thread". I
> believe that the current implementation is OK.

Ah, right. I've added documentation to the method to make it a little more obvious.
Comment 11 Marc Khouzam CLA 2011-01-21 11:16:14 EST 
Thanks John, and thanks for the tests.