Message ID | 20221207205734.9287-3-david.faust@oracle.com |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A74CC39484B2 for <patchwork@sourceware.org>; Wed, 7 Dec 2022 20:58:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A74CC39484B2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1670446709; bh=QBCCK1RUBPydLGVbu5Q+FDNuGw0ImgCf2bVapaO/8TQ=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=HHIupq/XZjUF1k00wy2leG6Qt4dt6kH0SHyTMFjeR+xu9t7ZhzgMC63tPQznMqtZV SyU+ShRQU9/P5+sBoyBC2KdcuShyk2eP5GdBNIGsL9CzEugwhmgnYDP6JQV+pxh/Re XsipfXfH78wHBfhlhiecoRVXW3V7O6xkII4QBZhw= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0a-00069f02.pphosted.com (mx0a-00069f02.pphosted.com [205.220.165.32]) by sourceware.org (Postfix) with ESMTPS id 2DF123899421 for <gcc-patches@gcc.gnu.org>; Wed, 7 Dec 2022 20:57:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2DF123899421 Received: from pps.filterd (m0246629.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 2B7GDZdu030072 for <gcc-patches@gcc.gnu.org>; Wed, 7 Dec 2022 20:57:57 GMT Received: from iadpaimrmta03.imrmtpd1.prodappiadaev1.oraclevcn.com (iadpaimrmta03.appoci.oracle.com [130.35.103.27]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3maudk9e1v-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gcc-patches@gcc.gnu.org>; Wed, 07 Dec 2022 20:57:57 +0000 Received: from pps.filterd (iadpaimrmta03.imrmtpd1.prodappiadaev1.oraclevcn.com [127.0.0.1]) by iadpaimrmta03.imrmtpd1.prodappiadaev1.oraclevcn.com (8.17.1.5/8.17.1.5) with ESMTP id 2B7JB0Mg033806 for <gcc-patches@gcc.gnu.org>; Wed, 7 Dec 2022 20:57:55 GMT Received: from nam12-bn8-obe.outbound.protection.outlook.com (mail-bn8nam12lp2169.outbound.protection.outlook.com [104.47.55.169]) by iadpaimrmta03.imrmtpd1.prodappiadaev1.oraclevcn.com (PPS) with ESMTPS id 3maa7x8g46-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gcc-patches@gcc.gnu.org>; Wed, 07 Dec 2022 20:57:55 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZSpCG01r2jnD0+zfge9v6KbBQR+R/hULuDDqwJNLedQJUUCcua7L4YsmqQmruY+qOSFEn1Ks7Z2VHl7aHTflWoe0wRIA/DAL8qzSxFa+54S5LKm2dUuxy6bX70lb2LqGg6fwu5kWB7XDZXkzQ7xJS3D9AlsN8yPTl6QontBKWP+6hBZ1EyEccrcd9ALhyI6IlhB2GLmqD9soVbscWP/LjOOLaoeSFR2tR1kEqLTvx55yhwR6ZMACWxKusuG+O9XANaFgE+Hgk/yw3vH2fwBXWb5FrUX1nycXsLY9v6kBA0Rs+Ppamn0XZit3pecYPkh+Q1XfAQiV2EyKJjH6ZcWstg== 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=QBCCK1RUBPydLGVbu5Q+FDNuGw0ImgCf2bVapaO/8TQ=; b=TRMjRtBwdjKhtkL0RZQP9+Jq9G57TvDZPQJWfs4lXj1FX38MNI1OW5PtamFSR1YTTwovhIHPYqKOOc5sTHc5JcW6S77ZRKrxgfnxXyw9BUmV3xytogFvxl52YgwpZ/sJINLkQSc+qi6s3LeMQ+QEIeGBtM5lj841dseigToqp4vCvfq6ZZlSyyqopBf8LloYpTIvmcenJXDxR3jRDWVPu9o8VDewfWMF8JjUKomBaO9kyW+7ir8nCuIf8ma39d4GvakMZmWX0Fiqrciw4Zh2yfmLEgB19ZG9wxGjPLbY7Uy58B5DrdSsG1ukXPe1csnjtRKW35qaXYv/jKprVrN2Hg== 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 MN2PR10MB3213.namprd10.prod.outlook.com (2603:10b6:208:131::33) by SA1PR10MB6493.namprd10.prod.outlook.com (2603:10b6:806:29c::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.14; Wed, 7 Dec 2022 20:57:54 +0000 Received: from MN2PR10MB3213.namprd10.prod.outlook.com ([fe80::dd41:a422:5763:8848]) by MN2PR10MB3213.namprd10.prod.outlook.com ([fe80::dd41:a422:5763:8848%7]) with mapi id 15.20.5880.014; Wed, 7 Dec 2022 20:57:54 +0000 To: gcc-patches@gcc.gnu.org Cc: indu.bhagat@oracle.com, jose.marchesi@oracle.com Subject: [PATCH 2/3] btf: fix 'extern const void' variables [PR106773] Date: Wed, 7 Dec 2022 12:57:33 -0800 Message-Id: <20221207205734.9287-3-david.faust@oracle.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221207205734.9287-1-david.faust@oracle.com> References: <20221207205734.9287-1-david.faust@oracle.com> Content-Transfer-Encoding: 8bit Content-Type: text/plain X-ClientProxiedBy: BY3PR05CA0031.namprd05.prod.outlook.com (2603:10b6:a03:39b::6) To MN2PR10MB3213.namprd10.prod.outlook.com (2603:10b6:208:131::33) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MN2PR10MB3213:EE_|SA1PR10MB6493:EE_ X-MS-Office365-Filtering-Correlation-Id: f201d984-9707-4db8-4089-08dad895b5ef X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 9qe0Yngpww7QsE4+Y0EpOoP9qByGHhZe4DwujQ7Mbqw9pTuSigQOZwqaTMBlfY1L0GOY7ZXxURsBJeV9speR9R2tXld8BmMdaAfIpdp01XYrQgkcIhahGcBI5Zr7WKyGmC0W5Ym3mV3vrkss/pBpacTFXThXkfJWlNYkklGhNqaWS0cNK1a60pTm9SY1iq9HqQSuLz2Yj5LNKpNn1IeVN9FOIjwMU4qodiJVzu35EhjVAM4+jcRfxUbGahDwkLeeUHjBUQmUW6r+xRb5aB4qXzoUfri/Ntdbubq41L/iNamWJdjP81hbQG+89t1qNIj9lrHNjdNTxhqS+8+tCXVIFy6tVaRRvAOeQqRX3h3wtXZltQz0Ln9apvvkG70dAdBxuEc4n+ooA7wO2riSLZ5dibuqLh/vtLltgxImpsBJoJeU2KuYMKjDmNsjMiGtLdSzsDtlq0+nKwTtKLUBhiTsPuCb1B12I28F7Wl+duTcNIGYw+eP6XIXKVVomyCDc/oTMwMhviQ4CYClXvFcanBKi8A7vlrgGxY2w8rbblYZopY4wAiM/5vxOVir5ZlVF1VOxOPyn4F4rx5av07n82inzgEQVwkvqKoooB3SEsh8FORoJmsNzl8/taZCoHfJdSwRpobtgsdJ9lM84YkQrJOqlZJLmd6lDhGU1hs5/44zWbQ= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR10MB3213.namprd10.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(136003)(39860400002)(346002)(396003)(366004)(376002)(451199015)(4326008)(316002)(6916009)(8676002)(66556008)(66946007)(66476007)(6512007)(478600001)(38100700002)(36756003)(6486002)(107886003)(6666004)(6506007)(186003)(26005)(1076003)(2616005)(41300700001)(86362001)(5660300002)(8936002)(44832011)(2906002)(83380400001)(84970400001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: F8plJBy/Ou+DJhGMLfQh/w1UDZk2ZBY9D/vflg7XUnlBYfjTvdeKDNItzHjSXkyjA2XsBZtr466fcnl0TSaKfD32WCXlp/DSCMFh4oZZAcjEnifeOEJtna9S5kzHUXhOZ9kIh3qSKsDHMDdz+En+ndf2xiW07t4V44LYXTrcMQJgo3qAMBCgSprWqUONA9LBLBZCEaeZD93aep1U0ENmR+s97A5f8/Q9OE6tidydCtNytX0Q6xdnMqV5T4pMffCRSrSMstEYT6QA5BQhNdD5Sxch7s2qqWVKRsuPwXV84Fsle40iS4HRJ0oXqvlpUI5zCwDHP08QxypGWQ38HPRn3B2zb+QeRISYPMySjo1ppM4rMZMeVOGj6TNqNsKnKnIMjWmAfBLO/QxavYSz2OHdvAp3pn6wXJ+XIv6GSTbZC15nWb49xAOLwF9WSrlqlgTTqnglBmb4GT7jGDUeHEcscKoV1Ji2qzzfVOwR0osVjal7dM7gC1mRQmTvJ87uXSGx8lT9qXwz2EmNyGFPGHulqbxmTRTr8hhxy/NV9CSK2DWPcSZ6x8gEeZzfKreutPITz2uo9xNy1e/TrV6duZDIV4jN3uSs87yZZIxLJgErhbVxwL+8WPq7T/X5NRgfutiZef1FroB3iqp4RJ+WWEdlcFdVXDicwUoItEQDnavRWO2vUfb8EColC97msuBgOQwgcUf8xk+8Nd92wBly//4yUfpHdkmJsLtpN7rPVSQujNETYMg6jo3VD+oq/uDALZ//rZEG+AmlRkn+uPOO+N5qBKlIfWe0f1JeP3w92mTTWp4+LINIhmMxcf//klChctv2B0eaom6nnZcKOyLsNSQKr8jkzuKI7swhfD1PTMFFl/KtQKFqG1sXg/T1EhChIlEhTTBzrOK6GJYX6pjeCuKGdRHzH14p+m3//NKJXdI+ppVcfrQXqJeuqXxQtLQDRBN0Ej6BwBHQNWV3kkNzIfKe8BsEHQqIgijjzPhETFgDCew58rWw5Cnb6JpKsadSkjyrURI6RFgeVT/2m8ZKbBVO8LfMqVjM3I5zuVH9WdHODpQgl5X3G6xHKaKxizzeav41nhY0ExAxVwv0p3IEPhcf/nxnVhjcaGpGWHztVc6GZg5PqI1m0YwjLXcWyE+buKK8H8u0bMYpVra8hHaeTanuVZ7z8DsmRzOfiIxdPHYgRISR3+Ly+SExOYiqOv2RcA9fPJ1Eb57D8SGc2YkOgbYXV0DZzOIWrM0R82G/t9/eVRSGBYAEU3gKv+lvpkz2ph89uGRK0CCgRDaemOlkV0ie7Na/lHWEsAJV9KhKzWlog/F5GYHyLvElT1OKuYdPXSyQ1yoDiWGUw8zN5znuiJ83KS7raUmeP6JO+C2QL3OKpSlA2B8XkwIkMC4P8ihDDdKFEMeLf3jF5MDQRKj5QGgDoEoO2NaKVkgsmTb54im/4BpcCl7F3JKtH9d5rKa46tBo0kNqalJAQ6H0k49sMWnECoWpUFns7CPiVs0fuNjai5UJXAS44BrZVRX4Yvq2nwf8AW0HSYxO+vEW9t65HbGVJWLCJbDek5arW76i9yPR3E/tfXfhmC83G6kt8oPPNK1j X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: f201d984-9707-4db8-4089-08dad895b5ef X-MS-Exchange-CrossTenant-AuthSource: MN2PR10MB3213.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Dec 2022 20:57:54.2147 (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: PgakTyk1fmv1nICSn+qyzrrvGC6LR+vC7hbu4CyOs1qEyHNXWil3iNb7Z2Vs7yvNQWxqZNzt6U0btTvv+oa/JA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR10MB6493 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-12-07_09,2022-12-07_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 spamscore=0 adultscore=0 suspectscore=0 mlxlogscore=999 malwarescore=0 phishscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000 definitions=main-2212070176 X-Proofpoint-GUID: gJYtmMoVZ51LkfV-QFh7-IoP0pZulwoG X-Proofpoint-ORIG-GUID: gJYtmMoVZ51LkfV-QFh7-IoP0pZulwoG X-Spam-Status: No, score=-13.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: David Faust via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: David Faust <david.faust@oracle.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
btf: fix BTF for extern items [PR106773]
|
|
Commit Message
David Faust
Dec. 7, 2022, 8:57 p.m. UTC
The eBPF loader expects to find BTF_KIND_VAR records for references to extern const void symbols. We were mistakenly identifing these as unsupported types, and as a result skipping emitting VAR records for them. In addition, the internal DWARF representation from which BTF is produced does not generate 'const' modifier DIEs for the void type, which meant in BTF the 'const' qualifier was dropped for 'extern const void' variables. This patch also adds support for generating a const void type in BTF to correct emission for these variables. PR target/106773 gcc/ * btfout.cc (btf_collect_datasec): Correct size of void entries. (btf_dvd_emit_preprocess_cb): Do not skip emitting variables which refer to void types. (btf_init_postprocess): Create 'const void' type record if needed and adjust variables to refer to it as appropriate. gcc/testsuite/ * gcc.dg/debug/btf/btf-pr106773.c: New test. --- gcc/btfout.cc | 44 +++++++++++++++++-- gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c | 25 +++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c
Comments
Looks OK to me overall. Minor comments below. Thanks On 12/7/22 12:57, David Faust wrote: > The eBPF loader expects to find BTF_KIND_VAR records for references to > extern const void symbols. We were mistakenly identifing these as > unsupported types, and as a result skipping emitting VAR records for > them. > > In addition, the internal DWARF representation from which BTF is > produced does not generate 'const' modifier DIEs for the void type, > which meant in BTF the 'const' qualifier was dropped for 'extern const > void' variables. This patch also adds support for generating a const > void type in BTF to correct emission for these variables. > > PR target/106773 > > gcc/ > > * btfout.cc (btf_collect_datasec): Correct size of void entries. > (btf_dvd_emit_preprocess_cb): Do not skip emitting variables which > refer to void types. > (btf_init_postprocess): Create 'const void' type record if needed and > adjust variables to refer to it as appropriate. > > gcc/testsuite/ > > * gcc.dg/debug/btf/btf-pr106773.c: New test. > --- > gcc/btfout.cc | 44 +++++++++++++++++-- > gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c | 25 +++++++++++ > 2 files changed, 65 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c > > diff --git a/gcc/btfout.cc b/gcc/btfout.cc > index a1c6266a7db..05f3a3f9b6e 100644 > --- a/gcc/btfout.cc > +++ b/gcc/btfout.cc > @@ -354,6 +354,8 @@ btf_collect_datasec (ctf_container_ref ctfc) > tree size = DECL_SIZE_UNIT (node->decl); > if (tree_fits_uhwi_p (size)) > info.size = tree_to_uhwi (size); > + else if (VOID_TYPE_P (TREE_TYPE (node->decl))) > + info.size = 1; > > /* Offset is left as 0 at compile time, to be filled in by loaders such > as libbpf. */ > @@ -439,7 +441,7 @@ btf_dvd_emit_preprocess_cb (ctf_dvdef_ref *slot, ctf_container_ref arg_ctfc) > ctf_dvdef_ref var = (ctf_dvdef_ref) * slot; > > /* Do not add variables which refer to unsupported types. */ > - if (btf_removed_type_p (var->dvd_type)) > + if (!voids.contains (var->dvd_type) && btf_removed_type_p (var->dvd_type)) > return 1; > > arg_ctfc->ctfc_vars_list[num_vars_added] = var; > @@ -1073,15 +1075,49 @@ btf_init_postprocess (void) > { > ctf_container_ref tu_ctfc = ctf_get_tu_ctfc (); > > - size_t i; > - size_t num_ctf_types = tu_ctfc->ctfc_types->elements (); > - > holes.create (0); > voids.create (0); > > num_types_added = 0; > num_types_created = 0; > > + /* Workaround for 'const void' variables. These variables are sometimes used > + in eBPF programs to address kernel symbols. DWARF does not generate const > + qualifier on void type, so we would incorrectly emit these variables > + without the const qualifier. > + Unfortunately we need the TREE node to know it was const, and we need > + to create the const modifier type (if needed) now, before making the types > + list. So we can't avoid iterating with FOR_EACH_VARIABLE here, and then > + again when creating the DATASEC entries. */ "Dot, space, space, new sentence." in 3 places. > + ctf_id_t constvoid_id = CTF_NULL_TYPEID; > + varpool_node *var; > + FOR_EACH_VARIABLE (var) > + { > + if (!var->decl) > + continue; > + > + tree type = TREE_TYPE (var->decl); > + if (type && VOID_TYPE_P (type) && TYPE_READONLY (type)) > + { > + dw_die_ref die = lookup_decl_die (var->decl); > + if (die == NULL) > + continue; > + > + ctf_dvdef_ref dvd = ctf_dvd_lookup (tu_ctfc, die); > + if (dvd == NULL) > + continue; > + > + /* Create the 'const' modifier type for void. */ > + if (constvoid_id == CTF_NULL_TYPEID) > + constvoid_id = ctf_add_reftype (tu_ctfc, CTF_ADD_ROOT, > + dvd->dvd_type, CTF_K_CONST, NULL); No de-duplication of the const void type. I assume libbpf will take care of this eventually. > + dvd->dvd_type = constvoid_id; > + } > + } > + > + size_t i; > + size_t num_ctf_types = tu_ctfc->ctfc_types->elements (); > + > if (num_ctf_types) > { > init_btf_id_map (num_ctf_types + 1); > diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c > new file mode 100644 > index 00000000000..f90fa773a4b > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c > @@ -0,0 +1,25 @@ > +/* Test BTF generation for extern const void symbols. > + BTF_KIND_VAR records should be emitted for such symbols if they are used, > + as well as a corresponding entry in the appropriate DATASEC record. */ > + > +/* { dg-do compile } */ > +/* { dg-options "-O0 -gbtf -dA" } */ > + > +/* Expect 1 variable record only for foo, with 'extern' (2) linkage. */ > +/* { dg-final { scan-assembler-times "\[\t \]0xe000000\[\t \]+\[^\n\]*btv_info" 1 } } */ > +/* { dg-final { scan-assembler-times "\[\t \]0x2\[\t \]+\[^\n\]*btv_linkage" 1 } } */ > + > +/* { dg-final { scan-assembler-times "ascii \"foo.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */ > + > +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 1 } } */ > +/* { dg-final { scan-assembler-times "1\[\t \]+\[^\n\]*bts_size" 1 } } */ > + > +extern const void foo __attribute__((weak)) __attribute__((section (".ksyms"))); > +extern const void bar __attribute__((weak)) __attribute__((section (".ksyms"))); > + > +unsigned long func () { > + unsigned long x = (unsigned long) &foo; > + > + return x; > +} > +
On 12/8/22 23:34, Indu Bhagat wrote: > Looks OK to me overall. Minor comments below. > > Thanks > > On 12/7/22 12:57, David Faust wrote: >> The eBPF loader expects to find BTF_KIND_VAR records for references to >> extern const void symbols. We were mistakenly identifing these as >> unsupported types, and as a result skipping emitting VAR records for >> them. >> >> In addition, the internal DWARF representation from which BTF is >> produced does not generate 'const' modifier DIEs for the void type, >> which meant in BTF the 'const' qualifier was dropped for 'extern const >> void' variables. This patch also adds support for generating a const >> void type in BTF to correct emission for these variables. >> >> PR target/106773 >> >> gcc/ >> >> * btfout.cc (btf_collect_datasec): Correct size of void entries. >> (btf_dvd_emit_preprocess_cb): Do not skip emitting variables which >> refer to void types. >> (btf_init_postprocess): Create 'const void' type record if needed and >> adjust variables to refer to it as appropriate. >> >> gcc/testsuite/ >> >> * gcc.dg/debug/btf/btf-pr106773.c: New test. >> --- >> gcc/btfout.cc | 44 +++++++++++++++++-- >> gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c | 25 +++++++++++ >> 2 files changed, 65 insertions(+), 4 deletions(-) >> create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c >> >> diff --git a/gcc/btfout.cc b/gcc/btfout.cc >> index a1c6266a7db..05f3a3f9b6e 100644 >> --- a/gcc/btfout.cc >> +++ b/gcc/btfout.cc >> @@ -354,6 +354,8 @@ btf_collect_datasec (ctf_container_ref ctfc) >> tree size = DECL_SIZE_UNIT (node->decl); >> if (tree_fits_uhwi_p (size)) >> info.size = tree_to_uhwi (size); >> + else if (VOID_TYPE_P (TREE_TYPE (node->decl))) >> + info.size = 1; >> >> /* Offset is left as 0 at compile time, to be filled in by loaders such >> as libbpf. */ >> @@ -439,7 +441,7 @@ btf_dvd_emit_preprocess_cb (ctf_dvdef_ref *slot, ctf_container_ref arg_ctfc) >> ctf_dvdef_ref var = (ctf_dvdef_ref) * slot; >> >> /* Do not add variables which refer to unsupported types. */ >> - if (btf_removed_type_p (var->dvd_type)) >> + if (!voids.contains (var->dvd_type) && btf_removed_type_p (var->dvd_type)) >> return 1; >> >> arg_ctfc->ctfc_vars_list[num_vars_added] = var; >> @@ -1073,15 +1075,49 @@ btf_init_postprocess (void) >> { >> ctf_container_ref tu_ctfc = ctf_get_tu_ctfc (); >> >> - size_t i; >> - size_t num_ctf_types = tu_ctfc->ctfc_types->elements (); >> - >> holes.create (0); >> voids.create (0); >> >> num_types_added = 0; >> num_types_created = 0; >> >> + /* Workaround for 'const void' variables. These variables are sometimes used >> + in eBPF programs to address kernel symbols. DWARF does not generate const >> + qualifier on void type, so we would incorrectly emit these variables >> + without the const qualifier. >> + Unfortunately we need the TREE node to know it was const, and we need >> + to create the const modifier type (if needed) now, before making the types >> + list. So we can't avoid iterating with FOR_EACH_VARIABLE here, and then >> + again when creating the DATASEC entries. */ > > "Dot, space, space, new sentence." in 3 places. > > >> + ctf_id_t constvoid_id = CTF_NULL_TYPEID; >> + varpool_node *var; >> + FOR_EACH_VARIABLE (var) >> + { >> + if (!var->decl) >> + continue; >> + >> + tree type = TREE_TYPE (var->decl); >> + if (type && VOID_TYPE_P (type) && TYPE_READONLY (type)) >> + { >> + dw_die_ref die = lookup_decl_die (var->decl); >> + if (die == NULL) >> + continue; >> + >> + ctf_dvdef_ref dvd = ctf_dvd_lookup (tu_ctfc, die); >> + if (dvd == NULL) >> + continue; >> + >> + /* Create the 'const' modifier type for void. */ >> + if (constvoid_id == CTF_NULL_TYPEID) >> + constvoid_id = ctf_add_reftype (tu_ctfc, CTF_ADD_ROOT, >> + dvd->dvd_type, CTF_K_CONST, NULL); > > No de-duplication of the const void type. I assume libbpf will take > care of this eventually. Hm, not sure I follow. Where is the duplication? The const void type is only created once here, for the first such variable which needs it, and reused for subsequent variables. And it does not already exist in the CTF which we are translating into BTF. In any case, yes libbpf can handle duplicated types. Though it would still be good to minimize that where we can to not bloat the BTF info. > >> + dvd->dvd_type = constvoid_id; >> + } >> + } >> + >> + size_t i; >> + size_t num_ctf_types = tu_ctfc->ctfc_types->elements (); >> + >> if (num_ctf_types) >> { >> init_btf_id_map (num_ctf_types + 1); >> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c >> new file mode 100644 >> index 00000000000..f90fa773a4b >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c >> @@ -0,0 +1,25 @@ >> +/* Test BTF generation for extern const void symbols. >> + BTF_KIND_VAR records should be emitted for such symbols if they are used, >> + as well as a corresponding entry in the appropriate DATASEC record. */ >> + >> +/* { dg-do compile } */ >> +/* { dg-options "-O0 -gbtf -dA" } */ >> + >> +/* Expect 1 variable record only for foo, with 'extern' (2) linkage. */ >> +/* { dg-final { scan-assembler-times "\[\t \]0xe000000\[\t \]+\[^\n\]*btv_info" 1 } } */ >> +/* { dg-final { scan-assembler-times "\[\t \]0x2\[\t \]+\[^\n\]*btv_linkage" 1 } } */ >> + >> +/* { dg-final { scan-assembler-times "ascii \"foo.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */ >> + >> +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 1 } } */ >> +/* { dg-final { scan-assembler-times "1\[\t \]+\[^\n\]*bts_size" 1 } } */ >> + >> +extern const void foo __attribute__((weak)) __attribute__((section (".ksyms"))); >> +extern const void bar __attribute__((weak)) __attribute__((section (".ksyms"))); >> + >> +unsigned long func () { >> + unsigned long x = (unsigned long) &foo; >> + >> + return x; >> +} >> + >
On 12/12/22 12:59, David Faust wrote: > > > On 12/8/22 23:34, Indu Bhagat wrote: >> Looks OK to me overall. Minor comments below. >> >> Thanks >> >> On 12/7/22 12:57, David Faust wrote: >>> The eBPF loader expects to find BTF_KIND_VAR records for references to >>> extern const void symbols. We were mistakenly identifing these as >>> unsupported types, and as a result skipping emitting VAR records for >>> them. >>> >>> In addition, the internal DWARF representation from which BTF is >>> produced does not generate 'const' modifier DIEs for the void type, >>> which meant in BTF the 'const' qualifier was dropped for 'extern const >>> void' variables. This patch also adds support for generating a const >>> void type in BTF to correct emission for these variables. >>> >>> PR target/106773 >>> >>> gcc/ >>> >>> * btfout.cc (btf_collect_datasec): Correct size of void entries. >>> (btf_dvd_emit_preprocess_cb): Do not skip emitting variables which >>> refer to void types. >>> (btf_init_postprocess): Create 'const void' type record if needed and >>> adjust variables to refer to it as appropriate. >>> >>> gcc/testsuite/ >>> >>> * gcc.dg/debug/btf/btf-pr106773.c: New test. >>> --- >>> gcc/btfout.cc | 44 +++++++++++++++++-- >>> gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c | 25 +++++++++++ >>> 2 files changed, 65 insertions(+), 4 deletions(-) >>> create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c >>> >>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc >>> index a1c6266a7db..05f3a3f9b6e 100644 >>> --- a/gcc/btfout.cc >>> +++ b/gcc/btfout.cc >>> @@ -354,6 +354,8 @@ btf_collect_datasec (ctf_container_ref ctfc) >>> tree size = DECL_SIZE_UNIT (node->decl); >>> if (tree_fits_uhwi_p (size)) >>> info.size = tree_to_uhwi (size); >>> + else if (VOID_TYPE_P (TREE_TYPE (node->decl))) >>> + info.size = 1; >>> >>> /* Offset is left as 0 at compile time, to be filled in by loaders such >>> as libbpf. */ >>> @@ -439,7 +441,7 @@ btf_dvd_emit_preprocess_cb (ctf_dvdef_ref *slot, ctf_container_ref arg_ctfc) >>> ctf_dvdef_ref var = (ctf_dvdef_ref) * slot; >>> >>> /* Do not add variables which refer to unsupported types. */ >>> - if (btf_removed_type_p (var->dvd_type)) >>> + if (!voids.contains (var->dvd_type) && btf_removed_type_p (var->dvd_type)) >>> return 1; >>> >>> arg_ctfc->ctfc_vars_list[num_vars_added] = var; >>> @@ -1073,15 +1075,49 @@ btf_init_postprocess (void) >>> { >>> ctf_container_ref tu_ctfc = ctf_get_tu_ctfc (); >>> >>> - size_t i; >>> - size_t num_ctf_types = tu_ctfc->ctfc_types->elements (); >>> - >>> holes.create (0); >>> voids.create (0); >>> >>> num_types_added = 0; >>> num_types_created = 0; >>> >>> + /* Workaround for 'const void' variables. These variables are sometimes used >>> + in eBPF programs to address kernel symbols. DWARF does not generate const >>> + qualifier on void type, so we would incorrectly emit these variables >>> + without the const qualifier. >>> + Unfortunately we need the TREE node to know it was const, and we need >>> + to create the const modifier type (if needed) now, before making the types >>> + list. So we can't avoid iterating with FOR_EACH_VARIABLE here, and then >>> + again when creating the DATASEC entries. */ >> >> "Dot, space, space, new sentence." in 3 places. >> >> >>> + ctf_id_t constvoid_id = CTF_NULL_TYPEID; >>> + varpool_node *var; >>> + FOR_EACH_VARIABLE (var) >>> + { >>> + if (!var->decl) >>> + continue; >>> + >>> + tree type = TREE_TYPE (var->decl); >>> + if (type && VOID_TYPE_P (type) && TYPE_READONLY (type)) >>> + { >>> + dw_die_ref die = lookup_decl_die (var->decl); >>> + if (die == NULL) >>> + continue; >>> + >>> + ctf_dvdef_ref dvd = ctf_dvd_lookup (tu_ctfc, die); >>> + if (dvd == NULL) >>> + continue; >>> + >>> + /* Create the 'const' modifier type for void. */ >>> + if (constvoid_id == CTF_NULL_TYPEID) >>> + constvoid_id = ctf_add_reftype (tu_ctfc, CTF_ADD_ROOT, >>> + dvd->dvd_type, CTF_K_CONST, NULL); >> >> No de-duplication of the const void type. I assume libbpf will take >> care of this eventually. > > Hm, not sure I follow. Where is the duplication? The const void type is > only created once here, for the first such variable which needs it, and > reused for subsequent variables. And it does not already exist in the > CTF which we are translating into BTF. > You're right - you are reusing the const void type once generated for a CU for each usage. My bad - I didnt follow the code properly :) > In any case, yes libbpf can handle duplicated types. Though it would > still be good to minimize that where we can to not bloat the BTF info. > >> >>> + dvd->dvd_type = constvoid_id; >>> + } >>> + } >>> + >>> + size_t i; >>> + size_t num_ctf_types = tu_ctfc->ctfc_types->elements (); >>> + >>> if (num_ctf_types) >>> { >>> init_btf_id_map (num_ctf_types + 1); >>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c >>> new file mode 100644 >>> index 00000000000..f90fa773a4b >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c >>> @@ -0,0 +1,25 @@ >>> +/* Test BTF generation for extern const void symbols. >>> + BTF_KIND_VAR records should be emitted for such symbols if they are used, >>> + as well as a corresponding entry in the appropriate DATASEC record. */ >>> + >>> +/* { dg-do compile } */ >>> +/* { dg-options "-O0 -gbtf -dA" } */ >>> + >>> +/* Expect 1 variable record only for foo, with 'extern' (2) linkage. */ >>> +/* { dg-final { scan-assembler-times "\[\t \]0xe000000\[\t \]+\[^\n\]*btv_info" 1 } } */ >>> +/* { dg-final { scan-assembler-times "\[\t \]0x2\[\t \]+\[^\n\]*btv_linkage" 1 } } */ >>> + >>> +/* { dg-final { scan-assembler-times "ascii \"foo.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */ >>> + >>> +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 1 } } */ >>> +/* { dg-final { scan-assembler-times "1\[\t \]+\[^\n\]*bts_size" 1 } } */ >>> + >>> +extern const void foo __attribute__((weak)) __attribute__((section (".ksyms"))); >>> +extern const void bar __attribute__((weak)) __attribute__((section (".ksyms"))); >>> + >>> +unsigned long func () { >>> + unsigned long x = (unsigned long) &foo; >>> + >>> + return x; >>> +} >>> + >>
diff --git a/gcc/btfout.cc b/gcc/btfout.cc index a1c6266a7db..05f3a3f9b6e 100644 --- a/gcc/btfout.cc +++ b/gcc/btfout.cc @@ -354,6 +354,8 @@ btf_collect_datasec (ctf_container_ref ctfc) tree size = DECL_SIZE_UNIT (node->decl); if (tree_fits_uhwi_p (size)) info.size = tree_to_uhwi (size); + else if (VOID_TYPE_P (TREE_TYPE (node->decl))) + info.size = 1; /* Offset is left as 0 at compile time, to be filled in by loaders such as libbpf. */ @@ -439,7 +441,7 @@ btf_dvd_emit_preprocess_cb (ctf_dvdef_ref *slot, ctf_container_ref arg_ctfc) ctf_dvdef_ref var = (ctf_dvdef_ref) * slot; /* Do not add variables which refer to unsupported types. */ - if (btf_removed_type_p (var->dvd_type)) + if (!voids.contains (var->dvd_type) && btf_removed_type_p (var->dvd_type)) return 1; arg_ctfc->ctfc_vars_list[num_vars_added] = var; @@ -1073,15 +1075,49 @@ btf_init_postprocess (void) { ctf_container_ref tu_ctfc = ctf_get_tu_ctfc (); - size_t i; - size_t num_ctf_types = tu_ctfc->ctfc_types->elements (); - holes.create (0); voids.create (0); num_types_added = 0; num_types_created = 0; + /* Workaround for 'const void' variables. These variables are sometimes used + in eBPF programs to address kernel symbols. DWARF does not generate const + qualifier on void type, so we would incorrectly emit these variables + without the const qualifier. + Unfortunately we need the TREE node to know it was const, and we need + to create the const modifier type (if needed) now, before making the types + list. So we can't avoid iterating with FOR_EACH_VARIABLE here, and then + again when creating the DATASEC entries. */ + ctf_id_t constvoid_id = CTF_NULL_TYPEID; + varpool_node *var; + FOR_EACH_VARIABLE (var) + { + if (!var->decl) + continue; + + tree type = TREE_TYPE (var->decl); + if (type && VOID_TYPE_P (type) && TYPE_READONLY (type)) + { + dw_die_ref die = lookup_decl_die (var->decl); + if (die == NULL) + continue; + + ctf_dvdef_ref dvd = ctf_dvd_lookup (tu_ctfc, die); + if (dvd == NULL) + continue; + + /* Create the 'const' modifier type for void. */ + if (constvoid_id == CTF_NULL_TYPEID) + constvoid_id = ctf_add_reftype (tu_ctfc, CTF_ADD_ROOT, + dvd->dvd_type, CTF_K_CONST, NULL); + dvd->dvd_type = constvoid_id; + } + } + + size_t i; + size_t num_ctf_types = tu_ctfc->ctfc_types->elements (); + if (num_ctf_types) { init_btf_id_map (num_ctf_types + 1); diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c new file mode 100644 index 00000000000..f90fa773a4b --- /dev/null +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-pr106773.c @@ -0,0 +1,25 @@ +/* Test BTF generation for extern const void symbols. + BTF_KIND_VAR records should be emitted for such symbols if they are used, + as well as a corresponding entry in the appropriate DATASEC record. */ + +/* { dg-do compile } */ +/* { dg-options "-O0 -gbtf -dA" } */ + +/* Expect 1 variable record only for foo, with 'extern' (2) linkage. */ +/* { dg-final { scan-assembler-times "\[\t \]0xe000000\[\t \]+\[^\n\]*btv_info" 1 } } */ +/* { dg-final { scan-assembler-times "\[\t \]0x2\[\t \]+\[^\n\]*btv_linkage" 1 } } */ + +/* { dg-final { scan-assembler-times "ascii \"foo.0\"\[\t \]+\[^\n\]*btf_string" 1 } } */ + +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 1 } } */ +/* { dg-final { scan-assembler-times "1\[\t \]+\[^\n\]*bts_size" 1 } } */ + +extern const void foo __attribute__((weak)) __attribute__((section (".ksyms"))); +extern const void bar __attribute__((weak)) __attribute__((section (".ksyms"))); + +unsigned long func () { + unsigned long x = (unsigned long) &foo; + + return x; +} +