From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-00069f02.pphosted.com (mx0a-00069f02.pphosted.com [205.220.165.32]) by sourceware.org (Postfix) with ESMTPS id 13CD53857831 for ; Wed, 4 May 2022 12:51:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 13CD53857831 Received: from pps.filterd (m0246629.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 244C0uAr032436; Wed, 4 May 2022 12:51:11 GMT Received: from phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com (phxpaimrmta02.appoci.oracle.com [147.154.114.232]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3frw0ar4p3-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 04 May 2022 12:51:11 +0000 Received: from pps.filterd (phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1]) by phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com (8.16.1.2/8.16.1.2) with SMTP id 244CfL8g003385; Wed, 4 May 2022 12:51:10 GMT Received: from nam11-co1-obe.outbound.protection.outlook.com (mail-co1nam11lp2177.outbound.protection.outlook.com [104.47.56.177]) by phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com with ESMTP id 3fs1a5pbdu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 04 May 2022 12:51:10 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hSaO05WUS2VVsH47W3ybO7XzLJeYrIDm1AXqe2/FkKfjhbhSMBk1ahqwKhay8dtPqmMz7HG76ohNx92/lhrPYPK4+XatFyPnSok4Sm8qZac9KTLNQwdVUYux8BPKTH2rGZ6avFDhqbtVBa2qNgiVsi39if7Ob1RsOgOcWwtW3ChzmxBKBFX5VVu7jtVWt0xjaYFQ6YjwCT25U4yFqypPzzQxLXZH99BGrZMyL5x/EYrZTy9tB9VxhnNPS1uMlyceQ8EaA67Rp0jZxSPCqcZhvHNtxQpms96kxe7XvdjxS9Y5o2mFpYj8kjeBuxpsFNtAkiqkvKgCZzCO3yZzZNLeaA== 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=nrBlwLPFIpR10aLWR+w6aWffU0usXme725R9p4aABNw=; b=Vbq1wR+edr+NCmaeRDuRfYbQ0q1/SIvBnWn8D1Wpe/8+QEeGf+IBibrzo/qoUhJ7/c+D6cX7jhcFHkgvrsTHLWzLBeptTtir1UCbjy2jyBXV7yAPAvR/L7L4a1K+lNbQ9sH2e3XwN/YeworWkHcvrRMSVB/kOIV4KCTJVz5xPcEUtV9oTXGIZQDjMJ7cXguqX8DKM2lN6zXpPAogz5VJaSHzqNmwSuH+Dlrp5GBWHdHmU7sIkk1WdG7aYUwqUqj56qUjaVhd01IA+gd9MetLai2qif4zYyZwUeSBRqDJu5hX9DXPHWw+oLVRzEiJ0li+XaWDPiBp/hBkbyZsCs9avw== 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 Received: from MWHPR10MB1407.namprd10.prod.outlook.com (2603:10b6:300:23::20) by CY4PR10MB1911.namprd10.prod.outlook.com (2603:10b6:903:120::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5206.24; Wed, 4 May 2022 12:51:07 +0000 Received: from MWHPR10MB1407.namprd10.prod.outlook.com ([fe80::107d:de35:fa5f:3cc0]) by MWHPR10MB1407.namprd10.prod.outlook.com ([fe80::107d:de35:fa5f:3cc0%7]) with mapi id 15.20.5206.024; Wed, 4 May 2022 12:51:07 +0000 From: "Guillermo E. Martinez" To: "Guillermo E. Martinez via Libabigail" , Dodji Seketeli Subject: Re: [PATCH v2] ctf-reader: Add support to read CTF information from Linux kernel Date: Wed, 04 May 2022 07:48:52 -0500 Message-ID: <2543885.7s5MMGUR32@sali> Organization: Oracle Corp In-Reply-To: <877d72fx7e.fsf@seketeli.org> References: <20220330153531.2769612-1-guillermo.e.martinez@oracle.com> <20220429141658.260012-1-guillermo.e.martinez@oracle.com> <877d72fx7e.fsf@seketeli.org> X-ClientProxiedBy: SA0PR13CA0010.namprd13.prod.outlook.com (2603:10b6:806:130::15) To MWHPR10MB1407.namprd10.prod.outlook.com (2603:10b6:300:23::20) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 2de4584c-3d72-419d-64c5-08da2dccc1b7 X-MS-TrafficTypeDiagnostic: CY4PR10MB1911:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: gRRs6tMqTLsftyYBcYyu/aHznaiETJKXGvMvpeLrSG0+l+timORaOtDKxNirLbG/2Le+i3V5y+KG9YkOk305n2zp2Ndo6QCCLJx/DT/zRXQUf8fGqK1DsaytJSpoqI/JAEw4bAbof4MOosfh3pelgcolheb5EqPJ+bfxIMBbeKbK2HQtojvEvbRgquwW98YtQ71BDd9aZH0Q7GHtBJ9uB2lQOB1raLB/GbDSBYZZhxb6ZrEqBu71ne4gfVlCPjKM1DLDFmCeJmHYbMt161i/b9WxWkvta2kLQdFflQPbSKediteOBpnsLxEzBoDIcPW+qVS2lT7A1BAnAbr5CGt2J8gDuOZecesNonJloukzHa4mJRIJ56fO5/9VgIPCKFUodM7qBhgaXPhCIlqqkYZRasCiV2VcFoGSBs9f22vHR55pxgfFkHUXo71ctLrLCcvS0lg57wGefVRHw+EDjjV/vq+LLinmj3Z5FXFF1YfaeD3AV5RA2+rtm6i2orq5phdit5WJwrhC0cb+xM1aaQsD2xTbvU8vp8Da8tbyMjbQIJ7eWCvufHFsXr09T5aHwMoaApIwW6vhGmbUViLmvDcVCRocDc55WhdUhHtM1o4Iyf4sBexTt38+3S3kr9CM6F+6qDKV+85AbCyBRw/tQuIyB3xKI2vN5ffo2EdTsQiV2WIHEHM+LEOrzxNsL0frtDKscuIaRv07i02ScHUtgFJAAT660uzuWVDlyfb3E96qQ4HvIMK2dQ4yjAv0haXbbkuLcY8UZV4CQVrDhxL86eoob6q3YCJ73MpPJYLyUrWd7tp+9ykvXDDxElm5Xjs/aJAg X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR10MB1407.namprd10.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(7916004)(366004)(6512007)(21615005)(26005)(53546011)(6506007)(83380400001)(30864003)(6666004)(52116002)(36916002)(9686003)(33716001)(186003)(508600001)(66946007)(8936002)(86362001)(2906002)(6486002)(966005)(166002)(316002)(38350700002)(38100700002)(66476007)(66556008)(110136005)(5660300002)(8676002)(39026012); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?psJS0UmQ2NILHrHiF1Sv4piW63sGcnEViBHeP0Pfd9b7yxQjH8qIYgKJSPjO?= =?us-ascii?Q?ikKN82E3nQJJO3SoJV9NHE0Xs47OfSMGS9RRFf6guOulyKUacQEx1Sn033Iu?= =?us-ascii?Q?k2ztzJaemkpnXPcR4JHAp9H/o928l3QuF3z/upKh2OmRjVwdr7+DKeSMMcIZ?= =?us-ascii?Q?k7YgIlHgJeGNNyHLLh29YjRmwoRjpbYjjyHoONSK3fclrvTnLSO2k5zncfBu?= =?us-ascii?Q?0aTwCYzxXp0SokD7K1cbezBSqnE1V89zt88lXJ5D+KYEJt59M16fBCb/qBjX?= =?us-ascii?Q?1wyGxagc2TFNmK8oxMXDAwsiYhRHYFJli7XeI3o9/xtd8pG1flfd8bGtqhW3?= =?us-ascii?Q?GhoTCKraqgnoru0aXsYq0QEM6Nqj3avFN760kBOh/fzRtJbyJSy+KPMQlAaq?= =?us-ascii?Q?3lkXHgt9p/9Oylu5v1gr+lCdMb791G1eXkgvmhVLa1Gy/ABjvhpVrVSlrSRm?= =?us-ascii?Q?6QN1eiZu6s0Vkb8MxxDs55eC+ll19lL7VHPfgOIhnYgg11jUfLR3fGOVsm7A?= =?us-ascii?Q?df7UDVK9YUnsyaT58h+X89RBrXoNryuOPDQHWKIQQ4z3jgzw4KzvMwOmym+g?= =?us-ascii?Q?het1XG6sJNyrimaoj8e2QQ1t03qaYAdkja+mpuTfrKZsGSxgctJfah9ToWa2?= =?us-ascii?Q?hoA/hls50YRkxR2ajQ50ukK+/Rv4ZPCv65PA60M3aQkb8TZd0lKbP3NrRcvb?= =?us-ascii?Q?QJghUXyfhYGIrWwMTdV1CDJQIB/W65dgpB1AH68jSmOa6xvutokxoQb/aMAb?= =?us-ascii?Q?xz8C/KSdmIuBK9bxyJpWvN5toLDbUqu7YqSti0WFyKPE0N4CU2+PqMOO/qaO?= =?us-ascii?Q?LYjr8KBaIKAVnFkT2i2XslacL2AstNT+rSfRqL7ILaxpYpiPaHQqacSWlu1F?= =?us-ascii?Q?eAKNDyhNQS8OecxHP3iQyT4uub0NzrlYABQAD5gq/tDciTMY74hMQHJEqZov?= =?us-ascii?Q?2Ib39/kEZyOTjqkTE+3k/oQr/5+V1PyCpGHHh8k7BE2wFW/X726Cy2/qJWyk?= =?us-ascii?Q?YfIYORqhjBUYhlSe25Z1V1RMobJ6OJVwCdNLXwS9vVoFyePyP14VHQCBOUhF?= =?us-ascii?Q?9lL2mGSufFcwPGIl6ZmESxHzN+WOpAghkHXJX+Xat5WlO13e4zpZTdx0kjUJ?= =?us-ascii?Q?eE6eqoOGUOjPRmSL8JEZmcuuf/NfmxqYDfDuzTwroBAvvOu44r+AC23f4U0z?= =?us-ascii?Q?QkGKdpibFUuSHAhVptZE2RIAbuBdx99H4tNCEOSe0W3T/GRMvr+Af9AVS4Wf?= =?us-ascii?Q?XYSrc9x2kFS2YOFYk/V6WKLGhDjWkMmsS4PJNTGF72OZrbLcdBqFKI0PMymy?= =?us-ascii?Q?XodfyBNSLxUe+2K3SzJNogKF57ILVVa5mUd7HheHviI/zUVRz+h4Q/mOM5TW?= =?us-ascii?Q?XH+xQZJqgZomxpfaw3VXQfgjtwUkAcFZL3pVXOGWcx/9v7EIyhfBFNj/8UV/?= =?us-ascii?Q?rlKgvAXTclTiuRA2cRXkMVluKa1SanaSapxb6jurk8Tpn60yrAFSUznYcASE?= =?us-ascii?Q?oXh5rxqWnH+W/5Gcaz6G2NvtspsAyZAHR98xxWEuRu38j9SYSM9+6jpQl86G?= =?us-ascii?Q?QkBLh3D7DpNaHVfiOsbFQo9+Bnvrajn/tHbUQhBItBE7PIBWOuuPLNAfC4k6?= =?us-ascii?Q?ZLi+hAvCTCai7plBoX4ZqqUSPkPsVHcUFwQOHpxNzKkWw3qslKilbs18LBWG?= =?us-ascii?Q?6V8dVdLCCMJRNe6MkkqYYKKqoqtRvdyhxCsLq4V4HZmRUXssKCAthSGiTd6/?= =?us-ascii?Q?G+Sj5eCIYUIitgUoHB3NrigzrSmbHmpWNPVe31RduosdDP0quBvx?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2de4584c-3d72-419d-64c5-08da2dccc1b7 X-MS-Exchange-CrossTenant-AuthSource: MWHPR10MB1407.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 May 2022 12:51:07.5602 (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: s40qQTyUB9iY2FQb2jPFDSzrc0wIuNcj2nJlpWHmgVhoZWGdy/8Y38xJvTYlQ8BZzqEu1sEcy/nwk0lfdXNuzaPwsVgJPNrAg0r+z00bpg8= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR10MB1911 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486, 18.0.858 definitions=2022-05-04_03:2022-05-04, 2022-05-04 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 bulkscore=0 malwarescore=0 spamscore=0 mlxlogscore=999 mlxscore=0 suspectscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000 definitions=main-2205040083 X-Proofpoint-GUID: ONsyL8QAlWFkrGMqlT8H27fm14AWt3_9 X-Proofpoint-ORIG-GUID: ONsyL8QAlWFkrGMqlT8H27fm14AWt3_9 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 May 2022 12:51:19 -0000 On Tuesday, May 3, 2022 10:32:53 AM CDT Dodji Seketeli wrote: > Hello Guillermo, > > Thanks a lot for this patch! I like it. oohh, thanks!! > I do have some comments however. > Please find them below. great, > [...] > > diff --git a/include/abg-ir.h b/include/abg-ir.h > index a2f4e1a7..033e3708 100644 > --- a/include/abg-ir.h > +++ b/include/abg-ir.h > @@ -136,7 +136,16 @@ class environment > public: > struct priv; > std::unique_ptr priv_; > + /// The possible debug format types. Default is DWARF_FORMAT_TYPE > + enum debug_format_type > + { > + DWARF_FORMAT_TYPE, > +#ifdef WITH_CTF > + CTF_FORMAT_TYPE, > +#endif > + }; I added this enum to handle the options::use_ctf (--ctf) from command line, this option is passed thought abigail::ir::environment instance to know which reader needs to be instantiated: if (opts.use_ctf) env->set_debug_format_type(environment::CTF_FORMAT_TYPE); if (env->get_debug_format_type() == environment::DWARF_FORMAT_TYPE) { // dwarf reader } else if (env->get_debug_format_type() == environment::CTF_FORMAT_TYPE) { // ctf reader } By default is environment::DWARF_FORMAT_TYPE in include/abg-ir.h Otherwise I think that we need to change the arguments in `build_corpus_group_from_kernel_dist_under'. For sure I can do this :-). > I'd prefer abg-ir.h to stay agnostic from front-end considerations > such as the kind of input (DWARF, CTF etc) as much as possible. Those > considerations are handled in abg-corpus.h with the > abigail::corpus::origin enum, which is currently defined as: > > /// This abstracts where the corpus comes from. That is, either it > /// has been read from the native xml format, from DWARF or built > /// artificially using the library's API. > enum origin > { > ARTIFICIAL_ORIGIN = 0, > NATIVE_XML_ORIGIN, > DWARF_ORIGIN, > CTF_ORIGIN, > LINUX_KERNEL_BINARY_ORIGIN > }; > > You can modify it to make it be a bitmap defined as: > > enum origin > { > ARTIFICIAL_ORIGIN = 0, > NATIVE_XML_ORIGIN = 1, > DWARF_ORIGIN = 1 << 1, > CTF_ORIGIN = 1 << 2, > LINUX_KERNEL_BINARY_ORIGIN = 1 << 3 > }; > > That way, you can modify abg-{ctf-reader,dwarf-reader,ir}.cc so that > instead of saying: > > if (corp->get_origin() == corpus::LINUX_KERNEL_BINARY_ORIGIN) > // blah > > it would instead say: > > if (corp->get_origin() & corpus::LINUX_KERNEL_BINARY_ORIGIN) > // blah > > or even: > > if (corp->get_origin() > & corpus::LINUX_KERNEL_BINARY_ORIGIN > & corpus:: CTF_ORIGIN) > // blah > > The different parts of the code that test for return of > corpus::get_origin() should be updated to take into account that the > value returned is now a bitmap. > > Of course, in abg-ctf-reader.cc, the origin of the corpus would be set by doing: > > corp->set_origin(corpus::LINUX_KERNEL_BINARY_ORIGIN > | corpus::CTF_ORIGIN); > > and in abg-dwarf-reader.cc, the origin of the corpus would be set by > doing: > > ctxt.current_corpus()->set_origin(corpus::LINUX_KERNEL_BINARY_ORIGIN > | corpus::DWARF_ORIGIN); > > [...] ok, totally agree. > + debug_format_type debug_format_; It is used to know which reader should be instantiated depending of command line. > So, the abigail::ir::environment type should not have a debug_format_ > data member. If anything, all the data members are hidden in the > abigail::ir::environment::priv_, which is a pointer to > abigail::ir::environment::priv, defined in abg-ir-priv.h. But I think > it's not neccessary to add anything there. More on that below. > > [...] > > diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc > index 2c6839cb..dcc65d4e 100644 > --- a/src/abg-ctf-reader.cc > +++ b/src/abg-ctf-reader.cc > > [...] > > + void initialize(const string& elf_path, ir::environment *env) > { > > Please, add a doxygen comment to this function. ok. > [...] > > +std::string > +dic_type_key(ctf_dict_t *dic, ctf_id_t ctf_type) > +{ > > Likewise. ok. > diff --git a/src/abg-ir.cc b/src/abg-ir.cc > index 0ef5e8b2..5eebcfa3 100644 > --- a/src/abg-ir.cc > +++ b/src/abg-ir.cc > @@ -3165,7 +3165,8 @@ typedef unordered_map > /// Default constructor of the @ref environment type. > environment::environment() > - :priv_(new priv) > + :priv_(new priv), > + debug_format_(DWARF_FORMAT_TYPE) > {} > > As said above, the debug_format_ data member should not be added to > the environment type. > > [...] > > diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc > index 1f0f6fa8..faa7243f 100644 > --- a/src/abg-tools-utils.cc > +++ b/src/abg-tools-utils.cc > > [...] > > @@ -2543,12 +2576,21 @@ build_corpus_group_from_kernel_dist_under(const string& root, > t.start(); > bool got_binary_paths = > get_binary_paths_from_kernel_dist(root, debug_info_root, vmlinux, modules); > +#ifdef WITH_CTF > + string vmlinux_ctfa; > + if (got_binary_paths && > + env->get_debug_format_type() == environment::CTF_FORMAT_TYPE) > + { > + got_binary_paths = get_vmlinux_ctfa_path_from_kernel_dist(root, vmlinux_ctfa); > + ABG_ASSERT(!vmlinux_ctfa.empty()); > + } > +#endif > + > > I think, rather than calling env->get_debug_format_type() > build_corpus_group_from_kernel_dist_under can be passed an additional > 'corpus_origin' parameter of type corpus::origin. The parameter would > have corpus::DWARF_ORIGIN as default argument, for instance. great!. So, consider it done. > Also, I think the rest of this function that follows could be > encapsulated into two functions that would be called: > > maybe_load_vmlinux_dwarf_corpus() ok. > This function would load the vmlinux corpus from the ELF information. > That function would contain the code that is inside the block: > > + if (env->get_debug_format_type() == environment::DWARF_FORMAT_TYPE) > + { > > The new condition inside the function would rather be something like: > > if (corpus_origin & corpus::DWARF_ORIGIN) > { > //blah > } > The function wouldn't do anything if the origin of the corpus is not DWARF. > > > The second function would be this one: > > maybe_load_vmlinux_ctf_corpus() > > it's invocation would be protected by an #ifdef WITH_CTF and its body > would be inside an if-block that would read: > > if (corpus_origin & corpus::CTF_ORIGIN) > { > // blah nice!. > I hope this all makes sense. If not, please do not hesitate to discuss > it. All make sense, > Thanks again. Thanks for these useful comments!!. I will send soon the patch v2 :-) I was thinking to merge here the patch: https://sourceware.org/pipermail/libabigail/2022q2/004310.html[1] Dodji, What do you think? > Cheers, Kind regards, Guillermo -------- [1] https://sourceware.org/pipermail/libabigail/2022q2/004310.html