Hi Bernd,
Thanks for the feedback!
> Patches need to be bootstrapped and regression tested, and patch submissions
> should include which target this was done on.
>
> Ideally you'd also want to include testcases along with your patches,
> although I'm not entirely sure how we can arrange for this type of problem to
> be tested.
Regression tested on x86_64-pc-linux-gnu (make check). Test cases added to
libiberty/testsuite/demangler-expected and checked PR70498 is resolved. Not
sure how to bootstrap the patch.
> Lastly, for this specific patch, I have trouble seeing how it fixes anything.
> I'd need a more detailed explanation of how the problem happens in the first
> place.
In the patched version, the values wrap around when they are parsed in
d_number. Since the mangled string may contain negative numbers, there is
usually proper handling of negative numbers in the clients of d_number. Without
the patch a value can become negative when cast from long to int *after* these
checks.
For instance, in d_source_name the length of the identifier is parsed as long
from the mangled string and checked whether it is negative. Since d_identifier
takes an int as length, d_identifier is called with a negative length after the
implicit cast:
static struct demangle_component *
d_source_name (struct d_info *di)
{
int len;
struct demangle_component *ret;
len = d_number (di);
if (len <= 0)
return NULL;
ret = d_identifier (di, len);
di->last_name = ret;
return ret;
}
--
Index: libiberty/cp-demangle.c
===================================================================
--- libiberty/cp-demangle.c (revision 234663)
+++ libiberty/cp-demangle.c (working copy)
@@ -398,7 +398,7 @@
struct demangle_component *);
static struct demangle_component *
-d_make_template_param (struct d_info *, long);
+d_make_template_param (struct d_info *, int);
static struct demangle_component *
d_make_sub (struct d_info *, const char *, int);
@@ -421,9 +421,9 @@
static struct demangle_component *d_source_name (struct d_info *);
-static long d_number (struct d_info *);
+static int d_number (struct d_info *);
-static struct demangle_component *d_identifier (struct d_info *, long);
+static struct demangle_component *d_identifier (struct d_info *, int);
static struct demangle_component *d_operator_name (struct d_info *);
@@ -1119,7 +1119,7 @@
/* Add a new template parameter. */
static struct demangle_component *
-d_make_template_param (struct d_info *di, long i)
+d_make_template_param (struct d_info *di, int i)
{
struct demangle_component *p;
@@ -1135,7 +1135,7 @@
/* Add a new function parameter. */
static struct demangle_component *
-d_make_function_param (struct d_info *di, long i)
+d_make_function_param (struct d_info *di, int i)
{
struct demangle_component *p;
@@ -1620,7 +1620,7 @@
static struct demangle_component *
d_source_name (struct d_info *di)
{
- long len;
+ int len;
struct demangle_component *ret;
len = d_number (di);
@@ -1633,12 +1633,12 @@
/* number ::= [n] <(non-negative decimal integer)> */
-static long
+static int
d_number (struct d_info *di)
{
int negative;
char peek;
- long ret;
+ int ret;
negative = 0;
peek = d_peek_char (di);
@@ -1681,7 +1681,7 @@
/* identifier ::= <(unqualified source code identifier)> */
static struct demangle_component *
-d_identifier (struct d_info *di, long len)
+d_identifier (struct d_info *di, int len)
{
const char *name;
@@ -1702,7 +1702,7 @@
/* Look for something which looks like a gcc encoding of an
anonymous namespace, and replace it with a more user friendly
name. */
- if (len >= (long) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2
+ if (len >= ANONYMOUS_NAMESPACE_PREFIX_LEN + 2
&& memcmp (name, ANONYMOUS_NAMESPACE_PREFIX,
ANONYMOUS_NAMESPACE_PREFIX_LEN) == 0)
{
@@ -1870,7 +1870,7 @@
{
struct demangle_component *p = NULL;
struct demangle_component *next = NULL;
- long len, i;
+ int len, i;
char c;
const char *str;
@@ -2012,7 +2012,7 @@
case 'C':
{
struct demangle_component *derived_type;
- long offset;
+ int offset;
struct demangle_component *base_type;
derived_type = cplus_demangle_type (di);
@@ -2946,10 +2946,10 @@
/* <non-negative number> _ */
-static long
+static int
d_compact_number (struct d_info *di)
{
- long num;
+ int num;
if (d_peek_char (di) == '_')
num = 0;
else if (d_peek_char (di) == 'n')
@@ -2969,7 +2969,7 @@
static struct demangle_component *
d_template_param (struct d_info *di)
{
- long param;
+ int param;
if (! d_check_char (di, 'T'))
return NULL;
@@ -3502,7 +3502,7 @@
static int
d_discriminator (struct d_info *di)
{
- long discrim;
+ int discrim;
if (d_peek_char (di) != '_')
return 1;
@@ -3558,7 +3558,7 @@
d_unnamed_type (struct d_info *di)
{
struct demangle_component *ret;
- long num;
+ int num;
if (! d_check_char (di, 'U'))
return NULL;
@@ -4086,7 +4086,7 @@
}
static inline void
-d_append_num (struct d_print_info *dpi, long l)
+d_append_num (struct d_print_info *dpi, int l)
{
char buf[25];
sprintf (buf,"%ld", l);
Index: libiberty/testsuite/demangle-expected
===================================================================
--- libiberty/testsuite/demangle-expected (revision 234663)
+++ libiberty/testsuite/demangle-expected (working copy)
@@ -4422,12 +4422,17 @@
_Z3fooI1FEN1XIXszdtcl1PclcvT__EEE5arrayEE4TypeEv
X<sizeof ((P(((F)())())).array)>::Type foo<F>()
#
-# Tests a use-after-free problem
+# Tests a use-after-free problem PR70481
_Q.__0
::Q.(void)
#
-# Tests a use-after-free problem
+# Tests a use-after-free problem PR70481
_Q10-__9cafebabe.
cafebabe.::-(void)
+#
+# Tests write access violation PR70498
+
+_Z80800000000000000000000
+_Z80800000000000000000000
