[v4,00/18] All-stop on top of non-stop

Message ID 20150812183208.GA24901@adacore.com
State New, archived
Headers

Commit Message

Joel Brobecker Aug. 12, 2015, 6:32 p.m. UTC
  Hi Pedro,

On Fri, Aug 07, 2015 at 05:58:37PM +0100, Pedro Alves wrote:
> On 05/22/2015 12:18 AM, Pedro Alves wrote:
> > v3: https://sourceware.org/ml/gdb-patches/2015-04/msg00662.html
> > v2: https://sourceware.org/ml/gdb-patches/2015-04/msg00198.html
> > v1: https://sourceware.org/ml/gdb-patches/2015-04/msg00073.html
> > 
> > Compared to v3, a set of minor changes accumulated, to address review
> > comments and discussions (i.e., no major functional/design changes),
> > plus fixes to a patch that touched all targets (that I noticed
> > converted a couple wrong).
> > 
> > I believe I addressed all review comments thus far.  Barring comments,
> > I'd like to push this in, and expose it to the build bots.
> 
> FYI, I pushed this in.  I'm keeping an eye on the build bots.
> 
> If you spot any related problem, please confirm whether
> "maint set target-non-stop off" fixes it.

This uncovered what I think is a latent bug in the "how-did-we-never-
see-this-before" category. Does this patch look OK to you?

gdb/ChangeLog:

        * amd64-tdep.c (amd64_displaced_step_fixup): Fix the mask used to
        compute RETADDR.

gdb/testsuite/ChangeLog:

        * gdb.base/dso2dso-impl.c, gdb.base/dso2dso-impl.h,
        gdb.base/dso2dso-pck.c, gdb.base/dso2dso-pck.h, gdb.base/dso2dso.c,
        gdb.base/dso2dso.exp: New files.

Tested on x86_64-linux, no regression.

Thanks!
  

Comments

Pedro Alves Aug. 12, 2015, 7:05 p.m. UTC | #1
On 08/12/2015 07:32 PM, Joel Brobecker wrote:

> This uncovered what I think is a latent bug in the "how-did-we-never-
> see-this-before" category. 

Wow!  Indeed.  Many thanks for tracking this down and fixing it.

A couple minor comments below, but this looks good to me.

> Does this patch look OK to you?

>         * gdb.base/dso2dso-impl.c, gdb.base/dso2dso-impl.h,
>         gdb.base/dso2dso-pck.c, gdb.base/dso2dso-pck.h, gdb.base/dso2dso.c,
>         gdb.base/dso2dso.exp: New files.
> 

Nit: the "pck" and "impl"s here confuse me -- I guess they meant something
in the original test?  Do they still have any meaning here?  I'd suggest
renaming to dso2dso-dso1.c, dso2dso-dso2.c, and then
 sub() -> sub1(), impl__initialize() -> sub2().  That way, it should
be clearer that a function in dso1 calls a function in dso2.

> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index 5e63b5e..5b4a8f4 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -1662,7 +1662,7 @@ amd64_displaced_step_fixup (struct gdbarch *gdbarch,
>  
>        regcache_cooked_read_unsigned (regs, AMD64_RSP_REGNUM, &rsp);
>        retaddr = read_memory_unsigned_integer (rsp, retaddr_len, byte_order);
> -      retaddr = (retaddr - insn_offset) & 0xffffffffUL;
> +      retaddr = (retaddr - insn_offset) & 0xffffffffffffffffUL;

I think this should use the ULL suffix now.

> +
> +if { [gdb_compile_shlib $srcfile_libimpl $binfile_libimpl \
> +	[list debug additional_flags=-fPIC]] != "" } {
> +  untested "Could not compile $binfile_libimpl."
> +  return -1
> +}
> +
> +if { [gdb_compile_shlib $srcfile_libpck $binfile_libpck \
> +	[list debug additional_flags=-fPIC]] != "" } {
> +  untested "Could not compile $binfile_libpck."
> +  return -1
> +}
> +
> +if { [gdb_compile $srcdir/$subdir/$srcfile $binfile executable \
> +	[list debug shlib=$binfile_libimpl shlib=$binfile_libpck]] != "" } {
> +  return -1
> +}
> +
> +clean_restart $binfile
> +

I think a

 gdb_load_shlibs $binfile_libimpl $binfile_libpck

is missing here.

> +if { ![runto_main] } {
> +  return -1
> +}
> +
> +set bp_location [gdb_get_line_number "impl__initialize" $srcfile_libpck]
> +gdb_breakpoint ${srcfile_libpck}:${bp_location}
> +
> +gdb_continue_to_breakpoint "at call to impl__initialize" \
> +    ".*impl__initialize ().*"
> +
> +gdb_test "next" \
> +    ".*return 5;.*" \
> +    "next over call to impl__initialize"
> -- 2.1.4

Thanks,
Pedro Alves
  
Luis Machado Aug. 12, 2015, 7:38 p.m. UTC | #2
Hi Joel,

On 08/12/2015 03:32 PM, Joel Brobecker wrote:
> Hi Pedro,
>
> On Fri, Aug 07, 2015 at 05:58:37PM +0100, Pedro Alves wrote:
>> On 05/22/2015 12:18 AM, Pedro Alves wrote:
>>> v3: https://sourceware.org/ml/gdb-patches/2015-04/msg00662.html
>>> v2: https://sourceware.org/ml/gdb-patches/2015-04/msg00198.html
>>> v1: https://sourceware.org/ml/gdb-patches/2015-04/msg00073.html
>>>
>>> Compared to v3, a set of minor changes accumulated, to address review
>>> comments and discussions (i.e., no major functional/design changes),
>>> plus fixes to a patch that touched all targets (that I noticed
>>> converted a couple wrong).
>>>
>>> I believe I addressed all review comments thus far.  Barring comments,
>>> I'd like to push this in, and expose it to the build bots.
>>
>> FYI, I pushed this in.  I'm keeping an eye on the build bots.
>>
>> If you spot any related problem, please confirm whether
>> "maint set target-non-stop off" fixes it.
>
> This uncovered what I think is a latent bug in the "how-did-we-never-
> see-this-before" category. Does this patch look OK to you?
>
> gdb/ChangeLog:
>
>          * amd64-tdep.c (amd64_displaced_step_fixup): Fix the mask used to
>          compute RETADDR.
>
> gdb/testsuite/ChangeLog:
>
>          * gdb.base/dso2dso-impl.c, gdb.base/dso2dso-impl.h,
>          gdb.base/dso2dso-pck.c, gdb.base/dso2dso-pck.h, gdb.base/dso2dso.c,
>          gdb.base/dso2dso.exp: New files.
>
> Tested on x86_64-linux, no regression.
>
> Thanks!
>

Two things about the patch. I see that the change to GDB's code is 
almost trivial, but the testcase looks quite involved.

The first thing is that one wouldn't be able to tell what the testcase 
does without looking at the commit log. dso2dso doesn't particularly 
translate to "displaced stepping instruction masking problem on amd64". 
Should we change the testcase name to something a bit more meaningful? 
Maybe document it a bit?

The second point is, should we restrict this testcase to be executed 
only for amd64? Maybe move it to gdb.arch? Unless you actually tried 
this testcase with different architectures and confirmed the testcase is 
sane there, it feels a bit iffy to add/execute this for non-amd64 targets.

In the worst case, we could have a failure due to different instruction 
scheduling schemes (or maybe even a displaced stepping bug) on non-amd64 
targets. At a minimum, we'd have a spurious PASS that wouldn't be 
meaningful for the overall test results since things never failed on 
said architecture in the first place.

Alternatively, for such a trivial fix, shouldn't we go without a testcase?

Luis
  
Sergio Durigan Junior Aug. 12, 2015, 7:51 p.m. UTC | #3
On Wednesday, August 12 2015, Luis Machado wrote:

> Alternatively, for such a trivial fix, shouldn't we go without a testcase?

Not commenting on the other points, but I don't think we should.  No
matter how trivial is the fix, a testcase is always welcome.  Of course,
I do agree about your suggestion to document better what the testcase
does.
  
Joel Brobecker Aug. 12, 2015, 7:59 p.m. UTC | #4
> Two things about the patch. I see that the change to GDB's code is almost
> trivial, but the testcase looks quite involved.
> 
> The first thing is that one wouldn't be able to tell what the testcase does
> without looking at the commit log. dso2dso doesn't particularly translate to
> "displaced stepping instruction masking problem on amd64". Should we change
> the testcase name to something a bit more meaningful? Maybe document it a
> bit?
> 
> The second point is, should we restrict this testcase to be executed only
> for amd64? Maybe move it to gdb.arch? Unless you actually tried this
> testcase with different architectures and confirmed the testcase is sane
> there, it feels a bit iffy to add/execute this for non-amd64 targets.
> 
> In the worst case, we could have a failure due to different instruction
> scheduling schemes (or maybe even a displaced stepping bug) on non-amd64
> targets. At a minimum, we'd have a spurious PASS that wouldn't be meaningful
> for the overall test results since things never failed on said architecture
> in the first place.
> 
> Alternatively, for such a trivial fix, shouldn't we go without a testcase?

Some good points. To me, the purpose of the test is to verify that
stepping from some code in one DSO over a call to a function in another
DSO, is working on all platforms that support shared libraries. That's
the main purpose of the test. If, one day, the implementation changes
so that displaced stepping is no longer necessary, then what matters
at the user level is that this testcase continues to behave the same.

Even if you wanted a gdb.arch test that verified this specific bug,
you'd have to find a way for the test to load at an address that's
high enough that the mask starts causing problems. I don't think
that's very easy, and I'm not sure it's worth our time.

Regarding the complexity of the test, I think we need to try it and
see.  At AdaCore, we run the equivalent Ada testcase nightly on Linux
(x86, x86_64 and ppc), Windows (x86 and x86_64), Solaris (sparc) and
Darwin (x86_64), and it passes everywhere. It passes with GDBserver
too. My take would be that we'd be fixing trivial errors in the testcase
itself, and just accept the failures on platforms where it reveals
a bona fide issue, as we've beeing doing with all the other tests.
  
Luis Machado Aug. 12, 2015, 8:18 p.m. UTC | #5
On 08/12/2015 04:59 PM, Joel Brobecker wrote:
>> Two things about the patch. I see that the change to GDB's code is almost
>> trivial, but the testcase looks quite involved.
>>
>> The first thing is that one wouldn't be able to tell what the testcase does
>> without looking at the commit log. dso2dso doesn't particularly translate to
>> "displaced stepping instruction masking problem on amd64". Should we change
>> the testcase name to something a bit more meaningful? Maybe document it a
>> bit?
>>
>> The second point is, should we restrict this testcase to be executed only
>> for amd64? Maybe move it to gdb.arch? Unless you actually tried this
>> testcase with different architectures and confirmed the testcase is sane
>> there, it feels a bit iffy to add/execute this for non-amd64 targets.
>>
>> In the worst case, we could have a failure due to different instruction
>> scheduling schemes (or maybe even a displaced stepping bug) on non-amd64
>> targets. At a minimum, we'd have a spurious PASS that wouldn't be meaningful
>> for the overall test results since things never failed on said architecture
>> in the first place.
>>
>> Alternatively, for such a trivial fix, shouldn't we go without a testcase?
>
> Some good points. To me, the purpose of the test is to verify that
> stepping from some code in one DSO over a call to a function in another
> DSO, is working on all platforms that support shared libraries. That's
> the main purpose of the test. If, one day, the implementation changes
> so that displaced stepping is no longer necessary, then what matters
> at the user level is that this testcase continues to behave the same.

Ok. So i think i misunderstood the purpose of the testcase there. In 
reality the testcase is not testing the fix itself, but rather 
introducing a new test not related to the problem, except in the amd64 
architecture, where it really tests the problem.

It just confused me that the test is generic for other non-amd64 
architectures and specific to the amd64 problem you saw.

I'd expect a generic solib test to be included in one of our shared 
library tests, but then you'd have to shape it in a way that would 
exercise your displaced stepping problem.

>
> Even if you wanted a gdb.arch test that verified this specific bug,
> you'd have to find a way for the test to load at an address that's
> high enough that the mask starts causing problems. I don't think
> that's very easy, and I'm not sure it's worth our time.
>
> Regarding the complexity of the test, I think we need to try it and
> see.  At AdaCore, we run the equivalent Ada testcase nightly on Linux
> (x86, x86_64 and ppc), Windows (x86 and x86_64), Solaris (sparc) and
> Darwin (x86_64), and it passes everywhere. It passes with GDBserver
> too. My take would be that we'd be fixing trivial errors in the testcase
> itself, and just accept the failures on platforms where it reveals
> a bona fide issue, as we've beeing doing with all the other tests.

It is not a big deal. In recent times i've been seeing a number of 
testcases that shouldn't be generally executed for all architectures. 
That ends up causing spurious failures and bringing the signal/noise 
ratio of the testsuite down for the affect architectures (usually not 
x86 or Linux).

I see your targets are mostly x86. I can give it a try on a few more 
(powerpc, mips, arm, nios) and let you know what i see. How does that sound?
  
Joel Brobecker Aug. 12, 2015, 8:33 p.m. UTC | #6
I tried to clarify the purpose of the testcase by adding a comment
inside it. Let me know if this isn't enough, and we'll try to improve.

> Ok. So i think i misunderstood the purpose of the testcase there. In reality
> the testcase is not testing the fix itself, but rather introducing a new
> test not related to the problem, except in the amd64 architecture, where it
> really tests the problem.
> 
> It just confused me that the test is generic for other non-amd64
> architectures and specific to the amd64 problem you saw.
> 
> I'd expect a generic solib test to be included in one of our shared library
> tests, but then you'd have to shape it in a way that would exercise your
> displaced stepping problem.

I understand the first paragraph, but I'm having trouble with the last
one. The testcase as I wrote it does exercise the issue being fixed
on amd64, and I verified that I get 1 FAIL without the patch. Did
I misunderstand you?

> I see your targets are mostly x86. I can give it a try on a few more
> (powerpc, mips, arm, nios) and let you know what i see. How does that sound?

Sounds good to me.

Bare in mind that the list of targets AdaCore tests on already includes
powerpc and sparc, and I forgot ARM that we also test (as a cross,
debugging via GDBserver). So I think it's fairly wide already.
Our testsuite uses a different technology than dejagnu, though, so we
might indeed get some surprises, but I think they will have more to do
with scripting than GDB: Except for the platforms that we don't test
at AdaCore, I'd expect that any error will mostly be about building
the program rather than debugging it.
  
Luis Machado Aug. 12, 2015, 8:40 p.m. UTC | #7
On 08/12/2015 05:33 PM, Joel Brobecker wrote:
> I tried to clarify the purpose of the testcase by adding a comment
> inside it. Let me know if this isn't enough, and we'll try to improve.
>
>> Ok. So i think i misunderstood the purpose of the testcase there. In reality
>> the testcase is not testing the fix itself, but rather introducing a new
>> test not related to the problem, except in the amd64 architecture, where it
>> really tests the problem.
>>
>> It just confused me that the test is generic for other non-amd64
>> architectures and specific to the amd64 problem you saw.
>>
>> I'd expect a generic solib test to be included in one of our shared library
>> tests, but then you'd have to shape it in a way that would exercise your
>> displaced stepping problem.
>
> I understand the first paragraph, but I'm having trouble with the last
> one. The testcase as I wrote it does exercise the issue being fixed
> on amd64, and I verified that I get 1 FAIL without the patch. Did
> I misunderstand you?

I was just pointing at the fact that we already have shared library 
tests, so those could be expanded to include this inter-dso call as 
opposed to having a different set of tests like your patch did. But 
you'd need to shape it in a way that exercises your amd64 failure mode then.

In any case, i'm good with the test. I just want to give it a try to be 
sure.
  
Joel Brobecker Aug. 12, 2015, 8:55 p.m. UTC | #8
> I was just pointing at the fact that we already have shared library tests,
> so those could be expanded to include this inter-dso call as opposed to
> having a different set of tests like your patch did. But you'd need to shape
> it in a way that exercises your amd64 failure mode then.

Ah, OK!

Speaking in general terms and for myself, I usually prefer to create new
testcases rather than piggy-back on existing ones, because I find it
simpler to do, and I also find it simpler to be certain that I'm not
altering the older testcases in a way that some tests are no longer
doing what they are supposed to do. And finally, it makes it easier to
investigate regressions, because the testcase is usually simpler that
way. Have you tried debugging a testcase where you have about 250 tests
before the failure, and you're not sure what's relevant and what is not?
;-)
  

