diff mbox series

Avoid seg fault in sim/cris/sim-if.c

Message ID 006501dd21f7$5fa6ab60$1ef40220$@nextmovesoftware.com
State New
Headers
Series Avoid seg fault in sim/cris/sim-if.c |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Roger Sayle Aug. 1, 2026, 8:50 p.m. UTC 
  Typing "target sim" in GDB built with --target=cris-elf, crashes immediately
after being launched with "gdb ./a.out".  The issue is that STATE_PROG_ARGV
may return NULL, and this isn't tested before dereferencing the pointer.

If approved, please could someone commit this for me.
Thanks in advance.

2026-08-01  Roger Sayle  <roger@nextmovesoftware.com>

ChangeLog
	* sim/cris/sim-if.c (sim_open): Check that prog_argv isn't NULL.

Comments

Andrew Burgess Aug. 3, 2026, 12:10 p.m. UTC | #1 
"Roger Sayle" <roger@nextmovesoftware.com> writes:

> Typing "target sim" in GDB built with --target=cris-elf, crashes immediately
> after being launched with "gdb ./a.out".  The issue is that STATE_PROG_ARGV
> may return NULL, and this isn't tested before dereferencing the pointer.
>
> If approved, please could someone commit this for me.
> Thanks in advance.

I pushed this.

But also, I owe you an apology.  For some reason I was not able to apply
this patch directly from my email client to a git tree as usual, so I
ended up copying the contents by hand.  In the process I forgot to make
you the patch author, something I only spotted after pushing to master.

This was not intentional, and I really do apologise, I did not intend to
take credit for this fix.  Unfortunately, there's no simple way I can
fix this mistake.

I will try to take greater care in future.

Thanks,
Andrew


>
> 2026-08-01  Roger Sayle  <roger@nextmovesoftware.com>
>
> ChangeLog
> 	* sim/cris/sim-if.c (sim_open): Check that prog_argv isn't NULL.
>
>
> diff --git a/sim/cris/sim-if.c b/sim/cris/sim-if.c
> index 12c80983208..3babbb058b7 100644
> --- a/sim/cris/sim-if.c
> +++ b/sim/cris/sim-if.c
> @@ -770,8 +770,9 @@ sim_open (SIM_OPEN_KIND kind, host_callback *callback, struct bfd *abfd,
>        for (envc = 0; environ[envc] != NULL; envc++)
>  	len += strlen (environ[envc]) + 1;
>  
> -      for (i = 0; prog_argv[i] != NULL; my_argc++, i++)
> -	len += strlen (prog_argv[i]) + 1;
> +      if (prog_argv != NULL)
> +	for (i = 0; prog_argv[i] != NULL; my_argc++, i++)
> +	  len += strlen (prog_argv[i]) + 1;
>  
>        envstart = (envtop - len) & ~8191;
>
Hans-Peter Nilsson Aug. 10, 2026, 2:07 p.m. UTC | #2 
> From: "Roger Sayle" <roger@nextmovesoftware.com>
> Date: Mon, 3 Aug 2026 13:28:41 +0100

> Hi Andrew,
> No worries.  The one line change is obvious, and there's really nothing
> to take credit for.  I'm far more pleased that you've pushed a fix.
> Thank you very much for taking care of this.
> 
> Cheers,
> Roger
> --
> 
> > -----Original Message-----
> > From: Andrew Burgess <aburgess@redhat.com>
> > Sent: 03 August 2026 13:10
> > To: Roger Sayle <roger@nextmovesoftware.com>; gdb-patches@sourceware.org
> > Cc: 'Hans-Peter Nilsson' <hp@axis.com>
> > Subject: Re: [PATCH] Avoid seg fault in sim/cris/sim-if.c
> > 
> > "Roger Sayle" <roger@nextmovesoftware.com> writes:
> > 
> > > Typing "target sim" in GDB built with --target=cris-elf, crashes
> > > immediately after being launched with "gdb ./a.out".  The issue is
> > > that STATE_PROG_ARGV may return NULL, and this isn't tested before
> > dereferencing the pointer.
> > >
> > > If approved, please could someone commit this for me.
> > > Thanks in advance.
> > 
> > I pushed this.
> > 

[Apology for mortal sin elided]

> > Thanks,
> > Andrew
> > 
> > 
> > >
> > > 2026-08-01  Roger Sayle  <roger@nextmovesoftware.com>
> > >
> > > ChangeLog
> > > 	* sim/cris/sim-if.c (sim_open): Check that prog_argv isn't NULL.

Thank you both!  I've (obviously) never run "target sim" in gdb for
cris-elf.  That sin may be more grave than forgetting to assign
authorship when applying a one-line patch...

It was always on the TODO-list to wire up the hooks and plugging in
the pieces, to get the simulator running built-in with gdb.  I'm glad
this got it working.  If there's actually something more needed, then
at least this serves as defensive programming.  I see it was already
rightly committed while I was happily on vacation, so just a thank
you.

brgds, H-P
diff mbox series

Patch

diff --git a/sim/cris/sim-if.c b/sim/cris/sim-if.c
index 12c80983208..3babbb058b7 100644
--- a/sim/cris/sim-if.c
+++ b/sim/cris/sim-if.c
@@ -770,8 +770,9 @@  sim_open (SIM_OPEN_KIND kind, host_callback *callback, struct bfd *abfd,
       for (envc = 0; environ[envc] != NULL; envc++)
 	len += strlen (environ[envc]) + 1;
 
-      for (i = 0; prog_argv[i] != NULL; my_argc++, i++)
-	len += strlen (prog_argv[i]) + 1;
+      if (prog_argv != NULL)
+	for (i = 0; prog_argv[i] != NULL; my_argc++, i++)
+	  len += strlen (prog_argv[i]) + 1;
 
       envstart = (envtop - len) & ~8191;