Bug 157755 - [patch] gdb(1) hardware watchpoints do not work correctly in multi-threaded programs
Summary: [patch] gdb(1) hardware watchpoints do not work correctly in multi-threaded p...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: gnu (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on: 210874
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-11 05:50 UTC by takahiro.kurosawa
Modified: 2016-07-27 16:27 UTC (History)
2 users (show)

See Also:


Attachments
file.txt (7.02 KB, patch)
2011-06-11 05:50 UTC, takahiro.kurosawa
no flags Details | Diff
patch for gdb-7.11.1 (for devel/gdb port) (6.61 KB, patch)
2016-06-24 23:49 UTC, takahiro.kurosawa
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description takahiro.kurosawa 2011-06-11 05:50:09 UTC
Hardware watchpoints are implemented on gdb in FreeBSD/i386 and
FreeBSD/amd64 by setting debug registers with ptrace(PT_SETDBREGS).
But watchpoints are effective only for one thread in the multi-threaded
program because debug registers are only set per process.

Fix: The attached patch fixes the problem.

----
(gdb) x/i thr_main
0x8048540 <thr_main>:   push   %ebp
(gdb) break *0x8048540
Breakpoint 1 at 0x8048540: file watch.c, line 10.
(gdb) run
Starting program: /home/kurosawa/a.out
[New LWP 100163]
[New Thread 28404300 (LWP 100163/initial thread)]
[New Thread 28404900 (LWP 100694/a.out)]
[Switching to Thread 28404900 (LWP 100694/a.out)]

Breakpoint 1, thr_main (arg=0x0) at watch.c:10
10      {
(gdb) watch watchpoint
Hardware watchpoint 2: watchpoint
(gdb) c
Continuing.
Hardware watchpoint 2: watchpoint

Old value = 0
New value = 2
thr_main (arg=0x0) at watch.c:13
13              return NULL;
(gdb) c
Continuing.
[Thread 28404900 (LWP 100694/a.out) exited]
[New Thread 28404900 (LWP 100694/a.out)]
[Switching to Thread 28404300 (LWP 100163/initial thread)]
Hardware watchpoint 2: watchpoint

Old value = 2
New value = 1
main () at watch.c:28
28              ret = 1;
(gdb) c
Continuing.

Program exited normally.
----

The patch only fixes FreeBSD/i386 but the same approach could be used
for FreeBSD/amd64.


Patch attached with submission follows:
How-To-Repeat: We can reproduce the problem by the following procedure:

% cat watch.c
#include <pthread.h>
#include <stdio.h>
#include <string.h>
#include <err.h>

static int watchpoint;

void *
thr_main(void *arg)
{

        watchpoint = 2;
        return NULL;
}

int
main(void)
{
        pthread_t pt;
        int ret = 0;

        if ((ret = pthread_create(&pt, NULL, thr_main, NULL)) != 0)
                errx(1, "pthread_create: %s.\n", strerror(ret));

        ret = 2;
        pthread_join(pt, NULL);
        watchpoint = 1;
        ret = 1;

        return 0;
}
% cc -pthread -g watch.c
% gdb a.out
GNU gdb 6.1.1 [FreeBSD]
Copyright 2004 Free Software Foundation, Inc.
   ....

# "break thr_main" sets a breakpoint at "watchpoint = 2"
(gdb) x/i thr_main
0x8048540 <thr_main>:   push   %ebp
(gdb) break *0x8048540
Breakpoint 1 at 0x8048540: file watch.c, line 10.

(gdb) run
Starting program: /home/kurosawa/a.out
[New LWP 100260]
[New Thread 28404300 (LWP 100260/initial thread)]
[New Thread 28404900 (LWP 100693/a.out)]
[Switching to Thread 28404900 (LWP 100693/a.out)]

Breakpoint 1, thr_main (arg=0x0) at watch.c:10
10      {

# set an watchpoint to the "watchpoint" variable
(gdb) watch watchpoint
Hardware watchpoint 2: watchpoint

# watchpoint hit in thr_main
(gdb) c
Continuing.
Hardware watchpoint 2: watchpoint

Old value = 0
New value = 2
thr_main (arg=0x0) at watch.c:13
13              return NULL;

(gdb) c
Continuing.
[Thread 28404900 (LWP 100693/a.out) exited]
[New Thread 28404900 (LWP 100693/a.out)]

Program exited normally.
# watchpoint must be hit also in main, but program runs without stopping.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2011-06-11 07:24:41 UTC
Responsible Changed
From-To: freebsd-threads->freebsd-bugs

Reclassify.
Comment 2 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-06-01 21:17:54 UTC
Comment on attachment 115873 [details]
file.txt

Looks like a unified diff (from mercurial!)
Comment 3 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-06-01 21:19:45 UTC
CC our gdb-threading expert.
Comment 4 Pedro F. Giffuni freebsd_committer freebsd_triage 2016-06-01 21:20:38 UTC
CC our gdb-threading expert.
Comment 5 John Baldwin freebsd_committer freebsd_triage 2016-06-14 18:32:20 UTC
The patch will have to be reworked for gdb7, but I can reproduce on 7.11.1.
Comment 6 takahiro.kurosawa 2016-06-24 23:49:47 UTC
Created attachment 171767 [details]
patch for gdb-7.11.1 (for devel/gdb port)

I've remade the patch for gdb (7.11.1) port.
This patch supports amd64 and i386.
Comment 7 John Baldwin freebsd_committer freebsd_triage 2016-06-28 01:40:46 UTC
Note that in theory dragonfly also supports PT_GET/SETDBREGS (though upstream gdb does not have a config for Dragonfly), so I think it should stay in the pan-BSD files rather than move to a FreeBSD-specific file.  I ended up doing something similar, but using an inline iteration rather than iterate_threads() (and I have an initial patch to move the debug register stuff into x86bsd-nat.c).

This is the patchset I plan to post for review upstream:

https://github.com/bsdjhb/gdb/compare/thread_dbreg
Comment 8 John Baldwin freebsd_committer freebsd_triage 2016-07-01 15:09:05 UTC
The changes from the thread_dbreg branch have been pushed upstream to gdb master.  They will be included in 7.12, though I'm not sure when that will be released.

We could bring the commits into the port as patches for now if desired.  If so, there are a few other changes we should bring in as well.
Comment 9 commit-hook freebsd_committer freebsd_triage 2016-07-15 01:10:52 UTC
A commit references this bug:

Author: jhb
Date: Fri Jul 15 01:10:16 UTC 2016
New revision: 418566
URL: https://svnweb.freebsd.org/changeset/ports/418566

Log:
  Import several patches recently merged upstream.

  - Fix fork following to honor 'detach-on-fork'
  - Fix vfork following to post a fake vfork_done event to fix breakpoints
    in vfork parents (a real vfork_done event is pending but requires kernel
    changes currently in review).
  - Fix x86 debug registers to work with multiple threads (PR 157755)
  - Add support for 'info auxv' on both live processes and cores.
  - Add support for 'catch syscall'.  Note that catching system calls by
    names requires parsing an XML file mapping system call names to
    numbers.  The port now installs the XML syscall files to the data
    directory.  In addition, the EXPAT option is now enabled by default as
    expat is used to parse the XML files.
  - Bump PORTREVISION.

  PR:		157755, 210874
  Approved by:	luca.pizzamiglio@gmail.com (maintainer), bdrewery

Changes:
  head/devel/gdb/Makefile
  head/devel/gdb/files/commit-21002a6
  head/devel/gdb/files/commit-2c5c2a3
  head/devel/gdb/files/commit-2faa344
  head/devel/gdb/files/commit-3350c5f
  head/devel/gdb/files/commit-5077bff
  head/devel/gdb/files/commit-7697fc9
  head/devel/gdb/files/commit-82372b2
  head/devel/gdb/files/commit-8607ea6
  head/devel/gdb/files/commit-a3405d1
  head/devel/gdb/files/commit-aa1ed4a
  head/devel/gdb/files/commit-b00f86d
  head/devel/gdb/files/commit-bb2a62e
  head/devel/gdb/files/commit-e6cdd38
  head/devel/gdb/files/commit-ee95032
  head/devel/gdb/pkg-plist
Comment 10 commit-hook freebsd_committer freebsd_triage 2016-07-27 16:27:32 UTC
A commit references this bug:

Author: jhb
Date: Wed Jul 27 16:27:07 UTC 2016
New revision: 419185
URL: https://svnweb.freebsd.org/changeset/ports/419185

Log:
  MFH: r418566 r418964

  Import several patches recently merged upstream.

  - Fix fork following to honor 'detach-on-fork'
  - Fix vfork following to post a fake vfork_done event to fix breakpoints
    in vfork parents (a real vfork_done event is pending but requires kernel
    changes currently in review).
  - Fix x86 debug registers to work with multiple threads (PR 157755)
  - Add support for 'info auxv' on both live processes and cores.
  - Add support for 'catch syscall'.  Note that catching system calls by
    names requires parsing an XML file mapping system call names to
    numbers.  The port now installs the XML syscall files to the data
    directory.  In addition, the EXPAT option is now enabled by default as
    expat is used to parse the XML files.
  - Handle version 1a of NT_PRPSINFO notes which include the pr_pid field.
  - Replace patch-sigev with upstream version.  Note that upstream GDB
    doesn't define SIGLIBRT on older OS versions, so do that in the port
    Makefile instead.
  - Use PT_GET_EVENT_MASK/PT_SET_EVENT_MASK (new in 12).
  - Fix a bug where fork and LWP events weren't enabled in new child
    processes when following child processes after a fork.
  - Handle "real" vfork done events via PTRACE_VFORK (new in 12).
  - Bump PORTREVISION.

  PR:		157755, 210874,	211254
  Approved by:	ports-secteam (feld)

Changes:
_U  branches/2016Q3/
  branches/2016Q3/devel/gdb/Makefile
  branches/2016Q3/devel/gdb/files/commit-0064d22
  branches/2016Q3/devel/gdb/files/commit-21002a6
  branches/2016Q3/devel/gdb/files/commit-2c5c2a3
  branches/2016Q3/devel/gdb/files/commit-2faa344
  branches/2016Q3/devel/gdb/files/commit-3350c5f
  branches/2016Q3/devel/gdb/files/commit-5077bff
  branches/2016Q3/devel/gdb/files/commit-5fa14c6
  branches/2016Q3/devel/gdb/files/commit-7697fc9
  branches/2016Q3/devel/gdb/files/commit-82372b2
  branches/2016Q3/devel/gdb/files/commit-8607ea6
  branches/2016Q3/devel/gdb/files/commit-a3405d1
  branches/2016Q3/devel/gdb/files/commit-aa1ed4a
  branches/2016Q3/devel/gdb/files/commit-b00f86d
  branches/2016Q3/devel/gdb/files/commit-bb2a62e
  branches/2016Q3/devel/gdb/files/commit-bc7b765
  branches/2016Q3/devel/gdb/files/commit-da95a26
  branches/2016Q3/devel/gdb/files/commit-dbaed38
  branches/2016Q3/devel/gdb/files/commit-e6cdd38
  branches/2016Q3/devel/gdb/files/commit-ee95032
  branches/2016Q3/devel/gdb/files/patch-sigev
  branches/2016Q3/devel/gdb/pkg-plist