This is an archive of the discontinued LLVM Phabricator instance.

[DWARFLinker] Prefix debug section names with '.' in the comments. NFC.
ClosedPublic

Authored by RamNalamothu on Aug 22 2021, 10:09 AM.

Details

Summary

In DWARFLinker.h, some comments prefix the debug section names
with '.' while others do not.

Diff Detail

Event Timeline

RamNalamothu requested review of this revision.Aug 22 2021, 10:09 AM
RamNalamothu created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2021, 10:09 AM
RamNalamothu edited the summary of this revision. (Show Details)Aug 22 2021, 10:17 AM
RamNalamothu added reviewers: avl, JDevlieghere.
RamNalamothu added a reviewer: scott.linder.
RamNalamothu added a subscriber: debug-info.
RamNalamothu added a subscriber: t-tye.
avl added a comment.Aug 22 2021, 10:32 AM

I think it would be better to not add any prefixes to section names. On Darwin, section names have "__" prefix. So using "." looks a bit incompatible. It seems, it is better always use section names without prefixes.

I think it would be better to not add any prefixes to section names. On Darwin, section names have "__" prefix. So using "." looks a bit incompatible. It seems, it is better always use section names without prefixes.

But this file implements DWARF support and the DWARF spec specified section names are prefixed with '.'.

In any case, I think the comments can be consistent in all the places on whether or not prefix debug section names with '.'.
Currently section names prefixed with '.' in some places and not prefixed with '.' in the rest.

I think it would be better to not add any prefixes to section names. On Darwin, section names have "__" prefix. So using "." looks a bit incompatible. It seems, it is better always use section names without prefixes.

Given the DWARF spec talks about the names in the ELF form (with the leading dot) and various other parts of LLVM do this - including messages in llvm-dwarfdump --verify, for instance - I think it's probably not bad to standardize on the dot forms when referring to these names. (the MachO forms also get abbreviated in some places, which would get a bit more weird).

avl added a comment.Aug 22 2021, 11:17 PM

I think it would be better to not add any prefixes to section names. On Darwin, section names have "__" prefix. So using "." looks a bit incompatible. It seems, it is better always use section names without prefixes.

But this file implements DWARF support and the DWARF spec specified section names are prefixed with '.'.

In any case, I think the comments can be consistent in all the places on whether or not prefix debug section names with '.'.
Currently section names prefixed with '.' in some places and not prefixed with '.' in the rest.

Right. Comments need to be consistent in all places. Avoiding prefixes would make names looking good for all platforms. But if the idea is to be conformant with Dwarf spec and others prefer dot form I am OK with it also.

If there are no concerns on standardizing the DWARF section names on the dot forms, can somebody approve these changes?

dblaikie accepted this revision.Aug 26 2021, 9:14 AM

Sounds good

This revision is now accepted and ready to land.Aug 26 2021, 9:14 AM