Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi

[Simon Marchi <simark@simark.ca>]

On 2020-10-20 6:05 p.m., Fredrik Hederstierna wrote:
>
>
>
>> From: Simon Marchi <simark@simark.ca>
>> Sent: Tuesday, October 20, 2020 5:04 PM
>> To: Fredrik Hederstierna <fredrik.hederstierna@verisure.com>; Paul Mathieu <paulmathieu@google.com>
>> Cc: gdb-patches@sourceware.org <gdb-patches@sourceware.org>; Luis Machado <luis.machado@linaro.org>; Alan Hayward > <Alan.Hayward@arm.com>
>> Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi
>
>> Ok, thanks for trying.  Is Outlook's SMTP not really SMTP?
>
> I think git-email now works with the SMTP server, but need to learn how to embed message etc. sorry for beginners mistakes..

Well, that's good news!

>> One thing that doesn't look right is this, in none_make_corefile_notes:
>>
>>       int global_id = 1;
>>       struct thread_info *info = find_thread_global_id (global_id);
>>
>> As you might have noticed, GDB now supports being connected to multiple
>> targets at the same time.  So, inferior 1 could be a local GNU/Linux
>> executable, while inferior 2 could be a remote bare-metal ARM program.
>
>
>> If so, I guess GDB should just dump all the current inferior's threads
>> in the core dump.
>
> I've tried to modify code so it should dump all threads,
> created a [PATCH v2] which includes this (sent with git-email).

Thanks, I'll take a look.

>
>> I see you have this commented out:
>>
>>     /* make_cleanup (xfree, note_data); */
>>
>> The way to do it now would be to make note_data a
>> gdb::unique_xmalloc_ptr<char>, and do "return note_data.release ();"
>> when returning.  This way, it gets automatically freed if something bad
>> happens.  And ideally, gdbarch_make_corefile_notes should be changed to
>> return a gdb::unique_xmalloc_ptr<char>, but that's out of the scope of
>> this patch (I'll give it a quick try).
>
> I tried to copy linux-tdep.c, and I could not find where data is unallocated, but maybe its a later problem.

If everything works well, your function returns allocated data to the
caller and gives up ownership.  So it becomes the caller's problem to
de-allocate it.  However, you need to make sure that your function
doesn't leak the data if an exception is thrown.

If you do:

  char *data = (char *) xmalloc (1234);
  call_function_that_may_throw ();
  return data;

... and call_function_that_may_throw throws, the allocated buffer leaks.

If you do:

  gdb::unique_xmalloc_ptr<char> data ((char *) xmalloc (1234));
  call_function_that_may_throw ();
  return data.release ();

... then data gets freed if the function throws.

I'll look at changing the callback type to make it return a
gdb::unique_xmalloc_ptr<char> instead of a `char *`, in which case it
would become:

  gdb::unique_xmalloc_ptr<char> data ((char *) xmalloc (1234));
  call_function_that_may_throw ();
  return data;

>> You don't need an _initialize_none_tdep if you don't do anything in it.
>
> Ok, I kept if for now, if something pops up that needs to be put there along the road..

If you don't have anything, just remove it.  We can easily add it later.

>> One last question: I see that you deal with AUXV stuff.  Will bare-metal
>> arm programs really have an auxiliary vector?
>
> I'm not sure what Auxv contains, but seems to be eg some system info that is handy, I do not know what/if its filled with something by default on a bare-metal arm-none target/arch?

On "standard" OSes, the auxiliary vector (auxv) is set up by the kernel
when it loads the executable in memory, to provide some information
about the ELF file how it was loaded.  I don't think it applies to bare
metal targets.  If not (I'd be happy to be proven wrong), you shouldn't
include it, it just makes the code unnecessarily more complicated.

Simon