From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8163 invoked by alias); 21 May 2014 08:13:23 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 8150 invoked by uid 89); 21 May 2014 08:13:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ie0-f178.google.com Received: from mail-ie0-f178.google.com (HELO mail-ie0-f178.google.com) (209.85.223.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 21 May 2014 08:13:21 +0000 Received: by mail-ie0-f178.google.com with SMTP id rl12so1629196iec.37 for ; Wed, 21 May 2014 01:13:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=k8nhnhTh2fRw6eLaEHNQ0+W9q2FjSx4YecAvJpxk7os=; b=T1PFfwCKAQIwiLP+Q/2arJ9ImLbHzm7MFl4zhcylHS5lI0uCIAWpLfESXE3aCZQulU FyehNYBmRd/wkwgfsmZDiikk6cEXdht1slKWDB25DPHHcsT6XXBF4Ldcj9NP9SM2AdNf /VfPLDIVe1cK2iIWdynBRAY8LMK7qKJppffk3Mzo+M/DqFJEzNczkt1KdEQB0CEiKKk+ 1+pPkZV2M3G5wY7oocfh4Pi24JqfALTQVxQ+P+1SudpNh0/KBFveyd3F5q6bxZHQSADC bTnuGE5AxRyCQgye+/bZ8YJlMMI8uuFZYUcx/QvNHvyoyP+Q94lP1UQKPKxLkaDQ9fq/ 8BiQ== X-Gm-Message-State: ALoCoQmUjnRhwGmwtYTfVRcn1rhpSsqAjgIEYaY0+kVSIuKyWjQI+Q63Hiybk+owwIU2VhMMIeh0 MIME-Version: 1.0 X-Received: by 10.42.98.1 with SMTP id q1mr731498icn.85.1400659999286; Wed, 21 May 2014 01:13:19 -0700 (PDT) Received: by 10.64.14.33 with HTTP; Wed, 21 May 2014 01:13:19 -0700 (PDT) In-Reply-To: <1400102354-9296-1-git-send-email-omair.javaid@linaro.org> References: <1400102354-9296-1-git-send-email-omair.javaid@linaro.org> Date: Wed, 21 May 2014 08:13:00 -0000 Message-ID: Subject: Re: [PATCH] aarch64 linux core dump support From: Will Newton To: Omair Javaid Cc: "binutils@sourceware.org" , Patch Tracking Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-05/txt/msg00193.txt.bz2 On 14 May 2014 22:19, Omair Javaid wrote: Hi Omair, > 2014-05-15 Omair Javaid > > bfd/ > * elfxx-aarch64.c (stdarg.h): Include. > (_bfd_aarch64_elf_grok_prstatus): Updated. > (_bfd_aarch64_elf_grok_psinfo): New function. > (_bfd_aarch64_elf_write_core_note): New function. > * elfxx-aarch64.h (elf_backend_grok_psinfo): Define. > (elf_backend_write_core_note): Define. > --- > bfd/elfxx-aarch64.c | 102 +++++++++++++++++++++++++++++++++++++++++++--------- > bfd/elfxx-aarch64.h | 12 +++++-- > 2 files changed, 96 insertions(+), 18 deletions(-) > > diff --git a/bfd/elfxx-aarch64.c b/bfd/elfxx-aarch64.c > index 7db6295..23e2ef1 100644 > --- a/bfd/elfxx-aarch64.c > +++ b/bfd/elfxx-aarch64.c > @@ -20,6 +20,7 @@ > > #include "sysdep.h" > #include "elfxx-aarch64.h" > +#include > > #define MASK(n) ((1u << (n)) - 1) > > @@ -498,25 +499,94 @@ _bfd_aarch64_elf_grok_prstatus (bfd *abfd, Elf_Internal_Note *note) > switch (note->descsz) > { > default: > - return FALSE; > + return FALSE; > + > + /* sizeof(struct elf_prstatus) on Linux/aarch64. */ > + case 392: > + elf_tdata (abfd)->core->signal = bfd_get_16 (abfd, note->descdata + 12); > + elf_tdata (abfd)->core->lwpid = bfd_get_32 (abfd, note->descdata + 32); > + offset = 112; > + size = 272; > + break; > + } Does anything actually change in this function? As far as I can tell it just gets reformatted. If the patch is just adding two functions and hooking them in then it would make it simpler to review if the formatting changes were left for a separate patch. > > - case 392: /* sizeof(struct elf_prstatus) on Linux/arm64. */ > - /* pr_cursig */ > - elf_tdata (abfd)->core->signal > - = bfd_get_16 (abfd, note->descdata + 12); > + /* Make a ".reg/999" section. */ > + return _bfd_elfcore_make_pseudosection (abfd, ".reg", > + size, note->descpos + offset); > +} > > - /* pr_pid */ > - elf_tdata (abfd)->core->lwpid > - = bfd_get_32 (abfd, note->descdata + 32); > +bfd_boolean > +_bfd_aarch64_elf_grok_psinfo (bfd *abfd, Elf_Internal_Note *note) > +{ > + switch (note->descsz) > + { > + default: > + return FALSE; > + > + case 136: /* sizeof(struct elf_prpsinfo) on Linux/x86_64 */ Should be aarch64? > + elf_tdata (abfd)->core->pid = bfd_get_32 (abfd, note->descdata + 24); > + elf_tdata (abfd)->core->program = > + _bfd_elfcore_strndup (abfd, note->descdata + 40, 16); > + elf_tdata (abfd)->core->command = > + _bfd_elfcore_strndup (abfd, note->descdata + 56, 80); > + } > > - /* pr_reg */ > - offset = 112; > - size = 272; > + /* Note that for some reason, a spurious space is tacked > + onto the end of the args in some (at least one anyway) > + implementations, so strip it off if it exists. */ I wonder if we (or anybody else) still needs this code. Bizarrely it was added with no commentary in this commit: commit eaa57a10aa324a06af1ac84ac8ffde8dc1b2bc87 Author: Ulrich Drepper Date: Tue Oct 27 00:00:50 1998 +0000 (bfd_elf_hash): Optimize the hash function a bit. And has been copy and pasted for ever more since. > - break; > - } > + { > + char *command = elf_tdata (abfd)->core->command; > + int n = strlen (command); I suppose we should really include string.h for this and memcpy. > > - /* Make a ".reg/999" section. */ > - return _bfd_elfcore_make_pseudosection (abfd, ".reg", > - size, note->descpos + offset); > + if (0 < n && command[n - 1] == ' ') > + command[n - 1] = '\0'; > + } > + > + return TRUE; > +} > + > +char * > +_bfd_aarch64_elf_write_core_note (bfd *abfd, char *buf, int *bufsiz, int note_type, > + ...) > +{ > + switch (note_type) > + { > + default: > + return NULL; > + > + case NT_PRPSINFO: > + { > + char data[136]; > + va_list ap; > + va_start (ap, note_type); > + memset (data, 0, sizeof (data)); > + strncpy (data + 40, va_arg (ap, const char *), 16); > + strncpy (data + 56, va_arg (ap, const char *), 80); > + va_end (ap); > + return elfcore_write_note (abfd, buf, bufsiz, > + "CORE", note_type, data, sizeof (data)); > + } > + > + case NT_PRSTATUS: > + { > + char data[392]; > + va_list ap; > + long pid; > + int cursig; > + const void *greg; > + > + va_start (ap, note_type); > + memset (data, 0, sizeof (data)); > + pid = va_arg (ap, long); > + bfd_put_32 (abfd, pid, data + 32); > + cursig = va_arg (ap, int); > + bfd_put_16 (abfd, cursig, data + 12); > + greg = va_arg (ap, const void *); > + memcpy (data + 112, greg, 272); > + va_end (ap); > + return elfcore_write_note (abfd, buf, bufsiz, > + "CORE", note_type, data, sizeof (data)); > + } > + } > } > diff --git a/bfd/elfxx-aarch64.h b/bfd/elfxx-aarch64.h > index 5ca3b7f..1d61d96 100644 > --- a/bfd/elfxx-aarch64.h > +++ b/bfd/elfxx-aarch64.h > @@ -42,6 +42,14 @@ _bfd_aarch64_elf_add_symbol_hook (bfd *, struct bfd_link_info *, > extern bfd_boolean > _bfd_aarch64_elf_grok_prstatus (bfd *, Elf_Internal_Note *); > > +extern bfd_boolean > +_bfd_aarch64_elf_grok_psinfo (bfd *, Elf_Internal_Note *); > + > +extern char * > +_bfd_aarch64_elf_write_core_note (bfd *abfd, char *buf, int *bufsiz, > + int note_type, ...); > > -#define elf_backend_add_symbol_hook _bfd_aarch64_elf_add_symbol_hook > -#define elf_backend_grok_prstatus _bfd_aarch64_elf_grok_prstatus > +#define elf_backend_add_symbol_hook _bfd_aarch64_elf_add_symbol_hook > +#define elf_backend_grok_prstatus _bfd_aarch64_elf_grok_prstatus > +#define elf_backend_grok_psinfo _bfd_aarch64_elf_grok_psinfo > +#define elf_backend_write_core_note _bfd_aarch64_elf_write_core_note There also seem to be some unrelated formatting changes here. -- Will Newton Toolchain Working Group, Linaro