Fix access to uninitialized variable in fill_in_stop_func
Commit Message
This patch changes fill_in_stop_func to check the return value of
find_pc_partial_function before accessing the block pointer that is only
written by find_pc_partial_function if it returns a success status.
gdb/ChangeLog:
YYYY-MM-DD Pedro Franco de Carvalho <pedromfc@linux.ibm.com>
* infrun.c (fill_in_stop_func): Use return value of
find_pc_partial_function, remove comment.
---
gdb/infrun.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
Comments
>>>>> "Pedro" == Pedro Franco de Carvalho <pedromfc@linux.ibm.com> writes:
Pedro> This patch changes fill_in_stop_func to check the return value of
Pedro> find_pc_partial_function before accessing the block pointer that is only
Pedro> written by find_pc_partial_function if it returns a success status.
Pedro> gdb/ChangeLog:
Pedro> YYYY-MM-DD Pedro Franco de Carvalho <pedromfc@linux.ibm.com>
Pedro> * infrun.c (fill_in_stop_func): Use return value of
Pedro> find_pc_partial_function, remove comment.
The comment for find_pc_partial_function says:
nullptr is used as a return value for *BLOCK if no block is found.
... which implies to me that the function was intended to set *BLOCK
unconditionally. Perhaps Kevin could say, as I think he added the block
parameter.
Tom
On Thu, 08 Aug 2019 11:14:22 -0600
Tom Tromey <tom@tromey.com> wrote:
> >>>>> "Pedro" == Pedro Franco de Carvalho <pedromfc@linux.ibm.com> writes:
>
> Pedro> This patch changes fill_in_stop_func to check the return value of
> Pedro> find_pc_partial_function before accessing the block pointer that is only
> Pedro> written by find_pc_partial_function if it returns a success status.
>
> Pedro> gdb/ChangeLog:
> Pedro> YYYY-MM-DD Pedro Franco de Carvalho <pedromfc@linux.ibm.com>
>
> Pedro> * infrun.c (fill_in_stop_func): Use return value of
> Pedro> find_pc_partial_function, remove comment.
>
> The comment for find_pc_partial_function says:
>
> nullptr is used as a return value for *BLOCK if no block is found.
>
> ... which implies to me that the function was intended to set *BLOCK
> unconditionally. Perhaps Kevin could say, as I think he added the block
> parameter.
Yes, that was the intent, but unfortunately, that's not what's happening.
find_pc_partial_function contains the following return path...
if (msymbol.minsym == NULL)
{
/* No available symbol. */
if (name != NULL)
*name = 0;
if (address != NULL)
*address = 0;
if (endaddr != NULL)
*endaddr = 0;
return 0;
}
...which does not set *BLOCK.
I think I'd prefer to see...
if (block != nullptr)
*block = nullptr;
...added to find_pc_partial_function instead.
Kevin
@@ -4117,13 +4117,12 @@ fill_in_stop_func (struct gdbarch *gdbarch,
{
const block *block;
- /* Don't care about return value; stop_func_start and stop_func_name
- will both be 0 if it doesn't work. */
- find_pc_partial_function (ecs->event_thread->suspend.stop_pc,
- &ecs->stop_func_name,
- &ecs->stop_func_start,
- &ecs->stop_func_end,
- &block);
+ bool status = (find_pc_partial_function
+ (ecs->event_thread->suspend.stop_pc,
+ &ecs->stop_func_name,
+ &ecs->stop_func_start,
+ &ecs->stop_func_end,
+ &block));
/* The call to find_pc_partial_function, above, will set
stop_func_start and stop_func_end to the start and end
@@ -4133,7 +4132,7 @@ fill_in_stop_func (struct gdbarch *gdbarch,
the function's start offset and entrypoint. Note that
stop_func_start is NOT advanced when in a range of a
non-contiguous block that does not contain the entry pc. */
- if (block != nullptr
+ if (status && block != nullptr
&& ecs->stop_func_start <= BLOCK_ENTRY_PC (block)
&& BLOCK_ENTRY_PC (block) < ecs->stop_func_end)
{