Patch

From 63cf63b54b7b303b07dcc3ab5206fef8b3bde418 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 12 Aug 2015 09:33:19 -0700
Subject: [PATCH] [amd64] Invalid return address after displaced stepping

Making all-stop run on top of non-stop caused a small regression
in behavior. This was observed on x86_64-linux. The attached testcase
is in C whereas the investigation was done with an Ada program,
but it's the same scenario, and using a C testcase allows wider testing.
Basically: I am debugging a single-threaded program, and currently
stopped inside a function provided by a shared-library, at a line
calling a subprogram provided by a second shared library, and trying
to "next" over that function call.

Before we changed the default all-stop behavior, we had:

    7             Impl_Initialize;  -- Stop here and try "next" over this line
    (gdb) n
    8             return 5;  <<-- OK

But now, "next" just stops much earlier:

    (gdb) n
    0x00007ffff7bd8560 in impl.initialize@plt () from /[...]/lib/libpck.so

What happens is that next stops at a call instruction, which calls
the function's PLT, and GDB fails to notice that the inferior stepped
into a subroutine, and so decides that we're done. We can see another
symptom of the same issue by looking at the backtrace at the point
GDB stopped:

    (gdb) bt
    #0  0x00007ffff7bd8560 in impl.initialize@plt ()
       from /[...]/lib/libpck.so
    #1  0x00000000f7bd86f9 in ?? ()
    #2  0x00007fffffffdf50 in ?? ()
    #3  0x0000000000401893 in a () at /[...]/a.adb:7
    Backtrace stopped: frame did not save the PC

