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 A804A3858431 for ; Tue, 25 Oct 2022 19:07:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A804A3858431 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 (m0246627.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 29PI4J6v013607 for ; Tue, 25 Oct 2022 19:07:28 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=from : to : cc : subject : references : date : in-reply-to : message-id : content-type : mime-version; s=corp-2022-7-12; bh=eD4iuRkM/C4NxiVwuNQWkJK+jMeHKQpFAPYaQ7MH9cA=; b=sVLFYgH5WazAQqKf1K4XS7D6wCF6I7ucPbQ970Dicw9fWqbNuozT/N//qBJ+GyVKQ++W pUFMx+Z2JEyFFGcTtrOVfCHOrw/MxTnCdFXxQt6wgCvapfPDnHZLeYPBt9gXtL7/7BYF 2yTZkpF5Z4nhVDAPsZLy9cVgd7W6qH5t16L1DbyZyEMC43Ahgeh2BoYec38tt7CfVobG fr4xUBdeLaloTWtJK1SgM0ZMdyAc+4exd7OAreLq+Bcu/Yryn7D2zBRzZYy/Hc5unNCZ f4eZLIJjTfb9BVFWkliO1N7z/v1Yh2vP8Q9Bnb17Wz3mJQSh2veA+V2R9cf2741+tYsm 6w== Received: from phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com (phxpaimrmta02.appoci.oracle.com [147.154.114.232]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3kc741w0bc-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 25 Oct 2022 19:07:28 +0000 Received: from pps.filterd (phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1]) by phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com (8.17.1.5/8.17.1.5) with ESMTP id 29PGculC012724 for ; Tue, 25 Oct 2022 19:07:27 GMT Received: from nam11-dm6-obe.outbound.protection.outlook.com (mail-dm6nam11lp2172.outbound.protection.outlook.com [104.47.57.172]) by phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com (PPS) with ESMTPS id 3kc6y4xdrv-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 25 Oct 2022 19:07:27 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Q6leWh9JMoo7MHSEuRuLjZlpFE562X4fDfZNFVthqZTOplkEAVhoP6HEkoFPhJ8XLIsRtiefn59la67sSObA4/cBhc8Fl6DEvblIfV1xaCnFx7LfYuXWRvxs0TOYvQs+13PUsLXwnl13goL7ZONKsn0cbLwTAyWc3l/HGuAIVlS21nghYo/HYVK2jpjIiEBVFgUTOS7CwZZVxymknlR2nZVZtD9ZE2XXg7oUbeN9AsatJxtvaqQ68mu+vwylMouG91uCuwaPWbk5alCywYAxbZDLYrhWjiKCDmAIxZcHxUWbG2mSIuXdO7DCDKSMVDjg+WIDBfhXiVAQkOfgNGpfhg== 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=eD4iuRkM/C4NxiVwuNQWkJK+jMeHKQpFAPYaQ7MH9cA=; b=GmyTvQAgIUxBkcGHqqp2qEcuZEy0L5tejNnA16u/sLrremKWOlJdfOpqVWWULiwSNYgejVHb6LK9gXOYXNpLYHXTAGmVAtwXTIF+HH/MMBgucVprBMEQ1QtKFGHXn6Dr28GZLjaNfRHG/1QcM4Lm/8byDydYRrqSU7gboQbzkvjd3azJaqULmNJxpUxBhFJhL0KMykHwCu57c+YhU2NTgY1L8RO6jRt7GMekT7fJt4Y3LzwdY/IZ5HhIU74N1oDgEAUDaOY5yrlvajn7p5vAe4pVPdiXXnvrUrwf2mpgSxDq0I4DZ/3NAM+F10lMf2+J6oRUBMqM3IJz3M6otWrexQ== 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=eD4iuRkM/C4NxiVwuNQWkJK+jMeHKQpFAPYaQ7MH9cA=; b=LFGGt+6hOFuk7WfFfsPD4qtrtnJPNGjqcu9nQw25iZ+AkIwdlz0mW7P4M4jmEql7YoPhOrvyMVZrDXKIi0QVp22aeZR3PeCait42wxxUkCCFgpezvG0BbY95FBjWbmLzpZ316dgy8QRe6/HpNVq4wNWz6BdOaw+cAyj6rDc8rWA= Received: from BYAPR10MB2888.namprd10.prod.outlook.com (2603:10b6:a03:88::32) by SJ2PR10MB7015.namprd10.prod.outlook.com (2603:10b6:a03:4c1::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5746.21; Tue, 25 Oct 2022 19:07:26 +0000 Received: from BYAPR10MB2888.namprd10.prod.outlook.com ([fe80::5095:b148:8def:1049]) by BYAPR10MB2888.namprd10.prod.outlook.com ([fe80::5095:b148:8def:1049%5]) with mapi id 15.20.5723.034; Tue, 25 Oct 2022 19:07:25 +0000 From: "Jose E. Marchesi" To: David Faust Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH v2] bpf: add preserve_field_info builtin References: <8735bifz4u.fsf@oracle.com> <20221025172443.6732-1-david.faust@oracle.com> Date: Tue, 25 Oct 2022 21:11:27 +0200 In-Reply-To: <20221025172443.6732-1-david.faust@oracle.com> (David Faust's message of "Tue, 25 Oct 2022 10:24:43 -0700") Message-ID: <87o7tzyazk.fsf@oracle.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Content-Type: text/plain X-ClientProxiedBy: LNXP265CA0029.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:5c::17) To BYAPR10MB2888.namprd10.prod.outlook.com (2603:10b6:a03:88::32) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BYAPR10MB2888:EE_|SJ2PR10MB7015:EE_ X-MS-Office365-Filtering-Correlation-Id: 956b62c0-430e-4cf3-df5b-08dab6bc26df X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: LrRKSdlWU+KmSWkmn3H8Pe0KQAHZnVmn3A18cbGBPSMMG7ZMyDOpLtdBEibxHuLW4JS3CR2Th95vZelgnn2OhmNLylTw1prO1lt8Be17kz79tpf6WzZb5c8NJpnPkIRN6LTTU3NhqBSP0ct4l9sNdf5U92r8ZdpLKEZS+0jCLcw0f5Zjej7feQ0LdMV5WhQbLHmvTfztiS9sUmKuGJYiV1JdP5dtfwgNjYI/Q+CaUmDo3buS0FWpzsQJMq2UCAp02o+hvPpkPs7/5SZnt6lzfZ+9TlfafGHOVPjH6ZPx+cAvc8R4IwOo1dpRzm0Xt9mZY8gLEibWFVUeVm6g71iheM6OFSV9JEBZmhPnP18qq7lLz61WFL4jRKhuksdCU/8gb7jSY1DBEYks+Ko4EIBcR5MpLX51W5PJtqvFuuOyX9VZDPfwIXQiSffznvhUWcExbqfGTbc0hxWk80HpmNF4+d0vwLUEPdlErXt2qP8ViHHY8KChMjdwoCB1foF0dyY6GZlooWWd/U4Zewa/xT56KYnFmqE6nM0wbB5s3Mh0LvdTlR/A2gK1WFy3wtFcAeRBhqjkFl+vWhZ6YopUCn/B9lybIQ1v/xj/SLfp196E9jC02RZlLquXYghvx/CE95zH0g7ZCjgu5vHa0X4nLLNGQWGDJ+7DzrPJgFkX2T8cZ7Rmxcd3yKqWTPrfE7HG2pVAeAakbHEQpFIvEGFxg4laeA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR10MB2888.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(346002)(136003)(39860400002)(396003)(376002)(366004)(451199015)(478600001)(38100700002)(83380400001)(6486002)(6636002)(2906002)(37006003)(186003)(36756003)(8676002)(66476007)(66946007)(4326008)(6506007)(8936002)(6862004)(66556008)(26005)(6512007)(86362001)(316002)(41300700001)(2616005)(5660300002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?j0+adb3D3gq7YpNnOBEMb1M/02TZWTEzhKMFnN0pnmEis/t2Ukqyg3szIqvP?= =?us-ascii?Q?eMSCFgGOHL9Ay9eFqpqqMr6PqwGhl7y5uhkMx1iC/4lge8PLUjAoX4SYiBA2?= =?us-ascii?Q?CnCkyEfCC0DRl6OOFucZs2VCy/708Lw4BdWiyglJN0b6HaPDQd4dCHCRDmcC?= =?us-ascii?Q?82c8WDq5Lgezpe5v8WpsE1DeuAjXaPUutg6eh3Suc4R/7VJp5nqTmjYHnMOY?= =?us-ascii?Q?74DMPK8rMh1AMNIMEuWvN3tVX5I639+mf2F4T8IYJQfnPWhNyused3yoYEih?= =?us-ascii?Q?dmv/Kl6BJ8YaG2AmYhSKxbHomVJifZP8rD2FDTFHdQXLyp89+9G58+WwiV4z?= =?us-ascii?Q?RtrIXzcQx4GlEg2HhS2NvUl4btGdECPyipZoiIT9Mh5X+BaTOoPqkubMV2e9?= =?us-ascii?Q?T/hmhTQL57WQB3uDQtg3sVu6lneh7lssvqMr6bybaR2/OpNPGIsQm9WIRc/e?= =?us-ascii?Q?j1IgWDIhPwcZHp51hWLWAASUpnDwi6iBP5lhn1G78gEEwJGXEmKkuEJm5WdQ?= =?us-ascii?Q?Z/Nf/QhNLPdUK3D7H1R33QeNvx+nNDriZyk5qM+pga29LoTLq6Kxz3UBzn+t?= =?us-ascii?Q?G9piCUMVkivmnje5GX4htEwsd9s6qvchcUEfYbWhwB/e0rBHUMWn9J1ZoU0v?= =?us-ascii?Q?SbaNUwugAf7Mn+WWSe6QbtnlzSwGY+z5imgMG+NUBgUcQTzlk6doLDdClFyj?= =?us-ascii?Q?ZG5nE3sZb6THsLWYyw0dV8VivCJ0o5/yodpI4MCr/gZg/jcba+XTwe8rXqTz?= =?us-ascii?Q?5sqM8IulB2hy6J6Ua8Nr76DRMrh0Q/sghwrb9xmDOYXYHc6V9zt+1dX+pCnE?= =?us-ascii?Q?JGxaXW1ZtdODE9nnjLVMMUuggJEYLzhPVVPn0YyetYt1tKXgA4UzqYLet6Ry?= =?us-ascii?Q?H1G/OytWuYykXkqpw0Sv1p+zfmdjmtLGsy6+HFTc1443bQW1q6NnPn6eEh1A?= =?us-ascii?Q?XOAD/BC0oot8DzlZWMxVCTFc9oa2raufeef9sxOxH4fmwLUVK1c4sWooYYtE?= =?us-ascii?Q?65MwENvYFgdIqQ2TjvII4fA3Bic37L2xORzYOdFSd6fhumm1c6TpmW87GoAu?= =?us-ascii?Q?oXq42RTr1IfoDDoKSDJA0g1BN+pM8N7zpKH99Ogsarnym6olzufdJMIImLlp?= =?us-ascii?Q?9zYw76kP8ALLsh27YR8JTpjdVpCNZ0CxiXb5KVuX31SAfWqixofRGnDZVluV?= =?us-ascii?Q?pGov+xE/6m2GllQVorLINpBWvzuwgaBAu1WtCr+AUZO1BK8MNUyj4zw71x5q?= =?us-ascii?Q?AiR56VDJHMir3hEz+YVptzq9xdCP1FitUy8mp/8BSoW+hIzFfgLe415NlJVS?= =?us-ascii?Q?TyFeYjSMFHWh32RR8btIG/Er3bxxocPpyshNJAEInqPChm2/Rgtrqx8MmEoc?= =?us-ascii?Q?qTKuT1qarT9lsEsLj4W/5zUCPAlsahh5T1Am8aWdWX5ypFlXMfQ0awX1qMnM?= =?us-ascii?Q?uyjrUPY566bRp5Hk7Yp3WsZEQeLAPXdTD/7QvMQZescPQyKYu6oBb428nhwM?= =?us-ascii?Q?tpklzwK9/Ptbzet+T85ETM4/hMxtdbxnbF4jKXaADC3HGHvyyzf0PydljNME?= =?us-ascii?Q?Pi9EjqbukbfkhJG7GcPAlKw8FI1Kei1XMV1JKfBRY52+6GAZHONbPt3vGez3?= =?us-ascii?Q?Hw=3D=3D?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 956b62c0-430e-4cf3-df5b-08dab6bc26df X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2888.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Oct 2022 19:07:25.7720 (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: Z04HnQ5X41PNkJ7mQgQbLc2B0epgTjYi7QotDpK1DirhBUjiGmkY1EyXudQXvRjht3gdR4ZEcj2unFXy4YjG/jnQD8rmxoG8C+mzf81eYFs= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR10MB7015 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-25_12,2022-10-25_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0 suspectscore=0 malwarescore=0 phishscore=0 mlxscore=0 spamscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000 definitions=main-2210250107 X-Proofpoint-GUID: Xg-op2N9aeln2C4OCZCD_79OwapOqtBP X-Proofpoint-ORIG-GUID: Xg-op2N9aeln2C4OCZCD_79OwapOqtBP X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SOMETLD_ARE_BAD_TLD,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > Hi Jose, > > Thanks for your comments. I think I've addressed them all in the updated > patch below. > >>>+ get_inner_reference (src, &bitsize, &bitpos, &var_off, &mode, &unsignedp, >>>+ &reversep, &volatilep); >> >>Since the information returned by the builtin is always constant >>(positions, sizes) I think you will want to adjust the code for the >>eventuality of variable positioned fields and also variable sized >>fields. >> >>get_inner_reference sets var_off to a tree if the position of the field >>is variable. In these cases `bitpos' is relative to that position. >> >>Likewise, get_inner_reference sets `mode' is set to BLKmode and >>`bitsize' will be set to -1. >> >>I'm not sure what the built-in is supposed to do/return in these cases. >>I guess it makes sense to error out, but what does LLVM do? > > I would have thought erroring out the only option, but it seems that > LLVM will return a value from the builtin and record a CO-RE relocation > as normal. > > What value will be returned depends of course on KIND, but from what > I can tell it seems that such fields are treated as having an offset of > 0 bits and/or a size of 0 bits. For example FIELD_BYTE_SIZE for a > flexible-length array will return 0. FIELD_RSHIFT_U64 will be > calculated as 64 - 0 = 64. > > This sort of makes sense if you expect that any BPF loader will honor > the CO-RE relocations and patch the return value before the program is > run, i.e. the actual values at compile time are irrelevant. > > But, I'm not sure that BPF loaders in practice actually _can_ patch the > return value correctly. The source of information for resolving the > relocations is the BTF. But the BTF won't have more information about > variable position/size members. A flexible-length array for example in > BTF is represented as an array type with 0 elements. So the size > calculation when patching the relocation (looking at the impl in > libbpf) will be elem_size * nelems = 0, and the 'patched' values will > be the same as the unpatched. > > I'm not sure whether this behavior is a known limitation or an > oversight. In my opinion it makes more sense to error at compile time, > becuase even after the loader patches the return value it still will > not be correct for these cases. > > So for now I've set these cases to error out, but it would be just as > simple to mimic the LLVM behavior. WDYT? I would say it makes more sense to error out than to return invalid data. However, the divergence wrt LLVM is a concern. What about keeping this behavior in the GCC backend and simultaneously raise the issue in bpf@vger? If it was a design oversight and the change doesn't impact kernel sources, they may be willing to change it. >>If I read this properly, for something like: >> >>__builtin_preserve_field_info (a = foo.bar + bar.baz, KIND) >> >>On one side CO-RE relocations are computed for both foo.bar and bar.baz >>(I see bpf_core_compute does that) as expected. >> >>But then the builtin returns information that can only apply to one >>access. Which one? > > Expressions like this should not be accepted by the builtin. I didn't > consider this case in v1 so it led to an ICE. Clang rejects this > outright and errors with "argument 1 is not a field access". It is > actually very strict about the expressions that are accepted, unlike > __builtin_preserve_access_index. > > I have updated this implementation to behave more like clang in that > it will reject any expression that isn't directly a field access. That > even includes rejecting things like: > > __builtin_preserve_field_info (&foo.bar, KIND) > > Since unlike preserve_access_index this builtin does not actually > perform the operation in EXPR, it makes sense to enforce that EXPR must > be exactly a single field access. Ok, thanks. > [...] > +@deftypefn {Built-in Function} unsigned int __builtin_preserve_field_info (@var{expr}, unsigned int @var{kind}) > +BPF Compile Once-Run Everywhere (CO-RE) support. This builtin is used to > +extract information to aid in struct/union relocations. @var{expr} is > +an access to a field of a struct or union. Depending on @var{kind}, different > +information is returned to the program. A CO-RE relocation for the access in > +@var{expr} with kind @var{kind} is recorded if @code{-mco-re} is in effect. > + > +The following values are supported for @var{kind}: > +@table @var > +@item FIELD_BYTE_OFFSET = 0 > +The returned value is the offset, in bytes, of the field from the > +beginning of the containing structure. What about bit fields? Is this the byte offset of the containing word? > +@item FIELD_BYTE_SIZE = 1 > +The returned value is the size, in bytes, of the field. For bit fields, is this the size of the containing word? > +@item FIELD_EXISTENCE = 2 > +The returned value is 1 if the field exists, 0 otherwise. Always 1 at > +compile time. > + > +@item FIELD_SIGNEDNESS = 3 > +The returned value is 1 if the field is signed, 0 otherwise. > + > +@item FIELD_LSHIFT_U64 = 4 > +@itemx FIELD_RSHIFT_U64 = 5 > +Suppose the field were loaded into a value of FIELD_BYTE_SIZE bytes > +and then zero or sign-extended to a 64-bit value. The returned value > +is the number of bits of left or right shifting respectively that > +would be needed to recover the original value of the field. What are the semantics for bit fields? > +@end table > + > +Note that the return value is a constant which is known at > +compile-time. If the field has a variable offset then > +FIELD_BYTE_OFFSET, FIELD_LSHIFT_U64 and FIELD_RSHIFT_U64 are not > +supported. Similarly, if the field has a variable size then > +FIELD_BYTE_SIZE, FIELD_LSHIFT_U64 and FIELD_RSHIFT_U64 are not > +supported. > + > +@end deftypefn > + > @node FR-V Built-in Functions > @subsection FR-V Built-in Functions