* Re: [PATCH] gdb: sim: automatically pass down sysroot [not found] <1306440200-25087-1-git-send-email-vapier__8251.52545371584$1306440225$gmane$org@gentoo.org> @ 2011-05-27 17:47 ` Tom Tromey 2011-05-27 18:37 ` Mike Frysinger 2011-06-01 16:18 ` [PATCH/RFC] new argv handlers to help with sim argv building Mike Frysinger 0 siblings, 2 replies; 21+ messages in thread From: Tom Tromey @ 2011-05-27 17:47 UTC (permalink / raw) To: Mike Frysinger; +Cc: gdb-patches, toolchain-devel >>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes: Mike> 2011-05-26 Mike Frysinger <vapier@gentoo.org> Mike> * remote-sim.c (gdbsim_open): Add the strlen of " --sysroot=" and Mike> gdb_sysroot to the "len" variable. Append both to "arg_buf". It seems reasonable to me. Ok. Mike> + strcat (arg_buf, gdb_sysroot); It seems like this will give wrong results if the sysroot needs quoting. But, this problem could affect other arguments here. Offhand it seems like it would be better if this code directly built sim_argv rather than building a string and converting it, but I don't know if any odd issue is lurking. Tom ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-05-27 17:47 ` [PATCH] gdb: sim: automatically pass down sysroot Tom Tromey @ 2011-05-27 18:37 ` Mike Frysinger 2011-06-01 16:18 ` [PATCH/RFC] new argv handlers to help with sim argv building Mike Frysinger 1 sibling, 0 replies; 21+ messages in thread From: Mike Frysinger @ 2011-05-27 18:37 UTC (permalink / raw) To: Tom Tromey; +Cc: toolchain-devel, gdb-patches On Fri, May 27, 2011 at 13:46, Tom Tromey wrote: > Mike> + strcat (arg_buf, gdb_sysroot); > > It seems like this will give wrong results if the sysroot needs quoting. > But, this problem could affect other arguments here. > Offhand it seems like it would be better if this code directly built > sim_argv rather than building a string and converting it, but I don't > know if any odd issue is lurking. i thought of that, but since as you say other parts have the same issue, and i couldnt find a helper func that would sanely take care of things, i didnt bother -mike ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH/RFC] new argv handlers to help with sim argv building 2011-05-27 17:47 ` [PATCH] gdb: sim: automatically pass down sysroot Tom Tromey 2011-05-27 18:37 ` Mike Frysinger @ 2011-06-01 16:18 ` Mike Frysinger 2011-06-03 16:56 ` Tom Tromey 1 sibling, 1 reply; 21+ messages in thread From: Mike Frysinger @ 2011-06-01 16:18 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, toolchain-devel [-- Attachment #1: Type: Text/Plain, Size: 4228 bytes --] On Friday, May 27, 2011 13:46:59 Tom Tromey wrote: > Offhand it seems like it would be better if this code directly built > sim_argv rather than building a string and converting it, but I don't > know if any odd issue is lurking. i slapped this together. what do you think (ignoring missing docs) ? -mike --- libiberty/argv.c 13 Aug 2010 11:36:10 -0000 1.22 +++ libiberty/argv.c 1 Jun 2011 16:17:06 -0000 @@ -301,6 +301,56 @@ char **buildargv (const char *input) return (argv); } +char **appendargv (char **argv, const char *arg, ...) +{ + va_list ap; + int argc; + + for (argc = 0; argv[argc]; ++argc) + continue; + + va_start (ap, arg); + while (arg) + { + argv = (char **) realloc (argv, sizeof (*argv) * (argc + 2)); + argv[argc] = strdup (arg); + ++argc; + arg = va_arg (ap, const char *); + } + va_end (ap); + + if (argv) + argv[argc] = NULL; + + return argv; +} + +char **mergeargv (char **iargv, ...) +{ + va_list ap; + char **argv = NULL; + int i, argc = 0; + + va_start (ap, iargv); + while (iargv) + { + for (i = 0; iargv[i]; ++i) + { + argv = (char **) realloc (argv, sizeof (*argv) * (argc + 2)); + argv[argc] = strdup (iargv[i]); + ++argc; + } + + iargv = va_arg (ap, char **); + } + va_end (ap); + + if (argv) + argv[argc] = NULL; + + return argv; +} + /* @deftypefn Extension int writeargv (const char **@var{argv}, FILE *@var{file}) --- include/libiberty.h 3 Jan 2011 21:05:50 -0000 1.63 +++ include/libiberty.h 1 Jun 2011 16:17:06 -0000 @@ -74,6 +74,9 @@ extern FILE *freopen_unlocked (const cha extern char **buildargv (const char *) ATTRIBUTE_MALLOC; +extern char **appendargv (char **, const char *, ...) ATTRIBUTE_MALLOC; +extern char **mergeargv (char **iargv, ...) ATTRIBUTE_MALLOC; + /* Free a vector returned by buildargv. */ extern void freeargv (char **); --- gdb/remote-sim.c 1 Jun 2011 15:29:07 -0000 1.103 +++ gdb/remote-sim.c 1 Jun 2011 16:17:06 -0000 @@ -664,7 +664,7 @@ static void gdbsim_open (char *args, int from_tty) { int len; - char *arg_buf; + char **argv; struct sim_inferior_data *sim_data; SIM_DESC gdbsim_desc; @@ -681,44 +681,43 @@ gdbsim_open (char *args, int from_tty) if (gdbsim_is_open) unpush_target (&gdbsim_ops); - len = (7 + 1 /* gdbsim */ - + strlen (" -E little") - + strlen (" --architecture=xxxxxxxxxx") - + strlen (" --sysroot=") + strlen (gdb_sysroot) + - + (args ? strlen (args) : 0) - + 50) /* slack */ ; - arg_buf = (char *) alloca (len); - strcpy (arg_buf, "gdbsim"); /* 7 */ + argv = buildargv ("gdbsim"); + /* Specify the byte order for the target when it is explicitly specified by the user (not auto detected). */ switch (selected_byte_order ()) { case BFD_ENDIAN_BIG: - strcat (arg_buf, " -E big"); + argv = appendargv (argv, "-E", "big", NULL); break; case BFD_ENDIAN_LITTLE: - strcat (arg_buf, " -E little"); + argv = appendargv (argv, "-E", "little", NULL); break; case BFD_ENDIAN_UNKNOWN: break; } + /* Specify the architecture of the target when it has been explicitly specified */ if (selected_architecture_name () != NULL) - { - strcat (arg_buf, " --architecture="); - strcat (arg_buf, selected_architecture_name ()); - } + argv = appendargv (argv, "--architecture", + selected_architecture_name (), NULL); + /* Pass along gdb's concept of the sysroot. */ - strcat (arg_buf, " --sysroot="); - strcat (arg_buf, gdb_sysroot); + argv = appendargv (argv, "--sysroot", gdb_sysroot, NULL); + /* finally, any explicit args */ if (args) { - strcat (arg_buf, " "); /* 1 */ - strcat (arg_buf, args); + char **uargv; + + uargv = gdb_buildargv (args); + sim_argv = mergeargv (argv, uargv, NULL); + freeargv (argv); + freeargv (uargv); } - sim_argv = gdb_buildargv (arg_buf); + else + sim_argv = argv; init_callbacks (); gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv); [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH/RFC] new argv handlers to help with sim argv building 2011-06-01 16:18 ` [PATCH/RFC] new argv handlers to help with sim argv building Mike Frysinger @ 2011-06-03 16:56 ` Tom Tromey 2011-06-03 18:34 ` Mike Frysinger 0 siblings, 1 reply; 21+ messages in thread From: Tom Tromey @ 2011-06-03 16:56 UTC (permalink / raw) To: Mike Frysinger; +Cc: gdb-patches, toolchain-devel >>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes: Mike> i slapped this together. what do you think (ignoring missing docs) ? It seems reasonable to me overall, though see below. Mike> +char **appendargv (char **argv, const char *arg, ...) Wrong formatting. The canonical source for libiberty is gcc, so changes have to go there. Mike> + argv = appendargv (argv, "--sysroot", gdb_sysroot, NULL); [...] Mike> + uargv = gdb_buildargv (args); Mike> + sim_argv = mergeargv (argv, uargv, NULL); Mike> + freeargv (argv); If there is only one argument potentially needing quoting from buildargv, and we have to do splitting and merging as well, then it seems like it would be simpler to just add a quoting function and use that. Tom ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH/RFC] new argv handlers to help with sim argv building 2011-06-03 16:56 ` Tom Tromey @ 2011-06-03 18:34 ` Mike Frysinger 0 siblings, 0 replies; 21+ messages in thread From: Mike Frysinger @ 2011-06-03 18:34 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, toolchain-devel [-- Attachment #1: Type: Text/Plain, Size: 1032 bytes --] On Friday, June 03, 2011 12:56:24 Tom Tromey wrote: > >>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes: > Mike> +char **appendargv (char **argv, const char *arg, ...) > > Wrong formatting. i dont know what you mean > The canonical source for libiberty is gcc, so changes have to go there. yes, but i'm just bouncing the idea right now > Mike> + argv = appendargv (argv, "--sysroot", gdb_sysroot, NULL); > [...] > Mike> + uargv = gdb_buildargv (args); > Mike> + sim_argv = mergeargv (argv, uargv, NULL); > Mike> + freeargv (argv); > > If there is only one argument potentially needing quoting from > buildargv, and we have to do splitting and merging as well, then it > seems like it would be simpler to just add a quoting function and use > that. quoting opens a whole can of worms i dont want to think about. simply sticking quotes around gdb_sysroot doesnt solve the whole problem. what if the sysroot has quotes in it ? i'd prefer to stick in the argv world. -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] gdb: sim: automatically pass down sysroot @ 2011-05-26 20:03 Mike Frysinger 2011-05-26 20:10 ` Marek Polacek 2011-06-08 16:50 ` Joel Brobecker 0 siblings, 2 replies; 21+ messages in thread From: Mike Frysinger @ 2011-05-26 20:03 UTC (permalink / raw) To: gdb-patches; +Cc: toolchain-devel Since gdb sets up a nice sysroot path for us by default, automatically pass it down to the sim target so it too gets a good default. This does not override anything the user explicitly specifies of course. Signed-off-by: Mike Frysinger <vapier@gentoo.org> 2011-05-26 Mike Frysinger <vapier@gentoo.org> * remote-sim.c (gdbsim_open): Add the strlen of " --sysroot=" and gdb_sysroot to the "len" variable. Append both to "arg_buf". --- gdb/remote-sim.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c index bf4e0ee..918d5cb 100644 --- a/gdb/remote-sim.c +++ b/gdb/remote-sim.c @@ -684,6 +684,7 @@ gdbsim_open (char *args, int from_tty) len = (7 + 1 /* gdbsim */ + strlen (" -E little") + strlen (" --architecture=xxxxxxxxxx") + + strlen (" --sysroot=") + strlen (gdb_sysroot) + + (args ? strlen (args) : 0) + 50) /* slack */ ; arg_buf = (char *) alloca (len); @@ -708,6 +709,9 @@ gdbsim_open (char *args, int from_tty) strcat (arg_buf, " --architecture="); strcat (arg_buf, selected_architecture_name ()); } + /* Pass along gdb's concept of the sysroot. */ + strcat (arg_buf, " --sysroot="); + strcat (arg_buf, gdb_sysroot); /* finally, any explicit args */ if (args) { -- 1.7.5.rc3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-05-26 20:03 [PATCH] gdb: sim: automatically pass down sysroot Mike Frysinger @ 2011-05-26 20:10 ` Marek Polacek 2011-05-26 22:32 ` Mike Frysinger 2011-05-27 17:48 ` Tom Tromey 2011-06-08 16:50 ` Joel Brobecker 1 sibling, 2 replies; 21+ messages in thread From: Marek Polacek @ 2011-05-26 20:10 UTC (permalink / raw) To: Mike Frysinger; +Cc: gdb-patches, toolchain-devel On 05/26/2011 10:03 PM, Mike Frysinger wrote: > + /* Pass along gdb's concept of the sysroot. */ > + strcat (arg_buf, " --sysroot="); > + strcat (arg_buf, gdb_sysroot); BTW, couldn't be those strcat()s replaced by, e.g., cheaper mempcpy()? Marek ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-05-26 20:10 ` Marek Polacek @ 2011-05-26 22:32 ` Mike Frysinger 2011-05-27 17:48 ` Tom Tromey 1 sibling, 0 replies; 21+ messages in thread From: Mike Frysinger @ 2011-05-26 22:32 UTC (permalink / raw) To: Marek Polacek; +Cc: gdb-patches, toolchain-devel On Thu, May 26, 2011 at 16:09, Marek Polacek wrote: > On 05/26/2011 10:03 PM, Mike Frysinger wrote: >> + /* Pass along gdb's concept of the sysroot. */ >> + strcat (arg_buf, " --sysroot="); >> + strcat (arg_buf, gdb_sysroot); > > BTW, couldn't be those strcat()s replaced by, e.g., cheaper mempcpy()? the whole func probably could be converted, but i was keeping existing style -mike ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-05-26 20:10 ` Marek Polacek 2011-05-26 22:32 ` Mike Frysinger @ 2011-05-27 17:48 ` Tom Tromey 2011-05-27 17:54 ` Pedro Alves 1 sibling, 1 reply; 21+ messages in thread From: Tom Tromey @ 2011-05-27 17:48 UTC (permalink / raw) To: Marek Polacek; +Cc: Mike Frysinger, gdb-patches, toolchain-devel >>>>> "Marek" == Marek Polacek <mpolacek@redhat.com> writes: Marek> BTW, couldn't be those strcat()s replaced by, e.g., cheaper mempcpy()? Thanks for noticing this. Normally my preference is to prefer the simplest code unless it is known to be performance-critical. In a case like this I doubt that it matters. Tom ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-05-27 17:48 ` Tom Tromey @ 2011-05-27 17:54 ` Pedro Alves 0 siblings, 0 replies; 21+ messages in thread From: Pedro Alves @ 2011-05-27 17:54 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey, Marek Polacek, Mike Frysinger, toolchain-devel On Friday 27 May 2011 18:47:49, Tom Tromey wrote: > >>>>> "Marek" == Marek Polacek <mpolacek@redhat.com> writes: > > Marek> BTW, couldn't be those strcat()s replaced by, e.g., cheaper mempcpy()? > > Thanks for noticing this. > > Normally my preference is to prefer the simplest code unless it is known > to be performance-critical. In a case like this I doubt that it > matters. And mempcpy is not portable --- we'd have to pull it from gnulib. -- Pedro Alves ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-05-26 20:03 [PATCH] gdb: sim: automatically pass down sysroot Mike Frysinger 2011-05-26 20:10 ` Marek Polacek @ 2011-06-08 16:50 ` Joel Brobecker 2011-06-08 17:08 ` Mike Frysinger ` (2 more replies) 1 sibling, 3 replies; 21+ messages in thread From: Joel Brobecker @ 2011-06-08 16:50 UTC (permalink / raw) To: Mike Frysinger; +Cc: gdb-patches, toolchain-devel [-- Attachment #1: Type: text/plain, Size: 847 bytes --] > 2011-05-26 Mike Frysinger <vapier@gentoo.org> > > * remote-sim.c (gdbsim_open): Add the strlen of " --sysroot=" and > gdb_sysroot to the "len" variable. Append both to "arg_buf". It turns out that this is breaking some simulators, at least the ppc sim: % powerpc-elf-gdb (gdb) target sim Unrecognized option The problem is that some sims have their own argument parsing, and they sometimes reject the --sysroot= option. Here is the patch I just checked in to fix the ppc, but I think that other simulators might have the same problem. I checked a few, and I think that the d10v sim, for instance, will also need to be updated). So, we have a decision to make: Either do a pass over all simulators, and fix the ones that need fixing, or revert (in which case my ppc patch also needs to be reverted). -- Joel [-- Attachment #2: sim-ppc-sysroot-option.diff --] [-- Type: text/x-diff, Size: 1488 bytes --] commit df31576192c6a95c3a56583d7de54a452dd52110 Author: Joel Brobecker <brobecker@adacore.com> Date: Wed Jun 8 09:17:09 2011 -0700 ppc sim: Allow --sysroot command-line option There was a recent change that cuased the "target sim" command to add a --sysroot option to the argument vector passed down to the simulator. This caused a failure in the powerpc simulator, as it did not recognize it. This patch fixes the problem by adding support for the --sysroot option (it ignores it). sim/ppc/ChangeLog: * psim.c (psim_options): Accept and ignore `--sysroot=...'. diff --git a/sim/ppc/ChangeLog b/sim/ppc/ChangeLog index 2b9017a..2536fa4 100644 --- a/sim/ppc/ChangeLog +++ b/sim/ppc/ChangeLog @@ -1,3 +1,7 @@ +2011-06-08 Joel Brobecker <brobecker@adacore.com> + + * psim.c (psim_options): Accept and ignore `--sysroot=...'. + 2011-06-03 Joel Brobecker <brobecker@adacore.com> (obvious fix) From Stephen Kitt <steve@sk2.org> diff --git a/sim/ppc/psim.c b/sim/ppc/psim.c index c311794..076a50c 100644 --- a/sim/ppc/psim.c +++ b/sim/ppc/psim.c @@ -357,6 +357,10 @@ psim_options(device *root, } else if (strcmp (argv[argp], "--help") == 0) psim_usage (0, 1); + else if (strncmp (argv[argp], "--sysroot=", + sizeof ("--sysroot=")) == 0) + /* Ignore this option. */ + p = argv[argp] + strlen(argv[argp]) - 1; else if (strcmp (argv[argp], "--version") == 0) { extern const char version[]; ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-08 16:50 ` Joel Brobecker @ 2011-06-08 17:08 ` Mike Frysinger 2011-06-08 17:15 ` Joel Brobecker 2011-06-09 5:19 ` Mike Frysinger 2011-06-09 5:19 ` Mike Frysinger 2 siblings, 1 reply; 21+ messages in thread From: Mike Frysinger @ 2011-06-08 17:08 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, toolchain-devel [-- Attachment #1: Type: Text/Plain, Size: 1133 bytes --] On Wednesday, June 08, 2011 12:49:39 Joel Brobecker wrote: > > 2011-05-26 Mike Frysinger <vapier@gentoo.org> > > > > * remote-sim.c (gdbsim_open): Add the strlen of " --sysroot=" and > > gdb_sysroot to the "len" variable. Append both to "arg_buf". > > It turns out that this is breaking some simulators, at least the ppc sim: > > % powerpc-elf-gdb > (gdb) target sim > Unrecognized option > > The problem is that some sims have their own argument parsing, > and they sometimes reject the --sysroot= option. Here is the patch > I just checked in to fix the ppc, but I think that other simulators > might have the same problem. I checked a few, and I think that > the d10v sim, for instance, will also need to be updated). > > So, we have a decision to make: Either do a pass over all simulators, > and fix the ones that need fixing, or revert (in which case my ppc > patch also needs to be reverted). i'll take a look over the sims to see which lack sysroot support and fix them up ... really wish people would use the common/ dir more as this keeps biting me :/ -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-08 17:08 ` Mike Frysinger @ 2011-06-08 17:15 ` Joel Brobecker 0 siblings, 0 replies; 21+ messages in thread From: Joel Brobecker @ 2011-06-08 17:15 UTC (permalink / raw) To: Mike Frysinger; +Cc: gdb-patches, toolchain-devel > really wish people would use the common/ dir more as this keeps biting > me :/ Same here. -- Joel ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-08 16:50 ` Joel Brobecker 2011-06-08 17:08 ` Mike Frysinger @ 2011-06-09 5:19 ` Mike Frysinger 2011-06-09 5:48 ` Joel Brobecker 2011-06-09 7:29 ` Tristan Gingold 2011-06-09 5:19 ` Mike Frysinger 2 siblings, 2 replies; 21+ messages in thread From: Mike Frysinger @ 2011-06-09 5:19 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, toolchain-devel [-- Attachment #1: Type: Text/Plain, Size: 659 bytes --] here's the fix for erc32. ok to commit ? (style is god awful, but seems the whole erc32 code base is buggered and i'm not about to try and fix that) -mike --- sim/erc32/interf.c 1 Jun 2011 17:35:02 -0000 1.10 +++ sim/erc32/interf.c 9 Jun 2011 05:03:32 -0000 @@ -234,6 +234,9 @@ sim_open (kind, callback, abfd, argv) if ((stat + 1) < argc) { freq = strtol(argv[++stat], (char **)NULL, 0); } + } else + if (strncmp(argv[stat], "--sysroot=", sizeof("--sysroot=")) == 0) { + /* Ignore until we start to support this. */ } else { (*sim_callback->printf_filtered) (sim_callback, "unknown option %s\n", [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-09 5:19 ` Mike Frysinger @ 2011-06-09 5:48 ` Joel Brobecker 2011-06-09 6:07 ` Mike Frysinger 2011-06-09 7:29 ` Tristan Gingold 1 sibling, 1 reply; 21+ messages in thread From: Joel Brobecker @ 2011-06-09 5:48 UTC (permalink / raw) To: Mike Frysinger; +Cc: gdb-patches, toolchain-devel > here's the fix for erc32. ok to commit ? Yes, please. Thank you for taking care of that. Funny I didn't notice, this problem, but it turns out that the error is non-fatal... And good to know that this is the last simulator that should be impacted. If there is a relatively mechanichal way to use sim/common/, we can look at that option, since the number of simulators to update would be small (2?). > (style is god awful, but seems the whole erc32 code base is buggered and i'm > not about to try and fix that) I had the exact same reaction - it's just amazing how style differences can make the reading so much more difficult. > --- sim/erc32/interf.c 1 Jun 2011 17:35:02 -0000 1.10 > +++ sim/erc32/interf.c 9 Jun 2011 05:03:32 -0000 > @@ -234,6 +234,9 @@ sim_open (kind, callback, abfd, argv) > if ((stat + 1) < argc) { > freq = strtol(argv[++stat], (char **)NULL, 0); > } > + } else > + if (strncmp(argv[stat], "--sysroot=", sizeof("--sysroot=")) == 0) { > + /* Ignore until we start to support this. */ > } else { > (*sim_callback->printf_filtered) (sim_callback, > "unknown option %s\n", -- Joel ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-09 5:48 ` Joel Brobecker @ 2011-06-09 6:07 ` Mike Frysinger 2011-06-09 14:26 ` Joel Brobecker 0 siblings, 1 reply; 21+ messages in thread From: Mike Frysinger @ 2011-06-09 6:07 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, toolchain-devel On Thu, Jun 9, 2011 at 01:47, Joel Brobecker wrote: > And good to know that this is the last simulator that should be > impacted. If there is a relatively mechanichal way to use sim/common/, > we can look at that option, since the number of simulators to update > would be small (2?). "no" to all parts unfortunately. the sim/common/ tree is a loose collection of files. sims get to opt in to different pieces or simply re-implement the core functions themselves. i'd say probably more than half do. you can tell quickly by looking at the sub-Makefile and see how many objects from common it refers to. and if the sim still uses "run.c" instead of "nrun.c", you're really screwed. and from what i recall from when i converted bfin from standalone to the common tree, the main loops, header files, and state are heavily impacted. fairly invasive to convert over, but much of it is simply code massage rather than rewriting new stuff. the upside is that the conversion tends to involve throwing out a lot of code because common/ takes care of it for you. the downside is that it isnt trivial, and i unfortunately have no interest in converting other sims. -mike ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-09 6:07 ` Mike Frysinger @ 2011-06-09 14:26 ` Joel Brobecker 0 siblings, 0 replies; 21+ messages in thread From: Joel Brobecker @ 2011-06-09 14:26 UTC (permalink / raw) To: Mike Frysinger; +Cc: gdb-patches, toolchain-devel > the upside is that the conversion tends to involve throwing out a lot > of code because common/ takes care of it for you. the downside is > that it isnt trivial, and i unfortunately have no interest in > converting other sims. Thanks. I would probably have taken care of erc32 and ppc, as I have some ways of testing them (I have the compilers). But if it isn't mechanical, I have more pressing work to do as well... -- Joel ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-09 5:19 ` Mike Frysinger 2011-06-09 5:48 ` Joel Brobecker @ 2011-06-09 7:29 ` Tristan Gingold 2011-06-09 14:24 ` Joel Brobecker 1 sibling, 1 reply; 21+ messages in thread From: Tristan Gingold @ 2011-06-09 7:29 UTC (permalink / raw) To: Mike Frysinger; +Cc: Joel Brobecker, gdb-patches, toolchain-devel On Jun 9, 2011, at 7:07 AM, Mike Frysinger wrote: > here's the fix for erc32. ok to commit ? Is it correct ? IIRC sizeof ("xxx") == strlen ("xxx") + 1. > (style is god awful, but seems the whole erc32 code base is buggered and i'm > not about to try and fix that) > -mike > > --- sim/erc32/interf.c 1 Jun 2011 17:35:02 -0000 1.10 > +++ sim/erc32/interf.c 9 Jun 2011 05:03:32 -0000 > @@ -234,6 +234,9 @@ sim_open (kind, callback, abfd, argv) > if ((stat + 1) < argc) { > freq = strtol(argv[++stat], (char **)NULL, 0); > } > + } else > + if (strncmp(argv[stat], "--sysroot=", sizeof("--sysroot=")) == 0) { > + /* Ignore until we start to support this. */ > } else { > (*sim_callback->printf_filtered) (sim_callback, > "unknown option %s\n", ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-09 7:29 ` Tristan Gingold @ 2011-06-09 14:24 ` Joel Brobecker 2011-06-09 15:32 ` Mike Frysinger 0 siblings, 1 reply; 21+ messages in thread From: Joel Brobecker @ 2011-06-09 14:24 UTC (permalink / raw) To: Tristan Gingold; +Cc: Mike Frysinger, gdb-patches, toolchain-devel [-- Attachment #1: Type: text/plain, Size: 349 bytes --] > > here's the fix for erc32. ok to commit ? > > Is it correct ? IIRC sizeof ("xxx") == strlen ("xxx") + 1. You're quite right. The weird thing is that I had the "- 1" in my code at some point, I'm sure of that. How it came to disappear, I'm really puzzled. Here is what I just checked in to fix the problem on ppc. Thanks, Tristan. -- Joel [-- Attachment #2: sim-ppc.diff --] [-- Type: text/x-diff, Size: 1435 bytes --] commit 1f192d6b369336703e07d74833f98a8bdc6c3361 Author: Joel Brobecker <brobecker@adacore.com> Date: Thu Jun 9 07:18:36 2011 -0700 sim/ppc: Fix check for --sysroot= option Fixes an error reported by Tristan and which can be evidenced by doing: % powerpc-elf-gdb (gdb) target sim --sysroot=var Invalid option: --sysroot=/var [...] sim/ppc/ChangeLog: * psim.c (psim_options): Fix length of comparison when checking for --sysroot= option. diff --git a/sim/ppc/ChangeLog b/sim/ppc/ChangeLog index 5e6fff6..455585a 100644 --- a/sim/ppc/ChangeLog +++ b/sim/ppc/ChangeLog @@ -1,3 +1,8 @@ +2011-06-09 Joel Brobecker <brobecker@adacore.com> + + * psim.c (psim_options): Fix length of comparison when checking + for --sysroot= option. + 2011-06-08 Joel Brobecker <brobecker@adacore.com> * psim.c (psim_options): Add option that cause the error diff --git a/sim/ppc/psim.c b/sim/ppc/psim.c index d814486..3e76386 100644 --- a/sim/ppc/psim.c +++ b/sim/ppc/psim.c @@ -359,7 +359,7 @@ psim_options(device *root, else if (strcmp (argv[argp], "--help") == 0) psim_usage (0, 1); else if (strncmp (argv[argp], "--sysroot=", - sizeof ("--sysroot=")) == 0) + sizeof ("--sysroot=") - 1) == 0) /* Ignore this option. */ p = argv[argp] + strlen(argv[argp]) - 1; else if (strcmp (argv[argp], "--version") == 0) ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-09 14:24 ` Joel Brobecker @ 2011-06-09 15:32 ` Mike Frysinger 0 siblings, 0 replies; 21+ messages in thread From: Mike Frysinger @ 2011-06-09 15:32 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tristan Gingold, toolchain-devel, gdb-patches On Thu, Jun 9, 2011 at 10:24, Joel Brobecker wrote: >> > here's the fix for erc32. ok to commit ? >> >> Is it correct ? IIRC sizeof ("xxx") == strlen ("xxx") + 1. > > You're quite right. The weird thing is that I had the "- 1" > in my code at some point, I'm sure of that. How it came to > disappear, I'm really puzzled. i was working with constants originally, but when i looked at Joel's patch, i copied & pasted what he did. so the erc32 version i committed includes his latest change. -mike --- interf.c 1 Jun 2011 17:35:02 -0000 1.10 +++ interf.c 9 Jun 2011 15:30:54 -0000 @@ -234,6 +234,9 @@ sim_open (kind, callback, abfd, argv) if ((stat + 1) < argc) { freq = strtol(argv[++stat], (char **)NULL, 0); } + } else + if (strncmp(argv[stat], "--sysroot=", sizeof("--sysroot=") - 1) == 0) { + /* Ignore until we start to support this. */ } else { (*sim_callback->printf_filtered) (sim_callback, "unknown option %s\n", ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] gdb: sim: automatically pass down sysroot 2011-06-08 16:50 ` Joel Brobecker 2011-06-08 17:08 ` Mike Frysinger 2011-06-09 5:19 ` Mike Frysinger @ 2011-06-09 5:19 ` Mike Frysinger 2 siblings, 0 replies; 21+ messages in thread From: Mike Frysinger @ 2011-06-09 5:19 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, toolchain-devel [-- Attachment #1: Type: Text/Plain, Size: 824 bytes --] On Wednesday, June 08, 2011 12:49:39 Joel Brobecker wrote: > > 2011-05-26 Mike Frysinger <vapier@gentoo.org> > > > > * remote-sim.c (gdbsim_open): Add the strlen of " --sysroot=" and > > gdb_sysroot to the "len" variable. Append both to "arg_buf". > > It turns out that this is breaking some simulators, at least the ppc sim: with your ppc fix and my erc32 patch, i dont see any other sims failing these guys are using sim_args_command() and sim_parse_args() which handles the --sysroot so they should "just work" already: bfin cris frv iq2000 lm32 m32r m68hc11 mips mn10300 moxie sh64 v850 these guys appear to ignore all and/or unknown options: arm avr cr16 m32c mcore microblaze rx sh these guys are no longer in gdb, so doesnt matter: d10v so i think ive covered everyone ... -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2011-06-09 15:32 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1306440200-25087-1-git-send-email-vapier__8251.52545371584$1306440225$gmane$org@gentoo.org>
2011-05-27 17:47 ` [PATCH] gdb: sim: automatically pass down sysroot Tom Tromey
2011-05-27 18:37 ` Mike Frysinger
2011-06-01 16:18 ` [PATCH/RFC] new argv handlers to help with sim argv building Mike Frysinger
2011-06-03 16:56 ` Tom Tromey
2011-06-03 18:34 ` Mike Frysinger
2011-05-26 20:03 [PATCH] gdb: sim: automatically pass down sysroot Mike Frysinger
2011-05-26 20:10 ` Marek Polacek
2011-05-26 22:32 ` Mike Frysinger
2011-05-27 17:48 ` Tom Tromey
2011-05-27 17:54 ` Pedro Alves
2011-06-08 16:50 ` Joel Brobecker
2011-06-08 17:08 ` Mike Frysinger
2011-06-08 17:15 ` Joel Brobecker
2011-06-09 5:19 ` Mike Frysinger
2011-06-09 5:48 ` Joel Brobecker
2011-06-09 6:07 ` Mike Frysinger
2011-06-09 14:26 ` Joel Brobecker
2011-06-09 7:29 ` Tristan Gingold
2011-06-09 14:24 ` Joel Brobecker
2011-06-09 15:32 ` Mike Frysinger
2011-06-09 5:19 ` Mike Frysinger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox