[v4,00/18] All-stop on top of non-stop
Commit Message
> My idea of a testcase comment is at the beginning of the testcase file,
> explaining what the test does and why it does it. I'd mention the amd64
> example as well, since it is part of why the test was created in the first
> place.
>
> That should give others enough background to pursue an investigation about
> why this potentially fails for them.
>
> My 2 cents anyway.
There is this perception that the testcase was created because
of the issue on amd64, but that's not true. The testcase was
created, albeit in AdaCore's infrastructure only, to test that
"next" in that context works as expected. Only later on did it
allow us to find another bug which actually has nothing to do
with the initial reason for creating the testcase. I hope I'm not
looking like I'm splitting hair, but I feel like there is a bit
of a misunderstanding somewhere, probably because the testcase
appears as new to the GDB community and was combined with an
amd64-specific fix.
That being said, I propose the attached patch. I confess I'm not
super convinced about the comment on amd64, as I think it might
become one day irrelevant. But I don't mind it that much; if
it is helpful to others...
What do you think?
gdb/testsuite/ChangeLog:
* gdb.base/dso2dso.exp: Improve the testcase's documentation.
Comments
On 08/12/2015 05:50 PM, Joel Brobecker wrote:
>> My idea of a testcase comment is at the beginning of the testcase file,
>> explaining what the test does and why it does it. I'd mention the amd64
>> example as well, since it is part of why the test was created in the first
>> place.
>>
>> That should give others enough background to pursue an investigation about
>> why this potentially fails for them.
>>
>> My 2 cents anyway.
>
> There is this perception that the testcase was created because
> of the issue on amd64, but that's not true. The testcase was
> created, albeit in AdaCore's infrastructure only, to test that
> "next" in that context works as expected. Only later on did it
> allow us to find another bug which actually has nothing to do
> with the initial reason for creating the testcase. I hope I'm not
> looking like I'm splitting hair, but I feel like there is a bit
> of a misunderstanding somewhere, probably because the testcase
> appears as new to the GDB community and was combined with an
> amd64-specific fix.
>
What caused confusion was the fact that the test was sent together with
a fix, which seemed to imply it was a fix-specific test as opposed to a
testcase-less fix and a generic new testcase in the same patch.
> That being said, I propose the attached patch. I confess I'm not
> super convinced about the comment on amd64, as I think it might
> become one day irrelevant. But I don't mind it that much; if
> it is helpful to others...
The description looks good. Feel free to drop the amd64 reference if you
think it is not worth it. I don't have a strong opinion on it.
It just felt like the testcase needed a bit more information on why it
was created in the first place.
On 08/12/2015 09:50 PM, Joel Brobecker wrote:
> That being said, I propose the attached patch. I confess I'm not
> super convinced about the comment on amd64, as I think it might
> become one day irrelevant. But I don't mind it that much; if
> it is helpful to others...
>
> What do you think?
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/dso2dso.exp: Improve the testcase's documentation.
>
Seems fine to me.
Thanks,
Pedro Alves
> > gdb/testsuite/ChangeLog:
> >
> > * gdb.base/dso2dso.exp: Improve the testcase's documentation.
> >
>
> Seems fine to me.
Thank you, pushed!
From bbb505f2f822997b4285c61cede194fe2617b295 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 12 Aug 2015 13:40:54 -0700
Subject: [PATCH] gdb.base/dso2dso.exp: Improve testcase documentation.
gdb/testsuite/ChangeLog:
* gdb.base/dso2dso.exp: Improve the testcase's documentation.
---
gdb/testsuite/gdb.base/dso2dso.exp | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
@@ -13,6 +13,16 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
+# The purpose of that testcase is to verify that we can "next" over
+# a call to a function provided by one shared library made from another
+# shared library, and that GDB stops at the expected location. In this
+# case, the call is made from sub1 (provided by libdso1) and we are
+# calling sub2 (provided by libdso2).
+#
+# Note that, while this is not the main purpose of this testcase, it
+# also happens to exercise an issue with displaced stepping on amd64
+# when libdso1 is mapped at an address greater than 0xffffffff.
+
if { [skip_shlib_tests] } {
return 0
}
@@ -53,10 +63,6 @@ if { ![runto_main] } {
return -1
}
-# Verify that we can "next" over the call to sub2 (provided by
-# libdso2) make from sub1 (provided by libdso1), and land at
-# the expected location.
-
set bp_location [gdb_get_line_number "STOP HERE" $srcfile_libdso1]
gdb_breakpoint ${srcfile_libdso1}:${bp_location}
--
2.1.4