[PATCH] Fix access to uninitialized variable in fill_in_stop_func

[PATCH] Fix access to uninitialized variable in fill_in_stop_func

[Pedro Franco de Carvalho]

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(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index a9588f896a..15c778c7f3 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -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)
 	{
-- 
2.20.1

Re: [PATCH] Fix access to uninitialized variable in fill_in_stop_func

[Tom Tromey]

>>>>> "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

Re: [PATCH] Fix access to uninitialized variable in fill_in_stop_func

[Kevin Buettner]

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

[PATCH] Fix access to uninitialized variable in fill_in_stop_func

[Pedro Franco de Carvalho]

This patch changes find_pc_partial_function so that *block is set to
nullptr when it fails, so that fill_in_stop_func won't access an
uninitialized variable.

gdb/ChangeLog:
YYYY-MM-DD  Pedro Franco de Carvalho  <pedromfc@linux.ibm.com>

	* blockframe.c (find_pc_partial_function): Set *block to nullptr
          when the function fails.
---
 gdb/blockframe.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index fe7807b87a..4462274f46 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -331,6 +331,8 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
 	*address = 0;
       if (endaddr != NULL)
 	*endaddr = 0;
+      if (block != nullptr)
+	*block = nullptr;
       return 0;
     }
 
-- 
2.20.1

Re: [PATCH] Fix access to uninitialized variable in fill_in_stop_func

[Kevin Buettner]

On Fri,  9 Aug 2019 16:13:13 -0300
Pedro Franco de Carvalho <pedromfc@linux.ibm.com> wrote:

> This patch changes find_pc_partial_function so that *block is set to
> nullptr when it fails, so that fill_in_stop_func won't access an
> uninitialized variable.
> 
> gdb/ChangeLog:
> YYYY-MM-DD  Pedro Franco de Carvalho  <pedromfc@linux.ibm.com>
> 
> 	* blockframe.c (find_pc_partial_function): Set *block to nullptr
>           when the function fails.

Okay.

Kevin

Re: [PATCH] Fix access to uninitialized variable in fill_in_stop_func

[Pedro Franco de Carvalho]

Kevin Buettner <kevinb@redhat.com> writes:

> Okay.

Thanks! Pushed.

--
Pedro Franco de Carvalho

Re: [PATCH] Fix access to uninitialized variable in fill_in_stop_func

[Tom Tromey]

>>>>> "Pedro" == Pedro Franco de Carvalho <pedromfc@linux.ibm.com> writes:

Pedro> This patch changes find_pc_partial_function so that *block is set to
Pedro> nullptr when it fails, so that fill_in_stop_func won't access an
Pedro> uninitialized variable.

Pedro> gdb/ChangeLog:
Pedro> YYYY-MM-DD  Pedro Franco de Carvalho  <pedromfc@linux.ibm.com>

Pedro> 	* blockframe.c (find_pc_partial_function): Set *block to nullptr
Pedro>           when the function fails.

Thanks.  This is ok.

Tom