Bug 254229 - gcore produces incorrect elf_prpsinfo note data
gcore produces incorrect elf_prpsinfo note data
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: gdb (Show other bugs)
9
All Linux
low Severity low
: ---
: ---
Assigned To: Jan Kratochvil
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-24 16:53 EDT by Phil Muldoon
Modified: 2008-08-01 03:10 EDT (History)
2 users (show)

See Also:
Fixed In Version: gdb-6.8-18.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-08-01 03:10:24 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Fills NT_PRPSINFO on Linux by parsing /proc/$PID/stat (5.55 KB, patch)
2008-07-23 11:10 EDT, Denys Vlasenko
no flags Details | Diff
Updated patch against Fedora 9 source (10.07 KB, patch)
2008-07-24 10:36 EDT, Denys Vlasenko
no flags Details | Diff
Silence gcc warning about probably uninitialized data in dbxread.c (463 bytes, patch)
2008-07-25 11:05 EDT, Denys Vlasenko
no flags Details | Diff
Updated patch against Fedora 9 source, v2 (11.06 KB, patch)
2008-07-25 11:09 EDT, Denys Vlasenko
no flags Details | Diff
Rawhide (=Fedora 9) patch, v3. (11.01 KB, patch)
2008-07-28 17:31 EDT, Jan Kratochvil
no flags Details | Diff
Updated patch according to Jan's comments. (11.54 KB, patch)
2008-07-29 07:41 EDT, Denys Vlasenko
no flags Details | Diff
Updated patch according to Jan's comments, v2 (11.55 KB, patch)
2008-07-29 07:59 EDT, Denys Vlasenko
no flags Details | Diff

  None (edit)
Description Phil Muldoon 2007-08-24 16:53:00 EDT
Description of problem:

gcore writes incorrect elf_prpsinfo data.

[pmuldoon@localhost bindir]$ ps
  PID TTY          TIME CMD
 3266 pts/0    00:00:01 bash
22496 pts/0    00:00:00 ps

[pmuldoon@localhost bindir]$ gcore 3266
Using host libthread_db library "/lib/libthread_db.so.1".
0x00110402 in __kernel_vsyscall ()
Saved corefile core.3266

[pmuldoon@localhost bindir]$ eu-readelf -n core.3266 

Note segment of 1132 bytes at offset 0x374:
  Owner          Data size  Type
  CORE                 124  PRPSINFO
    state:  (0),  zombie: 0,  nice: 0
    flags: 00000000,  uid: 0,  gid: 0
    pid: 0,  ppid: 0,  pgrp: 0,  sid: 0
    fname: bash
     args: /bin/bash 


Version-Release number of selected component (if applicable):

[pmuldoon@localhost bindir]$ rpm -q gdb
gdb-6.6-15.fc7


How reproducible:

Reproduced on many process.

Steps to Reproduce:
1. gcore <pid>
2. eu-readelf -n core.pid
3. Check incorrect elf_prpsinfo values
  
Actual results:


Expected results:


Additional info:
Comment 1 Jan Kratochvil 2007-08-24 16:57:10 EDT
Bug present even in CVS HEAD upstream.
Comment 2 Bug Zapper 2008-05-14 10:06:15 EDT
This message is a reminder that Fedora 7 is nearing the end of life. Approximately 30 (thirty) days from now Fedora will stop maintaining and issuing updates for Fedora 7. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as WONTFIX if it remains open with a Fedora 'version' of '7'.

Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version prior to Fedora 7's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that we may not be able to fix it before Fedora 7 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora please change the 'version' of this bug. If you are unable to change the version, please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. If possible, it is recommended that you try the newest available Fedora distribution to see if your bug still exists.

Please read the Release Notes for the newest Fedora distribution to make sure it will meet your needs:
http://docs.fedoraproject.org/release-notes/

The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 3 Bug Zapper 2008-06-16 22:13:59 EDT
Fedora 7 changed to end-of-life (EOL) status on June 13, 2008. 
Fedora 7 is no longer maintained, which means that it will not 
receive any further security or bug fix updates. As a result we 
are closing this bug. 

