diff mbox series

dwarf: properly update all_comp_units_without_ranges in stash_comp_unit

Message ID 20250918120424.418-2-oleg.tolmatcev@gmail.com
State New
Headers
Series dwarf: properly update all_comp_units_without_ranges in stash_comp_unit |

Commit Message

Oleg Tolmatcev Sept. 18, 2025, 12:04 p.m. UTC 
  each->next_unit_without_ranges = file->all_comp_units_without_ranges;
-	      file->all_comp_units_without_ranges = each->next_unit_without_ranges;

The second line of this code is probably a bug because it does nothing. It
was probably supposed to update "file->all_comp_units_without_ranges" to point
to "each" - the new head of the list.

Signed-off-by: oltolm <oleg.tolmatcev@gmail.com>
---
 bfd/dwarf2.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Jan Beulich Sept. 18, 2025, 2:40 p.m. UTC | #1 
On 18.09.2025 14:04, oltolm wrote:
>  	      each->next_unit_without_ranges = file->all_comp_units_without_ranges;
> -	      file->all_comp_units_without_ranges = each->next_unit_without_ranges;
> 
> The second line of this code is probably a bug because it does nothing. It
> was probably supposed to update "file->all_comp_units_without_ranges" to point
> to "each" - the new head of the list.

I agree, yet ...

> --- a/bfd/dwarf2.c
> +++ b/bfd/dwarf2.c
> @@ -5635,17 +5635,19 @@ stash_comp_unit (struct dwarf2_debug *stash, struct dwarf2_debug_file *file)
>  			     (splay_tree_value)each);
>  
>  	  if (file->all_comp_units)
> -	    file->all_comp_units->prev_unit = each;
> +	    {
> +	      file->all_comp_units->prev_unit = each;
> +	      each->next_unit = file->all_comp_units;
> +	    }
>  	  else
>  	    file->last_comp_unit = each;
>  
> -	  each->next_unit = file->all_comp_units;
>  	  file->all_comp_units = each;

... why this part of the change? The original code was correct afaics, while
the new code is correct only as long as what each points to starts out zero-
initialized (which right now it does, but which we may better not depend upon
here when it's easy to avoid such a dependency).

>  	  if (each->arange.high == 0)
>  	    {
>  	      each->next_unit_without_ranges = file->all_comp_units_without_ranges;
> -	      file->all_comp_units_without_ranges = each->next_unit_without_ranges;
> +	      file->all_comp_units_without_ranges = each;
>  	    }

As I assume the patch will need committing on your behalf, I could easily
reduce it to just this part while committing, provided you agree.

Jan
Oleg Tolmatcev Sept. 18, 2025, 3:23 p.m. UTC | #2 
Am Do., 18. Sept. 2025 um 16:40 Uhr schrieb Jan Beulich <jbeulich@suse.com>:

> ... why this part of the change? The original code was correct afaics, while
> the new code is correct only as long as what each points to starts out zero-
> initialized (which right now it does, but which we may better not depend upon
> here when it's easy to avoid such a dependency).

I made this change when trying to understand the code. Feel free to drop it.

> >         if (each->arange.high == 0)
> >           {
> >             each->next_unit_without_ranges = file->all_comp_units_without_ranges;
> > -           file->all_comp_units_without_ranges = each->next_unit_without_ranges;
> > +           file->all_comp_units_without_ranges = each;
> >           }
>
> As I assume the patch will need committing on your behalf, I could easily
> reduce it to just this part while committing, provided you agree.

I agree, thank you.
diff mbox series

Patch

diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index a62c952374..ab01958281 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -5635,17 +5635,19 @@  stash_comp_unit (struct dwarf2_debug *stash, struct dwarf2_debug_file *file)
 			     (splay_tree_value)each);
 
 	  if (file->all_comp_units)
-	    file->all_comp_units->prev_unit = each;
+	    {
+	      file->all_comp_units->prev_unit = each;
+	      each->next_unit = file->all_comp_units;
+	    }
 	  else
 	    file->last_comp_unit = each;
 
-	  each->next_unit = file->all_comp_units;
 	  file->all_comp_units = each;
 
 	  if (each->arange.high == 0)
 	    {
 	      each->next_unit_without_ranges = file->all_comp_units_without_ranges;
-	      file->all_comp_units_without_ranges = each->next_unit_without_ranges;
+	      file->all_comp_units_without_ranges = each;
 	    }
 
 	  file->info_ptr += length;