[PR,symtab/17911] Recognize bad file types
Commit Message
Hi.
I tripped over this this morning, and then found the bug report.
We came up with basically the same answer, but I like EINVAL
better than ENOEXEC if given neither a file nor a directory.
Tested on amd64-linux.
2015-09-19 Doug Evans <xdje42@gmail.com>
PR symtab/17911
* source.c (is_regular_file): Set errno.
(openp): Init errno to ENOENT before iterating over search path.
testsuite/
* gdb.base/bad-file.exp: New file.
Comments
On 09/19/2015 11:40 PM, Doug Evans wrote:
> diff --git a/gdb/source.c b/gdb/source.c
> index fab974c..34384fa 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -684,7 +684,10 @@ source_info (char *ignore, int from_tty)
> }
>
>
> -/* Return True if the file NAME exists and is a regular file. */
> +/* Return True if the file NAME exists and is a regular file.
> + If the result is false then errno is set to a useful value assuming we're
> + expecting a regular file. */
> +
> static int
> is_regular_file (const char *name)
> {
> @@ -699,7 +702,14 @@ is_regular_file (const char *name)
> if (status != 0)
> return (errno != ENOENT);
>
> - return S_ISREG (st.st_mode);
> + if (S_ISREG (st.st_mode))
> + return 1;
> +
> + if (S_ISDIR (st.st_mode))
> + errno = EISDIR;
> + else
> + errno = EINVAL;
> + return 0;
> }
>
> /* Open a file named STRING, searching path PATH (dir names sep by some char)
> @@ -785,6 +795,7 @@ openp (const char *path, int opts, const char *string,
> {
> filename = NULL;
> fd = -1;
> + /* Note: is_regular_file will have set errno appropriately. */
> }
>
> if (!(opts & OPF_SEARCH_IN_PATH))
> @@ -808,6 +819,7 @@ openp (const char *path, int opts, const char *string,
> alloclen = strlen (path) + strlen (string) + 2;
> filename = alloca (alloclen);
> fd = -1;
> + errno = ENOENT;
>
> dir_vec = dirnames_to_char_ptr_vec (path);
> back_to = make_cleanup_free_char_ptr_vec (dir_vec);
> @@ -879,6 +891,10 @@ openp (const char *path, int opts, const char *string,
> if (fd >= 0)
> break;
> }
> + else
> + {
> + /* Note: is_regular_file will have set errno appropriately. */
> + }
> }
Seems like there are function calls after these that may
clobber errno. I think it'd be safer to do the usual
save_errno = errno; / errno = save_errno; dance.
> +# Test passing bad files to gdb. PR symtab/17911
> +
> +# We watch for specific text for errno, so only test on systems we have
> +# confidence in the error text.
> +
> +if { ! [ishost "*-*-linux*"] } {
> + return 0
> +}
Same comment as to the other patch -- I think we should instead
remove this check and if the error message is different on other
hosts, then whoever cares about such hosts will adjust the
test. I don't think there will be that many cases of
different messages.
Otherwise LGTM.
Thanks,
Pedro Alves
Pedro Alves <palves@redhat.com> writes:
> Seems like there are function calls after these that may
> clobber errno. I think it'd be safer to do the usual
> save_errno = errno; / errno = save_errno; dance.
Does what you're saying apply to the current source base?
[I think so, just checking.
And if so, then let's fix that first.]
@@ -684,7 +684,10 @@ source_info (char *ignore, int from_tty)
}
-/* Return True if the file NAME exists and is a regular file. */
+/* Return True if the file NAME exists and is a regular file.
+ If the result is false then errno is set to a useful value assuming we're
+ expecting a regular file. */
+
static int
is_regular_file (const char *name)
{
@@ -699,7 +702,14 @@ is_regular_file (const char *name)
if (status != 0)
return (errno != ENOENT);
- return S_ISREG (st.st_mode);
+ if (S_ISREG (st.st_mode))
+ return 1;
+
+ if (S_ISDIR (st.st_mode))
+ errno = EISDIR;
+ else
+ errno = EINVAL;
+ return 0;
}
/* Open a file named STRING, searching path PATH (dir names sep by some char)
@@ -785,6 +795,7 @@ openp (const char *path, int opts, const char *string,
{
filename = NULL;
fd = -1;
+ /* Note: is_regular_file will have set errno appropriately. */
}
if (!(opts & OPF_SEARCH_IN_PATH))
@@ -808,6 +819,7 @@ openp (const char *path, int opts, const char *string,
alloclen = strlen (path) + strlen (string) + 2;
filename = alloca (alloclen);
fd = -1;
+ errno = ENOENT;
dir_vec = dirnames_to_char_ptr_vec (path);
back_to = make_cleanup_free_char_ptr_vec (dir_vec);
@@ -879,6 +891,10 @@ openp (const char *path, int opts, const char *string,
if (fd >= 0)
break;
}
+ else
+ {
+ /* Note: is_regular_file will have set errno appropriately. */
+ }
}
do_cleanups (back_to);
new file mode 100644
@@ -0,0 +1,56 @@
+# 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/>. */
+
+# Test passing bad files to gdb. PR symtab/17911
+
+# We watch for specific text for errno, so only test on systems we have
+# confidence in the error text.
+
+if { ! [ishost "*-*-linux*"] } {
+ return 0
+}
+
+# There is no such file, but we still use the normal mechanism to pick
+# its name and path.
+standard_testfile
+
+gdb_exit
+gdb_start
+
+set test "non-existent file"
+set bad_file $testfile
+remote_file host delete $bad_file
+gdb_test_multiple "file $bad_file" "$test" {
+ -re "No such file or directory.\[\r\n\]+$gdb_prompt $" {
+ pass $test
+ }
+}
+
+set test "directory"
+set bad_file [standard_output_file {}]
+remote_exec host "mkdir -p $bad_file"
+gdb_test_multiple "file $bad_file" "$test" {
+ -re "Is a directory.\[\r\n\]+$gdb_prompt $" {
+ pass $test
+ }
+}
+
+set test "neither file nor directory"
+set bad_file "/dev/null"
+gdb_test_multiple "file $bad_file" "$test" {
+ -re "Invalid argument.\[\r\n\]+$gdb_prompt $" {
+ pass $test
+ }
+}