From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 9E6FE3858401 for ; Tue, 18 Oct 2022 18:26:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9E6FE3858401 Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 29IIHKDY030863 for ; Tue, 18 Oct 2022 18:26:46 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3ka19sg9wr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 18 Oct 2022 18:26:46 +0000 Received: from m0098399.ppops.net (m0098399.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 29IIIcta006240 for ; Tue, 18 Oct 2022 18:26:46 GMT Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3ka19sg9w2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 18 Oct 2022 18:26:46 +0000 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 29IILOZm023987; Tue, 18 Oct 2022 18:26:44 GMT Received: from b01cxnp23034.gho.pok.ibm.com (b01cxnp23034.gho.pok.ibm.com [9.57.198.29]) by ppma04wdc.us.ibm.com with ESMTP id 3k7mg9g44m-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 18 Oct 2022 18:26:44 +0000 Received: from smtpav06.wdc07v.mail.ibm.com ([9.208.128.115]) by b01cxnp23034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 29IIQhLO9568856 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 18 Oct 2022 18:26:44 GMT Received: from smtpav06.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 966B158056; Tue, 18 Oct 2022 18:26:43 +0000 (GMT) Received: from smtpav06.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E007458055; Tue, 18 Oct 2022 18:26:42 +0000 (GMT) Received: from li-e362e14c-2378-11b2-a85c-87d605f3c641.ibm.com (unknown [9.211.71.228]) by smtpav06.wdc07v.mail.ibm.com (Postfix) with ESMTP; Tue, 18 Oct 2022 18:26:42 +0000 (GMT) Message-ID: From: Carl Love To: Kevin Buettner , blarsen@redhat.com Cc: gdb-patches@sourceware.org, Ulrich Weigand , cel@us.ibm.com Date: Tue, 18 Oct 2022 11:26:42 -0700 In-Reply-To: <20221017180657.54a9aeec@f35-zws-1> References: <28ce795ca489f69829207b2a7a535cf7f77f6dd8.camel@us.ibm.com> <8decd662134d57e8caf43960a1cdc47723e2bfe3.camel@us.ibm.com> <79d82ed277308ed5ce312bff398e770ab234390a.camel@us.ibm.com> <63f21a897f452d81a73fb386cb99110a359ef0b7.camel@de.ibm.com> <97275f61ef101a12cde8e5a45008ed8e479424eb.camel@us.ibm.com> <191f5826b228a7614c084c9704b086851d418c78.camel@us.ibm.com> <5405a79ecd6ed34646ad77eed0779063ee222d37.camel@de.ibm.com> <1d55efe113cdcf96b812055ee45196a554c4ca78.camel@us.ibm.com> <20221013194959.7164bb86@f35-zws-1> <41e8f94ed30fb22c94a05a0823de2f5ae2250334.camel@us.ibm.com> <20221017180657.54a9aeec@f35-zws-1> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-18.el8) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: MovhdmvGnhpCuCs0isN8ke9ZToVZNrBt X-Proofpoint-ORIG-GUID: UFuN1lke7oWJs8-YH6IauU3JXIGwKgMK Subject: RE: [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-10-18_07,2022-10-18_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 mlxlogscore=997 lowpriorityscore=0 phishscore=0 suspectscore=0 priorityscore=1501 bulkscore=0 impostorscore=0 clxscore=1015 mlxscore=0 malwarescore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000 definitions=main-2210180103 X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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, 18 Oct 2022 18:26:49 -0000 On Mon, 2022-10-17 at 18:06 -0700, Kevin Buettner wrote: > On Fri, 14 Oct 2022 16:23:58 -0700 > Carl Love wrote: > > > > 2) Whitespace complaints: > > > > > > Applying: PowerPC, fix support for printing the function return > > > value > > > for non-trivial values. > > > ...patch:161: indent with spaces. > > > "gdbarch_dump: get_return_buf_addr = > > > <%s>\n", > > > ...patch:162: indent with spaces. > > > host_address_to_string (gdbarch- > > > > get_return_buf_addr)); > > > ...patch:182: indent with spaces. > > > gdbarch_get_return_buf_addr_ftyp > > > e > > > get_return_buf_addr) > > > warning: 3 lines add whitespace errors. > > > > > > (I substituted some long paths local to my machines w/ '...'.) > > > > Yes, I have seen the complaints about the white spaces. The > > whitespace > > complaints come from file gdb/gdbarch.c which is a generated > > file. As > > I assume you know, the file was regenerated by the gdb/gdbarch.py > > based > > on the changes to gdb/gdbarch-components.py. The regenerated file > > is > > set to read only. Not sure what I can do to rectify the white > > spaces? > > Any ideas? > > If the unwanted white space is generated by gdbarch.py,maybe fix that > script? > > That said, if those files already have this problem, I'm okay with > it going in with the whitespace complaints. (It can be fixed in > a later patch addressing just that problem.) I see the same white space issues through out the file. The script generates the spaces so yes, it probably needs fixing and then cleanup the white spaces in the file. However, that is beyond the scope of this patch. > So, I tried to address all of the above comments about ABI, > > "DW_OP_entries" etc. The paragraph now reads: > > > > This patch adds a new gdbarch method to allow PowerPC to access the > > value > > of r3 on entry to a function. On PowerPC, the new gdb arch method > > attempts > > to use the DW_OP_entry_value for the DWARF entries, when exiting > > the > > function, to determine the value of r3 on entry to the > > function. This > > requires the use of the -fvar-tracking compiler option to compile > > the > > user application thus generating the DW_OP_entry_value in the > > binary. The > > DW_OP_entry_value entries in the binary file allows GDB to resolve > > the > > DW_TAG_call_site entries. This new gdbarch method is used to get > > the > > return buffer address, in the case of a function returning a > > nontrivial > > data type, on exit from the function. The GDB function should_stop > > checks > > to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be > > set to > > zero by the new gdbarch method call for all architectures except > > PowerPC. > > The get_return_value function will be used to obtain the return > > value on > > all other architectures as is currently being done if RETURN_BUF is > > zero. > > On PowerPC, the new gdbarch method will return a nonzero address in > > RETURN_BUF if the value can be determined. The value_at function > > uses the > > return buffer address to get the return value. > > > > Which I think covers all of the comments above. > > Yes, this looks good, though I'd use 'gdbarch' in place of 'gdb > arch'. > This is a small nit though; if you'd prefer to make them separate > words, I won't argue about it. I intended to use "gdbarch" not "gdb arch". Fixed. > > Thanks for reworking that comment. > > > > > + catch (const gdb_exception_error &e) > > > > + { > > > > + warning ("Cannot determine the function return value.\n" > > > > + "Try compiling with -fvar-tracking."); > > > > > > Will this warning be printed a lot? (I'm wondering if there > > > should > > > be a > > > way to shut it off...) > > > > Hmm, hard to say how often it will get printed. It is going to be > > up > > to what the user is trying to do. If they are stopping on the > > return > > of functions that return non-trivial structures, they will see the > > message for every function. Otherwise, the message should not get > > printed. > > > > Not aware of anyway we could prompt the user to ask them if they > > want > > to suppress this message from the command line. If there is, can > > you > > give me a pointer to an example? > > > > We could do something like: > > > > catch (const gdb_exception_error &e) > > { > > static int warned = 0; > > > > if (warned++ < 6) > > warning ("Cannot determine the function return value.\n" > > "Try compiling with -fvar-tracking."); > > } > > > > So the warning is only printed the first five or whatever number of > > times we want before suppressing the message. Not sure what the > > right > > number of times should be? > > > > Would adding something like this be agreeable? > > Maybe. If we go that route, I'd recommend printing an additional > message along side the last "Cannot determine the function return > value." > message indicating that these messages are now being suppressed. > > We have several mechanisms in gdb for suppressing messages already. > Probably the most used one is the "complaints" facility for > supressing warnings while reading symbols. (See > gdb/complaints.{h,c}.) I recently added a bfd error supression > mechanism in gdb_bfd.c. Look for gdb_bfd_error_handler() in that > file. There's also lim_warning() in ada-lang.c along with an old > FIXME remark indicating that it's doing something similar to that of > complaint(). There may be others, but I don't know for sure. > None of them prompt the user about whether to continue to print > warnings; I'd prefer not to do it that way. Hard coding it to six > (or > something) is okay. Making it user-settable with some default is > also okay. That said, this is just one message. It seems like > overkill > to add a setting for this one message. > > After thinking about this a bit more, I think we should go with your > original code which doesn't suppress any messages. If we find that > it's too noisy, we can add something along the lines of what you > suggested above. I think that sounds reasonable. We can always look at suppressing the messages in the future if it looks like an issue. I am leaving the code as it was originally. Thanks again for the feedback. Take care. Carl