With a functioning GDB, the backtrace looks like the following instead:

    #0  0x00007ffff7bd8560 in impl.initialize@plt ()
       from /[...]/lib/libpck.so
    #1  0x00007ffff7bd86f9 in sub () at /[...]/pck.adb:7
    #2  0x0000000000401893 in a () at /[...]/a.adb:7

Note how, for frame #1, the address looks quite similar, except
for the high-order bits not being set:

    #1  0x00007ffff7bd86f9 in sub () at /[...]/pck.adb:7   <<<--  OK
    #1  0x00000000f7bd86f9 in ?? ()                        <<<--  WRONG
              ^^^^
              ||||
              Wrong

Investigating this further led me to displaced stepping.
As we are "next"-ing from a location where a breakpoint is inserted,
we need to step out of it, and since we're on non-stop mode, we need
to do it using displaced stepping. And looking at
amd64-tdep.c:amd64_displaced_step_fixup, I found the code that handles
the return address:

    regcache_cooked_read_unsigned (regs, AMD64_RSP_REGNUM, &rsp);
    retaddr = read_memory_unsigned_integer (rsp, retaddr_len, byte_order);
    retaddr = (retaddr - insn_offset) & 0xffffffffUL;

The mask used to compute retaddr looks wrong to me, keeping only
4 bytes instead of 8, and explains why the high order bits of
the backtrace are unset. What happens is that, after the displaced
stepping has completed, GDB restores that return address at the location
where the program expects it.  But because the top half bits of
the address have been masked out, the return address is now invalid.
The incorrect behavior of the "next" command and the backtrace at
that location are the first symptoms of that.  Another symptom is
that this actually alters the behavior of the program, where a "cont"
from there soon leads to a SEGV when the inferior tries to jump back
to that incorrect return address:

    (gdb) c
    Continuing.

    Program received signal SIGSEGV, Segmentation fault.
    0x00000000f7bd86f9 in ?? ()
    ^^^^^^^^^^^^^^^^^^

