[PING PATCH] demangler, only access valid fields for DEMANGLE_COMPONENT_FIXED_TYPE.
Hi all,
I just retested this patch. The crash it fixes is still there,
and the patch still fixes it. Is this ok to commit?
Cheers,
Gary
Andrew Burgess wrote:
> In two places when a struct demangle_component is of type
> DEMANGLE_COMPONENT_FIXED_TYPE we fall back to accessing the default
> s_binary member of the union rather than the s_fixed member. This
> is incorrect and can cause the demangler to crash.
>
> In d_dump I've changed the code to only access the s_fixed member of
> the union, and also added printing of the remaining parts of the
> s_fixed struct, this felt like the most useful thing to do.
>
> I've added a new test, this causes a SIGSEGV for me before the
> patch, and is fine afterwords, however, this undefined, so might not
> cause a crash on all platforms.
>
> If this is approved then please could someone commit it for me, I
> don't have gcc write access.
>
> Thanks,
> Andrew
>
> libiberty/ChangeLog:
>
> * cp-demangle.c (d_dump): Only access field from s_fixed part of
> the union for DEMANGLE_COMPONENT_FIXED_TYPE.
> (d_count_templates_scopes): Likewise.
> * testsuite/demangle-expected: New test case.
> ---
> libiberty/cp-demangle.c | 10 +-
> libiberty/testsuite/demangle-expected | 6 ++
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
> index 68d8ee1..a31dad4 100644
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -710,7 +710,9 @@ d_dump (struct demangle_component *dc, int indent)
>printf ("pointer to member type\n");
>break;
> case DEMANGLE_COMPONENT_FIXED_TYPE:
> - printf ("fixed-point type\n");
> + printf ("fixed-point type, accum? %d, sat? %d\n",
> + dc->u.s_fixed.accum, dc->u.s_fixed.sat);
> + d_dump (dc->u.s_fixed.length, indent + 2)
>break;
> case DEMANGLE_COMPONENT_ARGLIST:
>printf ("argument list\n");
> @@ -3869,7 +3871,13 @@ d_count_templates_scopes (int *num_templates, int
> *num_scopes,
> case DEMANGLE_COMPONENT_FUNCTION_TYPE:
> case DEMANGLE_COMPONENT_ARRAY_TYPE:
> case DEMANGLE_COMPONENT_PTRMEM_TYPE:
> + goto recurse_left_right;
> +
> case DEMANGLE_COMPONENT_FIXED_TYPE:
> + d_count_templates_scopes (num_templates, num_scopes,
> +dc->u.s_fixed.length);
> + break;
> +
> case DEMANGLE_COMPONENT_VECTOR_TYPE:
> case DEMANGLE_COMPONENT_ARGLIST:
> case DEMANGLE_COMPONENT_TEMPLATE_ARGLIST:
> diff --git a/libiberty/testsuite/demangle-expected
> b/libiberty/testsuite/demangle-expected
> index 453f9a3..0e2bb12 100644
> --- a/libiberty/testsuite/demangle-expected
> +++ b/libiberty/testsuite/demangle-expected
> @@ -4343,3 +4343,9 @@
> cereal::detail::InputBindingMap::Serializers
> cereal::p
> --format=gnu-v3
> _ZNSt9_Any_data9_M_accessIPZ4postISt8functionIFvvEEEvOT_EUlvE_EERS5_v
> void post >(std::function&&)::{lambda()#1}*&
> std::_Any_data::_M_access >(void
> post >(std::function ()>&&)::{lambda()#1}*&&)::{lambda()#1}*>()
> +# The following input symbol was found during random, it caused a fault
> +# within the demangler, it's not a symbol we'd expect in the real world.
> +--format=auto --no-params
> +_Z3xxxDFyuVb
> +xxx(unsigned long long _Fract, bool volatile)
> +xxx
> --
> 1.8.1.3
[COMMITTED PATCH] Demangler fuzzer
Ian Lance Taylor wrote:
> On Tue, Aug 12, 2014 at 10:11 AM, Gary Benson wrote:
> > Ian Lance Taylor wrote:
> > > I think that by default the program should stop. That will make
> > > it possible to eventually run as part of "make check". Give it
> > > some number of iterations that stops it in a second or so. You
> > > can still have it run forever by using -m -1.
> >
> > On my machine it usually fails in 3-5 seconds, so a 1 second run
> > seems a little too short. How does 10 seconds sound?
>
> OK, we can start with that, I suppose.
I have committed the patch inlined below. By default it tries
7.5 million symbols, which takes roughly 10 seconds on my box.
A good seed for testing is 1407772345 which manages 30,123,441
symbols before crashing (about 45 seconds).
Cheers,
Gary
--
2014-08-13 Gary Benson
* testsuite/demangler-fuzzer.c: New file.
* testsuite/Makefile.in (fuzz-demangler): New rule.
(demangler-fuzzer): Likewise.
(mostlyclean): Clean up demangler fuzzer.
Index: libiberty/testsuite/Makefile.in
===
--- libiberty/testsuite/Makefile.in (revision 213911)
+++ libiberty/testsuite/Makefile.in (revision 213912)
@@ -59,6 +59,10 @@
check-expandargv: test-expandargv
./test-expandargv
+# Run the demangler fuzzer
+fuzz-demangler: demangler-fuzzer
+ ./demangler-fuzzer
+
TEST_COMPILE = $(CC) @DEFS@ $(LIBCFLAGS) -I.. -I$(INCDIR) $(HDEFINES)
test-demangle: $(srcdir)/test-demangle.c ../libiberty.a
$(TEST_COMPILE) -o test-demangle \
@@ -72,6 +76,10 @@
$(TEST_COMPILE) -DHAVE_CONFIG_H -I.. -o test-expandargv \
$(srcdir)/test-expandargv.c ../libiberty.a
+demangler-fuzzer: $(srcdir)/demangler-fuzzer.c ../libiberty.a
+ $(TEST_COMPILE) -o demangler-fuzzer \
+ $(srcdir)/demangler-fuzzer.c ../libiberty.a
+
# Standard (either GNU or Cygnus) rules we don't use.
html install-html info install-info clean-info dvi pdf install-pdf \
install etags tags installcheck:
@@ -81,6 +89,7 @@
rm -f test-demangle
rm -f test-pexecute
rm -f test-expandargv
+ rm -f demangler-fuzzer
rm -f core
clean: mostlyclean
distclean: clean
Index: libiberty/testsuite/demangler-fuzzer.c
===
--- libiberty/testsuite/demangler-fuzzer.c (revision 0)
+++ libiberty/testsuite/demangler-fuzzer.c (revision 213912)
@@ -0,0 +1,108 @@
+/* Demangler fuzzer.
+
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ This file is part of GNU libiberty.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+#include
+#include "demangle.h"
+
+#define MAXLEN 253
+#define ALPMIN 33
+#define ALPMAX 127
+
+static char *program_name;
+
+#define DEFAULT_MAXCOUNT 750
+
+static void
+print_usage (FILE *fp, int exit_value)
+{
+ fprintf (fp, "Usage: %s [OPTION]...\n", program_name);
+ fprintf (fp, "Options:\n");
+ fprintf (fp, " -h Display this message.\n");
+ fprintf (fp, " -s SEED Select the random seed to be used.\n");
+ fprintf (fp, " The default is to base one on the");
+ fprintf (fp, " current time.\n");
+ fprintf (fp, " -m MAXCOUNT Exit after MAXCOUNT symbols.\n");
+ fprintf (fp, " The default is %d.", DEFAULT_MAXCOUNT);
+ fprintf (fp, " Set to `-1' for no limit.\n");
+
+ exit (exit_value);
+}
+
+int
+main (int argc, char *argv[])
+{
+ char symbol[2 + MAXLEN + 1] = "_Z";
+ int seed = -1, seed_set = 0;
+ int count = 0, maxcount = DEFAULT_MAXCOUNT;
+ int optchr;
+
+ program_name = argv[0];
+
+ do
+{
+ optchr = getopt (argc, argv, "hs:m:t:");
+ switch (optchr)
+ {
+ case '?': /* Unrecognized option. */
+ print_usage (stderr, 1);
+ break;
+
+ case 'h':
+ print_usage (stdout, 0);
+ break;
+
+ case 's':
+ seed = atoi (optarg);
+ seed_set = 1;
+ break;
+
+ case 'm':
+ maxcount = atoi (optarg);
+ break;
+
Re: [PATCH] Demangler fuzzer
Ian Lance Taylor wrote:
> On Tue, Aug 12, 2014 at 2:02 AM, Gary Benson wrote:
> > +#include
>
> Include demangle.h with "".
Ok.
> > +int
> > +main (int argc, char *argv[])
> > +{
> > + char symbol[2 + MAXLEN + 1] = "_Z";
> > + int seed = -1, seed_set = 0;
> > + int count = 0, maxcount = -1;
>
> I think that by default the program should stop. That will make it
> possible to eventually run as part of "make check". Give it some
> number of iterations that stops it in a second or so. You can still
> have it run forever by using -m -1.
On my machine it usually fails in 3-5 seconds, so a 1 second run seems
a little too short. How does 10 seconds sound?
Thanks,
Gary
--
http://gbenson.net/
Re: [PATCH] Demangler fuzzer
Jakub Jelinek wrote: > On Tue, Aug 12, 2014 at 10:02:40AM +0100, Gary Benson wrote: > > I've removed the timeout code. Users can limit the run by setting a > > maximum number of iterations. That's more consistent for testing > > anyway: 500 iterations is 500 iterations wherever you run it. > > > > How about this one? > > LGTM, but I think it would be best to hear from Ian on this too. Cool, I'll wait for Ian's reply. > Perhaps MAXLEN could be a command line option (then you'd need to > allocate the buffer dynamically?), but not sure if it is worth it. Right now it generally crashes after a few hundred thousand iterations (usually under five seconds). It might be worth extending the fuzzer once the bugs are fixed such that it runs without crashing for longer. (I'll likely fix some of them myself in gaps between projects). Cheers, Gary -- http://gbenson.net/
Re: [PATCH] Demangler fuzzer
Jakub Jelinek wrote:
> On Mon, Aug 11, 2014 at 05:04:20PM +0100, Gary Benson wrote:
> > + case 's':
> > + seed = atoi (optarg);
> > + break;
> > +
> > + case 't':
> > + timeout = atoi (optarg);
> > + break;
> > +
> > + case 'm':
> > + maxcount = atoi (optarg);
> > + break;
> > + }
> > +}
> > + while (optchr != -1);
> > +
> > + if (seed == -1)
> > +seed = time (NULL);
> > + srand (seed);
> > + printf ("%s: seed = %d\n", program_name, seed);
>
> That will still make it non-reproduceable if time returns -1
> (well, as you cast time_t to int, that can be e.g. on 64-bit arches
> any time which has 0x in the low 32 bits). So perhaps do
> if (seed == -1)
> {
> seed = time (NULL);
> if (seed == -1) seed = 0;
> }
> or something similar?
I've added a flag, seed_set, so seed will only be set to time (NULL)
if the user has not specified a seed on the command line.
> > + if (timeout != -1)
> > +{
> > + signal (SIGALRM, alarm_handler);
> > + alarm (timeout);
> > +}
>
> Not sure how much portable signal/alarm is. So probably should be
> guarded by the existence of signal.h, SIGALRM being defined etc.
I've removed the timeout code. Users can limit the run by setting a
maximum number of iterations. That's more consistent for testing
anyway: 500 iterations is 500 iterations wherever you run it.
How about this one?
Thanks,
Gary
--
2014-08-12 Gary Benson
* testsuite/demangler-fuzzer.c: New file.
* testsuite/Makefile.in (fuzz-demangler): New rule.
(demangler-fuzzer): Likewise.
(mostlyclean): Clean up demangler fuzzer.
Index: libiberty/testsuite/Makefile.in
===
--- libiberty/testsuite/Makefile.in (revision 213809)
+++ libiberty/testsuite/Makefile.in (working copy)
@@ -59,6 +59,10 @@
check-expandargv: test-expandargv
./test-expandargv
+# Run the demangler fuzzer
+fuzz-demangler: demangler-fuzzer
+ ./demangler-fuzzer -m 5000
+
TEST_COMPILE = $(CC) @DEFS@ $(LIBCFLAGS) -I.. -I$(INCDIR) $(HDEFINES)
test-demangle: $(srcdir)/test-demangle.c ../libiberty.a
$(TEST_COMPILE) -o test-demangle \
@@ -72,6 +76,10 @@
$(TEST_COMPILE) -DHAVE_CONFIG_H -I.. -o test-expandargv \
$(srcdir)/test-expandargv.c ../libiberty.a
+demangler-fuzzer: $(srcdir)/demangler-fuzzer.c ../libiberty.a
+ $(TEST_COMPILE) -o demangler-fuzzer \
+ $(srcdir)/demangler-fuzzer.c ../libiberty.a
+
# Standard (either GNU or Cygnus) rules we don't use.
html install-html info install-info clean-info dvi pdf install-pdf \
install etags tags installcheck:
@@ -81,6 +89,7 @@
rm -f test-demangle
rm -f test-pexecute
rm -f test-expandargv
+ rm -f demangler-fuzzer
rm -f core
clean: mostlyclean
distclean: clean
Index: libiberty/testsuite/demangler-fuzzer.c
===
--- libiberty/testsuite/demangler-fuzzer.c (revision 0)
+++ libiberty/testsuite/demangler-fuzzer.c (revision 0)
@@ -0,0 +1,102 @@
+/* Demangler fuzzer.
+
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ This file is part of GNU libiberty.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+#include
+#include
+
+#define MAXLEN 253
+#define ALPMIN 33
+#define ALPMAX 127
+
+static char *program_name;
+
+static void
+print_usage (FILE *fp, int exit_value)
+{
+ fprintf (fp, "Usage: %s [OPTION]...\n", program_name);
+ fprintf (fp, "Options:\n");
+ fprintf (fp, " -h Display this message.\n");
+ fprintf (fp, " -s SEED Select the random seed to be used.\n");
+ fprintf (fp, " -m MAXCOUNT Exit after MAXCOUNT symbols.\n");
+
+ exit (exit_value);
+}
+
+int
+main (int argc, char *argv[])
+{
+ char symbol[2 + MAXLEN + 1] = "_Z";
+ int seed = -1, seed_set = 0;
+ int count = 0, maxcount = -1;
+ int optchr;
+
+ program_name = argv[0];
+
+ do
+{
+ opt
Re: [PATCH] Demangler fuzzer
David Malcolm wrote:
> On Mon, 2014-08-11 at 08:06 -0700, Andi Kleen wrote:
> > Gary Benson writes:
> > > srand(time(NULL));
> >
> > That's really bad, can never be reproduced. If you use a random
> > seed like this you need to at least print it.
>
> How about taking the random seed and the number of iterations as
> command-line arguments? (with defaults for each)
Well, the reproducer ends up in the corefile, but I take your point.
How about this patch?
Thanks,
Gary
--
Index: libiberty/ChangeLog
===
--- libiberty/ChangeLog (revision 213809)
+++ libiberty/ChangeLog (working copy)
@@ -1,3 +1,10 @@
+2014-08-11 Gary Benson
+
+ * testsuite/demangler-fuzzer.c: New file.
+ * testsuite/Makefile.in (fuzz-demangler): New rule.
+ (demangler-fuzzer): Likewise.
+ (mostlyclean): Clean up demangler fuzzer.
+
2014-06-11 Andrew Burgess
* cplus-dem.c (do_type): Call string_delete even if the call to
Index: libiberty/testsuite/Makefile.in
===
--- libiberty/testsuite/Makefile.in (revision 213809)
+++ libiberty/testsuite/Makefile.in (working copy)
@@ -59,6 +59,10 @@
check-expandargv: test-expandargv
./test-expandargv
+# Run the demangler fuzzer
+fuzz-demangler: demangler-fuzzer
+ ./demangler-fuzzer -m 5000
+
TEST_COMPILE = $(CC) @DEFS@ $(LIBCFLAGS) -I.. -I$(INCDIR) $(HDEFINES)
test-demangle: $(srcdir)/test-demangle.c ../libiberty.a
$(TEST_COMPILE) -o test-demangle \
@@ -72,6 +76,10 @@
$(TEST_COMPILE) -DHAVE_CONFIG_H -I.. -o test-expandargv \
$(srcdir)/test-expandargv.c ../libiberty.a
+demangler-fuzzer: $(srcdir)/demangler-fuzzer.c ../libiberty.a
+ $(TEST_COMPILE) -o demangler-fuzzer \
+ $(srcdir)/demangler-fuzzer.c ../libiberty.a
+
# Standard (either GNU or Cygnus) rules we don't use.
html install-html info install-info clean-info dvi pdf install-pdf \
install etags tags installcheck:
@@ -81,6 +89,7 @@
rm -f test-demangle
rm -f test-pexecute
rm -f test-expandargv
+ rm -f demangler-fuzzer
rm -f core
clean: mostlyclean
distclean: clean
Index: libiberty/testsuite/demangler-fuzzer.c
===
--- libiberty/testsuite/demangler-fuzzer.c (revision 0)
+++ libiberty/testsuite/demangler-fuzzer.c (revision 0)
@@ -0,0 +1,121 @@
+/* Demangler fuzzer.
+
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ This file is part of GNU libiberty.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAXLEN 253
+#define ALPMIN 33
+#define ALPMAX 127
+
+static char *program_name;
+
+static void
+print_usage (FILE *fp, int exit_value)
+{
+ fprintf (fp, "Usage: %s [OPTION]...\n", program_name);
+ fprintf (fp, "Options:\n");
+ fprintf (fp, " -h Display this message.\n");
+ fprintf (fp, " -s SEED Select the random seed to be used.\n");
+ fprintf (fp, " -t TIMEOUT Exit after TIMEOUT seconds.\n");
+ fprintf (fp, " -m MAXCOUNT Exit after MAXCOUNT symbols.\n");
+
+ exit (exit_value);
+}
+
+static int quit_flag;
+
+static void
+alarm_handler (int signum)
+{
+ quit_flag = 1;
+}
+
+int
+main (int argc, char *argv[])
+{
+ char symbol[2 + MAXLEN + 1] = "_Z";
+ int optchr;
+ int seed = -1, maxcount = -1, timeout = -1;
+ int count = 0;
+
+ program_name = argv[0];
+
+ do
+{
+ optchr = getopt (argc, argv, "hs:m:t:");
+ switch (optchr)
+ {
+ case '?': /* Unrecognized option. */
+ print_usage (stderr, 1);
+ break;
+
+ case 'h':
+ print_usage (stdout, 0);
+ break;
+
+ case 's':
+ seed = atoi (optarg);
+ break;
+
+ case 't':
+ timeout = atoi (optarg);
+ break;
+
+ case 'm':
+ maxcount = atoi (optarg);
+ break;
+ }
+}
+ while (optchr != -1);
+
+ if (seed == -1)
+seed = time (NULL);
+ srand (seed);
+ printf ("%s: seed = %d\n", program_na
Re: [PATCH] Demangler fuzzer
Jakub Jelinek wrote:
> On Mon, Aug 11, 2014 at 10:27:03AM +0100, Gary Benson wrote:
> > This patch adds a simple fuzzer for the libiberty C++ demangler.
> > You can run it like this:
> >
> > make -C /path/to/build/libiberty/testsuite fuzz-demangler
> >
> > It will run until it dumps core (usually only a few seconds).
> >
> > Is this ok to commit?
>
> I think it is bad when the command never succeeds in case of
> success. There should be some limit on the number of iterations
> (perhaps a parameter to the program), or timeout.
How about as inlined below, with a 60 second timeout?
> > + for (i = 0; i < length; i++)
> > + *(buffer++) = (rand () % (ALPMAX - ALPMIN)) + ALPMIN;
> > +
> > + *(buffer++) = '\0';
>
> Please use just *buffer++ instead of *(buffer++) in both places.
Changed below.
Thanks,
Gary
--
2014-08-11 Gary Benson
* testsuite/demangler-fuzzer.c: New file.
* testsuite/Makefile.in (fuzz-demangler): New rule.
(demangler-fuzzer): Likewise.
(mostlyclean): Clean up demangler fuzzer.
Index: libiberty/testsuite/Makefile.in
===
--- libiberty/testsuite/Makefile.in (revision 213809)
+++ libiberty/testsuite/Makefile.in (working copy)
@@ -59,6 +59,10 @@
check-expandargv: test-expandargv
./test-expandargv
+# Run the demangler fuzzer
+fuzz-demangler: demangler-fuzzer
+ ./demangler-fuzzer
+
TEST_COMPILE = $(CC) @DEFS@ $(LIBCFLAGS) -I.. -I$(INCDIR) $(HDEFINES)
test-demangle: $(srcdir)/test-demangle.c ../libiberty.a
$(TEST_COMPILE) -o test-demangle \
@@ -72,6 +76,10 @@
$(TEST_COMPILE) -DHAVE_CONFIG_H -I.. -o test-expandargv \
$(srcdir)/test-expandargv.c ../libiberty.a
+demangler-fuzzer: $(srcdir)/demangler-fuzzer.c ../libiberty.a
+ $(TEST_COMPILE) -o demangler-fuzzer \
+ $(srcdir)/demangler-fuzzer.c ../libiberty.a
+
# Standard (either GNU or Cygnus) rules we don't use.
html install-html info install-info clean-info dvi pdf install-pdf \
install etags tags installcheck:
@@ -81,6 +89,7 @@
rm -f test-demangle
rm -f test-pexecute
rm -f test-expandargv
+ rm -f demangler-fuzzer
rm -f core
clean: mostlyclean
distclean: clean
Index: libiberty/testsuite/demangler-fuzzer.c
===
--- libiberty/testsuite/demangler-fuzzer.c (revision 0)
+++ libiberty/testsuite/demangler-fuzzer.c (revision 0)
@@ -0,0 +1,67 @@
+/* Demangler fuzzer.
+
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ This file is part of GNU libiberty.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAXLEN 253
+#define ALPMIN 33
+#define ALPMAX 127
+
+static int quit_flag;
+
+static void
+alarm_handler (int signum)
+{
+ quit_flag = 1;
+}
+
+int
+main (int argc, char *argv[])
+{
+ char symbol[2 + MAXLEN + 1] = "_Z";
+ int count = 0;
+
+ srand (time (NULL));
+ signal (SIGALRM, alarm_handler);
+ alarm (60);
+
+ while (!quit_flag)
+{
+ char *buffer = symbol + 2;
+ int length, i;
+
+ length = rand () % MAXLEN;
+ for (i = 0; i < length; i++)
+ *buffer++ = (rand () % (ALPMAX - ALPMIN)) + ALPMIN;
+
+ *buffer++ = '\0';
+
+ cplus_demangle (symbol, DMGL_AUTO | DMGL_ANSI | DMGL_PARAMS);
+
+ count++;
+}
+
+ printf ("Successfully demangled %d symbols\n", count);
+ exit (0);
+}
[PATCH] Demangler fuzzer
Hi all,
This patch adds a simple fuzzer for the libiberty C++ demangler.
You can run it like this:
make -C /path/to/build/libiberty/testsuite fuzz-demangler
It will run until it dumps core (usually only a few seconds).
Is this ok to commit?
Thanks,
Gary
--
2014-08-11 Gary Benson
* testsuite/demangler-fuzzer.c: New file.
* testsuite/Makefile.in (fuzz-demangler): New rule.
(demangler-fuzzer): Likewise.
(mostlyclean): Clean up demangler fuzzer.
Index: libiberty/testsuite/Makefile.in
===
--- libiberty/testsuite/Makefile.in (revision 213809)
+++ libiberty/testsuite/Makefile.in (working copy)
@@ -59,6 +59,10 @@
check-expandargv: test-expandargv
./test-expandargv
+# Run the demangler fuzzer
+fuzz-demangler: demangler-fuzzer
+ ./demangler-fuzzer
+
TEST_COMPILE = $(CC) @DEFS@ $(LIBCFLAGS) -I.. -I$(INCDIR) $(HDEFINES)
test-demangle: $(srcdir)/test-demangle.c ../libiberty.a
$(TEST_COMPILE) -o test-demangle \
@@ -72,6 +76,10 @@
$(TEST_COMPILE) -DHAVE_CONFIG_H -I.. -o test-expandargv \
$(srcdir)/test-expandargv.c ../libiberty.a
+demangler-fuzzer: $(srcdir)/demangler-fuzzer.c ../libiberty.a
+ $(TEST_COMPILE) -o demangler-fuzzer \
+ $(srcdir)/demangler-fuzzer.c ../libiberty.a
+
# Standard (either GNU or Cygnus) rules we don't use.
html install-html info install-info clean-info dvi pdf install-pdf \
install etags tags installcheck:
@@ -81,6 +89,7 @@
rm -f test-demangle
rm -f test-pexecute
rm -f test-expandargv
+ rm -f demangler-fuzzer
rm -f core
clean: mostlyclean
distclean: clean
Index: libiberty/testsuite/demangler-fuzzer.c
===
--- libiberty/testsuite/demangler-fuzzer.c (revision 0)
+++ libiberty/testsuite/demangler-fuzzer.c (revision 0)
@@ -0,0 +1,47 @@
+/* Demangler fuzzer.
+
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ This file is part of GNU libiberty.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+
+#define MAXLEN 253
+#define ALPMIN 33
+#define ALPMAX 127
+
+int
+main (int argc, char *argv[])
+{
+ char symbol[2 + MAXLEN + 1] = "_Z";
+
+ srand (time (NULL));
+ while (1)
+{
+ char *buffer = symbol + 2;
+ int length, i;
+
+ length = rand () % MAXLEN;
+ for (i = 0; i < length; i++)
+ *(buffer++) = (rand () % (ALPMAX - ALPMIN)) + ALPMIN;
+
+ *(buffer++) = '\0';
+
+ cplus_demangle (symbol, DMGL_AUTO | DMGL_ANSI | DMGL_PARAMS);
+}
+}
Re: [PATCH] cplus-demangler, free resource after a failed call to gnu_special.
Thomas Schwinge wrote:
> On Thu, 22 May 2014 17:02:08 +0100, Gary Benson wrote:
> > Thomas Schwinge wrote:
> > > In GCC, I'm consistenly seeing the following new failure:
> > >
> > > ./test-demangle <
> > > ../../../source/libiberty/testsuite/demangle-expected
> > > FAIL at line 4350, options --format=auto --no-params:
> > > in: _QueueNotification_QueueController__$4PPPM_A_INotice___Z
> > > out: (null)
> > > exp:
> > > ./test-demangle: 895 tests, 1 failures
> > > make[2]: *** [check-cplus-dem] Error 1
> > >
> > > The patch was committed incompletely; I added the missing last line in
> > > r210803:
> > [snip]
> > > @@ -4347,3 +4347,4 @@ void post
> > > >(std::function&&)::{lambda()#1}*& std
> > > --format=auto --no-params
> > > _QueueNotification_QueueController__$4PPPM_A_INotice___Z
> > > _QueueNotification_QueueController__$4PPPM_A_INotice___Z
> > > +_QueueNotification_QueueController__$4PPPM_A_INotice___Z
> >
> > I thought that extra line was a mistake; I thought each test was
> > precisely three lines:
> >
> > # options
> > # input to be demangled
> > # expected output
> >
> > What is the extra line here?
>
> I too had to look it up -- see the explanation at the beginning of
> the file:
>
> #--no-params There are two lines of expected output; the first
> #is with DMGL_PARAMS, the second is without it.
Ah, I missed that. Thank you for fixing this!
Gary
--
http://gbenson.net/
Re: [PATCH] cplus-demangler, free resource after a failed call to gnu_special.
Hi Thomas,
Thomas Schwinge wrote:
> In GCC, I'm consistenly seeing the following new failure:
>
> ./test-demangle < ../../../source/libiberty/testsuite/demangle-expected
> FAIL at line 4350, options --format=auto --no-params:
> in: _QueueNotification_QueueController__$4PPPM_A_INotice___Z
> out: (null)
> exp:
> ./test-demangle: 895 tests, 1 failures
> make[2]: *** [check-cplus-dem] Error 1
>
> The patch was committed incompletely; I added the missing last line in
> r210803:
[snip]
> @@ -4347,3 +4347,4 @@ void post >(std::function ()>&&)::{lambda()#1}*& std
> --format=auto --no-params
> _QueueNotification_QueueController__$4PPPM_A_INotice___Z
> _QueueNotification_QueueController__$4PPPM_A_INotice___Z
> +_QueueNotification_QueueController__$4PPPM_A_INotice___Z
I thought that extra line was a mistake; I thought each test was
precisely three lines:
# options
# input to be demangled
# expected output
What is the extra line here?
Thanks,
Gary
--
http://gbenson.net/
Re: [PATCH] cplus-demangler, free resource after a failed call to gnu_special.
Andrew Burgess wrote:
> On 14/05/2014 10:01 AM, Gary Benson wrote:
> > Ian Lance Taylor wrote:
> > > Andrew Burgess wrote:
> > > > On 09/05/2014 9:53 PM, Ian Lance Taylor wrote:
> > > > > Andrew Burgess wrote:
> > > > > >if ((AUTO_DEMANGLING || GNU_DEMANGLING))
> > > > > > {
> > > > > > success = gnu_special (work, &mangled, &decl);
> > > > > > + if (!success)
> > > > > > +{
> > > > > > + delete_work_stuff (work);
> > > > > > + string_delete (&decl);
> > > > > > +}
> > > > >
> > >
> > > This patch is OK.
> >
> > Andrew, would you like me to commit this?
>
> Yes please.
Done:
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=210425
Thanks,
Gary
--
http://gbenson.net/
Re: [PATCH] cplus-demangler, free resource after a failed call to gnu_special.
Ian Lance Taylor wrote:
> Andrew Burgess wrote:
> > On 09/05/2014 9:53 PM, Ian Lance Taylor wrote:
> > > Andrew Burgess wrote:
> > > >if ((AUTO_DEMANGLING || GNU_DEMANGLING))
> > > > {
> > > > success = gnu_special (work, &mangled, &decl);
> > > > + if (!success)
> > > > +{
> > > > + delete_work_stuff (work);
> > > > + string_delete (&decl);
> > > > +}
> > >
> > > As far as I can see, decl may be uninitialized at this point. I
> > > don't think you can call string_delete. You need to ensure that
> > > decl is initialized somehow.
> >
> > There's a call to string_init on decl about 10 lines above the
> > above diff, just outside of context, but it's unconditional, so
> > I figured that would be enough.
> >
> > Also, if gnu_special returns false, and the call to
> > demangle_prefix returns false then we call (near the bottom of
> > internal_cplus_demangle) mop_up, which calls string_delete.
> >
> > Given that decl is initialised once, assuming that the string is
> > only released using delete_string then the internal state will
> > have been reset back to NULL, so calling delete_string should be
> > safe again.
>
> Right, sorry for the noise.
>
> This patch is OK.
Andrew, would you like me to commit this?
Thanks,
Gary
--
http://gbenson.net/
[C++ PATCH] demangler fix
Hi all,
A patch I committed to libiberty last year [1, 2] caused a regression
that caused the demangler to segfault on certain symbols [3, 4, 5, 6].
The attached patch fixes, and adds regression tests for all symbols
referenced in those bugs.
Ok to commit?
Thanks,
Gary
--
http://gbenson.net/
[1] http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01299.html
[2] http://gcc.gnu.org/ml/gcc-patches/2013-12/msg01755.html
[3] https://sourceware.org/bugzilla/show_bug.cgi?id=14963
[4] https://sourceware.org/bugzilla/show_bug.cgi?id=16593
[5] https://sourceware.org/bugzilla/show_bug.cgi?id=16752
[6] https://sourceware.org/bugzilla/show_bug.cgi?id=16845
2014-05-07 Gary Benson
* cp-demangle.c (struct d_component_stack): New structure.
(struct d_print_info): New field component_stack.
(d_print_init): Initialize the above.
(d_print_comp_inner): Renamed from d_print_comp.
Do not restore template stack if it would cause a loop.
(d_print_comp): New function.
* testsuite/demangle-expected: New test cases.
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index bf2ffa9..41c86c7 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -275,6 +275,16 @@ struct d_growable_string
int allocation_failure;
};
+/* Stack of components, innermost first, used to avoid loops. */
+
+struct d_component_stack
+{
+ /* This component. */
+ const struct demangle_component *dc;
+ /* This component's parent. */
+ const struct d_component_stack *parent;
+};
+
/* A demangle component and some scope captured when it was first
traversed. */
@@ -327,6 +337,8 @@ struct d_print_info
int pack_index;
/* Number of d_print_flush calls so far. */
unsigned long int flush_count;
+ /* Stack of components, innermost first, used to avoid loops. */
+ const struct d_component_stack *component_stack;
/* Array of saved scopes for evaluating substitutions. */
struct d_saved_scope *saved_scopes;
/* Index of the next unused saved scope in the above array. */
@@ -3934,6 +3946,8 @@ d_print_init (struct d_print_info *dpi,
demangle_callbackref callback,
dpi->demangle_failure = 0;
+ dpi->component_stack = NULL;
+
dpi->saved_scopes = NULL;
dpi->next_saved_scope = 0;
dpi->num_saved_scopes = 0;
@@ -4269,8 +4283,8 @@ d_get_saved_scope (struct d_print_info *dpi,
/* Subroutine to handle components. */
static void
-d_print_comp (struct d_print_info *dpi, int options,
- const struct demangle_component *dc)
+d_print_comp_inner (struct d_print_info *dpi, int options,
+ const struct demangle_component *dc)
{
/* Magic variable to let reference smashing skip over the next modifier
without needing to modify *dc. */
@@ -4673,11 +4687,30 @@ d_print_comp (struct d_print_info *dpi, int options,
}
else
{
+ const struct d_component_stack *dcse;
+ int found_self_or_parent = 0;
+
/* This traversal is reentering SUB as a substition.
- Restore the original templates temporarily. */
- saved_templates = dpi->templates;
- dpi->templates = scope->templates;
- need_template_restore = 1;
+ If we are not beneath SUB or DC in the tree then we
+ need to restore SUB's template stack temporarily. */
+ for (dcse = dpi->component_stack; dcse != NULL;
+dcse = dcse->parent)
+ {
+ if (dcse->dc == sub
+ || (dcse->dc == dc
+ && dcse != dpi->component_stack))
+ {
+ found_self_or_parent = 1;
+ break;
+ }
+ }
+
+ if (!found_self_or_parent)
+ {
+ saved_templates = dpi->templates;
+ dpi->templates = scope->templates;
+ need_template_restore = 1;
+ }
}
a = d_lookup_template_argument (dpi, sub);
@@ -5316,6 +5349,21 @@ d_print_comp (struct d_print_info *dpi, int options,
}
}
+static void
+d_print_comp (struct d_print_info *dpi, int options,
+ const struct demangle_component *dc)
+{
+ struct d_component_stack self;
+
+ self.dc = dc;
+ self.parent = dpi->component_stack;
+ dpi->component_stack = &self;
+
+ d_print_comp_inner (dpi, options, dc);
+
+ dpi->component_stack = self.parent;
+}
+
/* Print a Java dentifier. For Java we try to handle encoded extended
Unicode characters. The C++ ABI doesn't mention Unicode encoding,
so we don't it for C++. Characters are encoded as
diff --git a/libiberty/testsuite/demangle-expected
b/libiberty/testsuite/demangle-expected
index 3ff08e6..453f9a3 10064
[PATCH, libiberty] Remove malloc/realloc from demangler (was: Add a couple of missing casts)
Ian Lance Taylor wrote:
> On Wed, Nov 13, 2013 at 7:30 AM, Gary Benson wrote:
> > Richard Biener wrote:
> > > On Tue, Nov 12, 2013 at 8:55 PM, Ian Lance Taylor wrote:
> > > > On Tue, Nov 12, 2013 at 11:24 AM, Uros Bizjak wrote:
> > > > >
> > > > > This was uncovered by x86 lto-profiledbootstrap. The patch allows
> > > > > lto-profiledbootstrap to proceed further.
> > > > >
> > > > > 2013-11-12 Uros Bizjak
> > > > >
> > > > > * cp-demangle.c (d_copy_templates): Cast result of malloc
> > > > > to (struct d_print_template *).
> > > > > (d_print_comp): Cast result of realloc to (struct d_saved scope
> > > > > *).
> > > > >
> > > > > Tested on x86_64-pc-linux-gnu.
> > > > >
> > > > > OK for mainline?
> > > >
> > > > The patch is OK, but this code is troubling. I obviously
> > > > should have looked at it earlier. The C++ demangler is
> > > > sometimes used in panic situations, when malloc is not
> > > > available. The interface was designed to be usable without
> > > > requiring malloc, by passing in a sufficiently large buffer.
> > > > I'm concerned that we apparently now require malloc to work.
> > >
> > > That indeed looks like an important regression - Gary, can you
> > > please work to fix this?
> >
> > I'm on it.
>
> Thanks. See also the cplus_demangle_print_callback function.
The below patch should remedy this.
After the end of today I'll likely not check this email until
January 5. If this patch gets approved before then and you
want it in please either ping me on gary@ the domain in my sig
or just somebody else commit it for me.
Thanks,
Gary
--
http://gbenson.net/
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 825ddd2..78c1433 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,20 @@
+2013-12-20 Gary Benson
+
+ * cp-demangle.c (struct d_print_info): New fields
+ next_saved_scope, copy_templates, next_copy_template and
+ num_copy_templates.
+ (d_count_templates): New function.
+ (d_print_init): New parameter "dc".
+ Estimate numbers of templates and scopes required.
+ (d_print_free): Removed function.
+ (cplus_demangle_print_callback): Allocate stack for
+ templates and scopes. Removed call to d_print_free.
+ (d_copy_templates): Removed function.
+ (d_save_scope): New function.
+ (d_get_saved_scope): Likewise.
+ (d_print_comp): Replace state saving/restoring code with
+ calls to d_save_scope and d_get_saved_scope.
+
2013-11-22 Cary Coutant
PR other/59195
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 029151e..de08d94 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -329,8 +329,16 @@ struct d_print_info
unsigned long int flush_count;
/* Array of saved scopes for evaluating substitutions. */
struct d_saved_scope *saved_scopes;
+ /* Index of the next unused saved scope in the above array. */
+ int next_saved_scope;
/* Number of saved scopes in the above array. */
int num_saved_scopes;
+ /* Array of templates for saving into scopes. */
+ struct d_print_template *copy_templates;
+ /* Index of the next unused copy template in the above array. */
+ int next_copy_template;
+ /* Number of copy templates in the above array. */
+ int num_copy_templates;
/* The nearest enclosing template, if any. */
const struct demangle_component *current_template;
};
@@ -475,7 +483,8 @@ static void
d_growable_string_callback_adapter (const char *, size_t, void *);
static void
-d_print_init (struct d_print_info *, demangle_callbackref, void *);
+d_print_init (struct d_print_info *, demangle_callbackref, void *,
+ const struct demangle_component *);
static inline void d_print_error (struct d_print_info *);
@@ -3770,11 +3779,141 @@ d_growable_string_callback_adapter (const char *s,
size_t l, void *opaque)
d_growable_string_append_buffer (dgs, s, l);
}
+/* Walk the tree, counting the number of templates encountered, and
+ the number of times a scope might be saved. These counts will be
+ used to allocate data structures for d_print_comp, so the logic
+ here must mirror the logic d_print_comp will use. It is not
+ important that the resulting numbers are exact, so long as they
+ are larger than the actual numbers encountered. */
+
+static void
+d_count_templates_scopes (int *num_templates, int *num_scopes,
+ const struct demangle_component *dc)
+{
+ if (dc == NULL)
+return;
+
+ switch (dc->type)
+{
+case DEMANGLE_CO
Re: [PATCH, libiberty]: Add a couple of missing casts
Richard Biener wrote: > On Tue, Nov 12, 2013 at 8:55 PM, Ian Lance Taylor wrote: > > On Tue, Nov 12, 2013 at 11:24 AM, Uros Bizjak wrote: > > > > > > This was uncovered by x86 lto-profiledbootstrap. The patch allows > > > lto-profiledbootstrap to proceed further. > > > > > > 2013-11-12 Uros Bizjak > > > > > > * cp-demangle.c (d_copy_templates): Cast result of malloc > > > to (struct d_print_template *). > > > (d_print_comp): Cast result of realloc to (struct d_saved scope *). > > > > > > Tested on x86_64-pc-linux-gnu. > > > > > > OK for mainline? > > > > The patch is OK, but this code is troubling. I obviously should > > have looked at it earlier. The C++ demangler is sometimes used in > > panic situations, when malloc is not available. The interface was > > designed to be usable without requiring malloc, by passing in a > > sufficiently large buffer. I'm concerned that we apparently now > > require malloc to work. > > That indeed looks like an important regression - Gary, can you > please work to fix this? I'm on it. Thanks, Gary -- http://gbenson.net/
[2nd PING] [C++ PATCH] demangler fix (take 2)
Hi all,
This is a resubmission of my previous demangler fix [1] rewritten
to avoid using hashtables and other libiberty features.
From the above referenced email:
d_print_comp maintains a certain amount of scope across calls (namely
a stack of templates) which is used when evaluating references in
template argument lists. If such a reference is later used from a
subtitution then the scope in force at the time of the substitution is
used. This appears to be wrong (I say appears because I couldn't find
anything in the API [2] to clarify this).
The attached patch causes the demangler to capture the scope the first
time such a reference is traversed, and to use that captured scope on
subsequent traversals. This fixes GDB PR 14963 [3] whereby a
reference is resolved against the wrong template, causing an infinite
loop and eventual stack overflow and segmentation fault.
I've added the result to the demangler test suite, but I know of no
way to check the validity of the demangled symbol other than by
inspection (and I am no expert here!) If anybody knows a way to
check this then please let me know! Otherwise, I hope this
not-really-checked demangled version is acceptable.
Thanks,
Gary
[1] http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00215.html
[2] http://mentorembedded.github.io/cxx-abi/abi.html#mangling
[3] http://sourceware.org/bugzilla/show_bug.cgi?id=14963
--
http://gbenson.net/
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 89e108a..2ff8216 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,20 @@
+2013-09-17 Gary Benson
+
+ * cp-demangle.c (struct d_saved_scope): New structure.
+ (struct d_print_info): New fields saved_scopes and
+ num_saved_scopes.
+ (d_print_init): Initialize the above.
+ (d_print_free): New function.
+ (cplus_demangle_print_callback): Call the above.
+ (d_copy_templates): New function.
+ (d_print_comp): New variables saved_templates and
+ need_template_restore.
+ [DEMANGLE_COMPONENT_REFERENCE,
+ DEMANGLE_COMPONENT_RVALUE_REFERENCE]: Capture scope the first
+ time the component is traversed, and use the captured scope for
+ subsequent traversals.
+ * testsuite/demangle-expected: Add regression test.
+
2013-09-10 Paolo Carlini
PR bootstrap/58386
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 70f5438..a199f6d 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -275,6 +275,18 @@ struct d_growable_string
int allocation_failure;
};
+/* A demangle component and some scope captured when it was first
+ traversed. */
+
+struct d_saved_scope
+{
+ /* The component whose scope this is. */
+ const struct demangle_component *container;
+ /* The list of templates, if any, that was current when this
+ scope was captured. */
+ struct d_print_template *templates;
+};
+
enum { D_PRINT_BUFFER_LENGTH = 256 };
struct d_print_info
{
@@ -302,6 +314,10 @@ struct d_print_info
int pack_index;
/* Number of d_print_flush calls so far. */
unsigned long int flush_count;
+ /* Array of saved scopes for evaluating substitutions. */
+ struct d_saved_scope *saved_scopes;
+ /* Number of saved scopes in the above array. */
+ int num_saved_scopes;
};
#ifdef CP_DEMANGLE_DEBUG
@@ -3665,6 +3681,30 @@ d_print_init (struct d_print_info *dpi,
demangle_callbackref callback,
dpi->opaque = opaque;
dpi->demangle_failure = 0;
+
+ dpi->saved_scopes = NULL;
+ dpi->num_saved_scopes = 0;
+}
+
+/* Free a print information structure. */
+
+static void
+d_print_free (struct d_print_info *dpi)
+{
+ int i;
+
+ for (i = 0; i < dpi->num_saved_scopes; i++)
+{
+ struct d_print_template *ts, *tn;
+
+ for (ts = dpi->saved_scopes[i].templates; ts != NULL; ts = tn)
+ {
+ tn = ts->next;
+ free (ts);
+ }
+}
+
+ free (dpi->saved_scopes);
}
/* Indicate that an error occurred during printing, and test for error. */
@@ -3749,6 +3789,7 @@ cplus_demangle_print_callback (int options,
demangle_callbackref callback, void *opaque)
{
struct d_print_info dpi;
+ int success;
d_print_init (&dpi, callback, opaque);
@@ -3756,7 +3797,9 @@ cplus_demangle_print_callback (int options,
d_print_flush (&dpi);
- return ! d_print_saw_error (&dpi);
+ success = ! d_print_saw_error (&dpi);
+ d_print_free (&dpi);
+ return success;
}
/* Turn components into a human readable string. OPTIONS is the
@@ -3913,6 +3956,36 @@ d_print_subexpr (struct d_print_info *dpi, int options,
d_append_char (dpi, ')');
}
+/* Return a shallow copy of the current list of templates.
+ On error d_print_error is called and a partial list may
+ be returned. Whatever is returned must be freed. */
+
+static struct d_print_template *
+d_copy_templates (struct d_print_info *dpi)
+{
+
[PING] [C++ PATCH] demangler fix (take 2)
Gary Benson wrote:
> Hi all,
>
> This is a resubmission of my previous demangler fix [1] rewritten
> to avoid using hashtables and other libiberty features.
>
> From the above referenced email:
>
> d_print_comp maintains a certain amount of scope across calls (namely
> a stack of templates) which is used when evaluating references in
> template argument lists. If such a reference is later used from a
> subtitution then the scope in force at the time of the substitution is
> used. This appears to be wrong (I say appears because I couldn't find
> anything in the API [2] to clarify this).
>
> The attached patch causes the demangler to capture the scope the first
> time such a reference is traversed, and to use that captured scope on
> subsequent traversals. This fixes GDB PR 14963 [3] whereby a
> reference is resolved against the wrong template, causing an infinite
> loop and eventual stack overflow and segmentation fault.
>
> I've added the result to the demangler test suite, but I know of no
> way to check the validity of the demangled symbol other than by
> inspection (and I am no expert here!) If anybody knows a way to
> check this then please let me know! Otherwise, I hope this
> not-really-checked demangled version is acceptable.
>
> Thanks,
> Gary
>
> [1] http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00215.html
> [2] http://mentorembedded.github.io/cxx-abi/abi.html#mangling
> [3] http://sourceware.org/bugzilla/show_bug.cgi?id=14963
>
> --
> http://gbenson.net/
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 89e108a..2ff8216 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,20 @@
+2013-09-17 Gary Benson
+
+ * cp-demangle.c (struct d_saved_scope): New structure.
+ (struct d_print_info): New fields saved_scopes and
+ num_saved_scopes.
+ (d_print_init): Initialize the above.
+ (d_print_free): New function.
+ (cplus_demangle_print_callback): Call the above.
+ (d_copy_templates): New function.
+ (d_print_comp): New variables saved_templates and
+ need_template_restore.
+ [DEMANGLE_COMPONENT_REFERENCE,
+ DEMANGLE_COMPONENT_RVALUE_REFERENCE]: Capture scope the first
+ time the component is traversed, and use the captured scope for
+ subsequent traversals.
+ * testsuite/demangle-expected: Add regression test.
+
2013-09-10 Paolo Carlini
PR bootstrap/58386
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 70f5438..a199f6d 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -275,6 +275,18 @@ struct d_growable_string
int allocation_failure;
};
+/* A demangle component and some scope captured when it was first
+ traversed. */
+
+struct d_saved_scope
+{
+ /* The component whose scope this is. */
+ const struct demangle_component *container;
+ /* The list of templates, if any, that was current when this
+ scope was captured. */
+ struct d_print_template *templates;
+};
+
enum { D_PRINT_BUFFER_LENGTH = 256 };
struct d_print_info
{
@@ -302,6 +314,10 @@ struct d_print_info
int pack_index;
/* Number of d_print_flush calls so far. */
unsigned long int flush_count;
+ /* Array of saved scopes for evaluating substitutions. */
+ struct d_saved_scope *saved_scopes;
+ /* Number of saved scopes in the above array. */
+ int num_saved_scopes;
};
#ifdef CP_DEMANGLE_DEBUG
@@ -3665,6 +3681,30 @@ d_print_init (struct d_print_info *dpi,
demangle_callbackref callback,
dpi->opaque = opaque;
dpi->demangle_failure = 0;
+
+ dpi->saved_scopes = NULL;
+ dpi->num_saved_scopes = 0;
+}
+
+/* Free a print information structure. */
+
+static void
+d_print_free (struct d_print_info *dpi)
+{
+ int i;
+
+ for (i = 0; i < dpi->num_saved_scopes; i++)
+{
+ struct d_print_template *ts, *tn;
+
+ for (ts = dpi->saved_scopes[i].templates; ts != NULL; ts = tn)
+ {
+ tn = ts->next;
+ free (ts);
+ }
+}
+
+ free (dpi->saved_scopes);
}
/* Indicate that an error occurred during printing, and test for error. */
@@ -3749,6 +3789,7 @@ cplus_demangle_print_callback (int options,
demangle_callbackref callback, void *opaque)
{
struct d_print_info dpi;
+ int success;
d_print_init (&dpi, callback, opaque);
@@ -3756,7 +3797,9 @@ cplus_demangle_print_callback (int options,
d_print_flush (&dpi);
- return ! d_print_saw_error (&dpi);
+ success = ! d_print_saw_error (&dpi);
+ d_print_free (&dpi);
+ return success;
}
/* Turn components into a human readable string. OPTIONS is the
@@ -3913,6 +3956,36 @@ d_print_subexpr (struct d_print_info *dpi, int options,
d_append_char (dpi, ')');
}
+/* Return a shallow copy of the current list of templates.
+ On
[C++ PATCH] demangler fix (take 2)
Hi all,
This is a resubmission of my previous demangler fix [1] rewritten
to avoid using hashtables and other libiberty features.
>From the above referenced email:
d_print_comp maintains a certain amount of scope across calls (namely
a stack of templates) which is used when evaluating references in
template argument lists. If such a reference is later used from a
subtitution then the scope in force at the time of the substitution is
used. This appears to be wrong (I say appears because I couldn't find
anything in the API [2] to clarify this).
The attached patch causes the demangler to capture the scope the first
time such a reference is traversed, and to use that captured scope on
subsequent traversals. This fixes GDB PR 14963 [3] whereby a
reference is resolved against the wrong template, causing an infinite
loop and eventual stack overflow and segmentation fault.
I've added the result to the demangler test suite, but I know of no
way to check the validity of the demangled symbol other than by
inspection (and I am no expert here!) If anybody knows a way to
check this then please let me know! Otherwise, I hope this
not-really-checked demangled version is acceptable.
Thanks,
Gary
[1] http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00215.html
[2] http://mentorembedded.github.io/cxx-abi/abi.html#mangling
[3] http://sourceware.org/bugzilla/show_bug.cgi?id=14963
--
http://gbenson.net/
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 89e108a..2ff8216 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,20 @@
+2013-09-17 Gary Benson
+
+ * cp-demangle.c (struct d_saved_scope): New structure.
+ (struct d_print_info): New fields saved_scopes and
+ num_saved_scopes.
+ (d_print_init): Initialize the above.
+ (d_print_free): New function.
+ (cplus_demangle_print_callback): Call the above.
+ (d_copy_templates): New function.
+ (d_print_comp): New variables saved_templates and
+ need_template_restore.
+ [DEMANGLE_COMPONENT_REFERENCE,
+ DEMANGLE_COMPONENT_RVALUE_REFERENCE]: Capture scope the first
+ time the component is traversed, and use the captured scope for
+ subsequent traversals.
+ * testsuite/demangle-expected: Add regression test.
+
2013-09-10 Paolo Carlini
PR bootstrap/58386
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 70f5438..a199f6d 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -275,6 +275,18 @@ struct d_growable_string
int allocation_failure;
};
+/* A demangle component and some scope captured when it was first
+ traversed. */
+
+struct d_saved_scope
+{
+ /* The component whose scope this is. */
+ const struct demangle_component *container;
+ /* The list of templates, if any, that was current when this
+ scope was captured. */
+ struct d_print_template *templates;
+};
+
enum { D_PRINT_BUFFER_LENGTH = 256 };
struct d_print_info
{
@@ -302,6 +314,10 @@ struct d_print_info
int pack_index;
/* Number of d_print_flush calls so far. */
unsigned long int flush_count;
+ /* Array of saved scopes for evaluating substitutions. */
+ struct d_saved_scope *saved_scopes;
+ /* Number of saved scopes in the above array. */
+ int num_saved_scopes;
};
#ifdef CP_DEMANGLE_DEBUG
@@ -3665,6 +3681,30 @@ d_print_init (struct d_print_info *dpi,
demangle_callbackref callback,
dpi->opaque = opaque;
dpi->demangle_failure = 0;
+
+ dpi->saved_scopes = NULL;
+ dpi->num_saved_scopes = 0;
+}
+
+/* Free a print information structure. */
+
+static void
+d_print_free (struct d_print_info *dpi)
+{
+ int i;
+
+ for (i = 0; i < dpi->num_saved_scopes; i++)
+{
+ struct d_print_template *ts, *tn;
+
+ for (ts = dpi->saved_scopes[i].templates; ts != NULL; ts = tn)
+ {
+ tn = ts->next;
+ free (ts);
+ }
+}
+
+ free (dpi->saved_scopes);
}
/* Indicate that an error occurred during printing, and test for error. */
@@ -3749,6 +3789,7 @@ cplus_demangle_print_callback (int options,
demangle_callbackref callback, void *opaque)
{
struct d_print_info dpi;
+ int success;
d_print_init (&dpi, callback, opaque);
@@ -3756,7 +3797,9 @@ cplus_demangle_print_callback (int options,
d_print_flush (&dpi);
- return ! d_print_saw_error (&dpi);
+ success = ! d_print_saw_error (&dpi);
+ d_print_free (&dpi);
+ return success;
}
/* Turn components into a human readable string. OPTIONS is the
@@ -3913,6 +3956,36 @@ d_print_subexpr (struct d_print_info *dpi, int options,
d_append_char (dpi, ')');
}
+/* Return a shallow copy of the current list of templates.
+ On error d_print_error is called and a partial list may
+ be returned. Whatever is returned must be freed. */
+
+static struct d_print_template *
+d_copy_templates (struct d_print_info *dpi)
+
Re: C++ demangler fix
Jakub Jelinek wrote: > cp-demangle.c isn't used just in libiberty, where using hashtab, > xcalloc, XNEW etc. is fine, but also in libsupc++/libstdc++, where > none of that is fine. That is why cp-demangle.c only uses > e.g. realloc, checks for allocation failures and propagates those to > the caller if they happen (see allocation_failure field). hashtab.o > isn't linked into libstdc++ nor libsupc++, and the question is if we > really do want to link all the hashtable code into libstdc++. > How many hash table entries are there typically? Is a hashtable > required? Three entries were required for the symbol in the testcase: "_ZSt7forwardIRN1x14refobjiteratorINS0_3refINS0_4mime30multipart_se" \ "ction_processorObjIZ15get_body_parserIZZN14mime_processor21make_se" \ "ction_iteratorERKNS2_INS3_10sectionObjENS0_10ptrrefBaseEEEbENKUlvE" \ "_clEvEUlSB_bE_ZZNS6_21make_section_iteratorESB_bENKSC_clEvEUlSB_E0" \ "_ENS1_INS2_INS0_20outputrefiteratorObjIiEES8_RKSsSB_OT_OT0_EUl" \ "mE_NS3_32make_multipart_default_discarderISP_S8_EOT_RNSt16" \ "remove_referenceISW_E4typeE" I don't think there will many symbols with very many entries required. I'm guessing that most symbols will require zero (which is why I made it defer hashtable creation until it was required). What kind of data structure would you like to see here, a realloc'd array? Do libsupc++ and libstdc++ use the demangler for anything more performance-sensitive than exception printing? Thanks, Gary -- http://gbenson.net/
C++ demangler fix
Hi all,
d_print_comp maintains a certain amount of scope across calls (namely
a stack of templates) which is used when evaluating references in
template argument lists. If such a reference is later used from a
subtitution then the scope in force at the time of the substitution is
used. This appears to be wrong (I say appears because I couldn't find
anything in http://mentorembedded.github.io/cxx-abi/abi.html#mangling
to clarify this).
The attached patch causes the demangler to capture the scope the first
time such a reference is traversed, and to use that captured scope on
subsequent traversals. This fixes
http://sourceware.org/bugzilla/show_bug.cgi?id=14963 whereby a
reference is resolved against the wrong template, causing an infinite
loop and eventual stack overflow and segmentation fault.
I've added the result to the demangler test suite, but I know of
no way to check the validity of the demangled symbol other than by
inspection (and I am no expert here!) If anybody knows a way to
check this then please let me know! Otherwise, I hope this
not-really-checked demangled version is acceptable.
Thanks,
Gary
--
http://gbenson.net/
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index e4ce0b9..a084282 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,22 @@
+2013-09-04 Gary Benson
+
+ * cp-demangle.c: Include hashtab.h.
+ (struct d_print_info): New field saved_scopes.
+ (d_print_init): Initialize the above.
+ (d_print_free): New function.
+ (cplus_demangle_print_callback): Call the above.
+ (struct d_saved_scope): New structure.
+ (d_store_scope): New function.
+ (d_free_scope) Likewise.
+ (d_restore_scope) Likewise.
+ (d_hash_saved_scope) Likewise.
+ (d_equal_saved_scope) Likewise.
+ (d_print_comp): New variable saved_scope.
+ [DEMANGLE_COMPONENT_REFERENCE,
+ DEMANGLE_COMPONENT_RVALUE_REFERENCE]: Capture scope the first
+ time the component is traversed, and use the captured scope for
+ subsequent traversals.
+
2013-08-20 Alan Modra
* floatformat.c (floatformat_ibm_long_double): Rename to..
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 70f5438..d44e82a 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -128,6 +128,7 @@ extern char *alloca ();
#include "libiberty.h"
#include "demangle.h"
#include "cp-demangle.h"
+#include "hashtab.h"
/* If IN_GLIBCPP_V3 is defined, some functions are made static. We
also rename them via #define to avoid compiler errors when the
@@ -302,6 +303,10 @@ struct d_print_info
int pack_index;
/* Number of d_print_flush calls so far. */
unsigned long int flush_count;
+ /* Table mapping demangle components to scopes saved when first
+ traversing those components. These are used while evaluating
+ substitutions. */
+ htab_t saved_scopes;
};
#ifdef CP_DEMANGLE_DEBUG
@@ -3665,6 +3670,17 @@ d_print_init (struct d_print_info *dpi,
demangle_callbackref callback,
dpi->opaque = opaque;
dpi->demangle_failure = 0;
+
+ dpi->saved_scopes = NULL;
+}
+
+/* Free a print information structure. */
+
+static void
+d_print_free (struct d_print_info *dpi)
+{
+ if (dpi->saved_scopes != NULL)
+htab_delete (dpi->saved_scopes);
}
/* Indicate that an error occurred during printing, and test for error. */
@@ -3749,6 +3765,7 @@ cplus_demangle_print_callback (int options,
demangle_callbackref callback, void *opaque)
{
struct d_print_info dpi;
+ int success;
d_print_init (&dpi, callback, opaque);
@@ -3756,7 +3773,9 @@ cplus_demangle_print_callback (int options,
d_print_flush (&dpi);
- return ! d_print_saw_error (&dpi);
+ success = ! d_print_saw_error (&dpi);
+ d_print_free (&dpi);
+ return success;
}
/* Turn components into a human readable string. OPTIONS is the
@@ -3913,6 +3932,114 @@ d_print_subexpr (struct d_print_info *dpi, int options,
d_append_char (dpi, ')');
}
+/* A demangle component and some scope captured when it was first
+ traversed. */
+
+struct d_saved_scope
+{
+ /* The component whose scope this is. Used as the key for the
+ saved_scopes hashtable in d_print_info. May be NULL if this
+ scope will not be inserted into that table. */
+ const struct demangle_component *container;
+ /* Nonzero if the below items are copies and require freeing
+ when this scope is freed. */
+ int is_copy;
+ /* The list of templates, if any, that was current when this
+ scope was captured. */
+ struct d_print_template *templates;
+};
+
+/* Allocate a scope and populate it with the current values from DPI.
+ CONTAINER is the demangle component to which the scope refers, and
+ is used as the key for the saved_scopes hashtable in d_print_info.
+ CONTAINER may be NULL if this sc
