From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62d.google.com (mail-ej1-x62d.google.com [IPv6:2a00:1450:4864:20::62d]) by sourceware.org (Postfix) with ESMTPS id 615743858434 for ; Fri, 27 Aug 2021 06:13:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 615743858434 Received: by mail-ej1-x62d.google.com with SMTP id me10so11457660ejb.11 for ; Thu, 26 Aug 2021 23:13:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=bGdIQmjNYro0B7lBnYg3RKst8f5PiAPpuqPnneOYbdM=; b=YOF4h2EVCwyaBWusK3dOMYK/bjuJXTjQD6+D0tlTNfTnsHpZ3ijJedv7D/IWxg3HI1 /SKPY9RTAAXXANSCjFbOt6mzN3IhBP6LFoda6nQx15Wd4OnclxlhohXcvidIQIsGLnJt a/qwx7TbubTyc9skpanEobSmzYSZJG2O5vL+8ko2LehnqJg0znE0+jjqXoB1gMOxFuAH T0ViyacBnPsODNE9smF+fMRrX2OskYaai4DiHmSsLVmQYri03MDFZcxrTpZmWjyzYHHS d2oPPTEmCEo6aVcRIMCPgrhpFI+SyeTeUMBhR8sVLs9MHJMIpvYN7WYBVCpHZ6TTRIbV SmwA== X-Gm-Message-State: AOAM531a+3ARZ7d/pZydOhaclK56XCdsjNfkF6AXnkmAnEcunc6Amn97 WI53V1m2BZd5BihErLo+uZ2eH0gCybCTULzC+y8= X-Google-Smtp-Source: ABdhPJw5up7AXoY5i1odjZDcYIfHyPN8BizmAQT8kxgK6mAoKIMegXsXfPeJAU4Gx7r1CHATOH2HJtpwEUpzvnMx71I= X-Received: by 2002:a17:906:27d5:: with SMTP id k21mr8596053ejc.235.1630044787246; Thu, 26 Aug 2021 23:13:07 -0700 (PDT) MIME-Version: 1.0 References: <1628124628-28706-1-git-send-email-indu.bhagat@oracle.com> <1628124628-28706-3-git-send-email-indu.bhagat@oracle.com> <85d5d4ff-a65d-03ab-d888-493fbd7f451b@oracle.com> <24a1be82-bae8-d5d0-4022-822bc48a9a3a@oracle.com> In-Reply-To: From: Richard Biener Date: Fri, 27 Aug 2021 08:12:56 +0200 Message-ID: Subject: Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission To: Indu Bhagat Cc: GCC Patches , David Faust Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Aug 2021 06:13:10 -0000 On Thu, Aug 26, 2021 at 8:55 PM Indu Bhagat wrote: > > On 8/26/21 3:03 AM, Richard Biener wrote: > > On Tue, Aug 24, 2021 at 7:07 PM Indu Bhagat wrote: > >> > >> On 8/18/21 12:00 AM, Richard Biener wrote: > >>> On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat wrote: > >>>> > >>>> On 8/17/21 1:04 AM, Richard Biener wrote: > >>>>> On Mon, Aug 16, 2021 at 7:39 PM Indu Bhagat wrote: > >>>>>> > >>>>>> On 8/10/21 4:54 AM, Richard Biener wrote: > >>>>>>> On Thu, Aug 5, 2021 at 2:52 AM Indu Bhagat via Gcc-patches > >>>>>>> wrote: > >>>>>>>> > >>>>>>>> This patch adds a new target hook to detect if the CTF container can allow the > >>>>>>>> emission of CTF/BTF debug info at DWARF debug info early finish time. Some > >>>>>>>> backends, e.g., BPF when generating code for CO-RE usecase, may need to emit > >>>>>>>> the CTF/BTF debug info sections around the time when late DWARF debug is > >>>>>>>> finalized (dwarf2out_finish). > >>>>>>> > >>>>>>> Without looking at the dwarf2out.c usage in the next patch - I think > >>>>>>> the CTF part > >>>>>>> should be always emitted from dwarf2out_early_finish, the "hooks" should somehow > >>>>>>> arrange for the alternate output specific data to be preserved until > >>>>>>> dwarf2out_finish > >>>>>>> time so the late BTF data can be emitted from there. > >>>>>>> > >>>>>>> Lumping everything together now just makes it harder to see what info > >>>>>>> is required > >>>>>>> to persist and thus make LTO support more intrusive than necessary. > >>>>>> > >>>>>> In principle, I agree the approach to split generate/emit CTF/BTF like > >>>>>> you mention is ideal. But, the BTF CO-RE relocations format is such > >>>>>> that the .BTF section cannot be finalized until .BTF.ext contents are > >>>>>> all fully known (David Faust summarizes this issue in the other thread > >>>>>> "[PATCH, V2 3/3] dwarf2out: Emit BTF in dwarf2out_finish for BPF CO-RE > >>>>>> usecase".) > >>>>>> > >>>>>> In summary, the .BTF.ext section refers to strings in the .BTF section. > >>>>>> These strings are added at the time the CO-RE relocations are added. > >>>>>> Recall that the .BTF section's header has information about the .BTF > >>>>>> string table start offset and length. So, this means the "CTF part" (or > >>>>>> the .BTF section) cannot simply be emitted in the dwarf2out_early_finish > >>>>>> because it's not ready yet. If it is still unclear, please let me know. > >>>>>> > >>>>>> My judgement here is that the BTF format itself is not amenable to split > >>>>>> early/late emission like DWARF. BTF has no linker support yet either. > >>>>> > >>>>> But are the strings used for the CO-RE relocations not all present already? > >>>>> Or does the "CTF part" have only "foo", "bar" and "baz" while the CO-RE > >>>>> part wants to output sth like "foo->bar.baz" (which IMHO would be quite > >>>>> stupid also for size purposes)? > >>>>> > >>>> > >>>> Yes, the latter ("foo->bar.baz") is closer to what the format does for > >>>> CO-RE relocations! > >>>> > >>>>> That said, fix the format. > >>>>> > >>>>> Alternatively hand the CO-RE part its own string table (what's the fuss > >>>>> with re-using the CTF string table if there's nothing to share ...) > >>>>> > >>>> > >>>> BTF and .BTF.ext formats are specified already by implementations in the > >>>> kernel, libbpf, and LLVM. For that matter, I should add BPF CO-RE to the > >>>> mix and say that BPF CO-RE capability _and_ .BTF/.BTF.ext debug formats > >>>> have been defined already by the BPF kernel developers/associated > >>>> entities. At this time, we as GCC developers simply extending the BPF > >>>> backend/BTF generation support in GCC, cannot fix the format. That ship > >>>> has sailed. > >>> > >>> Hmm, well. How about emitting .BTF.ext.string from GCC and have the linker > >>> merge the .BTF.ext.string section with the CTF string section then? You can't > >>> really say "the ship has sailed" if I read the CTF webpage - there seems to be > >>> many format changes planned. > >>> > >>> Well. Guess that was it from my side on the topic of ranting about the > >>> not well thought out debug format ;) > >>> > >>> Richard. > >> > >> Hello Richard, > >> > >> As we clarified in this thread, BTF/CO-RE format cannot be changed. What > >> are your thoughts on this patch set now ? Is this OK ? > > > > Since the issue is intrinsic to BTF/CO-RE and not the actual target can we > > do w/o a target hook by just gating on BTF_WITH_CORE as debug format > > or so? > > > > Richard. > > > > The issue is intrinsic to BTF debug format *when* CO-RE is in effect, so > it is not entirely target independent because the whole "Compile Once - > Run Everywhere" scheme is BPF backend specific. I see. > The debug information generation routines need to know if CO-RE is in > effect (to finalize BTF debug info generation late and not early). Now, > because it is the user who selects it via the -mco-re option, we need to > have a way to detect this at run-time. Guarding it with a definition > like BTF_WITH_CORE (is this what you meant?) will not work. I was thinking about having BTF_CORE_DEBUG in addition to BTF_DEBUG and thus have this part of the debug info format. That would be straight-forward in case the option to enable it were not backend specific but I guess it might be valid for the backend to alter ops->x_write_symbols in the backend option processing code. > But, yes, we can do without a target hook. We can keep a global var in > the BTF context in btfout.c / CTF container (CTFC) which can be updated > by the backend when BPF CO-RE is in effect (the -mco-re option). This > was also considered as an option but the target hook option was chosen > because it appeared to be the GCC's preferred way of conveying > information from the backend. Is keeping global var preferable in this > specific case ? No, a global variable would be worse. Richard. > > Indu