If you can reproduce this bug against a currently maintained version 
of Fedora please feel free to reopen this bug against that version.

Thank you for reporting this bug and we are sorry it could not be fixed.
Comment 4 Denys Vlasenko 2008-07-23 11:10:16 EDT
Created attachment 312485 [details]
Fills NT_PRPSINFO on Linux by parsing /proc/$PID/stat
Comment 5 Denys Vlasenko 2008-07-23 11:10:51 EDT
The above patch is against gdb-6.8
Comment 6 Jan Kratochvil 2008-07-24 03:04:21 EDT
(In reply to comment #5)
> The above patch is against gdb-6.8

Please update the patch for F9 GDB as the patch currently does not support
cross-arch gcore.  Try attaching to a 32-bit process on x86_64 and gcore it - it
creates 32-bit core file from 64-bit GDB.  Thanks.
Comment 7 Denys Vlasenko 2008-07-24 10:36:53 EDT
Created attachment 312566 [details]
Updated patch against Fedora 9 source

Taking coredump of 64-bit process on x86_64:

# ./gcore.sleep
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
[Thread debugging using libthread_db enabled]
[New Thread 0x7f837a29e6f0 (LWP 25786)]
(no debugging symbols found)
(no debugging symbols found)
0x0000003d4d6a6220 in __nanosleep_nocancel () from /lib64/libc.so.6
Saved corefile core.gcore.sleep.25786

# eu-readelf -n core.gcore.sleep.25786

Note section [ 1] 'note0' of 1336 bytes at offset 0x580:
  Owner 	 Data size  Type
  CORE		       136  PRPSINFO
    state: 0, sname: T, zomb: 0, nice: 0, flag: 0x0000000000000000
    uid: 0, gid: 0, pid: 25786, ppid: 25785, pgrp: 16965, sid: 16187
    fname: sleep, psargs: /bin/sleep
  CORE		       336  PRSTATUS
...

Taking coredump on 32-bit process on x86_64:

# ./gcore.sleep32
(no debugging symbols found)
(no debugging symbols found)
0x080cfd94 in ?? ()
Saved corefile core.gcore.sleep32.25797

# eu-readelf -n core.gcore.sleep32.25797

Note section [ 1] 'note0' of 1148 bytes at offset 0xf4:
  Owner 	 Data size  Type
  CORE		       124  PRPSINFO
    state: 0, sname: T, zomb: 0, nice: 0, flag: 0x00000000
    uid: 0, gid: 0, pid: 25797, ppid: 25796, pgrp: 16965, sid: 16187
    fname: sleep, psargs: /root/srcdevel/gdb/fix/tmp/sleep
  CORE		       144  PRSTATUS
...
Comment 8 Jan Kratochvil 2008-07-25 04:48:32 EDT
IMO elfcore_write_prpsinfo() should not touch the INFO content (strncpys below)
if it is already provided as non-NULL (strncpys should be in its caller).
You have unrelated dbxread.c patch there.
IMO HAVE_PRPSINFO_T in linux-nat.c is over-engineered and should be removed.

Anyway it can go to Rawhide now (I will commit it so far).  Feel free to push it
upstream (sure the bfd/ part should go to the binutils list).
Comment 9 Denys Vlasenko 2008-07-25 11:05:00 EDT
Created attachment 312652 [details]
Silence gcc warning about probably uninitialized data in dbxread.c
Comment 10 Denys Vlasenko 2008-07-25 11:08:10 EDT
Respectfully disagree on HAVE_PRPSINFO_T. Having code to sit in #ifdef
HAVE_PRPSINFO_T / #endif clearly shows the reader that this code _must_ have
prpsinfo_t available.

Fully agree with elfcore_write_prpsinfo comment. This way I can actually improve
info generation to show complete command line.
Comment 11 Denys Vlasenko 2008-07-25 11:09:55 EDT
Created attachment 312653 [details]
Updated patch against Fedora 9 source, v2

Again dumping 64-bit and 32-bit processes. Note better command line reporting.

# eu-readelf -n core.gcore.sleep.12134

Note section [ 1] 'note0' of 1336 bytes at offset 0x580:
  Owner 	 Data size  Type
  CORE		       136  PRPSINFO
    state: 0, sname: T, zomb: 0, nice: 0, flag: 0x0000000000000000
    uid: 0, gid: 0, pid: 12134, ppid: 12133, pgrp: 7736, sid: 7714
    fname: sleep, psargs: sleep 2
  CORE		       336  PRSTATUS
...

# eu-readelf -n core.gcore.sleep32.12145

Note section [ 1] 'note0' of 1148 bytes at offset 0xf4:
  Owner 	 Data size  Type
  CORE		       124  PRPSINFO
    state: 0, sname: T, zomb: 0, nice: 0, flag: 0x00000000
    uid: 1, gid: 2, pid: 12145, ppid: 12144, pgrp: 7736, sid: 7714
    fname: sleep, psargs: ./sleep 2
  CORE		       144  PRSTATUS
...
Comment 12 Jan Kratochvil 2008-07-28 17:31:44 EDT
Created attachment 312823 [details]
Rawhide (=Fedora 9) patch, v3.

Did some update I would like there.
Unfortunately I have some problems with RHTS (partially with YUM) now so it
still has not been verifified as regression-free.

Always use xfree(), not free().
GNU indentation: func() -> func ().
Always use NULL-protected allocations: xmalloc/xzalloc/...
Empty line between declarations and code.
/* Two spaces after each sentence dot.	Like here.  */
`return val;' (with any cleanups) is preceded by an empty line.
(Not fixed.) GDB and I believe all the GNU programs limit the line length to 80

  characters; some GDB code violates it, though.
Write complete sentences in the comments.
(Not fixed.)  `#if ...\n#define ...'should be indented `#if ...\n# define ...'.

Function parameters should be aligned behind '(' (at `sscanf').
Comment 13 Denys Vlasenko 2008-07-29 07:41:05 EDT
Created attachment 312860 [details]
Updated patch according to Jan's comments.

Jan, I believe I addressed all your concerns in this patch. Let me know if you
see something in need of further work.
Comment 14 Denys Vlasenko 2008-07-29 07:59:51 EDT
Created attachment 312861 [details]
Updated patch according to Jan's comments, v2

Fixed one 80 column rule violation, don't know what to do with this one:

 char *(*linux_elfcore_write_prpsinfo)
-  (bfd *, char *, int *, const char *, const char *) = elfcore_write_prpsinfo;

+  (bfd *, char *, int *, void *, const char *, const char *) =
elfcore_write_prpsinfo;

Should it be wrapped, or would it impact readability too much.
Comment 15 Jan Kratochvil 2008-07-29 08:14:33 EDT
(In reply to comment #14)
> Fixed one 80 column rule violation, don't know what to do with this one:

http://www.gnu.org/prep/standards/standards.html
does not talk about 80 columns at all.

I hope it is just really a detail how to format it.
`indent -gnu' will do:
char *(*linux_elfcore_write_prpsinfo)
  (bfd *, char *, int *, void *, const char *, const char *) = 
  elfcore_write_prpsinfo;
but it will leave the other forms without reformatting.
I would find better to follow the existing style around in the code there:
char *(*linux_elfcore_write_prpsinfo)
  (bfd *, char *, int *, void *, const char *, const char *)
  = elfcore_write_prpsinfo;
and while writing it as GNU compliant new I would probably choose:
char *(*linux_elfcore_write_prpsinfo) (bfd *, char *, int *, void *, 
                                       const char *, const char *)
  = elfcore_write_prpsinfo;

but I hope it should not matter much (as long as it is <=80) to have better
chance of an upstream acceptance (not that I would be too much successful).

Note You need to log in before you can comment on or make changes to this bug.