[PING 4][PATCH 1/1] gdb: add check for empty array
Andrew Burgess
aburgess@redhat.com
Fri Jan 24 16:30:51 GMT 2025
"Rudnicki, Piotr" <piotr.rudnicki@intel.com> writes:
> "Andrew Burgess" <aburgess@redhat.com> writes:
>
> Thanks for your reply.
>
> Tom > Where does the crash occur?
>
> The crash I've found is reproducible only via gdb CLI. The scenario is:
> (gdb) p 1 == { }
> Fatal signal: Segmentation fault
>
> Here is the full backtrace:
> Fatal signal: Segmentation fault
> ----- Backtrace -----
> 0x5588afd72348 gdb_internal_backtrace_1
> /binutils-gdb/gdb/bt-utils.c:121
> 0x5588afd72348 _Z22gdb_internal_backtracev
> /binutils-gdb/gdb/bt-utils.c:182
> 0x5588afe9bcc4 handle_fatal_signal
> /binutils-gdb/gdb/event-top.c:1018
> 0x5588afe9beb8 handle_sigsegv
> /binutils-gdb/gdb/event-top.c:1089
> 0x7f805218651f ???
> ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
> 0x5588b01b4c48 _ZNK5value14enclosing_typeEv
> /binutils-gdb/gdb/value.h:326
> 0x5588b01b4c48 _Z11value_arrayiN3gdb10array_viewIP5valueEE
> /binutils-gdb/gdb/valops.c:1698
> 0x5588afe979ff _ZN4expr15array_operation8evaluateEP4typeP10expression6noside
> /binutils-gdb/gdb/eval.c:2553
> 0x5588afd890ca _ZN4expr20comparison_operationIL10exp_opcode14EXadL_Z13eval_op_equalP4typeP10expression6nosideS1_P5valueS8_EEE8evaluateES3_S5_S6_
> /binutils-gdb/gdb/expop.h:1339
> 0x5588afe94e83 _ZN10expression8evaluateEP4type6noside
> /binutils-gdb/gdb/eval.c:110
> 0x5588affdbf23 process_print_command_args
> /binutils-gdb/gdb/printcmd.c:1327
> 0x5588affdc53f print_command_1
> /binutils-gdb/gdb/printcmd.c:1340
> 0x5588afda8e74 _Z8cmd_funcP16cmd_list_elementPKci
> cli/cli-decode.c:2830
> 0x5588b013a2e9 _Z15execute_commandPKci
> /binutils-gdb/gdb/top.c:566
> 0x5588afe9c6b7 _Z15command_handlerPKc
> /binutils-gdb/gdb/event-top.c:613
> 0x5588afe9e113 _Z20command_line_handlerOSt10unique_ptrIcN3gdb13xfree_deleterIcEEE
> /binutils-gdb/gdb/event-top.c:849
> 0x5588afe9d614 gdb_rl_callback_handler
> /binutils-gdb/gdb/event-top.c:288
> 0x5588b01fa4cb rl_callback_read_char
> /binutils-gdb/readline/readline/callback.c:302
> 0x5588afe9d2ad gdb_rl_callback_read_char_wrapper_sjlj
> /binutils-gdb/gdb/event-top.c:197
> 0x5588afe9d483 gdb_rl_callback_read_char_wrapper_noexcept
> /binutils-gdb/gdb/event-top.c:240
> 0x5588afe9d483 gdb_rl_callback_read_char_wrapper
> /binutils-gdb/gdb/event-top.c:252
> 0x5588b017467f stdin_event_handler
> /binutils-gdb/gdb/ui.c:154
> 0x5588b030e535 gdb_wait_for_event
> /binutils-gdb/gdbsupport/event-loop.cc:672
> 0x5588b030eeef gdb_wait_for_event
> /binutils-gdb/gdbsupport/event-loop.cc:571
> 0x5588b030eeef _Z16gdb_do_one_eventi
> /binutils-gdb/gdbsupport/event-loop.cc:263
> 0x5588aff7fb99 start_event_loop
> /binutils-gdb/gdb/main.c:402
> 0x5588aff7fb99 captured_command_loop
> /binutils-gdb/gdb/main.c:466
> 0x5588aff81fe4 captured_main
> /binutils-gdb/gdb/main.c:1343
> 0x5588aff81fe4 _Z8gdb_mainP18captured_main_args
> /binutils-gdb/gdb/main.c:1362
> 0x5588afcaed7b main
> /binutils-gdb/gdb/gdb.c:38
>
> Tom > Anyway my question is whether this can be fixed to instead allow a 0-length array.
>
> As far as I understand, it is not allowed in c and rust languages to
> have arrays with unspecified elements size.
I suspect there's a difference between unspecified element size, and a
zero length array.
Though in the C case, where you say '{ }', clearly the element size
itself is unspecified.
But, according to[1] rust does support zero sized arrays with the syntax
Tom supplied: [a;0].
So, I guess Tom's question was really: how does your patch impact that
case? Could we support that case?
[1] https://doc.rust-lang.org/stable/std/primitive.array.html
Thanks,
Andrew
> In such case it would be fine to just give error info to the user instead of crashing.
> So please let me know if you think otherwise.
>
> Andrew > You'll need to give some answer to his questions in order to move this patch forward.
>
> Sorry, I thought in my understanding I already replied.
> In case backtrace information was missing for the crash in my reply then adding it also here.
> So please let me know if anything else yet not answered.
>
> Regards,
> Piotr R.
>
>
> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Monday, January 20, 2025 5:27 PM
> To: Rudnicki, Piotr <piotr.rudnicki@intel.com>; gdb-patches@sourceware.org
> Cc: Tom Tromey <tom@tromey.com>
> Subject: Re: [PING 4][PATCH 1/1] gdb: add check for empty array
>
> "Rudnicki, Piotr" <piotr.rudnicki@intel.com> writes:
>
>> Polite ping #4.
>
> You replied to Tom's feedback email[1], but didn't seem to address his questions. You'll need to give some answer to his questions in order to move this patch forward.
>
> [1] https://inbox.sourceware.org/gdb-patches/87seqmcr16.fsf@tromey.com
>
> Thanks,
> Andrew
>
>
>>
>> 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.
>
> ---------------------------------------------------------------------
> 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.
More information about the Gdb-patches
mailing list