[PATCH] [gdb/build] Fix Wmaybe-uninitialized in tid_range_parser::get_tid_or_range

[PATCH] [gdb/build] Fix Wmaybe-uninitialized in tid_range_parser::get_tid_or_range

[Tom de Vries]

On aarch64-linux with an -O3 build, I run into:
...
gdb/tid-parse.c: In function ‘int tid_is_in_list(const char*, int, int, int)’:
gdb/tid-parse.c:375:39: warning: ‘tmp_thr_end’ may be used uninitialized \
  [-Wmaybe-uninitialized]
  374 |       if (tmp_inf == inf_num
      |           ~~~~~~~~~~~~~~~~~~
  375 |           && tmp_thr_start <= thr_num && thr_num <= tmp_thr_end)
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
gdb/tid-parse.c:370:35: note: ‘tmp_thr_end’ was declared here
  370 |       int tmp_inf, tmp_thr_start, tmp_thr_end;
      |                                   ^~~~~~~~~~~
...

This is a false positive.

The warning is triggered in the tail of function
tid_range_parser::get_tid_or_range:
...
  /* If we successfully parsed a thread number or finished parsing a
     thread range, switch back to assuming the next TID is
     inferior-qualified.  */
  if (!m_range_parser.in_range ())
    {
      m_state = STATE_INFERIOR;
      m_cur_tok = m_range_parser.cur_tok ();

      if (thr_end != NULL)
	*thr_end = *thr_start;
    }

  /* If we're midway through a range, and the caller wants the end
     value, return it and skip to the end of the range.  */
  if (thr_end != NULL
      && (m_state == STATE_THREAD_RANGE
          || m_state == STATE_STAR_RANGE))
    {
      *thr_end = m_range_parser.end_value ();

      skip_range ();
    }

  return (*inf_num != 0 && *thr_start != 0);
}
...

To understand what's happening, we need to know two things:
- m_state has enum type with 3 valid values: STATE_INFERIOR,
  STATE_THREAD_RANGE, STATE_STAR_RANGE, and
- at the start of the code snippet, m_state != STATE_INFERIOR.

The first lets us reduce this:
...
  if (thr_end != NULL
      && (m_state == STATE_THREAD_RANGE
          || m_state == STATE_STAR_RANGE))
...
to:
...
  if (thr_end != NULL && m_state != STATE_INFERIOR)
...

The second lets us reduce it to:
...
  else if (thr_end != NULL)
...
at which point the warning is no longer triggered.

And if we copy the return to the first if clause, we simply get:
...
  if (thr_end != NULL)
...

Do this simplification and get rid of the warning.

Tested on aarch64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34892
---
 gdb/tid-parse.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/tid-parse.c b/gdb/tid-parse.c
index 767a125974e..0cfd6737c14 100644
--- a/gdb/tid-parse.c
+++ b/gdb/tid-parse.c
@@ -302,13 +302,13 @@ tid_range_parser::get_tid_or_range (int *inf_num,
 
       if (thr_end != NULL)
 	*thr_end = *thr_start;
+
+      return (*inf_num != 0 && *thr_start != 0);
     }
 
   /* If we're midway through a range, and the caller wants the end
      value, return it and skip to the end of the range.  */
-  if (thr_end != NULL
-      && (m_state == STATE_THREAD_RANGE
-	  || m_state == STATE_STAR_RANGE))
+  if (thr_end != NULL)
     {
       *thr_end = m_range_parser.end_value ();
 

base-commit: aec50b130daf9c6882017a9d5424943fb41991e5
-- 
2.51.0

Re: [PATCH] [gdb/build] Fix Wmaybe-uninitialized in tid_range_parser::get_tid_or_range

[Simon Marchi]

On 10/27/26 5:49 AM, Tom de Vries wrote:
> On aarch64-linux with an -O3 build, I run into:
> ...
> gdb/tid-parse.c: In function ‘int tid_is_in_list(const char*, int, int, int)’:
> gdb/tid-parse.c:375:39: warning: ‘tmp_thr_end’ may be used uninitialized \
>   [-Wmaybe-uninitialized]
>   374 |       if (tmp_inf == inf_num
>       |           ~~~~~~~~~~~~~~~~~~
>   375 |           && tmp_thr_start <= thr_num && thr_num <= tmp_thr_end)
>       |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
> gdb/tid-parse.c:370:35: note: ‘tmp_thr_end’ was declared here
>   370 |       int tmp_inf, tmp_thr_start, tmp_thr_end;
>       |                                   ^~~~~~~~~~~
> ...
> 
> This is a false positive.
> 
> The warning is triggered in the tail of function
> tid_range_parser::get_tid_or_range:
> ...
>   /* If we successfully parsed a thread number or finished parsing a
>      thread range, switch back to assuming the next TID is
>      inferior-qualified.  */
>   if (!m_range_parser.in_range ())
>     {
>       m_state = STATE_INFERIOR;
>       m_cur_tok = m_range_parser.cur_tok ();
> 
>       if (thr_end != NULL)
> 	*thr_end = *thr_start;
>     }
> 
>   /* If we're midway through a range, and the caller wants the end
>      value, return it and skip to the end of the range.  */
>   if (thr_end != NULL
>       && (m_state == STATE_THREAD_RANGE
>           || m_state == STATE_STAR_RANGE))
>     {
>       *thr_end = m_range_parser.end_value ();
> 
>       skip_range ();
>     }
> 
>   return (*inf_num != 0 && *thr_start != 0);
> }
> ...
> 
> To understand what's happening, we need to know two things:
> - m_state has enum type with 3 valid values: STATE_INFERIOR,
>   STATE_THREAD_RANGE, STATE_STAR_RANGE, and
> - at the start of the code snippet, m_state != STATE_INFERIOR.
> 
> The first lets us reduce this:
> ...
>   if (thr_end != NULL
>       && (m_state == STATE_THREAD_RANGE
>           || m_state == STATE_STAR_RANGE))
> ...
> to:
> ...
>   if (thr_end != NULL && m_state != STATE_INFERIOR)
> ...
> 
> The second lets us reduce it to:
> ...
>   else if (thr_end != NULL)
> ...
> at which point the warning is no longer triggered.
> 
> And if we copy the return to the first if clause, we simply get:
> ...
>   if (thr_end != NULL)
> ...
> 
> Do this simplification and get rid of the warning.
> 
> Tested on aarch64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34892

Ok, I think I followed that, with the new structure it's more obvious to
the compiler that *thr_end is always set (if thr_end != nullptr).

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon