From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.netbsd.org (mail.NetBSD.org [IPv6:2001:470:a085:999::25]) by sourceware.org (Postfix) with ESMTPS id 95553388A836 for ; Tue, 28 Jul 2020 15:10:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 95553388A836 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=netbsd.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kamil@netbsd.org Received: from [IPv6:::1] (localhost [127.0.0.1]) by mail.netbsd.org (Postfix) with ESMTP id 32A5E84D02; Tue, 28 Jul 2020 15:10:03 +0000 (UTC) Subject: Re: [PATCH] Implement xfer_partial TARGET_OBJECT_SIGNAL_INFO for NetBSD To: Simon Marchi , gdb-patches@sourceware.org References: <20200727074118.4030-1-n54@gmx.com> <48da991a-84ea-012f-8a6c-e21d90dcf079@simark.ca> From: Kamil Rytarowski Autocrypt: addr=kamil@netbsd.org; keydata= mQINBFVwUF8BEADHmOg7PFLIcSDdMx5HNDYr8MY2ExGfUTrKwPndbt3peaa5lHsK+UGoPG48 KiWkhEaMmjaXHFa7XgVpJHhFmNoJXfPgjI/sOKTMCPQ5DEHEHTibC4mta7IBAk+rmnaOF0k8 bxHfP8Qbls66wvicrAfTRXn/1ReeNc3NP4Sq39PoVHkfQTlnQiD4eAqBdq61B7DhzjhbKAZ4 RsNtLfB6eOv9qvmblUzs50ChYewM9hvn+c7MdDH+x2UXoSDhkBDkKcJGkX91evos8s9AuoEd D32X5e+bmdUGe8Cr3cAZJ8IEXR6F9828/kxzPliMsCWVRx1Fr28baCJOUGgFPNr3ips78m9+ Iw8PdQ101jU0dvucDFxw/1SCGYEZzV+O/237oRPuLCiDX5nhQoxf6dn9ukQleLBMNy2BLI4H g342NhF21HLA+KlyLOHaMKQCKzlal+zVNZTRTCh/ikMhsxWQjBfnqTDbMj85DnWwtump27SI qhPjUnS0a6MKoS/A+hbi64k5zztkvloELfCSrX7NyBTT0jgF2IGFIxZMrKCtQ9StcGMCV9MX tjcBy6fj7QMontEaIDRJEMjg8UIGw1B687OhalOv1ISia4xOWvpYAM6ipgqh6tBQmFzasL9P h1RtcVdFpFbhwVlr1Bly8c25gBNQHL5GUjLMn45LlQz50OzrkwARAQABtCNLYW1pbCBSeXRh cm93c2tpIDxrYW1pbEBOZXRCU0Qub3JnPokCOQQTAQgAIwUCVbKF6wIbIwcLCQgHAwIBBhUI AgkKCwQWAgMBAh4BAheAAAoJEEuzCOmwLnZsrgwQAMdXTXDWkxtUciFgBnioE6hvZYOBV7Xa Gh3dwgVvS5rLwwq5ob1R9qdtCGMYxdaCAQCzo2hhUfe9ts11/Q4Pg0aDAb5CfdVVTmyvLMu+ gtK99t/sG4SfCdn8Bb8rCfRRDpkTq1cAGy6pp7rxyMrFBITTbdBWVcWdEdlMhEZtV8Z1BNDI kwEwZkYnM1UxOGW4rJNjNU+hBjNAscCTwBSbpG6NV1oBbgmgJ1PfaPCeAmGTLZyI57VLuFJy kR0Jlj8Ui7dAaJgO1WYdmvL+48s0N2QGEoHnrf50xoO34LlrIBUsCLmhtjWhZiuj0meCxNTr 5YpdBP13b2i64OCruH8/M4IO85GAIWxIMMv510rge9qSe38NHCzSmn9zcjFwVXIh9flZi7PK eqOP3yah6r1ZIBY68If/2FtvwDptUi1NHoSpN+dt0kRg26hDqMFOg+Jc6o7Wtm+3vFNDhU4I 8HkjDr62VlbHBxe6gDgVELcecWgXOydKgdrQhOPwCBJkPJigifsIz4EZQnyI3CchFja3qR9J Vo4iXwqAi6xN4RD0PS775JYDh56qUaaUsEctQ/D6Xm7Bbdv1VPlsYs/9uXxc/jWVhkd1sDn2 KZ3kv7uo04DoejVGWK9B4XEZ1ufRPzmlV0SYohX34ouLBq5Q6wbyw6+hUM+yM9RcvgkOCVgB laejuQINBFVwUF8BEAC61vNvzAAcYvkU89YoStDcGyun1ENNWpHOnuQEw613/Xgys6xZbKKa Xhee8Fiwm6FlaiYWh66Vw5cA+hMna9PDp6tZi106JnKZ9DcYxanHOCQ5V42OwUX0BDfwUIwq YgOz12Cf4pdIheVkDfiSEot3XrdI3lT8od9iWeehx5zfW77utVrWGUXkMFJKmiKzxyzjV+gF gLk2wH+L7KoYiV/MfLukLa7mTJAK4mi0sfjLStPlf5gELvPtyooKG0gs0MbDSG2qmzb1/A4Y ET8Vaa7wJulIePym+Du5TJBwptls0KEF9a04kp2Oc2zlUd/Z5z3lLBiZaXpfProbz3Ydjg4O 2+XTn+SHSq10l3agjiAkGwHH83Xnzn/clg3iTvwYgdOcwvfEnJ1FGz3DAzcBd/+IMaszJjuo dBVckt07mc97sseDjy+vIIyQGdMzDmI0U9UK7nDUFpnIfG5LYe+myBS1CgFrZAQ/WNg0j7aq CiIgbhVAOFi2sPRYlph2L8LZRUPFHLTt23vdJXdFDuKM6JSvPiDf914UpjXr/WSwT43lJzlO O3zgKGM7eclFsetDF3p0I4SVHvR7dHbIC5IHibssmk7bQgH0K1aGUX/QC18v3VY7wYYaotYH RnTiGbBGz+XxPhZYiXKQuyFu6dY3qOw/VjbsV6KVNn49z2Zg4RQV8QARAQABiQIfBBgBCAAJ BQJVcFBfAhsMAAoJEEuzCOmwLnZs9rIP/2MTyZ0252u51LFsMHa9/ylTyvl+UKq8iR852lkZ X9bH9nH4cUcen5vZo0EZI3IVOemHUq71u+DTq8PSj5vtJ0DW+sGBEbjW3Q4IjJ+96PPrlemK fYS0KWVwEzzNQLEejjduU43x83DvQ/URzSWgGnhMBqXUyJdsHyTFFNFwQ9U71gX00+wXHJyh aXRlK+7gRKtCWuNFtW/5bQXL9epxDAS0POIVAdBc1FtPLwg08Pj0KwHsGQpEr5/W8ybDtLF+ zISHIKCj1lZ8dv/7D1PmH5SEXzsv+bbzvPtb6zhoIA8HONshaG2eAYknAiCJZ0gj0npgktwc u9VkvDvHMD9+VyNzRV/M6Ak4nDeEG6QecTPv8IqCcAHDI27nY/49BvFVOJOMwqbTp5Xvfa71 ksP1mARrN+bIYMfOy7OhfCxGeZydvBhgCLKdL698aXmgy0xrmrOw+GXO69GVcebOvxWMXxz1 FOG/JnLIe1ZgCo2YF5wy8zTCGKCMx6gAwnku2nJmDGNsePVedV00FmB8mQ7Oxz+3B9+LtFim FHHR33PlRnViXlG+XTm9NontiGE0LvG4TzIY5CYNSw8PBao795dQMSsmMI4FHlvTIgupE9g1 PQWP+2H2C0RtnLUanRNUGRkze1+MNG7jc+fqJIo5s7+PSs26rUvA38QzEOJ95k7hdJty Message-ID: <2ee982e9-fb65-1adc-2d4d-d5045d09c4c3@netbsd.org> Date: Tue, 28 Jul 2020 17:07:32 +0200 User-Agent: Mozilla/5.0 (X11; NetBSD amd64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <48da991a-84ea-012f-8a6c-e21d90dcf079@simark.ca> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="fh01gkHGcHfQjB34w8H6ZWVFKC2Pvfjfi" X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Jul 2020 15:10:07 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --fh01gkHGcHfQjB34w8H6ZWVFKC2Pvfjfi Content-Type: multipart/mixed; boundary="FToWWyyg97ytZW2m6QxX6jc4u1TaGufHu"; protected-headers="v1" From: Kamil Rytarowski To: Simon Marchi , gdb-patches@sourceware.org Message-ID: <2ee982e9-fb65-1adc-2d4d-d5045d09c4c3@netbsd.org> Subject: Re: [PATCH] Implement xfer_partial TARGET_OBJECT_SIGNAL_INFO for NetBSD References: <20200727074118.4030-1-n54@gmx.com> <48da991a-84ea-012f-8a6c-e21d90dcf079@simark.ca> In-Reply-To: <48da991a-84ea-012f-8a6c-e21d90dcf079@simark.ca> --FToWWyyg97ytZW2m6QxX6jc4u1TaGufHu Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 28.07.2020 15:26, Simon Marchi wrote: > A few comments, the patch LGTM with these fixed. >=20 > On 2020-07-27 3:41 a.m., Kamil Rytarowski wrote: >> NetBSD implementes reading and overwriting siginfo_t received by the >=20 > "implementes" >=20 Done. >> tracee. With TARGET_OBJECT_SIGNAL_INFO signal information can be >> examined and modified through the special variable $_siginfo. >> >> Implement the "get_siginfo_type" gdbarch method for NetBSD architectur= es. >> >> As with Linux architectures, cache the created type in the gdbarch whe= n it >> is first created. Currently NetBSD uses an identical siginfo type on >> all architectures, so there is no support for architecture-specific fi= elds. >=20 > Watch out tabs vs spaces at a few places in the patch. >=20 Please specify exact lines if there is still anything left. >> +{ >> + pid_t pid =3D inferior_ptid.pid (); >> + >> + switch (object) >> + { >> + case TARGET_OBJECT_SIGNAL_INFO: >> + { >> + ptrace_siginfo_t psi; >> + >> + if (offset > sizeof(siginfo_t)) >> + return TARGET_XFER_E_IO; >> + >> + if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) =3D=3D -1) >> + return TARGET_XFER_E_IO; >> + >> + if (offset + len > sizeof(siginfo_t)) >> + len =3D sizeof(siginfo_t) - offset; >=20 > Space after all instances of `sizeof`. >=20 Done. >> + } >> +} >> diff --git a/gdb/nbsd-nat.h b/gdb/nbsd-nat.h >> index 0a7048ecf35..59210ad1d03 100644 >> --- a/gdb/nbsd-nat.h >> +++ b/gdb/nbsd-nat.h >> @@ -49,6 +49,12 @@ struct nbsd_nat_target : public inf_ptrace_target >> override; >> >> bool supports_multi_process () override; >> + enum target_xfer_status xfer_partial (enum target_object object, >> + const char *annex, >> + gdb_byte *readbuf, >> + const gdb_byte *writebuf, >> + ULONGEST offset, ULONGEST len= , >> + ULONGEST *xfered_len) overrid= e; >> }; >> >> #endif /* nbsd-nat.h */ >> diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c >> index 4fdfe476b59..faf41313f68 100644 >> --- a/gdb/nbsd-tdep.c >> +++ b/gdb/nbsd-tdep.c >> @@ -373,6 +373,169 @@ nbsd_skip_solib_resolver (struct gdbarch *gdbarc= h, CORE_ADDR pc) >> return find_solib_trampoline_target (get_current_frame (), pc); >> } >> >> +static struct gdbarch_data *nbsd_gdbarch_data_handle; >> + >> +struct nbsd_gdbarch_data >> + { >> + struct type *siginfo_type; >> + }; >=20 > Remove leading indentation (`{` and `}` on column 0). >=20 Done. >> + >> +static void * >> +init_nbsd_gdbarch_data (struct gdbarch *gdbarch) >> +{ >> + return GDBARCH_OBSTACK_ZALLOC (gdbarch, struct nbsd_gdbarch_data); >> +} >> + >> +static struct nbsd_gdbarch_data * >> +get_nbsd_gdbarch_data (struct gdbarch *gdbarch) >> +{ >> + return ((struct nbsd_gdbarch_data *) >> + gdbarch_data (gdbarch, nbsd_gdbarch_data_handle)); >> +} >> + >> +/* Implement the "get_siginfo_type" gdbarch method. */ >> + >> +static struct type * >> +nbsd_get_siginfo_type (struct gdbarch *gdbarch) >> +{ >> + struct nbsd_gdbarch_data *nbsd_gdbarch_data; >> + struct type *char_type, *int_type, *long_type; >> + struct type *void_ptr_type; >> + struct type *int32_type, *uint32_type, *uint64_type; >> + struct type *pid_type, *lwpid_type, *uid_type, *clock_type; >> + struct type *sigval_type, *option_type; >> + struct type *siginfo_type; >> + struct type *ksiginfo_type; >> + struct type *reason_type; >> + struct type *type; >> + bool lp64; >=20 > Feel free to declare the variables when first using them and skip the `= struct` keyword. >=20 Done. >> + >> + nbsd_gdbarch_data =3D get_nbsd_gdbarch_data (gdbarch); >> + if (nbsd_gdbarch_data->siginfo_type !=3D NULL) >> + return nbsd_gdbarch_data->siginfo_type; >> + >> + char_type =3D builtin_type (gdbarch)->builtin_char; >> + int_type =3D builtin_type (gdbarch)->builtin_int; >> + long_type =3D builtin_type (gdbarch)->builtin_long; >> + >> + void_ptr_type =3D lookup_pointer_type (builtin_type (gdbarch)->buil= tin_void); >> + >> + int32_type =3D builtin_type (gdbarch)->builtin_int32; >> + uint32_type =3D builtin_type (gdbarch)->builtin_uint32; >> + uint64_type =3D builtin_type (gdbarch)->builtin_uint64; >> + >> + lp64 =3D TYPE_LENGTH (void_ptr_type) =3D=3D 8; >> + >> + /* pid_t */ >> + pid_type =3D arch_type (gdbarch, TYPE_CODE_TYPEDEF, >> + TYPE_LENGTH (int32_type) * TARGET_CHAR_BIT, "pid_t"); >=20 > TARGET_CHAR_BIT is bogus, since GDB can have multiple targets with poss= ibly different > target char sizes. It should really be removed. >=20 > Use gdbarch_addressable_memory_unit_size, and multiply that value by 8 = to get > the target char size in bits. >=20 Done. >> + TYPE_TARGET_TYPE (pid_type) =3D int32_type; >> + TYPE_TARGET_STUB (pid_type) =3D 1; >=20 > Is STUB really needed here? The TYPE_TARGET_STUB comment says it only = applies to > arrays and ranges. >=20 Done. >> + >> + /* uid_t */ >> + uid_type =3D arch_type (gdbarch, TYPE_CODE_TYPEDEF, >> + TYPE_LENGTH (uint32_type) * TARGET_CHAR_BIT, >> + "uid_t"); >> + TYPE_TARGET_TYPE (uid_type) =3D uint32_type; >> + TYPE_TARGET_STUB (uid_type) =3D 1; >> + >> + /* clock_t */ >> + clock_type =3D arch_type (gdbarch, TYPE_CODE_TYPEDEF, >> + TYPE_LENGTH (int_type) * TARGET_CHAR_BIT, >> + "clock_t"); >> + TYPE_TARGET_TYPE (clock_type) =3D int_type; >> + TYPE_TARGET_STUB (clock_type) =3D 1; >> + >> + /* lwpid_t */ >> + lwpid_type =3D arch_type (gdbarch, TYPE_CODE_TYPEDEF, >> + TYPE_LENGTH (int32_type) * TARGET_CHAR_BIT, >> + "lwpid_t"); >> + TYPE_TARGET_TYPE (lwpid_type) =3D int32_type; >> + TYPE_TARGET_STUB (lwpid_type) =3D 1; >> + >> + /* union sigval */ >> + sigval_type =3D arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION= ); >> + sigval_type->set_name (xstrdup ("sigval")); >=20 > If these types are tied to the gdbarch, maybe allocate it on the gdbarc= h's > obstack, just like the type is? You would use gdbarch_obstack_strdup. >=20 > You could also pass the name directly to arch_composite_type. You woul= d > still need to strdup it. Oddly enough, arch_type strdups its `name` ar= gument, > but arch_composite_type does not. >=20 This looks like a generic GDB bug wiith this arch_composite_type vs arch_type. For now, I have used set_name + gdbarch_obstack_strdup. Feel free to fix the problem in GDB later and pass the name directly in arch_composite_type. >> + >> +void _initialize_nbsd_tdep (); >> +void >> +_initialize_nbsd_tdep () >> +{ >> + nbsd_gdbarch_data_handle =3D >> + gdbarch_data_register_post_init (init_nbsd_gdbarch_data); >=20 > Assignment operator on the next line: >=20 > nbsd_gdbarch_data_handle > =3D ... >=20 Done. > Simon >=20 --FToWWyyg97ytZW2m6QxX6jc4u1TaGufHu-- --fh01gkHGcHfQjB34w8H6ZWVFKC2Pvfjfi Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEELaxVpweEzw+lMDwuS7MI6bAudmwFAl8gP0EACgkQS7MI6bAu dmyFQg//eCRWxAw8uXeHcipX3tKJEuPEeFk2x1qPE48N6+4DPp+6wv0Me711Zx+4 VTNwkwWrewVy5O8Zn4CP5/aiOh44ylkRgQvVcAYOrgGy/i89mlwFsrAz3nHXaZ+D gEO9mEUdmLQqy3EKE8UHFG4DzKLnozSvGHxexhr1RuNUX2JLDl1dT5bQS46GrfKO IIlwtjcGasgB1HuMN1nL7oqtxDufqBodZSVSDk7e063aIxOF3TW3sz1rvprv6wCS FFijwlAHsREQvPwOJU5SrYTLL6bq9fm+BpF2cC+bkNXvHIzHRSlHGHGuhFwUqjUw 3wVNd0ehS8OAV8DtzH2987+n1/8th2TlA2T1tauVcAwlj7QdxMamYaleIhCyg/SQ /w3YFBNavhW3UmWrypJpSjBbgwmoRqnwH7EG49ilCiJLM8U15GfMeRhf0wHWfYhq pRiWzg3mEgMmxAA6vBxL/iBZgrLWycMamoUwsXqjB860uw2DxMUTE2C1FlBfAvS9 Qkciin1Rsthh/UNbaZwmp4EYy35+krWvgWlrBJ+21G1ufOYqjlfcOF69nzqg4NtX Prjb0t5AY/aeArHo6z1O6APZ+q48rLf2Bff0y9erD/x11I1JWtoUA0dw7kPng7fK 8tpsbUj3ioJBEUo6bTg5HGmRHe8WGbM3UBpVAB1y+e48XWW36DE= =GCU4 -----END PGP SIGNATURE----- --fh01gkHGcHfQjB34w8H6ZWVFKC2Pvfjfi--