This is an archive of the discontinued LLVM Phabricator instance.

Differential D101890

[LLD] Improve --strip-all help text
ClosedPublic

Authored by bd1976llvm on May 5 2021, 3:52 AM.

Details

Reviewers
MaskRay
grimar
peter.smith
Commits
rG5dd9f44c17ec: [LLD] Improve --strip-all help text
Summary

This is a slight improvement to the help text, as I was slightly surprised when --strip-all did more than remove the symbol table.

Currently, we match gold's help text for --strip-all and --strip-debug. I think that the GNU documentation for these options is not particularly clear. However, I have opted to make only a minor change here and keep the help text similar to gold's as these are mature options that are well understood.

Diff Detail

Event Timeline

bd1976llvm created this revision.May 5 2021, 3:52 AM
Herald added subscribers: dang, arichardson, emaste. · View Herald TranscriptMay 5 2021, 3:52 AM
bd1976llvm requested review of this revision.May 5 2021, 3:52 AM
Harbormaster completed remote builds in B102697: Diff 342983.May 5 2021, 4:22 AM
peter.smith accepted this revision.May 5 2021, 5:42 AM

LGTM. Will be worth waiting before committing to see if there are any comments from MaskRay.

ld.bfd (https://sourceware.org/binutils/docs/ld/Options.html) has a similar implication although it defines strip-debug as a subset of strip-all.

This revision is now accepted and ready to land.May 5 2021, 5:42 AM
peter.smith added a comment.May 5 2021, 5:45 AM

Just remembered; worth making the same change in the man page in docs/ld.lld.1 as well.

bd1976llvm updated this revision to Diff 343048.EditedMay 5 2021, 7:58 AM

Thanks @peter.smith. I have added the man page update.

ld.bfd (https://sourceware.org/binutils/docs/ld/Options.html) has a similar implication although it defines strip-debug as a subset of strip-all.

I think this is the wrong way round as you have to read both the --strip-debug and the --strip-all help text to understand the behaviour of --strip-all, and the --strip-all help text doesn't indicate that he --strip-debug help text is related. I think what I have proposed in this review is better.

Harbormaster completed remote builds in B102747: Diff 343048.May 5 2021, 8:37 AM
emaste added inline comments.May 5 2021, 10:03 AM
lld/docs/ld.lld.1
505–507

FWIW mandoc style (from FreeBSD where this originated) is to start a new sentence on a new line. Not critical, but the rest of this file follows the convention.

MaskRay accepted this revision.May 5 2021, 10:16 AM

Looks great!

bd1976llvm updated this revision to Diff 343104.May 5 2021, 10:48 AM

Conform to mandoc style.

bd1976llvm marked an inline comment as done.May 5 2021, 10:48 AM
bd1976llvm added inline comments.
lld/docs/ld.lld.1
505–507

Thanks :)

Harbormaster completed remote builds in B102789: Diff 343104.May 5 2021, 11:18 AM
Closed by commit rG5dd9f44c17ec: [LLD] Improve --strip-all help text (authored by Ben Dunbobbin <Ben.Dunbobbin@sony.com>). · Explain WhyMay 6 2021, 4:34 AM
This revision was automatically updated to reflect the committed changes.
Ben Dunbobbin <Ben.Dunbobbin@sony.com> added a commit: rG5dd9f44c17ec: [LLD] Improve --strip-all help text.
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2021, 4:34 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
bd1976llvm marked an inline comment as done.May 6 2021, 4:40 AM
bd1976llvm added a subscriber: tstellar.

@tstellar this is a very safe change and we did have a customer confused by this option. Would it be possible to put this change onto the 12 release branch? Thanks.

tstellar added a comment.May 6 2021, 6:15 AM

Can you file a bug for this?

bd1976llvm added a comment.May 7 2021, 8:42 AM
In D101890#2741842, @tstellar wrote:

Can you file a bug for this?

https://bugs.llvm.org/show_bug.cgi?id=50264.. I have messaged you about putting this onto the release branch on that bug.

bd1976llvm added a comment.May 10 2021, 3:07 AM

@tstellar sorry to be a pain but could you give a go or no-go for putting this on the release branch please? For internal reasons I have to finish LLVM 12 changes this week. Thanks!

Revision Contents

PathSize
lld/
ELF/
2 lines
docs/
2 lines

Diff 343358

lld/ELF/Options.td

Show First 20 Lines • Show All 375 Lines • ▼ Show 20 Linesdefm sort_section:
Eq<"sort-section", "Specifies sections sorting rule when linkerscript is used">; Eq<"sort-section", "Specifies sections sorting rule when linkerscript is used">;
def start_group: F<"start-group">, def start_group: F<"start-group">,
HelpText<"Ignored for compatibility with GNU unless you pass --warn-backrefs">; HelpText<"Ignored for compatibility with GNU unless you pass --warn-backrefs">;
def start_lib: F<"start-lib">, def start_lib: F<"start-lib">,
HelpText<"Start a grouping of objects that should be treated as if they were together in an archive">; HelpText<"Start a grouping of objects that should be treated as if they were together in an archive">;
def strip_all: F<"strip-all">, HelpText<"Strip all symbols">; def strip_all: F<"strip-all">, HelpText<"Strip all symbols. Implies --strip-debug">;
def strip_debug: F<"strip-debug">, HelpText<"Strip debugging information">; def strip_debug: F<"strip-debug">, HelpText<"Strip debugging information">;
defm symbol_ordering_file: defm symbol_ordering_file:
Eq<"symbol-ordering-file", "Layout sections to place symbols in the order specified by symbol ordering file">; Eq<"symbol-ordering-file", "Layout sections to place symbols in the order specified by symbol ordering file">;
defm sysroot: Eq<"sysroot", "Set the system root">; defm sysroot: Eq<"sysroot", "Set the system root">;
▲ Show 20 Lines • Show All 320 Lines • Show Last 20 Lines

lld/docs/ld.lld.1

Show First 20 Lines • Show All 496 Lines • ▼ Show 20 Lines
.It Fl -sort-common .It Fl -sort-common
This option is ignored for GNU compatibility. This option is ignored for GNU compatibility.
.It Fl -sort-section Ns = Ns Ar value .It Fl -sort-section Ns = Ns Ar value
Specifies sections sorting rule when linkerscript is used. Specifies sections sorting rule when linkerscript is used.
.It Fl -start-lib .It Fl -start-lib
Start a grouping of objects that should be treated as if they were together Start a grouping of objects that should be treated as if they were together
in an archive. in an archive.
.It Fl -strip-all , Fl s .It Fl -strip-all , Fl s
Strip all symbols. Strip all symbols.
Implies
.Fl -strip-debug .
emasteUnsubmitted
Done
ReplyInline Actions

FWIW mandoc style (from FreeBSD where this originated) is to start a new sentence on a new line. Not critical, but the rest of this file follows the convention.

emaste: FWIW mandoc style (from FreeBSD where this originated) is to start a new sentence on a new line.
bd1976llvmAuthorUnsubmitted
Done
ReplyInline Actions

Thanks :)

bd1976llvm: Thanks :)
.It Fl -strip-debug , Fl S .It Fl -strip-debug , Fl S
Strip debugging information. Strip debugging information.
.It Fl -symbol-ordering-file Ns = Ns Ar file .It Fl -symbol-ordering-file Ns = Ns Ar file
Lay out sections in the order specified by Lay out sections in the order specified by
.Ar file . .Ar file .
.It Fl -sysroot Ns = Ns Ar value .It Fl -sysroot Ns = Ns Ar value
Set the system root. Set the system root.
.It Fl -target1-abs .It Fl -target1-abs
▲ Show 20 Lines • Show All 350 Lines • Show Last 20 Lines