[v3,2/2] Make gdbserver work with filename-only binaries
Commit Message
Changes from v2:
- Updated commit log.
- s/True/true
- Implement Simon's idea to not modify the path is there's any path
separator char.
- Implement Simon's idea to use a class for "program_path".
- s/warning/error when getcwd returns NULL. Reorder error message.
- Don't restrict the test to native-gdbserver.
- New testsuite helper "with_cwd" which takes care of switching to a
directory, executing commands, and restoring the original CWD.
- Don't use runto_main; instead, use gdb_breakpoint + continue.
Simon mentioned on IRC that, after the startup-with-shell feature has
been implemented on gdbserver, it is not possible to specify a
filename-only binary, like:
$ gdbserver :1234 a.out
/bin/bash: line 0: exec: a.out: not found
During startup program exited with code 127.
Exiting
This happens on systems where the current directory "." is not listed
in the PATH environment variable. Although including "." in the PATH
variable is a possible workaround, this can be considered a regression
because before startup-with-shell it was possible to use only the
filename (due to reason that gdbserver used "exec*" directly).
The idea of the patch is to verify if the program path provided by the
user (or by the remote protocol) contains a directory separator
character. If it doesn't, it means we're dealing with a filename-only
binary, so we call "gdb_abspath" to properly expand it and transform
it into a full path. Otherwise, we leave the program path untouched.
This mimicks the behaviour seen on GDB (look at "openp" and
"attach_inferior", for example).
I am also submitting a testcase which exercises the scenario described
above. This test requires gdbserver to be executed in a different CWD
than the original, so I also created a helper function, "with_cwd" (on
testsuite/lib/gdb.exp), which takes care of cd'ing into and out of the
specified dir.
Built and regtested on BuildBot, without regressions.
gdb/gdbserver/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
Simon Marchi <simon.marchi@polymtl.ca>
* common/common-utils.c: Include "sys/stat.h".
(is_regular_file): Move here from "source.c"; change return
type to "bool".
* common/common-utils.h (is_regular_file): New prototype.
* common/pathstuff.c (contains_dir_separator): New function.
* common/pathstuff.h (contains_dir_separator): New prototype.
* source.c: Don't include "sys/stat.h".
(is_regular_file): Move to "common/common-utils.c".
gdb/gdbserver/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
* server.c: Include "filenames.h" and "pathstuff.h".
(program_name): Delete variable.
(program_path): New anonymous class.
(get_exec_wrapper): Use "program_path" instead of
"program_name".
(handle_v_run): Likewise.
(captured_main): Likewise.
(process_serial_event): Likewise.
gdb/testsuite/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.server/abspath.exp: New file.
* lib/gdb.exp (with_cwd): New procedure.
---
gdb/common/common-utils.c | 32 ++++++++++++++++++++++
gdb/common/common-utils.h | 5 ++++
gdb/common/pathstuff.c | 14 ++++++++++
gdb/common/pathstuff.h | 4 +++
gdb/gdbserver/server.c | 53 +++++++++++++++++++++++++++---------
gdb/source.c | 34 -----------------------
gdb/testsuite/gdb.server/abspath.exp | 51 ++++++++++++++++++++++++++++++++++
gdb/testsuite/lib/gdb.exp | 24 ++++++++++++++++
8 files changed, 170 insertions(+), 47 deletions(-)
create mode 100644 gdb/testsuite/gdb.server/abspath.exp
Comments
On Tuesday, February 27 2018, I wrote:
> Simon mentioned on IRC that, after the startup-with-shell feature has
> been implemented on gdbserver, it is not possible to specify a
> filename-only binary, like:
>
> $ gdbserver :1234 a.out
> /bin/bash: line 0: exec: a.out: not found
> During startup program exited with code 127.
> Exiting
>
> This happens on systems where the current directory "." is not listed
> in the PATH environment variable. Although including "." in the PATH
> variable is a possible workaround, this can be considered a regression
> because before startup-with-shell it was possible to use only the
> filename (due to reason that gdbserver used "exec*" directly).
>
> The idea of the patch is to verify if the program path provided by the
> user (or by the remote protocol) contains a directory separator
> character. If it doesn't, it means we're dealing with a filename-only
> binary, so we call "gdb_abspath" to properly expand it and transform
> it into a full path. Otherwise, we leave the program path untouched.
> This mimicks the behaviour seen on GDB (look at "openp" and
> "attach_inferior", for example).
>
> I am also submitting a testcase which exercises the scenario described
> above. This test requires gdbserver to be executed in a different CWD
> than the original, so I also created a helper function, "with_cwd" (on
> testsuite/lib/gdb.exp), which takes care of cd'ing into and out of the
> specified dir.
This part is still giving me a few headaches. I've just noticed that
two builders reported the new test as FAIL. When I run it here, I can't
reproduce it, which makes me wonder that it's racy. I don't know if the
culprit is the new "with_cwd" logic or not. Curiously, both builders
reporting the failure are running on s390x
(Debian-s390x-native-extended-gdbserver-m64 and RHEL-s390x-m64).
Here's an excerpt of report from one of them,
Debian-s390x-native-extended-gdbserver-m64:
----------------
(gdb) file /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath
Reading symbols from /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath...done.
(gdb) set remote exec-file /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath
(gdb) set remote exec-file /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath
(gdb) disconnect
Ending remote debugging.
(gdb) PASS: gdb.server/abspath.exp: disconnect
Switching to directory /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath (saved CWD: /home/dje/debian-jessie-s390x-1/debian
-s390x-native-extended-gdbserver/build/gdb/testsuite).
spawn /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/../gdbserver/gdbserver --once :2347 abspath
Can't bind address: Address already in use.
Exiting
Port 2347 is already in use.
spawn /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/../gdbserver/gdbserver --once :2348 abspath
Process /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath created; pid = 21406
Listening on port 2348
target extended-remote localhost:2348
Remote debugging using localhost:2348
Reading /lib/ld64.so.1 from remote target...
warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
Reading /lib/ld64.so.1 from remote target...
Reading symbols from target:/lib/ld64.so.1...Reading /lib/ld-2.19.so from remote target...
Reading /lib/.debug/ld-2.19.so from remote target...
(no debugging symbols found)...done.
0x000003fffdfd9280 in ?? () from target:/lib/ld64.so.1
Protocol error: qXfer:btrace-conf (read-btrace-conf) conflicting enabled responses.
(gdb) break main
Breakpoint 1 at 0x800005bc: file /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.server/normal.c, line 23.
(gdb) continue
The program is not being run.
(gdb) FAIL: gdb.server/abspath.exp: continue to main (the program is no longer running)
Switching back to /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite.
----------------
They're both running the testsuite in parallel mode (-j8), but when I
run it locally in parallel I can't reproduce (even when I run in a
loop).
Anyway, I'll try to find out why this happens. I don't see how changing
the CWD in a test could impact its results, so I'm guessing there's
something else at play here.
Thanks,
On 2018-02-27 10:58 PM, Sergio Durigan Junior wrote:
> On Tuesday, February 27 2018, I wrote:
>
>> Simon mentioned on IRC that, after the startup-with-shell feature has
>> been implemented on gdbserver, it is not possible to specify a
>> filename-only binary, like:
>>
>> $ gdbserver :1234 a.out
>> /bin/bash: line 0: exec: a.out: not found
>> During startup program exited with code 127.
>> Exiting
>>
>> This happens on systems where the current directory "." is not listed
>> in the PATH environment variable. Although including "." in the PATH
>> variable is a possible workaround, this can be considered a regression
>> because before startup-with-shell it was possible to use only the
>> filename (due to reason that gdbserver used "exec*" directly).
>>
>> The idea of the patch is to verify if the program path provided by the
>> user (or by the remote protocol) contains a directory separator
>> character. If it doesn't, it means we're dealing with a filename-only
>> binary, so we call "gdb_abspath" to properly expand it and transform
>> it into a full path. Otherwise, we leave the program path untouched.
>> This mimicks the behaviour seen on GDB (look at "openp" and
>> "attach_inferior", for example).
>>
>> I am also submitting a testcase which exercises the scenario described
>> above. This test requires gdbserver to be executed in a different CWD
>> than the original, so I also created a helper function, "with_cwd" (on
>> testsuite/lib/gdb.exp), which takes care of cd'ing into and out of the
>> specified dir.
>
> This part is still giving me a few headaches. I've just noticed that
> two builders reported the new test as FAIL. When I run it here, I can't
> reproduce it, which makes me wonder that it's racy. I don't know if the
> culprit is the new "with_cwd" logic or not. Curiously, both builders
> reporting the failure are running on s390x
> (Debian-s390x-native-extended-gdbserver-m64 and RHEL-s390x-m64).
>
> Here's an excerpt of report from one of them,
> Debian-s390x-native-extended-gdbserver-m64:
>
> ----------------
> (gdb) file /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath
> Reading symbols from /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath...done.
> (gdb) set remote exec-file /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath
> (gdb) set remote exec-file /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath
> (gdb) disconnect
> Ending remote debugging.
> (gdb) PASS: gdb.server/abspath.exp: disconnect
> Switching to directory /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath (saved CWD: /home/dje/debian-jessie-s390x-1/debian
> -s390x-native-extended-gdbserver/build/gdb/testsuite).
> spawn /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/../gdbserver/gdbserver --once :2347 abspath
> Can't bind address: Address already in use.
> Exiting
> Port 2347 is already in use.
> spawn /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/../gdbserver/gdbserver --once :2348 abspath
> Process /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath created; pid = 21406
> Listening on port 2348
> target extended-remote localhost:2348
> Remote debugging using localhost:2348
> Reading /lib/ld64.so.1 from remote target...
> warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> Reading /lib/ld64.so.1 from remote target...
> Reading symbols from target:/lib/ld64.so.1...Reading /lib/ld-2.19.so from remote target...
> Reading /lib/.debug/ld-2.19.so from remote target...
> (no debugging symbols found)...done.
> 0x000003fffdfd9280 in ?? () from target:/lib/ld64.so.1
> Protocol error: qXfer:btrace-conf (read-btrace-conf) conflicting enabled responses.
> (gdb) break main
> Breakpoint 1 at 0x800005bc: file /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.server/normal.c, line 23.
> (gdb) continue
> The program is not being run.
> (gdb) FAIL: gdb.server/abspath.exp: continue to main (the program is no longer running)
> Switching back to /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver/build/gdb/testsuite.
> ----------------
>
> They're both running the testsuite in parallel mode (-j8), but when I
> run it locally in parallel I can't reproduce (even when I run in a
> loop).
>
> Anyway, I'll try to find out why this happens. I don't see how changing
> the CWD in a test could impact its results, so I'm guessing there's
> something else at play here.
>
> Thanks,
>
Maybe it's just because of this issue? Note the btrace-related
error message.
https://sourceware.org/ml/gdb-patches/2018-02/msg00352.html
On 2018-02-27 10:27 PM, Sergio Durigan Junior wrote:
> diff --git a/gdb/testsuite/gdb.server/abspath.exp b/gdb/testsuite/gdb.server/abspath.exp
> new file mode 100644
> index 0000000000..beb1d96209
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/abspath.exp
> @@ -0,0 +1,51 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2018 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/>.
> +
> +# Test that gdbserver performs path expansion/adjustment when we
> +# provide just a filename (without any path specifications) to it.
> +
> +load_lib gdbserver-support.exp
> +
> +standard_testfile normal.c
> +
> +if { [skip_gdbserver_tests] } {
> + return 0
> +}
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
> + return -1
> +}
> +
> +# Make sure we're disconnected, in case we're testing with an
> +# extended-remote board, therefore already connected.
> +gdb_test "disconnect" ".*"
> +
> +set target_exec [gdbserver_download_current_prog]
> +
> +# Switch to where $target_exec lives, and execute gdbserver from
> +# there.
> +with_cwd "[file dirname $target_exec]" {
> + set target_execname [file tail $target_exec]
> + set res [gdbserver_start "" $target_execname]
> +
> + set gdbserver_protocol [lindex $res 0]
> + set gdbserver_gdbport [lindex $res 1]
> + gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
> +
> + gdb_breakpoint main
> + gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
> +}
The patch LGTM, just a note about the test.
I think this won't work when testing with a remote target (wher gdbserver
is started on another machine. with_cwd will try to cd into an directory
that exists on the other machine, or an "empty string" directory (I am not sure
what gdbserver_download_current_prog returns), so it will probably crash.
It would complicate the test quite a bit to make it work with a remote
target, and wouldn't add much value: when testing with a remote target,
gdbserver is started with a filename-only argument (that's how I
stumbled on the issue). So if that breaks, all the other tests will fail.
A counter argument for that would be that the remote board file is subject
to change. if we change it so that it invokes gdbserver by passing
non-filename-only paths, the feature would not be tested anymore...
At the minimum, I think we need to skip this test is target is a remote
one ([is_remote target]). If you want to go further and make the test
work with a remote target, then go ahead, but I would be fine with
just skipping it.
Simon
Hello,
This ...
> > 0x000003fffdfd9280 in ?? () from target:/lib/ld64.so.1 Protocol error:
> > qXfer:btrace-conf (read-btrace-conf) conflicting enabled responses.
... is caused by a regression from removing the btrace_supported target
method for targets that do not define HAVE_LINUX_BTRACE.
A proposed fix is here:
https://sourceware.org/ml/gdb-patches/2018-02/msg00420.html
Maciej already noted the missing _() around string literals.
Regards,
Markus.
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Simon Marchi
> Sent: 28 February 2018 06:33
> To: Sergio Durigan Junior <sergiodj@redhat.com>; GDB Patches <gdb-
> patches@sourceware.org>
> Cc: Pedro Alves <palves@redhat.com>; Joel Brobecker
> <brobecker@adacore.com>
> Subject: Re: [PATCH v3 2/2] Make gdbserver work with filename-only binaries
>
> On 2018-02-27 10:58 PM, Sergio Durigan Junior wrote:
> > On Tuesday, February 27 2018, I wrote:
> >
> >> Simon mentioned on IRC that, after the startup-with-shell feature has
> >> been implemented on gdbserver, it is not possible to specify a
> >> filename-only binary, like:
> >>
> >> $ gdbserver :1234 a.out
> >> /bin/bash: line 0: exec: a.out: not found
> >> During startup program exited with code 127.
> >> Exiting
> >>
> >> This happens on systems where the current directory "." is not listed
> >> in the PATH environment variable. Although including "." in the PATH
> >> variable is a possible workaround, this can be considered a
> >> regression because before startup-with-shell it was possible to use
> >> only the filename (due to reason that gdbserver used "exec*" directly).
> >>
> >> The idea of the patch is to verify if the program path provided by
> >> the user (or by the remote protocol) contains a directory separator
> >> character. If it doesn't, it means we're dealing with a
> >> filename-only binary, so we call "gdb_abspath" to properly expand it
> >> and transform it into a full path. Otherwise, we leave the program path
> untouched.
> >> This mimicks the behaviour seen on GDB (look at "openp" and
> >> "attach_inferior", for example).
> >>
> >> I am also submitting a testcase which exercises the scenario
> >> described above. This test requires gdbserver to be executed in a
> >> different CWD than the original, so I also created a helper function,
> >> "with_cwd" (on testsuite/lib/gdb.exp), which takes care of cd'ing
> >> into and out of the specified dir.
> >
> > This part is still giving me a few headaches. I've just noticed that
> > two builders reported the new test as FAIL. When I run it here, I
> > can't reproduce it, which makes me wonder that it's racy. I don't
> > know if the culprit is the new "with_cwd" logic or not. Curiously,
> > both builders reporting the failure are running on s390x
> > (Debian-s390x-native-extended-gdbserver-m64 and RHEL-s390x-m64).
> >
> > Here's an excerpt of report from one of them,
> > Debian-s390x-native-extended-gdbserver-m64:
> >
> > ----------------
> > (gdb) file
> > /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver
> > /build/gdb/testsuite/outputs/gdb.server/abspath/abspath
> > Reading symbols from /home/dje/debian-jessie-s390x-1/debian-s390x-native-
> extended-
> gdbserver/build/gdb/testsuite/outputs/gdb.server/abspath/abspath...done.
> > (gdb) set remote exec-file
> > /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver
> > /build/gdb/testsuite/outputs/gdb.server/abspath/abspath
> > (gdb) set remote exec-file
> > /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver
> > /build/gdb/testsuite/outputs/gdb.server/abspath/abspath
> > (gdb) disconnect
> > Ending remote debugging.
> > (gdb) PASS: gdb.server/abspath.exp: disconnect Switching to directory
> > /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver
> > /build/gdb/testsuite/outputs/gdb.server/abspath (saved CWD:
> > /home/dje/debian-jessie-s390x-1/debian
> > -s390x-native-extended-gdbserver/build/gdb/testsuite).
> > spawn
> > /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-
> gdbserver/build/gdb/testsuite/../gdbserver/gdbserver --once :2347 abspath
> Can't bind address: Address already in use.
> > Exiting
> > Port 2347 is already in use.
> > spawn
> > /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver
> > /build/gdb/testsuite/../gdbserver/gdbserver --once :2348 abspath
> > Process
> > /home/dje/debian-jessie-s390x-1/debian-s390x-native-extended-gdbserver
> > /build/gdb/testsuite/outputs/gdb.server/abspath/abspath created; pid =
> 21406 Listening on port 2348 target extended-remote localhost:2348 Remote
> debugging using localhost:2348 Reading /lib/ld64.so.1 from remote target...
> > warning: File transfers from remote targets can be slow. Use "set sysroot" to
> access files locally instead.
> > Reading /lib/ld64.so.1 from remote target...
> > Reading symbols from target:/lib/ld64.so.1...Reading /lib/ld-2.19.so from
> remote target...
> > Reading /lib/.debug/ld-2.19.so from remote target...
> > (no debugging symbols found)...done.
> > 0x000003fffdfd9280 in ?? () from target:/lib/ld64.so.1 Protocol error:
> > qXfer:btrace-conf (read-btrace-conf) conflicting enabled responses.
> > (gdb) break main
> > Breakpoint 1 at 0x800005bc: file /home/dje/debian-jessie-s390x-1/debian-
> s390x-native-extended-gdbserver/build/gdb/testsuite/../../../binutils-
> gdb/gdb/testsuite/gdb.server/normal.c, line 23.
> > (gdb) continue
> > The program is not being run.
> > (gdb) FAIL: gdb.server/abspath.exp: continue to main (the program is
> > no longer running) Switching back to /home/dje/debian-jessie-s390x-1/debian-
> s390x-native-extended-gdbserver/build/gdb/testsuite.
> > ----------------
> >
> > They're both running the testsuite in parallel mode (-j8), but when I
> > run it locally in parallel I can't reproduce (even when I run in a
> > loop).
> >
> > Anyway, I'll try to find out why this happens. I don't see how
> > changing the CWD in a test could impact its results, so I'm guessing
> > there's something else at play here.
> >
> > Thanks,
> >
>
> Maybe it's just because of this issue? Note the btrace-related error message.
>
> https://sourceware.org/ml/gdb-patches/2018-02/msg00352.html
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
On Wednesday, February 28 2018, Simon Marchi wrote:
> On 2018-02-27 10:27 PM, Sergio Durigan Junior wrote:
>> diff --git a/gdb/testsuite/gdb.server/abspath.exp b/gdb/testsuite/gdb.server/abspath.exp
>> new file mode 100644
>> index 0000000000..beb1d96209
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.server/abspath.exp
>> @@ -0,0 +1,51 @@
>> +# This testcase is part of GDB, the GNU debugger.
>> +
>> +# Copyright 2018 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/>.
>> +
>> +# Test that gdbserver performs path expansion/adjustment when we
>> +# provide just a filename (without any path specifications) to it.
>> +
>> +load_lib gdbserver-support.exp
>> +
>> +standard_testfile normal.c
>> +
>> +if { [skip_gdbserver_tests] } {
>> + return 0
>> +}
>> +
>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
>> + return -1
>> +}
>> +
>> +# Make sure we're disconnected, in case we're testing with an
>> +# extended-remote board, therefore already connected.
>> +gdb_test "disconnect" ".*"
>> +
>> +set target_exec [gdbserver_download_current_prog]
>> +
>> +# Switch to where $target_exec lives, and execute gdbserver from
>> +# there.
>> +with_cwd "[file dirname $target_exec]" {
>> + set target_execname [file tail $target_exec]
>> + set res [gdbserver_start "" $target_execname]
>> +
>> + set gdbserver_protocol [lindex $res 0]
>> + set gdbserver_gdbport [lindex $res 1]
>> + gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
>> +
>> + gdb_breakpoint main
>> + gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
>> +}
>
> The patch LGTM, just a note about the test.
>
> I think this won't work when testing with a remote target (wher gdbserver
> is started on another machine. with_cwd will try to cd into an directory
> that exists on the other machine, or an "empty string" directory (I am not sure
> what gdbserver_download_current_prog returns), so it will probably crash.
>
> It would complicate the test quite a bit to make it work with a remote
> target, and wouldn't add much value: when testing with a remote target,
> gdbserver is started with a filename-only argument (that's how I
> stumbled on the issue). So if that breaks, all the other tests will fail.
>
> A counter argument for that would be that the remote board file is subject
> to change. if we change it so that it invokes gdbserver by passing
> non-filename-only paths, the feature would not be tested anymore...
>
> At the minimum, I think we need to skip this test is target is a remote
> one ([is_remote target]). If you want to go further and make the test
> work with a remote target, then go ahead, but I would be fine with
> just skipping it.
Thanks, Simon. I'll skip the test for remote targets for now. I'll
push it afterwards.
Thanks,
On Wednesday, February 28 2018, Markus T. Metzger wrote:
> Hello,
>
> This ...
>
>> > 0x000003fffdfd9280 in ?? () from target:/lib/ld64.so.1 Protocol error:
>> > qXfer:btrace-conf (read-btrace-conf) conflicting enabled responses.
>
> ... is caused by a regression from removing the btrace_supported target
> method for targets that do not define HAVE_LINUX_BTRACE.
>
> A proposed fix is here:
> https://sourceware.org/ml/gdb-patches/2018-02/msg00420.html
>
> Maciej already noted the missing _() around string literals.
Thanks, Markus and Simon. Indeed, the problem lies elsewhere.
On Wednesday, February 28 2018, I wrote:
> On Wednesday, February 28 2018, Simon Marchi wrote:
>
>> On 2018-02-27 10:27 PM, Sergio Durigan Junior wrote:
>>> diff --git a/gdb/testsuite/gdb.server/abspath.exp b/gdb/testsuite/gdb.server/abspath.exp
>>> new file mode 100644
>>> index 0000000000..beb1d96209
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.server/abspath.exp
>>> @@ -0,0 +1,51 @@
>>> +# This testcase is part of GDB, the GNU debugger.
>>> +
>>> +# Copyright 2018 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/>.
>>> +
>>> +# Test that gdbserver performs path expansion/adjustment when we
>>> +# provide just a filename (without any path specifications) to it.
>>> +
>>> +load_lib gdbserver-support.exp
>>> +
>>> +standard_testfile normal.c
>>> +
>>> +if { [skip_gdbserver_tests] } {
>>> + return 0
>>> +}
>>> +
>>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
>>> + return -1
>>> +}
>>> +
>>> +# Make sure we're disconnected, in case we're testing with an
>>> +# extended-remote board, therefore already connected.
>>> +gdb_test "disconnect" ".*"
>>> +
>>> +set target_exec [gdbserver_download_current_prog]
>>> +
>>> +# Switch to where $target_exec lives, and execute gdbserver from
>>> +# there.
>>> +with_cwd "[file dirname $target_exec]" {
>>> + set target_execname [file tail $target_exec]
>>> + set res [gdbserver_start "" $target_execname]
>>> +
>>> + set gdbserver_protocol [lindex $res 0]
>>> + set gdbserver_gdbport [lindex $res 1]
>>> + gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
>>> +
>>> + gdb_breakpoint main
>>> + gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
>>> +}
>>
>> The patch LGTM, just a note about the test.
>>
>> I think this won't work when testing with a remote target (wher gdbserver
>> is started on another machine. with_cwd will try to cd into an directory
>> that exists on the other machine, or an "empty string" directory (I am not sure
>> what gdbserver_download_current_prog returns), so it will probably crash.
>>
>> It would complicate the test quite a bit to make it work with a remote
>> target, and wouldn't add much value: when testing with a remote target,
>> gdbserver is started with a filename-only argument (that's how I
>> stumbled on the issue). So if that breaks, all the other tests will fail.
>>
>> A counter argument for that would be that the remote board file is subject
>> to change. if we change it so that it invokes gdbserver by passing
>> non-filename-only paths, the feature would not be tested anymore...
>>
>> At the minimum, I think we need to skip this test is target is a remote
>> one ([is_remote target]). If you want to go further and make the test
>> work with a remote target, then go ahead, but I would be fine with
>> just skipping it.
>
> Thanks, Simon. I'll skip the test for remote targets for now. I'll
> push it afterwards.
Pushed.
25e3c82c0e927398e759e2d5e35623012b8683f7
Thanks,
@@ -20,6 +20,7 @@
#include "common-defs.h"
#include "common-utils.h"
#include "host-defs.h"
+#include <sys/stat.h>
#include <ctype.h>
/* The xmalloc() (libiberty.h) family of memory management routines.
@@ -408,3 +409,34 @@ stringify_argv (const std::vector<char *> &args)
return ret;
}
+
+/* See common/common-utils.h. */
+
+bool
+is_regular_file (const char *name, int *errno_ptr)
+{
+ struct stat st;
+ const int status = stat (name, &st);
+
+ /* Stat should never fail except when the file does not exist.
+ If stat fails, analyze the source of error and return true
+ unless the file does not exist, to avoid returning false results
+ on obscure systems where stat does not work as expected. */
+
+ if (status != 0)
+ {
+ if (errno != ENOENT)
+ return true;
+ *errno_ptr = ENOENT;
+ return false;
+ }
+
+ if (S_ISREG (st.st_mode))
+ return true;
+
+ if (S_ISDIR (st.st_mode))
+ *errno_ptr = EISDIR;
+ else
+ *errno_ptr = EINVAL;
+ return false;
+}
@@ -146,4 +146,9 @@ in_inclusive_range (T value, T low, T high)
return value >= low && value <= high;
}
+/* Return true if the file NAME exists and is a regular file.
+ If the result is false then *ERRNO_PTR is set to a useful value assuming
+ we're expecting a regular file. */
+extern bool is_regular_file (const char *name, int *errno_ptr);
+
#endif
@@ -140,3 +140,17 @@ gdb_abspath (const char *path)
? "" : SLASH_STRING,
path, (char *) NULL));
}
+
+/* See common/pathstuff.h. */
+
+bool
+contains_dir_separator (const char *path)
+{
+ for (; *path != '\0'; path++)
+ {
+ if (IS_DIR_SEPARATOR (*path))
+ return true;
+ }
+
+ return false;
+}
@@ -46,4 +46,8 @@ extern gdb::unique_xmalloc_ptr<char>
extern gdb::unique_xmalloc_ptr<char> gdb_abspath (const char *path);
+/* Return whether PATH contains a directory separator character. */
+
+extern bool contains_dir_separator (const char *path);
+
#endif /* PATHSTUFF_H */
@@ -39,6 +39,8 @@
#include "common-inferior.h"
#include "job-control.h"
#include "environ.h"
+#include "filenames.h"
+#include "pathstuff.h"
#include "common/selftest.h"
@@ -112,7 +114,35 @@ static int vCont_supported;
space randomization feature before starting an inferior. */
int disable_randomization = 1;
-static char *program_name = NULL;
+static struct {
+ /* Set the PROGRAM_PATH. Here we adjust the path of the provided
+ binary if needed. */
+ void set (gdb::unique_xmalloc_ptr<char> &&path)
+ {
+ m_path = std::move (path);
+
+ /* Make sure we're using the absolute path of the inferior when
+ creating it. */
+ if (!contains_dir_separator (m_path.get ()))
+ {
+ int reg_file_errno;
+
+ /* Check if the file is in our CWD. If it is, then we prefix
+ its name with CURRENT_DIRECTORY. Otherwise, we leave the
+ name as-is because we'll try searching for it in $PATH. */
+ if (is_regular_file (m_path.get (), ®_file_errno))
+ m_path = gdb_abspath (m_path.get ());
+ }
+ }
+
+ /* Return the PROGRAM_PATH. */
+ char *get ()
+ { return m_path.get (); }
+
+private:
+ /* The program name, adjusted if needed. */
+ gdb::unique_xmalloc_ptr<char> m_path;
+} program_path;
static std::vector<char *> program_args;
static std::string wrapper_argv;
@@ -269,10 +299,10 @@ get_exec_wrapper ()
char *
get_exec_file (int err)
{
- if (err && program_name == NULL)
+ if (err && program_path.get () == NULL)
error (_("No executable file specified."));
- return program_name;
+ return program_path.get ();
}
/* See server.h. */
@@ -3003,7 +3033,7 @@ handle_v_run (char *own_buf)
{
/* GDB didn't specify a program to run. Use the program from the
last run with the new argument list. */
- if (program_name == NULL)
+ if (program_path.get () == NULL)
{
write_enn (own_buf);
free_vector_argv (new_argv);
@@ -3011,16 +3041,13 @@ handle_v_run (char *own_buf)
}
}
else
- {
- xfree (program_name);
- program_name = new_program_name;
- }
+ program_path.set (gdb::unique_xmalloc_ptr<char> (new_program_name));
/* Free the old argv and install the new one. */
free_vector_argv (program_args);
program_args = new_argv;
- create_inferior (program_name, program_args);
+ create_inferior (program_path.get (), program_args);
if (last_status.kind == TARGET_WAITKIND_STOPPED)
{
@@ -3765,13 +3792,13 @@ captured_main (int argc, char *argv[])
int i, n;
n = argc - (next_arg - argv);
- program_name = xstrdup (next_arg[0]);
+ program_path.set (gdb::unique_xmalloc_ptr<char> (xstrdup (next_arg[0])));
for (i = 1; i < n; i++)
program_args.push_back (xstrdup (next_arg[i]));
program_args.push_back (NULL);
/* Wait till we are at first instruction in program. */
- create_inferior (program_name, program_args);
+ create_inferior (program_path.get (), program_args);
/* We are now (hopefully) stopped at the first instruction of
the target process. This assumes that the target process was
@@ -4288,9 +4315,9 @@ process_serial_event (void)
fprintf (stderr, "GDBserver restarting\n");
/* Wait till we are at 1st instruction in prog. */
- if (program_name != NULL)
+ if (program_path.get () != NULL)
{
- create_inferior (program_name, program_args);
+ create_inferior (program_path.get (), program_args);
if (last_status.kind == TARGET_WAITKIND_STOPPED)
{
@@ -29,7 +29,6 @@
#include "filestuff.h"
#include <sys/types.h>
-#include <sys/stat.h>
#include <fcntl.h>
#include "gdbcore.h"
#include "gdb_regex.h"
@@ -670,39 +669,6 @@ info_source_command (const char *ignore, int from_tty)
}
-/* Return True if the file NAME exists and is a regular file.
- If the result is false then *ERRNO_PTR is set to a useful value assuming
- we're expecting a regular file. */
-
-static int
-is_regular_file (const char *name, int *errno_ptr)
-{
- struct stat st;
- const int status = stat (name, &st);
-
- /* Stat should never fail except when the file does not exist.
- If stat fails, analyze the source of error and return True
- unless the file does not exist, to avoid returning false results
- on obscure systems where stat does not work as expected. */
-
- if (status != 0)
- {
- if (errno != ENOENT)
- return 1;
- *errno_ptr = ENOENT;
- return 0;
- }
-
- if (S_ISREG (st.st_mode))
- return 1;
-
- if (S_ISDIR (st.st_mode))
- *errno_ptr = EISDIR;
- else
- *errno_ptr = EINVAL;
- return 0;
-}
-
/* Open a file named STRING, searching path PATH (dir names sep by some char)
using mode MODE in the calls to open. You cannot use this function to
create files (O_CREAT).
new file mode 100644
@@ -0,0 +1,51 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2018 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/>.
+
+# Test that gdbserver performs path expansion/adjustment when we
+# provide just a filename (without any path specifications) to it.
+
+load_lib gdbserver-support.exp
+
+standard_testfile normal.c
+
+if { [skip_gdbserver_tests] } {
+ return 0
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
+ return -1
+}
+
+# Make sure we're disconnected, in case we're testing with an
+# extended-remote board, therefore already connected.
+gdb_test "disconnect" ".*"
+
+set target_exec [gdbserver_download_current_prog]
+
+# Switch to where $target_exec lives, and execute gdbserver from
+# there.
+with_cwd "[file dirname $target_exec]" {
+ set target_execname [file tail $target_exec]
+ set res [gdbserver_start "" $target_execname]
+
+ set gdbserver_protocol [lindex $res 0]
+ set gdbserver_gdbport [lindex $res 1]
+ gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
+
+ gdb_breakpoint main
+ gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
+}
@@ -2054,6 +2054,30 @@ proc save_vars { vars body } {
}
}
+# Run tests in BODY with the current working directory (CWD) set to
+# DIR. When BODY is finished, restore the original CWD. Return the
+# result of BODY.
+#
+# This procedure doesn't check if DIR is a valid directory, so you
+# have to make sure of that.
+
+proc with_cwd { dir body } {
+ set saved_dir [pwd]
+ verbose -log "Switching to directory $dir (saved CWD: $saved_dir)."
+ cd $dir
+
+ set code [catch {uplevel 1 $body} result]
+
+ verbose -log "Switching back to $saved_dir."
+ cd $saved_dir
+
+ if {$code == 1} {
+ global errorInfo errorCode
+ return -code $code -errorinfo $errorInfo -errorcode $errorCode $result
+ } else {
+ return -code $code $result
+ }
+}
# Run tests in BODY with GDB prompt and variable $gdb_prompt set to
# PROMPT. When BODY is finished, restore GDB prompt and variable