[PATCH] D88199: Introduce and use a new section type for the bb_addr_map section.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D88199#2297495 <https://reviews.llvm.org/D88199#2297495>, @jhenderson wrote:

> In D88199#2295596 <https://reviews.llvm.org/D88199#2295596>, @MaskRay wrote:
>
>> In D88199#2292193 <https://reviews.llvm.org/D88199#2292193>, @jhenderson wrote:
>>
>>> Mostly looks okay. I think there is no testing for the assembly parser being able to parse the new section type?
>>>
>>> I'm also keen to hear what others think (e.g. @MaskRay).
>>
>>
>>
>>> This patch lets the bb_addr_map (renamed to llvm_bb_addr_map) section use a special section type (SHT_LLVM_BB_ADDR_MAP) instead of SHT_PROGBITS. This would help linker and other tools to use the sh_type ELF field to identify this section rather than relying on string comparison on the section name.
>>
>> This description implies that some special processing is needed in the linker. (As you can see, when llvm_call_graph_profile and llvm_dependent_libraries were proposed, they had clear patches on the linker side.)
>
> Maybe pivot the statement to say "dumpers and other tools"? Dumpers will need to know the section type for simpler processing of the code, even if the linker has no special handling currently.

`xray_instr_map` and `xray_fn_idx` are also an LLVM specific section types but they do not get their own section types. They use a dedicated dumper (llvm-xray), instead of llvm-readobj.

If the eventual behavior is to let LLD interpret `.bb_addr_map` differently (like `.deplibs` and `.llvm_sympart*`) differently (not "Rules for Linking Unrecognized Sections" on http://www.sco.com/developers/gabi/latest/ch4.sheader.html), having a dedicated section type seems fine. Though, I'd really like to know whether auto-defined `__start_bb_addr_map` and `__stop_bb_addr_map` are needed. If yes, the section name should not start with a period. And I'd like to know how the section is differently interpreted. `xray_instr_map` and `xray_fn_idx` only use generic ELF features.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88199/new/

https://reviews.llvm.org/D88199