Re: [PATCH, V2 2/3] targhooks: New target hook for CTF/BTF debug info emission

[Indu Bhagat <indu.bhagat@oracle.com>]

On 8/26/21 11:12 PM, Richard Biener wrote:
> On Thu, Aug 26, 2021 at 8:55 PM Indu Bhagat <indu.bhagat@oracle.com> wrote:
>>
>> On 8/26/21 3:03 AM, Richard Biener wrote:
>>> On Tue, Aug 24, 2021 at 7:07 PM Indu Bhagat <indu.bhagat@oracle.com> wrote:
>>>>
>>>> On 8/18/21 12:00 AM, Richard Biener wrote:
>>>>> On Tue, Aug 17, 2021 at 7:26 PM Indu Bhagat <indu.bhagat@oracle.com> wrote:
>>>>>>
>>>>>> On 8/17/21 1:04 AM, Richard Biener wrote:
>>>>>>> On Mon, Aug 16, 2021 at 7:39 PM Indu Bhagat <indu.bhagat@oracle.com> 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
>>>>>>>>> <gcc-patches@gcc.gnu.org> 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.
> 

This is doable. I updated the patch series and have posted V3.

Thanks
Indu

>> 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