From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.15.18]) by sourceware.org (Postfix) with ESMTPS id 95BB43887023 for ; Wed, 8 Apr 2020 15:00:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 95BB43887023 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gmx.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=n54@gmx.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1586358022; bh=hcB6uC1QPcBRnKdqdYF7ds3YQafSPrNLse28lMxRdKM=; h=X-UI-Sender-Class:Subject:To:Cc:References:From:Date:In-Reply-To; b=Dxb73eupS5AVJQIgVGB9mtFx3UY0tupKTEShhucbZvcj+zBsiiX72chfKRu0aV+68 Y0C3vwzE8gcFl17liOz/8QBg2fhnETsJVBsdLx4p/vNEFl1y/DaqODCDsgXzMFHbLB riLscSOP2h8JO2rxqUOTx0/AABfd5A+/FsG7yqKA= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.0.241] ([89.79.191.25]) by mail.gmx.com (mrgmx005 [212.227.17.184]) with ESMTPSA (Nemesis) id 1N0XCw-1j7hi61DuO-00wWPB; Wed, 08 Apr 2020 17:00:22 +0200 Subject: Re: [PATCH] Add SVR4 psABI specific parser for AUXV entries To: Simon Marchi , gdb-patches@sourceware.org Cc: tom@tromey.com References: <20200408021426.28406-1-n54@gmx.com> <5b988c52-2bd0-6af9-5c6e-147c639d52a5@simark.ca> From: Kamil Rytarowski Autocrypt: addr=n54@gmx.com; prefer-encrypt=mutual; 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 h1RtcVdFpFbhwVlr1Bly8c25gBNQHL5GUjLMn45LlQz50OzrkwARAQABtCdLYW1pbCBSeXRh cm93c2tpIChOZXRCU0QpIDxuNTRAZ214LmNvbT6JAjwEEwEIACYCGyMHCwkIBwMCAQYVCAIJ CgsEFgIDAQIeAQIXgAUCVbKGFwIZAQAKCRBLswjpsC52bIVpD/9i8npieI91xMIVvAHIUMeo cQO0IrNb+b/PuTj2qNemdwU7dhVJ7tVU5O1H2hI2M4rHGzjzDTxYzdxka0+A8CVEuvFdf6sF lXlXF0wM7rC6MoaB0QLAKxkZB5OtCILxLx7Bl2Y4cTPMU9v+qSL6yrdmhxogkufa4d6O9Zl/ FCWO2kH/BphKOiDtbyvdo2WULSLWP2IXN+0rCpNL4wbTfYLgV9JtMf8f0naGsdy7BFuDWsIE vtHh8dkQZP7dz6Qy67kx8negZaehSEgXwiae0HwQIn3xTQrFmBDALDsCgXuLWPTvglSkqTak uG+8X5fyTy0cU10TNKsU+rFBO+/xsUoIQOGrARwfWOIfJNPelzh/qigSnyNQNH8u5vFRPg9n fqB/AcvvAvtOYOo8EN9Ofx11gNj397NXc5HBQTrX6k5GNAeBWE3Ng1uO6scIwAS7qGnqGezU ABmQKLN37gmJiiGwhQAnSE6HILLBC5Z2b0S2rQsPKg8WgUmPa1YIcDkDtNB/LJcDsdU4Fm+r U2ksKU7tGD2ZfBt8H2nqfPKKeB+Uv/TBigjRvx/m70vjhqVxwCZA9Fqr9vkQkZroNfqP+3dp Z5V5fjmxO5abE2+IikSvFagwMtgx56i8Yrr2BzE8P5/S4cKq1kgyQoF+lVGDKRkUKCv1i4Fo aftnSxN8jTFZDbkCDQRVcFBfARAAutbzb8wAHGL5FPPWKErQ3Bsrp9RDTVqRzp7kBMOtd/14 MrOsWWyiml4XnvBYsJuhZWomFoeulcOXAPoTJ2vTw6erWYtdOiZymfQ3GMWpxzgkOVeNjsFF 9AQ38FCMKmIDs9dgn+KXSIXlZA34khKLd163SN5U/KHfYlnnocec31u+7rVa1hlF5DBSSpoi s8cs41foBYC5NsB/i+yqGIlfzHy7pC2u5kyQCuJotLH4y0rT5X+YBC7z7cqKChtILNDGw0ht qps29fwOGBE/FWmu8CbpSHj8pvg7uUyQcKbZbNChBfWtOJKdjnNs5VHf2ec95SwYmWl6Xz66 G892HY4ODtvl05/kh0qtdJd2oI4gJBsBx/N1585/3JYN4k78GIHTnML3xJydRRs9wwM3AXf/ iDGrMyY7qHQVXJLdO5nPe7LHg48vryCMkBnTMw5iNFPVCu5w1BaZyHxuS2HvpsgUtQoBa2QE P1jYNI+2qgoiIG4VQDhYtrD0WJaYdi/C2UVDxRy07dt73SV3RQ7ijOiUrz4g3/deFKY16/1k sE+N5Sc5Tjt84ChjO3nJRbHrQxd6dCOElR70e3R2yAuSB4m7LJpO20IB9CtWhlF/0AtfL91W O8GGGqLWB0Z04hmwRs/l8T4WWIlykLshbunWN6jsP1Y27FeilTZ+Pc9mYOEUFfEAEQEAAYkC HwQYAQgACQUCVXBQXwIbDAAKCRBLswjpsC52bPayD/9jE8mdNudrudSxbDB2vf8pU8r5flCq vIkfOdpZGV/Wx/Zx+HFHHp+b2aNBGSNyFTnph1Ku9bvg06vD0o+b7SdA1vrBgRG41t0OCIyf vejz65Xpin2EtCllcBM8zUCxHo43blON8fNw70P1Ec0loBp4TAal1MiXbB8kxRTRcEPVO9YF 9NPsFxycoWl0ZSvu4ESrQlrjRbVv+W0Fy/XqcQwEtDziFQHQXNRbTy8INPD49CsB7BkKRK+f 1vMmw7SxfsyEhyCgo9ZWfHb/+w9T5h+UhF87L/m287z7W+s4aCAPBzjbIWhtngGJJwIgiWdI I9J6YJLcHLvVZLw7xzA/flcjc0VfzOgJOJw3hBukHnEz7/CKgnABwyNu52P+PQbxVTiTjMKm 06eV732u9ZLD9ZgEazfmyGDHzsuzoXwsRnmcnbwYYAiynS+vfGl5oMtMa5qzsPhlzuvRlXHm zr8VjF8c9RThvyZyyHtWYAqNmBecMvM0whigjMeoAMJ5LtpyZgxjbHj1XnVdNBZgfJkOzsc/ twffi7RYphRx0d9z5UZ1Yl5Rvl05vTaJ7YhhNC7xuE8yGOQmDUsPDwWqO/eXUDErJjCOBR5b 0yILqRPYNT0Fj/th9gtEbZy1Gp0TVBkZM3tfjDRu43Pn6iSKObO/j0rNuq1LwN/EMxDifeZO 4XSbcg== Message-ID: Date: Wed, 8 Apr 2020 16:59:11 +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: <5b988c52-2bd0-6af9-5c6e-147c639d52a5@simark.ca> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:+6xxJH+JeEcDo1adpNMcfSQKvZuic85lqwKkzXTrlkqKnBmxIr2 0Z8HvH856uBwvze83vpeQCKl+yX1FcZ6KBCFzA8RsYdeaXYPEtZYUVtR13gMRlY6M1v7Vym VJGmA35VAWZA1uRrQL/37MRDIWgoPVd0iQYAEhEI5e2104j3q7V5gVZshFgmertyEJnZaId pw6n7wUkiN+yP6k72N5qA== X-UI-Out-Filterresults: notjunk:1;V03:K0:jqjC9FOX6ug=:mgCCe7BI1abc1T1azdRNmY iKtIm2yMOTsFBC0sUzvXda3pcijzCLLmigj0adKh2FyzViWqbTnJzqR2Qa/SvFGvSAZHNsOdk HhXp7dy5y1t8SA89kYkBFHOsxQGD+WpQsWYz8I4LrNLZzGXftSULX5kf8m8PdhIMj1BTEFNSK ROQj8WPI7Gf0jbcJ3vLRh1A/hC7DnT6S/n91oawwVWlKuNz+0zQpomHUMASEa5c9naiBajoj+ g7o7BuNggNWxo316FmpmXLmMRmo+sBWGffKc+OWetCa9JypXH8eUjEQTrf8aMNKm2Pj7oxRAy JFRE8PBp4s66TbtQ8LK6xjUUFpQn6GRj3VlNtJie/j8YRW6zl4tAyUsORyvatZulhxIb4gDtt Nmvk52w5QKBA3DkXBBWzrLd12qwE7onKegvmA1dATdJCkOdnbKHE8ZSIFpEpsYKpzAKmRgdL+ LLEzBVTFzjS5usBVQwD92wUrfUgwNh88zYHpkvKLV5yBtN6x+wPUV2DY6+imVTkPznc/YSs/l tQRRmEFP9Waw4tBP1aJACJPoVdIGoNjiJa22tXYqZSWAkGxKYa0ChQFskHxNDWcNTouQC+cNl vw+iRGdhQSH8Te28OuJII/4G5mt3s9tV+hNVm3fG4YSpSpNZVv1DezJ/e2glfymxrRyQyk5/K 5t07qP9ZJOuG0MGYVEQmSxqi93GFW8S9GX9Ys636fGMuH4NI9OHkW/AHxTVML48jP3CkWqmNY RXqzGjVebdrbWHdyvcZ93cMoyXi1/MSNK2mGMeaBd3eX6doOChkP5vnqBzgx83oeIKvA/gnZt 6Ljnwhb+GpVCuHFAdQ1Y/IyolH74VkrE1zDhPJR/zUvNFpIjBFbTgC6VwFD8aeu+Vr5P1nVsm IRMjEInPG9eL2c2IN7/VUr6tEr+bWKGKz7BwDEGK2MT8FtLpAaOvJm3uX93jUiSIDYje1iF6k 2QdvdnhNVxZ2x89hPre62wsgPLKZzcfaWxvhF7C3o6CEhRSkINZXERt3fIQEmBvF5I0g8hViS XP7aeQJL8HsKmC88e2Y6RBDs+ejN5dj8o+AeEyG3gGOgqLiPGJIl9GMYne0Iv5BHI1ooB2CNr 0L98UimtWsSoYEm25sGMFIKJZ5P+FyCbbwLVNDCSo5c6QV2kxFbtpxKyXmI4JNgddejty89lu 4x6TLY5p3/DkQPXXMoRG7iuShckQ59t3F3ccnOr1aj0ByuH0nmtIAXevo6PDTblUJwRT0= X-Spam-Status: No, score=-28.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, 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: Wed, 08 Apr 2020 15:00:41 -0000 On 08.04.2020 06:11, Simon Marchi wrote: > On 2020-04-07 10:14 p.m., Kamil Rytarowski wrote: >> NetBSD and OpenBSD always use an int to store the type as >> defined in the SVR4 psABI specifications rather than long >> as assumed by the default parser. >> >> Define svr4_auxv_parse() that shares code with default_auxv_parse(). >> >> Remove obsd_auxv_parse() and switch OpenBSD to svr4_auxv_parse(). >> Remove not fully accurate comment from obsd-tdep.c. >> >> Use svr4_auxv_parse() on NetBSD. >> >> gdb/ChangeLog: >> >> * auxv.h (svr4_auxv_parse): New. >> * auxv.c (default_auxv_parse): Split into default_auxv_parse >> and generic_auxv_parse. >> (svr4_auxv_parse): Add. >> * obsd-tdep.c: Include "auxv.h". >> (obsd_auxv_parse): Remove. >> (obsd_init_abi): Remove comment. >> (obsd_init_abi): Change set_gdbarch_auxv_parse passed argument >> from `obsd_auxv_parse' to `svr4_auxv_parse'. >> * nbsd-tdep.c: Include "auxv.h". >> (nbsd_init_abi): Call set_gdbarch_auxv_parse. > > Thanks, just a few minor comments. > Please see v2. >> diff --git a/gdb/auxv.c b/gdb/auxv.c >> index c13d7a22eb9..240f32236b3 100644 >> --- a/gdb/auxv.c >> +++ b/gdb/auxv.c >> @@ -248,34 +248,64 @@ memory_xfer_auxv (struct target_ops *ops, >> return procfs_xfer_auxv (readbuf, writebuf, offset, len, xfered_len)= ; >> } >> >> -/* Read one auxv entry from *READPTR, not reading locations >=3D ENDPT= R. >> - Return 0 if *READPTR is already at the end of the buffer. >> - Return -1 if there is insufficient buffer for a whole entry. >> - Return 1 if an entry was read into *TYPEP and *VALP. */ >> -int >> -default_auxv_parse (struct target_ops *ops, gdb_byte **readptr, >> - gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp) >> +/* Generic auxv_parse body shared with default_auxv_parse() and >> + svr4_auxv_parse(). */ > > Instead of saying that it's used by this and that function (which is doo= med > to become obsolete very easily anyway), say what is the particularity of= this > function, compared to other auxv_parse functions: it takes the size of t= he > auxv type field as a parameter. > >> + >> +static int >> +generic_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr, >> + gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp, >> + int sizeof_auxv_type) >> { >> - const int sizeof_auxv_field =3D gdbarch_ptr_bit (target_gdbarch ()) >> - / TARGET_CHAR_BIT; >> - const enum bfd_endian byte_order =3D gdbarch_byte_order (target_gdba= rch ()); >> + struct type *ptr_type =3D builtin_type (gdbarch)->builtin_data_ptr; >> + const int sizeof_auxv_val =3D TYPE_LENGTH (ptr_type); >> + enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); >> gdb_byte *ptr =3D *readptr; >> >> if (endptr =3D=3D ptr) >> return 0; >> >> - if (endptr - ptr < sizeof_auxv_field * 2) >> + if (endptr - ptr < 2 * sizeof_auxv_val) >> return -1; >> >> - *typep =3D extract_unsigned_integer (ptr, sizeof_auxv_field, byte_or= der); >> - ptr +=3D sizeof_auxv_field; >> - *valp =3D extract_unsigned_integer (ptr, sizeof_auxv_field, byte_ord= er); >> - ptr +=3D sizeof_auxv_field; >> + *typep =3D extract_unsigned_integer (ptr, sizeof_auxv_type, byte_ord= er); >> + ptr +=3D sizeof_auxv_val; /* Alignment. */ > > Instead of just "Alignment", please add a comment explaining that even i= f the > auxv type takes less space than an auxv value, there is padding after th= e type > such that the value is aligned on a multiple of its size (and this is wh= y we > advance by `sizeof_auxv_val` and not `sizeof_auxv_type`. I think that c= an > be surprising for somebody who does not know this fact. > >> + *valp =3D extract_unsigned_integer (ptr, sizeof_auxv_val, byte_order= ); >> + ptr +=3D sizeof_auxv_val; >> >> *readptr =3D ptr; >> return 1; >> } >> >> +/* Read one auxv entry from *READPTR, not reading locations >=3D ENDPT= R. >> + Return 0 if *READPTR is already at the end of the buffer. >> + Return -1 if there is insufficient buffer for a whole entry. >> + Return 1 if an entry was read into *TYPEP and *VALP. */ > > Since this is the implementation of an exported function, replace the co= mment > here with: > > /* See auxv.h. */ > > and document in the header file. I think the documentation you have put= in the > header file for svr4_auxv_parse is good, it explains what the function d= oes and > how it's different from default_auxv_parse. I don't think it's particul= arly > useful to describe all parameters here, since it's a function that imple= ments > an interface. The parameters should be documented where the interface i= s > defined. > >> +int >> +default_auxv_parse (struct target_ops *ops, gdb_byte **readptr, >> + gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp) > > This last line is not properly aligned on the parenthesis. > >> +{ >> + struct type *ptr_type =3D builtin_type (target_gdbarch ())->builtin_= data_ptr; >> + const int sizeof_auxv_type =3D TYPE_LENGTH (ptr_type); >> + >> + return generic_auxv_parse (target_gdbarch (), readptr, endptr, typep= , valp, >> + sizeof_auxv_type); >> +} >> + >> +/* Read one auxv entry from *READPTR, not reading locations >=3D ENDPT= R. >> + Return 0 if *READPTR is already at the end of the buffer. >> + Return -1 if there is insufficient buffer for a whole entry. >> + Return 1 if an entry was read into *TYPEP and *VALP. */ > > Same comment about the comment. > >> +int >> +svr4_auxv_parse (struct gdbarch *gdbarch, gdb_byte **readptr, >> + gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp) > > This last line is not properly aligned on the parenthesis. > > Simon >