This is an archive of the discontinued LLVM Phabricator instance.

[BPF] Support for "btf_decl_tag" annotations for arguments of extern functions
AbandonedPublic

Authored by eddyz87 on Jan 3 2023, 4:29 PM.

Details

Reviewers
yonghong-song
Summary

This adds support for "btf_decl_tag" annotations for arguments of extern functions. The changes are split into several commits:

  • Triple::isBPF() utility method -- adds a simple utility method for llvm::Triple class;
  • preserve btf_decl_tag for parameters of extern functions -- modifies CLang frontend to generate DILocalVariable entries with attached "btf_dec_tag" annotations for parameters of extern functions;
  • generate btf_decl_tag records for params of extern functions -- modifies BPF backend to process "btf_decl_tag" annotations for parameters of extern functions.

Below are descriptions for the last two commits:

Preserve btf_decl_tag for parameters of extern functions

Generate DILocalVariable entries for parameters of extern functions, the "annotations" field of DILocalVariable is used to link "btf_decl_tag" annotation with the parameter.

Do this only for BPF backend as there are no other users for this information. Final DWARF is valid as "Appendix A" is very much lax in
what is allowed as attributes for "DW_TAG_formal_parameter":

DWARF does not in general require that a given debugging information entry contain a particular attribute or set of attributes. Instead, a DWARF producer is free to generate any, all, or none of the attributes ... other attributes ... may also appear in a given debugging information entry.

DWARF Debugging Information Format Version 5,
Appendix A: Attributes by Tag Value (Informative)
Page 251, Line 3.

Generate btf_decl_tag records for params of extern functions

After frontend changes in the following commit: "BPF: preserve btf_decl_tag for parameters of extern functions" same mechanics could be used to get the list of function parameters and associated btf_decl_tag entries for both extern and non-extern functions.

This commit extracts this mechanics as a separate auxiliary function BTFDebug::processDISubprogram(). The function is called for both
extern and non-extern functions in order to generated corresponding BTF_DECL_TAG records.

Diff Detail

Event Timeline

eddyz87 created this revision.Jan 3 2023, 4:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 4:29 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
eddyz87 published this revision for review.Jan 3 2023, 4:39 PM
eddyz87 retitled this revision from This series of patches adds support for "btf_decl_tag" annotations for arguments of extern functions. Patch descriptions follow. [BPF] Triple::isBPF() utility method to [BPF] Support for "btf_decl_tag" annotations for arguments of extern functions.
eddyz87 edited the summary of this revision. (Show Details)
eddyz87 added a reviewer: yonghong-song.
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 3 2023, 4:39 PM

Please separate the patch to clang and llvm part.

clang/lib/CodeGen/CGDebugInfo.cpp
4218 ↗(On Diff #486116)

Looks like this is for bpf only? Can we extend it to non-bpf as well? Currently, I think most use case, if any, will be in vmlinux (e.g., for struct_ops callback, etc).

yonghong-song added inline comments.Jan 3 2023, 11:51 PM
clang/lib/CodeGen/CGDebugInfo.cpp
4218 ↗(On Diff #486116)

sorry, the above comment is not right about struct_ops, I thought this is for function pointer parameters, but it is for extern functions. Yes, for vmlinux, we don't need to support extern func arg decl tags since we always have real function definitions. So bpf only support is okay.

Please separate the patch to clang and llvm part.

It's already split in three commits: two for clang, one for llvm.
Not sure if this is possible to see in the Phabricator UI, though.

Please separate the patch to clang and llvm part.

It's already split in three commits: two for clang, one for llvm.
Not sure if this is possible to see in the Phabricator UI, though.

However arc patch D140929 applies this revision as a single commit. Maybe I messed something up while creating revision.

eddyz87 planned changes to this revision.Jan 4 2023, 4:32 AM
eddyz87 updated this revision to Diff 486245.Jan 4 2023, 5:06 AM

Reorganize commits as a stack.

eddyz87 abandoned this revision.Jan 4 2023, 5:13 AM

I think I messed up while creating this revision. I've created a proper stack of revisions with a tip at D140971. Sorry for all the spam in emails.