From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84126 invoked by alias); 18 Dec 2015 13:11:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 84116 invoked by uid 89); 18 Dec 2015 13:11:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=UD:cz, 2517, Hx-languages-length:3563, 3446 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 18 Dec 2015 13:11:18 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 9A4E018AEA6; Fri, 18 Dec 2015 13:11:17 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tBIDBGMJ013766; Fri, 18 Dec 2015 08:11:16 -0500 Message-ID: <567405F4.30707@redhat.com> Date: Fri, 18 Dec 2015 13:11:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Walfred Tedeschi , brobecker@adacore.com CC: gdb-patches@sourceware.org Subject: Re: [PATCH V2 4/5] Add bound related fields to the siginfo structure. References: <1450371416-24270-1-git-send-email-walfred.tedeschi@intel.com> <1450371416-24270-4-git-send-email-walfred.tedeschi@intel.com> In-Reply-To: <1450371416-24270-4-git-send-email-walfred.tedeschi@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-12/txt/msg00362.txt.bz2 On 12/17/2015 04:56 PM, Walfred Tedeschi wrote: > New kernel and glibc patches have introduced fields in the > segmentation fault fields. New fields will be conditional and requested > on demand by the customers. I think that by "customers" you mean that the patch adds them to x86 only, right? "API client" is more usual than "API customer", which otherwise sounds like you're talking about Intel's customers. :-) I'd suggest writing: Both Linux and glibc have introduced bound related fields in the segmentation fault fields of the siginfo_t type. Add the new fields to our x86's siginfo_t type too. > > Kernel patch: > http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=ee1b58d36aa1b5a79eaba11f5c3633c88231da83 > > Glibc patch: > http://repo.or.cz/w/glibc.git/commit/d4358b51c26a634eb885955aea06cad26af6f696 > > 2015-12-08 Walfred Tedeschi > > * linux-tdep.c (linux_get_siginfo_type): Add the _addr_bnd > structure to the siginfo conditionally if extra_fields is > LINUX_SIGINFO_FIELD_ADDR_BND. "conditionally" is redundant with if. Should be "contains" rather than "is". When talking about a variable's value, use uppercase: * linux-tdep.c (linux_get_siginfo_type): Add the _addr_bnd structure to the siginfo type if EXTRA_FIELDS contains LINUX_SIGINFO_FIELD_ADDR_BND. > > --- > gdb/linux-tdep.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c > index e41b6f4..73c70cb 100644 > --- a/gdb/linux-tdep.c > +++ b/gdb/linux-tdep.c > @@ -251,7 +251,7 @@ linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch, > linux_siginfo_extra_fields extra_fields) > { > struct linux_gdbarch_data *linux_gdbarch_data; > - struct type *int_type, *uint_type, *long_type, *void_ptr_type; > + struct type *int_type, *uint_type, *long_type, *void_ptr_type, *short_type; > struct type *uid_type, *pid_type; > struct type *sigval_type, *clock_type; > struct type *siginfo_type, *sifields_type; > @@ -267,6 +267,8 @@ linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch, > 1, "unsigned int"); > long_type = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch), > 0, "long"); > + short_type = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch), > + 0, "short"); > void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void); > > /* sival_t */ > @@ -344,6 +346,18 @@ linux_get_siginfo_type_with_fields (struct gdbarch *gdbarch, > append_composite_type_field (type, "si_addr", void_ptr_type); > append_composite_type_field (sifields_type, "_sigfault", type); > > + if (extra_fields == LINUX_SIGINFO_FIELD_ADDR_BND) This is a bit flags type so that other fields can be added in the future. Write: if ((extra_fields & LINUX_SIGINFO_FIELD_ADDR_BND) != 0) > + { > + struct type *sigfault_bnd_fields; > + > + append_composite_type_field (type, "_addr_lsb", short_type); > + sigfault_bnd_fields = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT); > + append_composite_type_field (sigfault_bnd_fields, "_lower", void_ptr_type); > + append_composite_type_field (sigfault_bnd_fields, "_upper", void_ptr_type); > + append_composite_type_field (type, "_addr_bnd", sigfault_bnd_fields); > + append_composite_type_field (sifields_type, "_sigfault", type); We add "_sigfault" twice? > + } > + > /* _sigpoll */ > type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT); > append_composite_type_field (type, "si_band", long_type); > Thanks, Pedro Alves