[PATCH] Fix malloc allocation size sanity check

[PATCH] Fix malloc allocation size sanity check

[Luis Machado]

During debugging of PR26362, it was noticed that the malloc size check
in check_type_length_before_alloc wasn't detecting an allocation attempt
of a huge amount of bytes, making GDB run into an internal error.

This happens because we're using an int to store a type's length. When the
type length is large enough, the int will overflow and the max_value_size
check won't work anymore.

The following patch fixes this by making the length variable a size_t.

Printing statements were also updated to show the correct number of bytes.

gdb/ChangeLog:

YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

	* value.c (check_type_length_before_alloc): Use size_t to store a
	type's length.
	Use %s and pulongest to print the length.
---
 gdb/value.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/value.c b/gdb/value.c
index aac9baaaf5..4efd75fa25 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -997,16 +997,16 @@ show_max_value_size (struct ui_file *file, int from_tty,
 static void
 check_type_length_before_alloc (const struct type *type)
 {
-  unsigned int length = TYPE_LENGTH (type);
+  size_t length = TYPE_LENGTH (type);
 
   if (max_value_size > -1 && length > max_value_size)
     {
       if (type->name () != NULL)
-	error (_("value of type `%s' requires %u bytes, which is more "
-		 "than max-value-size"), type->name (), length);
+	error (_("value of type `%s' requires %s bytes, which is more "
+		 "than max-value-size"), type->name (), pulongest (length));
       else
-	error (_("value requires %u bytes, which is more than "
-		 "max-value-size"), length);
+	error (_("value requires %s bytes, which is more than "
+		 "max-value-size"), pulongest (length));
     }
 }
 
-- 
2.17.1

Re: [PATCH] Fix malloc allocation size sanity check

[Tom Tromey]

>>>>> "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:

Luis> The following patch fixes this by making the length variable a size_t.

main_type::length is a ULONGEST, so maybe that would be better?

Tom

Re: [PATCH] Fix malloc allocation size sanity check

[Tom Tromey]

Tom> main_type::length is a ULONGEST, so maybe that would be better?

Oops, I mean type::length.

Tom

Re: [PATCH] Fix malloc allocation size sanity check

[Luis Machado]

On 8/12/20 4:31 PM, Tom Tromey wrote:
>>>>>> "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Luis> The following patch fixes this by making the length variable a size_t.
> 
> main_type::length is a ULONGEST, so maybe that would be better?

Yeah, I guess. I'll change it.

I noticed xzalloc takes a size_t though.

Re: [PATCH] Fix malloc allocation size sanity check

[Gary Benson]

gdb-patches wrote:
> On 8/12/20 4:31 PM, Tom Tromey wrote:
> > > > > > > "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:
> > 
> > Luis> The following patch fixes this by making the length variable a size_t.
> > 
> > main_type::length is a ULONGEST, so maybe that would be better?
> 
> Yeah, I guess. I'll change it.
> 
> I noticed xzalloc takes a size_t though.

We're allocating a buffer on the host, to hold a value from the
target.  size_t is for sizes on the host, so it's appropriate
for xzalloc, and main_type::length came from the the target, so
ULONGEST is appropriate there.  The boundary stops things being
neat :(

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat