[Bug remote/25111] [Zephyr/thread aware debugging] remote: write_ptid returns negative tid.

[Bug remote/25111] [Zephyr/thread aware debugging] remote: write_ptid returns negative tid.

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25111

Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fredrik.hederstierna@securi
                   |                            |tas-direct.com

--- Comment #1 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
When looking into Zephyr and OpenOCD I just saw this ticket. Isn't there a
discrepancy in <gdb/remote.c> for ptid_t write/read when it comes to types. The
'lwp' is used as 'tid', but has a wider type:

  int   ptid_get_pid (ptid_t ptid)
  long  ptid_get_lwp (ptid_t ptid)
  long  ptid_get_tid (ptid_t ptid)

so in the <read_ptid> it reads into ULONGEST, which I guess is uint64_t, but
the <write_tpid> instead implicit cast long to int when using 'lwp as 'tid'
(which I believe is often int32_t on a normal host target machine):

  int tid;
  tid = ptid.lwp ();

Wouldn't it be better to keep the wider long type, and keep calling 'tid' for
'lwp' which it actually is? And also keeping the long-type in the write
function? In this case there won't be any overflow if an 32bit int representing
'tid' is becoming >0x80000000. The read function I guess will still handle it
as it is using ULONGEST to my understanding.

Attaching idea of a possible patch.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/25111] [Zephyr/thread aware debugging] remote: write_ptid returns negative tid.

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25111

--- Comment #2 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
Created attachment 13765
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13765&action=edit
write_ptid_long_type.patch

Extends type of 'tid' when writing ptid.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/25111] [Zephyr/thread aware debugging] remote: write_ptid returns negative tid.

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25111

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simark at simark dot ca

--- Comment #3 from Simon Marchi <simark at simark dot ca> ---
At first glance, it seems you're right.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/25111] [Zephyr/thread aware debugging] remote: write_ptid returns negative tid.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25111

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |obilaniu at yahoo dot com

--- Comment #4 from Tom Tromey <tromey at sourceware dot org> ---
*** Bug 14314 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/25111] [Zephyr/thread aware debugging] remote: write_ptid returns negative tid.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25111

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |tromey at sourceware dot org
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-01-07

--- Comment #5 from Tom Tromey <tromey at sourceware dot org> ---
Can you send the patch to gdb-patches?
See the contribution checklist for all the details:
https://sourceware.org/gdb/wiki/ContributionChecklist

I think this is a longstanding bug, see the dup I just closed...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/25111] [Zephyr/thread aware debugging] remote: write_ptid returns negative tid.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25111

--- Comment #6 from Tom Tromey <tromey at sourceware dot org> ---
I guess that never happened, the bug is still there.
It seems weird to me that we used signed types in ptid_t.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/25111] [Zephyr/thread aware debugging] remote: write_ptid returns negative tid.

[fredrik.hederstierna@securitas-direct.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25111

--- Comment #7 from Fredrik Hederstierna <fredrik.hederstierna@securitas-direct.com> ---
Unfortunately I am currently not using Zephyr myself, so I would probably need
some help to get a better formatting and testing the patch. Anyone is welcome
to assist in improving the packaging/testing the patch, if interested in
getting this patch merged. Unfortunately I haven't any good Zephyr test setup
myself.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/25111] [Zephyr/thread aware debugging] remote: write_ptid returns negative tid.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25111

--- Comment #8 from Tom Tromey <tromey at sourceware dot org> ---
I looked a little.

Changing ptid_t::pid_type to be unsigned is probably a colossal
change, bordering on the impossible.

However, it does seem that negative values aren't really intended --
except for the special case of -1.  The manual says:

The PID (process) and TID
(thread) components each have the format described above: a positive
number with target-specific interpretation formatted as a big-endian hex
string, literal ‘-1’ to indicate all processes or threads
(respectively), or ‘0’ to indicate an arbitrary process or thread.

So in remote_target::write_ptid, I wonder if it's sufficient to
change the "pid < 0" check to "pid == -1" (and likewise for tid).

-- 
You are receiving this mail because:
You are on the CC list for the bug.