diff mbox

[FYI/pushed] (Ada/tasking) fix array or string index out of range warning

Message ID 1541626357-41376-3-git-send-email-brobecker@adacore.com
State New
Headers show

Commit Message

Joel Brobecker Nov. 7, 2018, 9:32 p.m. UTC
A recent change in the compiler highlighted a small weakness in
the function reading the contents of the Ada Task Control Block
(ATCB -- the data that allows us to inspect Ada tasks). As a result,
anytime we read it, we started getting some warnings. For instance,
using the gdb.ada/tasks.exp testcase...

        $ gnatmake -g foo.adb
        $ gdb foo
        (gdb) b foo.adb:60
        Breakpoint 1 at 0x403e07: file foo.adb, line 60.
        (gdb) run
        [...]
        Thread 1 "foo" hit Breakpoint 1, foo () at foo.adb:60
        60         for J in Task_List'Range loop  -- STOP_HERE

... we can see that the "info tasks" command produces some warnings,
followed by the correct output.

        (gdb) info tasks
  !! ->  warning: array or string index out of range
  !! ->  warning: array or string index out of range
  !! ->  warning: array or string index out of range
  !! ->  warning: array or string index out of range
           ID       TID P-ID Pri State                  Name
        *   1    654050       48 Runnable               main_task
            2    654ef0    1  48 Accept or Select Term  task_list(1)
            3    658680    1  48 Accept or Select Term  task_list(2)
            4    65be10    1  48 Accept or Select Term  task_list(3)

The problem comes from the fact that read_atcb, the function responsible
for loading the contents of the ATCB, blindly tries to read some data
which is only relevant when a task is waiting for another task on
an entry call. A comment in that code's section gives a hint as to
how the information is meant to be decoded:

      /* Let My_ATCB be the Ada task control block of a task calling the
         entry of another task; then the Task_Id of the called task is
         in My_ATCB.Entry_Calls (My_ATCB.ATC_Nesting_Level).Called_Task.  */

What the comment shows is that, to get the Id of the task being called,
one has to go through the entry calls field, which is an array pointer.
Up to now, we were lucky that, for tasks that are _not_ waiting on an
entry call, its ATCB atc_nesting_level used to be set to 1, and so
we were able to silently read some irrelevant data. But a recent change
now causes this field to be zero instead, and this triggers the warning,
since we are now trying to read outside of the array's range (arrays
in Ada often start at index 1, as is the case here).

We avoid this issue by simply only reading that data when the data
is actually known to be relevant (state == Entry_Caller_Sleep).

This, in turn, allows us to simplify a bit the use of the task_info->state
field, where we no longer need to check task the task has a state equal
to Entry_Caller_Sleep before using this field. Indeed, with this new
approach, we now know that, unless task_info->state == Entry_Caller_Sleep,
the state is now guaranteed to be zero. In other words, we no longer set
task_info->called_task to some random value, forcing to check the task's
state first as a way to verify that the data is not random.

gdb/ChangeLog:

        * ada-lang.c (read_atcb): Only set task_info->called_task if
        task_info->state == Entry_Caller_Sleep.
        (print_ada_task_info): Do not check task_info->state before
        checking task_info->called_task.
        (info_task): Likewise.
---
 gdb/ChangeLog   |  8 ++++++++
 gdb/ada-tasks.c | 12 ++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c94321b..9c277f5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@ 
 2018-11-07  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-lang.c (read_atcb): Only set task_info->called_task if
+	task_info->state == Entry_Caller_Sleep.
+	(print_ada_task_info): Do not check task_info->state before
+	checking task_info->called_task.
+	(info_task): Likewise.
+
+2018-11-07  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-tasks.c (read_atcb): Clear task_info before computing
 	the value of each of its fields.
 
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 5b97e93..71af616 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -709,10 +709,11 @@  read_atcb (CORE_ADDR task_id, struct ada_task_info *task_info)
       value_as_address (value_field (common_value,
 				     pspace_data->atcb_fieldno.parent));
 
-  /* If the ATCB contains some information about entry calls, then
-     compute the "called_task" as well.  Otherwise, zero.  */
+  /* If the task is in an entry call waiting for another task,
+     then determine which task it is.  */
 
-  if (pspace_data->atcb_fieldno.atc_nesting_level > 0
+  if (task_info->state == Entry_Caller_Sleep
+      && pspace_data->atcb_fieldno.atc_nesting_level > 0
       && pspace_data->atcb_fieldno.entry_calls > 0)
     {
       /* Let My_ATCB be the Ada task control block of a task calling the
@@ -1126,8 +1127,7 @@  print_ada_task_info (struct ui_out *uiout,
 			  _("Accepting RV with %-4d"),
 			  get_task_number_from_id (task_info->caller_task,
 						   inf));
-      else if (task_info->state == Entry_Caller_Sleep
-	       && task_info->called_task)
+      else if (task_info->called_task)
 	uiout->field_fmt ("state",
 			  _("Waiting on RV with %-3d"),
 			  get_task_number_from_id (task_info->called_task,
@@ -1213,7 +1213,7 @@  info_task (struct ui_out *uiout, const char *taskno_str, struct inferior *inf)
         printf_filtered (_("State: Accepting rendezvous with %d"),
                          target_taskno);
       }
-    else if (task_info->state == Entry_Caller_Sleep && task_info->called_task)
+    else if (task_info->called_task)
       {
         target_taskno = get_task_number_from_id (task_info->called_task, inf);
         printf_filtered (_("State: Waiting on task %d's entry"),