[PING 4][PATCH 1/1] gdb: add check for empty array

[Rudnicki, Piotr]

Polite ping #4.

Regards, 
Piotr R.


-----Original Message-----
From: Rudnicki, Piotr 
Sent: Monday, January 13, 2025 11:35 AM
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>; Rudnicki, Piotr <piotr.rudnicki@intel.com>
Subject: [PING 3][PATCH 1/1] gdb: add check for empty array

Polite ping #3.

Regards, 
Piotr R.


-----Original Message-----
From: Rudnicki, Piotr 
Sent: Tuesday, January 7, 2025 8:50 PM
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>; Rudnicki, Piotr <piotr.rudnicki@intel.com>
Subject: [PING 2][PATCH 1/1] gdb: add check for empty array

Polite ping #2.

Regards, 
Piotr R.


-----Original Message-----
From: Rudnicki, Piotr <piotr.rudnicki@intel.com> 
Sent: Friday, December 27, 2024 8:27 PM
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>; Rudnicki, Piotr <piotr.rudnicki@intel.com>
Subject: [PING][PATCH 1/1] gdb: add check for empty array

Polite ping.

Regards, 
Piotr R.


-----Original Message-----
From: Rudnicki, Piotr <piotr.rudnicki@intel.com> 
Sent: Wednesday, December 18, 2024 1:01 PM
To: Tom Tromey <tom@tromey.com>; gdb-patches@sourceware.org
Cc: Rudnicki, Piotr <piotr.rudnicki@intel.com>
Subject: RE: [PATCH 1/1] gdb: add check for empty array

Hi Tom, 
Thanks for your quick response and please see my feedback below!

Piotr>> (gdb) p 1 == { }
Piotr>> Fatal signal: Segmentation fault

Tom>Where does the crash occur?

The crash I've found is reproducible only via gdb CLI.
It is forbidden in c and rust languages to have arrays with unspecified elements size.

Tom>FWIW I can reproduce in Rust:
Tom>set lang rust
Tom>print [0;0]

Thanks for catching the issue for rust language.

Tom> Anyway my question is whether this can be fixed to instead allow a 0-length array.

The function value_array () expects to get valid size of element to allocate space for array values.
For example, in c lang:
(gdb) set $a = { }
The element size of array is not specified, and we see a crash. 

For rust lang:
(gdb) set lang rust
(gdb) print [0;0]
Element size is specified, but equal to 0, so the function also crash as allocation can't be done.

I would like to change the error message:
from "array must not be empty" to "array element size must not be empty"

For c lang:
(gdb) p 1 == { }
array element size must not be empty

For rust lang:
(gdb) set lang rust
(gdb) print [0;0]
array element size must not be empty

What do you think?
I want also to add a new test case for rust lang to gdb.rust/expr.exp file.

Regards,
Piotr R.


-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Tuesday, December 17, 2024 3:40 PM
To: Rudnicki, Piotr <piotr.rudnicki@intel.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 1/1] gdb: add check for empty array

>>>>> "Piotr" == Piotr Rudnicki <piotr.rudnicki@intel.com> writes:

Piotr> With the command before the change, gdb crashes with message:
Piotr> (gdb) p 1 == { }
Piotr> Fatal signal: Segmentation fault

Where does the crash occur?

Piotr> Add a new test file gdb.base/array-value/exp to test this change.

Thank you.

Piotr> diff --git a/gdb/valops.c b/gdb/valops.c index 
Piotr> 88a42d38660..9e4fe2844d2 100644
Piotr> --- a/gdb/valops.c
Piotr> +++ b/gdb/valops.c
Piotr> @@ -1695,6 +1695,9 @@ value_array (int lowbound, gdb::array_view<struct value *> elemvec)
Piotr>    /* Validate that the bounds are reasonable and that each of the
Piotr>       elements have the same size.  */
 
Piotr> +  if (elemvec.empty ())
Piotr> +    error (_("array must not be empty"));

I don't have an issue with the patch itself but I wonder if it's correct to always disallow arrays with 0 length.  For instance, I think they are valid in Rust.

FWIW I can reproduce in Rust:

set lang rust
print [0;0]

Anyway my question is whether this can be fixed to instead allow a 0-length array.

Tom
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.