From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-00069f02.pphosted.com (mx0b-00069f02.pphosted.com [205.220.177.32]) by sourceware.org (Postfix) with ESMTPS id 972CA3858D1E for ; Tue, 1 Nov 2022 22:36:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 972CA3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=oracle.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=oracle.com Received: from pps.filterd (m0246632.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 2A1M41xM016271 for ; Tue, 1 Nov 2022 22:36:17 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=message-id : date : subject : to : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=corp-2022-7-12; bh=pCO/8sCXHtHPiAeZ8kSXvz5FxlFb83OUBIVW+bYs/0U=; b=lFRtJONUy7zNvbP+EqHC6xYpv+aAzx4j2F99tzilpMhVUrLdNeGQDZyHc+3Ki+UyIIPA vE57U0d0XVIGN7JUyU4vAoc6BTjoRw96PsqDdj2xSp+IwJQzdDlxY1U3KBahW78+gTG5 j8vqrBwQtQCRFvfHTQQwr7bae3tj49BqLkvoKop/49XnhFeBgrsSV1HUWzVokbVtqbsG n0IT65LM8g7xu1qe+NrOPqFD91xt9HjkpYE5yR8+8zTbPaJlzKULB7fiUm5Q47PJwW0U aPn3bIDtWE5iSuj/FTwLDX92yotkNJ44d/8mrrOwU3shJEBWFDPhAVyBjMPBRzAqpiV6 YA== Received: from iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com (iadpaimrmta01.appoci.oracle.com [130.35.100.223]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3kgussr23v-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 01 Nov 2022 22:36:16 +0000 Received: from pps.filterd (iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com [127.0.0.1]) by iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com (8.17.1.5/8.17.1.5) with ESMTP id 2A1KKuhI002861 for ; Tue, 1 Nov 2022 22:36:16 GMT Received: from nam10-bn7-obe.outbound.protection.outlook.com (mail-bn7nam10lp2104.outbound.protection.outlook.com [104.47.70.104]) by iadpaimrmta01.imrmtpd1.prodappiadaev1.oraclevcn.com (PPS) with ESMTPS id 3kgtmaydjw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 01 Nov 2022 22:36:15 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TABWR8lmOzqEPF3un/POWiB3qXcSlh1mK88dINKcaT6gJ07j+SPrJLaxoWNLk7wIkMs2vQ+7vRXln5D/FUcsfmajm5vWMrXy8U8IiGTGh4Qn+LE4oZoBWl8GkdnP9DRpHIDRCTTN4erSVkAMLJhAXeQUe/vvWvjXCAiA/Jf5MFx/oqEBcjFtoMSMVBP3LVn29smcrUzud6M9jskH9bc/9FtN+GH7/jt0+I5iUhlGs/NpHYXMHXXG59juv2gKmV6BLgOTSHTEu6eZO3eBttqiYEVIn/pspRuz7oXJonyHVO3uupsHfYp2+pWw++f2/KiPW2yphoGtnTAcZMIRIMs7pw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=pCO/8sCXHtHPiAeZ8kSXvz5FxlFb83OUBIVW+bYs/0U=; b=ET8lvUvLj105kzmcH/j0SieDMK5ktunq2iJ7SGFzyD69J59rbqVVmdIb9ZbCJpfWCS6G7CQGK7mI8HCg6HMWGNZl+dEfu3NaSzI9mXd4eFPbld3l4bobhQweGAMqHQNO9IIgf981JIk48YCFelPFFq8o7qXTFRrVsXAW4ZragfxJbJtMorcS9HNePXw8d0qm72Vfzf9ckUY5E6BoflL+nSkviBwy2Q3nmH8umQhEbDwOXT/bdKsJarbDsrmNlY/qfU1WsLoJqLuGSt7NKNo8coBcAzVYzdy4lOahemVvBpgxrrpN0Y5gInA1x916NthGJZu2pUTJ/gxSv5X5vEfWig== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.onmicrosoft.com; s=selector2-oracle-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=pCO/8sCXHtHPiAeZ8kSXvz5FxlFb83OUBIVW+bYs/0U=; b=jWTG8b9Sb1R0NHHxjIfjYVwodw7k1vhbm8Ws6bGHiTpr9p3hFcsBW6seHFGtFUMg9hk1Y/m1hlnSu6uC71loDECD3VWelcLe/TFWKbvTKae497u9cSFifDXOSm7lUkri1aHmS8xfpGg65UNQtwdwOvDPuAFyiIp6flS5TPeW0oc= Received: from SJ0PR10MB4655.namprd10.prod.outlook.com (2603:10b6:a03:2df::23) by DM6PR10MB4284.namprd10.prod.outlook.com (2603:10b6:5:21f::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5769.19; Tue, 1 Nov 2022 22:36:14 +0000 Received: from SJ0PR10MB4655.namprd10.prod.outlook.com ([fe80::8e00:7668:70b5:7854]) by SJ0PR10MB4655.namprd10.prod.outlook.com ([fe80::8e00:7668:70b5:7854%9]) with mapi id 15.20.5769.021; Tue, 1 Nov 2022 22:36:14 +0000 Message-ID: <34f06d43-3db9-b16d-95e4-805c24ad258a@oracle.com> Date: Tue, 1 Nov 2022 15:36:11 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH, V3 08/15] unwinder: generate backtrace using SFrame format To: binutils@sourceware.org References: <20221030074450.1956074-1-indu.bhagat@oracle.com> <20221030074450.1956074-9-indu.bhagat@oracle.com> Content-Language: en-US From: Weimin Pan Organization: Oracle Corporation In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: DS7PR06CA0030.namprd06.prod.outlook.com (2603:10b6:8:54::22) To SJ0PR10MB4655.namprd10.prod.outlook.com (2603:10b6:a03:2df::23) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: SJ0PR10MB4655:EE_|DM6PR10MB4284:EE_ X-MS-Office365-Filtering-Correlation-Id: d0a9ca9f-f10b-451b-e6b2-08dabc597bd6 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: g2i09+GaCU663yE3jFTSZpoAtdNhnSB0ydATbPqgj6XjjGVuuCZnsmygAzqxIimx1XDLrZvWsSrqKE0J3Iyqp7kWNiNnuV+nXF6m4P4EDiL+QXEKBm0Clqa+yJ+JkFx5JMD6OgXlNKzfMwb7tWYpvTki8ozdMVhRNCb6DEYhTRHp+myq5AD/HFGRmEglJuThoGJbmNht7BvE/aHhHPTNUoZkb7dK7yHsn/oNxdyYjbmG6CyGN+XZL0eJmG8c0/h2i5uZ7szzOZTdQgiG5U1QE8H6/YiuebtVxWj5SfybAUT+8mQYFhGG4jYelSTH2ghlJRm0gAmgpTptcKpMm0Vfngdjy3cJv270iSRT1mwlnKw7RqT/M5/4HofP/BVmTXNIffW91ZfMGbZRiKExS+IQgnhJonmHYoY9JrqutZVzLMb5YdZN0PKyNcMFCCkijK4iiTcD7+i7w1uxPBLRaA8u2owdiuPk2gbfZVEkwB0rd9PhouDEh5KsDzVPAe5l9iV+9zdUjjzyYfDzFng+eBjAM+gCGFaj3dLXaphfla0KsIp77mOfBxJ7fQa3xDVcab0Kvht52/3r8NBjs9XXq9YAV4b0u7zhgEs/KNaVr7Wiev5H/a+xBfUUX1RC/9MBVhktia+0vokxN5RTea+M5tjLfrAhJlwtrTO4hmLMUM2FWdt0L0CgpuO9GLGjA0rBYjkmGIY3SlOZmdSqbC/SKrMKpuvn6tsfT/6s52Z4HrjChv02OsT0/almVNFfo0CvwdkeunKo5cqe/azLq2mEn3HnbptfI9YZuFRZJNPrfvGKGBI= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SJ0PR10MB4655.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(39860400002)(396003)(376002)(346002)(366004)(136003)(451199015)(86362001)(83380400001)(478600001)(31686004)(6486002)(66476007)(6506007)(66946007)(53546011)(8676002)(6512007)(36916002)(41300700001)(8936002)(6666004)(44832011)(36756003)(2616005)(5660300002)(66556008)(6916009)(316002)(31696002)(186003)(2906002)(38100700002)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MjB0VEdjVDNvMUp0bU5DaEpYNnNEeFBDVG5wN0tQRHBmVWVVT1ovK2pNN0pF?= =?utf-8?B?YXFVK3ZaVUJrR05PZ0NrMjIrZTNlek5JbjNOdDkxeWRnOWhsZXJER0ZKNzZO?= =?utf-8?B?Y0FFNDUzWHNhVmtkbVYwWlVMd1dqWnBnaUpTN3hySTdzUmNVMFhwSjZQUkFL?= =?utf-8?B?MEpxV1VoaFUyQm52Z1dXM05VVUYvZ1R5WE5mSHNGUWJGa21STzBqUWxhUm1J?= =?utf-8?B?TnlJYjVVYUhoR20wOHlUSU44U0RHT0tNUjIrZFVoQjk0aWplb2dtYUtDYm1R?= =?utf-8?B?bEM3eXdVV21nV2VHMU5oVHR0NDREYjBSZ2hXSFhVdFU4ZEwxcU9JTDdwNE13?= =?utf-8?B?UU02RU5uU3NrMXlkblBkQmYvVHVRWUpHK0N1V2Y3MlZaS3IvaHpac2Mvc2dp?= =?utf-8?B?dFc2SVV3dDU2YnVKOWRMTGFVbERnbDdBNEVMV0JhUjM2UjR3NkFDQkU4cE5x?= =?utf-8?B?cXFsMkcwWU95WWdrckFwM1E0Q0NqaTgxLzJCM1FyTzVRdUtaMWZ2YmpDNFRN?= =?utf-8?B?eVpGL0ErLzlXWkVFY3lxaVhpVVNSRmtkV2RsZjlZZ2pKa1BVUGcwR3FEMTZ1?= =?utf-8?B?SDZkUmlDdDI4a0xaL2I0OXMrSHY5d2JzVXhzelJweDh4ZzZGZWhpdnVUMjZL?= =?utf-8?B?NTVxZFRHdUNMbkRGc2tEMFlTNUtOZ2liVXFJSnRsOWNmMUdyQ0dwWTlEdXZ5?= =?utf-8?B?QWNPWkl5ekRqZnE3d2lyYm1VNmN1N1pVdWNrNW8vME4zZEN1NVBNY3dOZGNV?= =?utf-8?B?VWxmMVFPcUszV1d1NzNxWWswMXllRVJDaXJiODVmSTl6TStLWUJjeVBtL0Ns?= =?utf-8?B?UWg3K1E2QkpQR05nYjlIOHgxaHZia2pjTEVGOHo4bnUzZDFtOGhtSUtnMTlI?= =?utf-8?B?NG5KZ1dFVXZEMlQ5Z3Y3ZE9vMFhCSDFEYzBMRVRjWDI0d3RsSndHbzlVL0Z1?= =?utf-8?B?YUlLWFFZcDJ5L0JJVG13TUhCNksrdCtRem94SG13OStwcVFNK3pXY0xLVzZJ?= =?utf-8?B?dVlkNUN3dmtKcmdDT3drTjdsS280SUJCRDNiQ01sSlNtWUVRTFBnMW1mYTFv?= =?utf-8?B?ZW9zRytyb3VDV2tuVllCUW9yQ1ZPZkN4OUVQc21pQWk3WDhPNUs5cm5wNmVM?= =?utf-8?B?Yk9Kekp4S2tnZkRyeWpDcUo3MmtPRkwvZGJSMHVHRXRmNTMrSnROcWFQams0?= =?utf-8?B?SmlpWEViSEh1YUwxNnlLYzNlbEEwYXo4T2dOSGpBR0JGY0t6REFralljeTdX?= =?utf-8?B?bGdHSnVROVJHZFllSE9ucFM1aWdKNXpWSUd0Qzk1TllseTF5eWxSQWtkTE9V?= =?utf-8?B?WWdzM01ScHVBVHI0aVhRZk9CRkZHNUM4b0VSYXRuVERWd0k1b3RpR2YxeWNU?= =?utf-8?B?dHhKU1N1UnJjSkgrUHhBa3VyTE5mQjN5Tnl0MG02enkwOWlDZUlVZ0hjOHlp?= =?utf-8?B?VmNtWFM3MXR4ME1DQVZRVnNFSVRSVHI1czZZNzlhVEIzYmNOZ3pHYTVBVVky?= =?utf-8?B?VklKQzNSWXd0UEdQMnRHbldkRzM2cUorUkVuSnlTRW9EemhXWGlXd3BkT1Nx?= =?utf-8?B?L29zZ00vdUU3SkI2dldmUVlmbllueE5rVUZoM0ZJUklhYlM0RE55bG1HUTRU?= =?utf-8?B?MXFXcy9YQTgrTDJyMjU5UFFuWWRWMEhURGdOc1FRRDNRMm5XMllZMXYwUkc4?= =?utf-8?B?eEhMRjUydno1R1NTYklSZ3NibGpSelh4c2lQNzUwZnRJM2FnMzUvWERhZGpF?= =?utf-8?B?S1B3QmpzTVN3b0RQbjNTVUxqMXV1eVd6RDZacXQrSXZPOTRwZ1c1REMyWFJP?= =?utf-8?B?ZVJPdHBDS2lSSHEveVA1WUZTaFVwREh5YTFjZ0R6L3FIZHdGMVBxMkw3dGdj?= =?utf-8?B?RkhlSHRMR0QrSFFXeTZuVXJ1WDRhL1dlcWNibmNsWVlzTGwwZFUxa0FFeExX?= =?utf-8?B?bEsvWjhsVVY1UGVFb2loeUNnUXJ1SDI2ZmFvcDR5SmprRytRR2dWK25ROXJZ?= =?utf-8?B?SFZDQmpXTmtiMzJWOUNqb2VjQ0NIOWl2N0lMT0czdVc3bW50VmJOdEgva3BB?= =?utf-8?B?dStJN2p0czlsbGF6R2wzTEJCTmVHZXovQk1nMGtUcjFZWUpaNUFFQll0bC95?= =?utf-8?B?cjkrSS9kVzVWZVNLa253Rlh6N0NlWlhZeTFHTXRkbUNNT3Y0YXdNK3lBT0dX?= =?utf-8?Q?hrHbVWQbj3/TLRPWcr7Gm5Q=3D?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: d0a9ca9f-f10b-451b-e6b2-08dabc597bd6 X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4655.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Nov 2022 22:36:14.4013 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Zu1GidJ5uhWGe1j5QymzVlTjqa5NGv+u8ewNxrWEqo0kOGrNkP3f7/fUOYn5q/8jGt8/hvGv1LNMKBDYXIllEg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4284 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-11-01_10,2022-11-01_02,2022-06-22_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 adultscore=0 mlxscore=0 suspectscore=0 malwarescore=0 bulkscore=0 phishscore=0 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000 definitions=main-2211010153 X-Proofpoint-ORIG-GUID: 2enQRJgYp3pDhLGkPgBgxdIenztj-jxb X-Proofpoint-GUID: 2enQRJgYp3pDhLGkPgBgxdIenztj-jxb X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,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 List-Id: Thanks for your comments. On 10/30/2022 7:03 AM, Mike Frysinger via Binutils wrote: > On 30 Oct 2022 00:44, Indu Bhagat via Binutils wrote: >> --- /dev/null >> +++ b/config/sframe.m4 >> @@ -0,0 +1,16 @@ >> +# SFRAME_CHECK_AS_SFRAME >> +# ---------------------- >> +# Check whether the assembler supports generation of SFrame >> +# unwind information. >> +# >> +# Defines: >> +# ac_cv_have_sframe >> + > you should be using `dnl` for comments in m4 files so they aren't copied > into the generated output. OK. >> +AC_DEFUN([SFRAME_CHECK_AS_SFRAME],[ > space after the , OK. >> + ac_save_CFLAGS="$CFLAGS" >> + CFLAGS="$CFLAGS -Wa,--gsframe" >> + AC_MSG_CHECKING([for as that supports --gsframe]) >> + AC_TRY_COMPILE([], [return 0;], [ac_cv_have_sframe=yes], [ac_cv_have_sframe=no]) >> + AC_MSG_RESULT($ac_cv_have_sframe) >> + CFLAGS="$ac_save_CFLAGS" >> +]) > you call it "ac_cv_have_sframe" which implies it's an autoconf cached var, > but you aren't actually using the AC_CACHE_CHECK macro. > > i'm guessing this isn't actually coming from autoconf, or will be merged > there, so shouldn't this be using a "gcc_cv_" prefix instead ? i'm not > sure what the policy is on config/ when it comes to home-grown cache vars. Indu should be able to address issue better. > similarly, should the macro name lacks scoping ... > >> --- a/libsframe/configure.ac >> +++ b/libsframe/configure.ac >> >> COMPAT_DEJAGNU=$ac_cv_dejagnu_compat >> AC_SUBST(COMPAT_DEJAGNU) >> >> +dnl The libsframebt library needs to be built with SFrame info. >> +dnl If the build assembler is not capable of generate SFrame then >> +dnl the library is not built. >> + >> +SFRAME_CHECK_AS_SFRAME >> +AM_CONDITIONAL([HAVE_SFRAME_AS], [test "x$ac_cv_have_sframe" = "xyes"]) > hmm, is this macro only used by libsframe/ ? if no one else is going to use > this macro, config/ isn't the right place for it. you should put it into > libsframe/acinclude.m4 instead. Looks like libsframe/ is the only place that uses this macro. >> --- /dev/null >> +++ b/libsframe/sframe-backtrace-err.c >> >> +/* SFrame backtrace error messages. */ >> +static const char *const sframe_bt_errlist[] = >> +{ >> + "", >> + "File does not contain SFrame data", >> + "Iterating shared object reading error", >> + "Failed to malloc memory space", >> + "Failed to realloc memory space", >> + "Failed to open file", >> + "Failed on resolve canonical file name", >> + "Failed to reposition file offset", >> + "Failed to read from a file descriptor", >> + "Failed to get the user context", >> + "Failed to set up decode data", >> + "Illegal CFA offset" >> +}; >> + >> +/* Return the error message associated with the error code. */ >> + >> +const char * >> +sframe_bt_errmsg (enum sframe_bt_errcode ecode) >> +{ >> + return sframe_bt_errlist[ecode]; >> +} > this needs to make sure the ecode is within range, and if it isn't, at the > very least perform an assert/abort. > assert((unsigned int)ecode < ARRAY_SIZE (sframe_bt_errlist)) OK, added code to validate ecode. It prints "Unknown error" for invalid ecode. >> --- /dev/null >> +++ b/libsframe/sframe-backtrace.c >> >> +#ifndef _GNU_SOURCE >> +#define _GNU_SOURCE >> +#endif > i thought your configure script already took care of this ? Yes,  the above code is taken out. >> +#ifndef PT_SFRAME >> +#define PT_SFRAME 0x6474e554 /* FIXME. */ >> +#endif > what excatly is the fix ? that it should be defined in include/elf/common.h > first (among other places) ? you should do that then instead of keeping this. Since , which is needed for dl_iterate_phdr, also includes . using include/elf/common.h will cause multiple defined warnings, e.g. PT_HIPROC. Looks like it should be defined in /usr/include/elf.h. >> +#define _sf_printflike_(string_index,first_to_check) \ >> + __attribute__ ((__format__ (__printf__, (string_index), (first_to_check)))) > use ATTRIBUTE_PRINTF from ansidecl instead of defining your own OK. >> +static int _sframe_unwind_debug; /* Control for printing out debug info. */ > this should be a proper bool instead of an int-pretending-to-be-bool Change its type to bool. >> +static int no_of_entries = 32; > shouldn't this be a const ? OK, done. >> +static int >> +sframe_bt_errno (int *errp) >> +{ >> + if (errp == NULL) >> + return 0; >> + >> + return (*errp != SFRAME_BT_OK); >> +} > shouldn't errp be const ? Done. >> +static int >> +sframe_fd_open (int *errp) >> +{ >> + char filename[PATH_MAX]; > don't rely on PATH_MAX. use asprintf. > >> + pid = getpid (); >> + snprintf (filename, sizeof filename, "/proc/%d/task/%d/mem", pid, pid); > /proc//task// is the same thing as /proc// > > and /proc// is simply /proc/self/ > > so can't you use the constant path /proc/self/mem ? > am i missing something obvious here ? Nice. We can get rid of asprintf and filename[] altogether and just do:         if ((fd = open ("/proc/self/mem", O_CLOEXEC)) == -1) >> + if ((fd = open (filename, O_RDONLY)) == -1) > you should always use O_CLOEXEC when calling open(). if you don't want > that flag, you should always leave a comment explaining why. > >> +static int >> +sframe_callback (struct dl_phdr_info *info, > this seems to be returning a bool, so use a bool instead of an int Function sframe_callback is the "callback" of        int dl_iterate_phdr(                  int (*callback) (struct dl_phdr_info *info,                                   size_t size, void *data), void *data); which returns an int: 0 for success and 1 otherwise. >> + for (i = 0; i < info->dlpi_phnum; i++) >> + { >> + debug_printf(" %2d: [%14p; memsz:%7lx] flags: 0x%x; \n", i, > GNU style puts space before the ( OK >> + (void *) info->dlpi_phdr[i].p_vaddr, > p_vaddr is an Elf_Addr right ? you can't assume that sizeof(void*) is >> = sizeof(Elf_Addr). a 64-bit BFD on a 32-bit host will break this. > cast it to a proper uint64_t and use proper stdint.h PRIx64 types. > >> + info->dlpi_phdr[i].p_memsz, > i think this also has broken assumptions -- that sizeof(long) == > sizeof(p_memsz). cast it to 64-bit too. OK, cast it to uint64_t and use PRIu64 for both p_vaddr and p_memsz. >> + sf->sui_ctx.sfdd_data = (char *) malloc (info->dlpi_phdr[i].p_memsz); > malloc returns void*. in C, we don't have to cast that. so you can omit it. OK, done. > >> +static void >> +sframe_unwind (struct sframe_unwind_info *sf, void **ra_lst, >> + int *ra_size, int *errp) >> +{ >> ... >> +#ifdef __x86_64__ > this really needs better delegation for porting than inlined in the middle > of a large portable file. can you at least factor it out into a header ? Sorry, not sure I follow this comment. Would you please elaborate on "factoring it out into a header"? >> + pc = cp->uc_mcontext.gregs[REG_RIP]; >> + rsp = cp->uc_mcontext.gregs[REG_RSP]; >> + rfp = cp->uc_mcontext.gregs[REG_RBP]; >> +#else >> +#ifdef __aarch64__ > use `#elif defined` to avoid an ever-growing set of nested #else/#endif OK. Thanks again. > -mike