This patch fixes the issue by using a mask that seems more appropriate
for this architecture.

gdb/ChangeLog:

        * amd64-tdep.c (amd64_displaced_step_fixup): Fix the mask used to
        compute RETADDR.

gdb/testsuite/ChangeLog:

        * gdb.base/dso2dso-impl.c, gdb.base/dso2dso-impl.h,
        gdb.base/dso2dso-pck.c, gdb.base/dso2dso-pck.h, gdb.base/dso2dso.c,
        gdb.base/dso2dso.exp: New files.

Tested on x86_64-linux, no regression.
---
 gdb/amd64-tdep.c                      |  2 +-
 gdb/testsuite/gdb.base/dso2dso-impl.c | 24 +++++++++++++
 gdb/testsuite/gdb.base/dso2dso-impl.h | 23 +++++++++++++
 gdb/testsuite/gdb.base/dso2dso-pck.c  | 26 +++++++++++++++
 gdb/testsuite/gdb.base/dso2dso-pck.h  | 23 +++++++++++++
 gdb/testsuite/gdb.base/dso2dso.c      | 25 ++++++++++++++
 gdb/testsuite/gdb.base/dso2dso.exp    | 63 +++++++++++++++++++++++++++++++++++
 7 files changed, 185 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/dso2dso-impl.c
 create mode 100644 gdb/testsuite/gdb.base/dso2dso-impl.h
 create mode 100644 gdb/testsuite/gdb.base/dso2dso-pck.c
 create mode 100644 gdb/testsuite/gdb.base/dso2dso-pck.h
 create mode 100644 gdb/testsuite/gdb.base/dso2dso.c
 create mode 100644 gdb/testsuite/gdb.base/dso2dso.exp

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 5e63b5e..5b4a8f4 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1662,7 +1662,7 @@  amd64_displaced_step_fixup (struct gdbarch *gdbarch,
 
       regcache_cooked_read_unsigned (regs, AMD64_RSP_REGNUM, &rsp);
       retaddr = read_memory_unsigned_integer (rsp, retaddr_len, byte_order);
-      retaddr = (retaddr - insn_offset) & 0xffffffffUL;
+      retaddr = (retaddr - insn_offset) & 0xffffffffffffffffUL;
       write_memory_unsigned_integer (rsp, retaddr_len, byte_order, retaddr);
 
       if (debug_displaced)
diff --git a/gdb/testsuite/gdb.base/dso2dso-impl.c b/gdb/testsuite/gdb.base/dso2dso-impl.c
new file mode 100644
index 0000000..0b34aa9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dso2dso-impl.c
@@ -0,0 +1,24 @@ 
+/* Copyright 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "dso2dso-impl.h"
+
+void
+impl__initialize (void)
+{
+  /* Do nothing.  */
+}
diff --git a/gdb/testsuite/gdb.base/dso2dso-impl.h b/gdb/testsuite/gdb.base/dso2dso-impl.h
new file mode 100644
index 0000000..58b9d13
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dso2dso-impl.h
@@ -0,0 +1,23 @@ 
+/* Copyright 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef DSO2DSO_IMPL_H
+#define DSO2DSO_IMPL_H
+
+extern void impl__initialize (void);
+
+#endif
diff --git a/gdb/testsuite/gdb.base/dso2dso-pck.c b/gdb/testsuite/gdb.base/dso2dso-pck.c
new file mode 100644
index 0000000..a6a38f9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dso2dso-pck.c
@@ -0,0 +1,26 @@ 
+/* Copyright 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "dso2dso-pck.h"
+#include "dso2dso-impl.h"
+
+int
+sub (void)
+{
+  impl__initialize ();
+  return 5;
+}
diff --git a/gdb/testsuite/gdb.base/dso2dso-pck.h b/gdb/testsuite/gdb.base/dso2dso-pck.h
new file mode 100644
index 0000000..11e8643
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dso2dso-pck.h
@@ -0,0 +1,23 @@ 
+/* Copyright 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef DSO2DSO_PCK_H
+#define DSO2DSO_PCK_H
+
+extern int sub (void);
+
+#endif
diff --git a/gdb/testsuite/gdb.base/dso2dso.c b/gdb/testsuite/gdb.base/dso2dso.c
new file mode 100644
index 0000000..a085c00
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dso2dso.c
@@ -0,0 +1,25 @@ 
+/* Copyright 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "dso2dso-pck.h"
+
+int
+main (void)
+{
+  int ignored = sub ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/dso2dso.exp b/gdb/testsuite/gdb.base/dso2dso.exp
new file mode 100644
index 0000000..3d08b79
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dso2dso.exp
@@ -0,0 +1,63 @@ 
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if { [skip_shlib_tests] } {
+    return 0
+}
+
+standard_testfile
+
+set output_dir [standard_output_file {}]
+
+set libimpl $testfile-impl
+set srcfile_libimpl $srcdir/$subdir/$libimpl.c
+set binfile_libimpl [standard_output_file $libimpl.so]
+
+set libpck $testfile-pck
+set srcfile_libpck $srcdir/$subdir/$libpck.c
+set binfile_libpck [standard_output_file $libpck.so]
+
+if { [gdb_compile_shlib $srcfile_libimpl $binfile_libimpl \
+	[list debug additional_flags=-fPIC]] != "" } {
+  untested "Could not compile $binfile_libimpl."
+  return -1
+}
+
+if { [gdb_compile_shlib $srcfile_libpck $binfile_libpck \
+	[list debug additional_flags=-fPIC]] != "" } {
+  untested "Could not compile $binfile_libpck."
+  return -1
+}
+
+if { [gdb_compile $srcdir/$subdir/$srcfile $binfile executable \
+	[list debug shlib=$binfile_libimpl shlib=$binfile_libpck]] != "" } {
+  return -1
+}
+
+clean_restart $binfile
+
+if { ![runto_main] } {
+  return -1
+}
+
+set bp_location [gdb_get_line_number "impl__initialize" $srcfile_libpck]
+gdb_breakpoint ${srcfile_libpck}:${bp_location}
+
+gdb_continue_to_breakpoint "at call to impl__initialize" \
+    ".*impl__initialize ().*"
+
+gdb_test "next" \
+    ".*return 5;.*" \
+    "next over call to impl__initialize"
-- 
2.1.4