From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 36381384AB58 for ; Fri, 3 May 2024 17:49:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 36381384AB58 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 36381384AB58 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.158.5 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714758556; cv=none; b=Ycafqf0dUvg6yTWOHPkDTpTHoTPIE8hAn/EMs72qERAEFrlF5Cktd19WvhutWckgj7d5wR/6qHCWCWmzh1sV1gZZbzJSWZ0Dt+UyiQeAT8rvHGyrjC51UTVifDmQ+mR6q2HbT+663aYYBPOgH+ZmUUBbZ/J/0ySIlY+BCKkrKP8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1714758556; c=relaxed/simple; bh=fvW/EVVP7O//kewoPvAltRFdjagNlYkSC+WkzVaSPFA=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=CIWF34ed2WxBFNd4leM6E406uDNJO2zSpDyTTMRLvvwFVh77xO40rwyGxjpYDH7Wwmvfy3SK7eylPYyiFMLM5qeHy9WgPRfz9OlpG+ExfAxXtC0leDbzgyaa6ngq/Rf6MA5w8WLDFhYHINzwqg5+cIs2Y6uLuzv/PdAd1kb1b7k= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353725.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 443G41mr023280; Fri, 3 May 2024 16:40:31 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pp1; bh=eG3fI2pYlwPCfl+9A1JwwiHlcEKkMAZJ412S6giCokk=; b=Q+gjupHIHtwtB7Td33El5JCnOqcSRlBMh5wjzqsBkrhw83b8EuW4yt9MpRg7xfvdXHeW tJAi0SAPXoNDKQ2FAa/G+R7TF9KE58BzEtaPN75Cnr1YXwveDIz6rhQrq6HKcKDy0++9 dvYVPw42z8b9tAncFZYJ/g7/H90oAkktwA7s0qpyw8tBYIKCeyj9UYe+Kod+K+KVbhMK gKvFSKfcpCeSGvEBensik0YZ7DeqDDBDUWdBdUg1N1lXAxq+S7W+pS0ouvztpbDA2dgP uPNbZEWeWcPLhIf5++hxHTtWuc4sMyULWgkMunsPG3A9I52BRDdLemLSIQfxBoHUs9cx pQ== Received: from ppma23.wdc07v.mail.ibm.com (5d.69.3da9.ip4.static.sl-reverse.com [169.61.105.93]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3xw2tf84vn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 03 May 2024 16:40:30 +0000 Received: from pps.filterd (ppma23.wdc07v.mail.ibm.com [127.0.0.1]) by ppma23.wdc07v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 443E7fjs022177; Fri, 3 May 2024 16:40:30 GMT Received: from smtprelay04.fra02v.mail.ibm.com ([9.218.2.228]) by ppma23.wdc07v.mail.ibm.com (PPS) with ESMTPS id 3xsd6n6jut-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 03 May 2024 16:40:30 +0000 Received: from smtpav02.fra02v.mail.ibm.com (smtpav02.fra02v.mail.ibm.com [10.20.54.101]) by smtprelay04.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 443GeOZ716122196 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 3 May 2024 16:40:26 GMT Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 25C1620043; Fri, 3 May 2024 16:40:24 +0000 (GMT) Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id BFEEF20040; Fri, 3 May 2024 16:40:23 +0000 (GMT) Received: from [9.171.46.7] (unknown [9.171.46.7]) by smtpav02.fra02v.mail.ibm.com (Postfix) with ESMTP; Fri, 3 May 2024 16:40:23 +0000 (GMT) Message-ID: Date: Fri, 3 May 2024 18:40:23 +0200 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 15/15] gas: Validate SFrame RA tracking and fixed RA offset To: Indu Bhagat , binutils@sourceware.org Cc: Andreas Krebbel References: <20240412144718.4191286-1-jremus@linux.ibm.com> <20240412144718.4191286-16-jremus@linux.ibm.com> From: Jens Remus Content-Language: en-US Organization: IBM Deutschland Research & Development GmbH In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: eM_NBq5j_S5Mhjhn07E1bRr5fZvxjUPb X-Proofpoint-GUID: eM_NBq5j_S5Mhjhn07E1bRr5fZvxjUPb Content-Transfer-Encoding: 8bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26 definitions=2024-05-03_10,2024-05-03_02,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 priorityscore=1501 clxscore=1015 impostorscore=0 bulkscore=0 mlxlogscore=999 lowpriorityscore=0 mlxscore=0 spamscore=0 adultscore=0 suspectscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000 definitions=main-2405030115 X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,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 List-Id: Am 18.04.2024 um 22:38 schrieb Indu Bhagat: > On 4/12/24 07:47, Jens Remus wrote: >> If an architecture uses SFrame return-address (RA) tracking it must >> specify the fixed RA offset as invalid. Otherwise, if an architecture >> does not use RA tracking, it must specify a valid fixed RA offset. >> >> gas/ >>     * gen-sframe.c: Validate SFrame RA tracking and fixed >>     RA offset. >> >> Signed-off-by: Jens Remus >> --- >> >> Notes (jremus): >>      Changes v2 -> v3: >>      - New patch. >>      This could be made dependent on ENABLE_CHECKING (configure option >>      --enable-checking). >> >>   gas/gen-sframe.c | 12 ++++++++++++ >>   1 file changed, 12 insertions(+) >> >> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c >> index ca6565b0e45e..7e815f9603ef 100644 >> --- a/gas/gen-sframe.c >> +++ b/gas/gen-sframe.c >> @@ -1532,6 +1532,18 @@ output_sframe (segT sframe_seg) >>     /* Setup the version specific access functions.  */ >>     sframe_set_version (SFRAME_VERSION_2); >> +#ifdef SFRAME_FRE_RA_TRACKING >> +  if (sframe_ra_tracking_p ()) >> +    /* With RA tracking the fixed RA offset must be invalid.  */ >> +    gas_assert (sframe_cfa_ra_offset () == SFRAME_CFA_FIXED_RA_INVALID); >> +  else >> +    /* Without RA tracking the fixed RA offset may not be invalid.  */ >> +    gas_assert (sframe_cfa_ra_offset () != SFRAME_CFA_FIXED_RA_INVALID); >> +#else >> +  /* Without RA tracking the fixed RA offset may not be invalid.  */ >> +  gas_assert (sframe_cfa_ra_offset () != SFRAME_CFA_FIXED_RA_INVALID); >> +#endif >> + > > I am not sure if the detailed checks are worth it here (simply because > of code patterns that follow). I agree, provided the checks are performed elsewhere as you suggest. My intention was to have checks that assist with getting SFrame support for another architecture implemented correctly, without having to chase subtle issues. > > We use the sframe_cfa_ra_offset () function later and only in > output_sframe_internal () (shown below).  How about we simply put an > assert there (and get rid of the proposed thunk above): > > #ifdef sframe_ra_tracking_p >   if (!sframe_ra_tracking_p ()) See below. >     { >       fixed_ra_offset = sframe_cfa_ra_offset (); >       gas_assert (fixed_ra_offset != SFRAME_CFA_FIXED_RA_INVALID); That is clever and accounts for one potential implementation issue! >     } > #endif >   out_one (fixed_ra_offset); > > fixed_ra_offset is initialized to SFRAME_CFA_FIXED_RA_INVALID in > output_sframe_internal (). Above logic requires sframe_ra_tracking_p to be defined by an architecture that is not using RA tracking. Not defining sframe_ra_tracking_p would result in fixed_ra_offset being unexpectedly initialized to SFRAME_CFA_FIXED_RA_INVALID instead of being set to sframe_cfa_ra_offset(). All checks but this do test SFRAME_FRE_RA_TRACKING first, which ensures both sframe_ra_tracking_p and SFRAME_CFA_RA_REG are defined, and then the predicate sframe_ra_tracking_p to determine whether RA tracking is used. If SFRAME_FRE_RA_TRACKING is defined and sframe_ra_tracking_p returns true, then RA tracking is used. Likewise, if SFRAME_FRE_RA_TRACKING is not defined or if sframe_ra_tracking_p returns false (evaluating lazily) RA tracking is not used. What about making the following change to make all RA tracking tests consistent depend on SFRAME_FRE_RA_TRACKING? #ifdef SFRAME_FRE_RA_TRACKING if (!sframe_ra_tracking_p ()) #endif { fixed_ra_offset = sframe_cfa_ra_offset (); /* Without RA tracking the fixed RA offset may not be invalid. */ gas_assert (fixed_ra_offset != SFRAME_CFA_FIXED_RA_INVALID); } out_one (fixed_ra_offset); What would still not be checked is the implementation error to define sframe_ra_tracking_p and have it return true without also defining SFRAME_CFA_RA_REG. This would be treated as if RA tracking was not used. Would it therefore make sense to add the following? #if defined (sframe_ra_tracking_p) && !defined (SFRAME_CFA_RA_REG) gas_assert (!sframe_ra_tracking_p ()) #endif Also when using RA tracking an architecture should implement sframe_cfa_ra_offset to return SFRAME_CFA_FIXED_RA_INVALID. Would it therefore make sense to add the following? #ifdef SFRAME_FRE_RA_TRACKING if (sframe_ra_tracking_p ()) gas_assert (sframe_cfa_ra_offset () == SFRAME_CFA_FIXED_RA_INVALID); #endif > >>     /* Process all fdes and create SFrame stack trace information.  */ >>     create_sframe_all (); > Thanks and regards, Jens -- Jens Remus Linux on Z Development (D3303) and z/VSE Support +49-7031-16-1128 Office jremus@de.ibm.com IBM IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294 IBM Data Privacy Statement: https://www.ibm.com/privacy/