diff mbox series

Make sure DW_CFA_advance_loc4 is in the same frag

Message ID 20230810022140.3030-1-hejinyang@loongson.cn
State Accepted
Headers
Series Make sure DW_CFA_advance_loc4 is in the same frag |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Jinyang He Aug. 10, 2023, 2:21 a.m. UTC 
  The DW_CFA_advance_loc4 may be in different frags. Then fr_fix
may caused something wrong. Referenced by commit b9d8f5601bcf
("Re: Optimise away eh_frame advance_loc 0").

gas/ChangeLog:

	* ehopt.c (check_eh_frame): Don't allow DW_CFA_advance_loc4
	to be placed in a different frag to the rs_cfa.
---
BTW, it is a bug I triggered when compiling a LoongArch.asm file with
relaxation being enabled. I hope it could be merged to 2.41 as bug-fix.
 
 gas/ehopt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alan Modra Aug. 10, 2023, 7:51 a.m. UTC | #1 
On Thu, Aug 10, 2023 at 10:21:40AM +0800, Jinyang He wrote:
> The DW_CFA_advance_loc4 may be in different frags. Then fr_fix
> may caused something wrong. Referenced by commit b9d8f5601bcf
> ("Re: Optimise away eh_frame advance_loc 0").
> 
> gas/ChangeLog:
> 
> 	* ehopt.c (check_eh_frame): Don't allow DW_CFA_advance_loc4
> 	to be placed in a different frag to the rs_cfa.

OK.
Jan Beulich Aug. 10, 2023, 7:55 a.m. UTC | #2 
On 10.08.2023 04:21, Jinyang He wrote:
> The DW_CFA_advance_loc4 may be in different frags. Then fr_fix
> may caused something wrong. Referenced by commit b9d8f5601bcf
> ("Re: Optimise away eh_frame advance_loc 0").

I'm afraid I don't understand that earlier fix: frag_more(1) there
ought to guarantee fr_fix (once the frag is closed) to be >= 1.
It would then seem to me that ...

> --- a/gas/ehopt.c
> +++ b/gas/ehopt.c
> @@ -386,7 +386,7 @@ check_eh_frame (expressionS *exp, unsigned int *pnbytes)
>  	{
>  	  /* This might be a DW_CFA_advance_loc4.  Record the frag and the
>  	     position within the frag, so that we can change it later.  */
> -	  frag_grow (1);
> +	  frag_grow (1 + 4);
>  	  d->state = state_saw_loc4;
>  	  d->loc4_frag = frag_now;
>  	  d->loc4_fix = frag_now_fix ();

... here instead we also need

	  frag_more (1);
	  d->state = state_saw_loc4;
	  d->loc4_frag = frag_now;
	  d->loc4_fix = frag_now_fix () - 1;

Otoh if frags were always grown (there and here), couldn't we do away
with loc4_frag and loc4_fix?

As to your 2.41 question: You realize the release was already cut?
Putting this (or whichever else) fix there would be likely be okay, but
would have a real effect only if 2.41.1 was ever made.

Jan
diff mbox series

Patch

diff --git a/gas/ehopt.c b/gas/ehopt.c
index feea61b92..9d6606adf 100644
--- a/gas/ehopt.c
+++ b/gas/ehopt.c
@@ -386,7 +386,7 @@  check_eh_frame (expressionS *exp, unsigned int *pnbytes)
 	{
 	  /* This might be a DW_CFA_advance_loc4.  Record the frag and the
 	     position within the frag, so that we can change it later.  */
-	  frag_grow (1);
+	  frag_grow (1 + 4);
 	  d->state = state_saw_loc4;
 	  d->loc4_frag = frag_now;
 	  d->loc4_fix = frag_now_fix ();