[PR,symtab/17911] Recognize bad file types
Commit Message
On 10/26/2015 11:44 PM, Doug Evans wrote:
> 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.]
>
Like so.
From 0becb0a09f6a18e4edfe35bf088a0dd41e949796 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 27 Oct 2015 12:54:22 +0000
Subject: [PATCH] source.c:openp: save/restore errno
openp's return is documented as:
~~~
If a file is found, return the descriptor.
Otherwise, return -1, with errno set for the last name we tried to open. */
~~~
By inspection, I noticed that there are function calls after the ones
that first set errno, and those may clobber errno. It's safer to save
errno when see an open fail, and restore it on exit.
Tested on x86_64 Fedora 20.
gdb/ChangeLog:
2015-10-27 Pedro Alves <palves@redhat.com>
* source.c (openp): New local 'last_errno'. Use it to
save/restore errno.
---
gdb/source.c | 7 +++++++
1 file changed, 7 insertions(+)
Comments
On Tue, Oct 27, 2015 at 6:11 AM, Pedro Alves <palves@redhat.com> wrote:
> On 10/26/2015 11:44 PM, Doug Evans wrote:
>> 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.]
>>
>
> Like so.
So you were referring to the existing code.
[One can see this bug easily enough, but it wasn't clear
whether you were saying my bug was introducing
another bug.]
gdb is always more broken than one imagines. :-)
LGTM.
On 10/27/2015 03:47 PM, Doug Evans wrote:
> On Tue, Oct 27, 2015 at 6:11 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 10/26/2015 11:44 PM, Doug Evans wrote:
>>> 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.]
>>>
>>
>> Like so.
>
> So you were referring to the existing code.
I wasn't ...
> [One can see this bug easily enough, but it wasn't clear
> whether you were saying my bug was introducing
> another bug.]
... because it actually first looked to me that it was your
patch that introduced the issue. So let's say your code
made it stand out. ;-)
> gdb is always more broken than one imagines. :-)
>
> LGTM.
I'll push in a bit.
Thanks,
Pedro Alves
@@ -746,6 +746,9 @@ openp (const char *path, int opts, const char *string,
struct cleanup *back_to;
int ix;
char *dir;
+ /* The errno set for the last name we tried to open (and
+ failed). */
+ int last_errno = 0;
/* The open syscall MODE parameter is not specified. */
gdb_assert ((mode & O_CREAT) == 0);
@@ -786,6 +789,7 @@ openp (const char *path, int opts, const char *string,
filename = NULL;
fd = -1;
}
+ last_errno = errno;
if (!(opts & OPF_SEARCH_IN_PATH))
for (i = 0; string[i]; i++)
@@ -808,6 +812,7 @@ openp (const char *path, int opts, const char *string,
alloclen = strlen (path) + strlen (string) + 2;
filename = (char *) alloca (alloclen);
fd = -1;
+ last_errno = ENOENT;
dir_vec = dirnames_to_char_ptr_vec (path);
back_to = make_cleanup_free_char_ptr_vec (dir_vec);
@@ -878,6 +883,7 @@ openp (const char *path, int opts, const char *string,
fd = gdb_open_cloexec (filename, mode, 0);
if (fd >= 0)
break;
+ last_errno = errno;
}
}
@@ -895,6 +901,7 @@ done:
*filename_opened = gdb_abspath (filename);
}
+ errno = last_errno;
return fd;
}