[PATCH] gas/Dwarf: properly skip zero-size functions

[PATCH] gas/Dwarf: properly skip zero-size functions

[Jan Beulich]

PR gas/29451

While out_debug_abbrev() properly skips such functions, out_debug_info()
mistakenly didn't. It needs to calculate the high_pc expression ahead of
time, in order to skip emitting any data for the function if the value
is zero.

The one case which would still leave a zero-size entry is when
symbol_get_obj(symp)->size ends up evaluating to zero. I hope we can
expect that to not be the case, otherwise we'd need to have a way to
post-process .debug_info contents between resolving expressions and
actually writing the data out to the file. Even then it wouldn't be
entirely obvious in which way to alter the data.

--- a/gas/dwarf2dbg.c
+++ b/gas/dwarf2dbg.c
@@ -2882,6 +2882,7 @@ out_debug_info (segT info_seg, segT abbr
 	{
 	  const char *name;
 	  size_t len;
+	  expressionS size = { .X_op = O_constant };
 
 	  /* Skip warning constructs (see above).  */
 	  if (symbol_get_bfdsym (symp)->flags & BSF_WARNING)
@@ -2895,6 +2896,18 @@ out_debug_info (segT info_seg, segT abbr
 	  if (!S_IS_DEFINED (symp) || !S_IS_FUNCTION (symp))
 	    continue;
 
+#if defined (OBJ_ELF) /* || defined (OBJ_MAYBE_ELF) */
+	  size.X_add_number = S_GET_SIZE (symp);
+	  if (size.X_add_number == 0 && IS_ELF
+	      && symbol_get_obj (symp)->size != NULL)
+	    {
+	      size.X_op = O_add;
+	      size.X_op_symbol = make_expr_symbol (symbol_get_obj (symp)->size);
+	    }
+#endif
+	  if (size.X_op == O_constant && size.X_add_number == 0)
+	    continue;
+
 	  subseg_set (str_seg, 0);
 	  name_sym = symbol_temp_new_now_octets ();
 	  name = S_GET_NAME (symp);
@@ -2920,29 +2933,17 @@ out_debug_info (segT info_seg, segT abbr
 	  emit_expr (&exp, sizeof_address);
 
 	  /* DW_AT_high_pc */
-	  exp.X_op = O_constant;
-#if defined (OBJ_ELF) /* || defined (OBJ_MAYBE_ELF) */
-	  exp.X_add_number = S_GET_SIZE (symp);
-	  if (exp.X_add_number == 0 && IS_ELF
-	      && symbol_get_obj (symp)->size != NULL)
-	    {
-	      exp.X_op = O_add;
-	      exp.X_op_symbol = make_expr_symbol (symbol_get_obj (symp)->size);
-	    }
-#else
-	  exp.X_add_number = 0;
-#endif
 	  if (DWARF2_VERSION < 4)
 	    {
-	      if (exp.X_op == O_constant)
-		exp.X_op = O_symbol;
-	      exp.X_add_symbol = symp;
-	      emit_expr (&exp, sizeof_address);
+	      if (size.X_op == O_constant)
+		size.X_op = O_symbol;
+	      size.X_add_symbol = symp;
+	      emit_expr (&size, sizeof_address);
 	    }
-	  else if (exp.X_op == O_constant)
-	    out_uleb128 (exp.X_add_number);
+	  else if (size.X_op == O_constant)
+	    out_uleb128 (size.X_add_number);
 	  else
-	    emit_leb128_expr (symbol_get_value_expression (exp.X_op_symbol), 0);
+	    emit_leb128_expr (symbol_get_value_expression (size.X_op_symbol), 0);
 	}
 
       /* End of children.  */

Re: [PATCH] gas/Dwarf: properly skip zero-size functions

[Nick Clifton]

Hi Jan,

> +	  expressionS size = { .X_op = O_constant };

Does this expression guarantee that the other fields in size are
initialised to zero ?  If not, then:

> +#endif
> +	  if (size.X_op == O_constant && size.X_add_number == 0)
> +	    continue;

size.X_add_number could be uninitialized here.

Apart from fixing that, (if I am right about designated initializers), the
rest of the patch looks fine to me.

Cheers
   Nick

Re: [PATCH] gas/Dwarf: properly skip zero-size functions

[Jan Beulich]

On 09.08.2022 16:51, Nick Clifton wrote:
>> +	  expressionS size = { .X_op = O_constant };
> 
> Does this expression guarantee that the other fields in size are
> initialised to zero ?

Designated initializers set all unmentioned fields to zero just like
initializers of static variables would do: "... all subobjects that
are not initialized explicitly shall be initialized implicitly the
same as objects that have static storage duration" (quoted from C99).

>  If not, then:
> 
>> +#endif
>> +	  if (size.X_op == O_constant && size.X_add_number == 0)
>> +	    continue;
> 
> size.X_add_number could be uninitialized here.
> 
> Apart from fixing that, (if I am right about designated initializers), the
> rest of the patch looks fine to me.

Thanks. I simply didn't want to commit it right away, but give
people already involved in the bug to potentially actually try it
out first. But I'm not going to wait too long with actually
committing it (and also putting it on the branch).

Jan

Re: [PATCH] gas/Dwarf: properly skip zero-size functions

[Martin Liška]

On 8/9/22 17:05, Jan Beulich via Binutils wrote:
> Thanks. I simply didn't want to commit it right away, but give
> people already involved in the bug to potentially actually try it
> out first. But I'm not going to wait too long with actually
> committing it (and also putting it on the branch).

Thanks for the patch.

I can only confirm it fixes the problematic assembly snippet isolated
from crti.S.

Cheers,
Martin