-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[LLD][Docs] Document -z gcs= option in the man page #146522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add documentation for the `-z gcs` option to the LLD man page. This flag controls how the GCS bit is set in the output: - implicit (default): inferred from input objects - never: GCS bit is never set - always: GCS bit is always set Clarifies behavior for users and aligns the man page with existing functionality.
Add documentation for the -z gcs= option to the LLD man page. This flag controls how the GCS bit is set in the output: - implicit (default): inferred from input objects - never: GCS bit is never set - always: GCS bit is always set Clarifies behavior for users and aligns the man page with existing functionality.
lld/docs/ld.lld.1
Outdated
@@ -953,6 +958,15 @@ disallows overlap. | |||
.It Cm shstk | |||
x86 only, use shadow stack. | |||
.Pp | |||
.It Cm gcs Ns = Ns Ar [implicit|never|always] | |||
Specify how the GCS bit is set. A set GCS bit indicates that the object file supports the Guarded Control Stack security feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to say
Specify how the GNU_PROPERTY_AARCH64_FEATURE_1_GCS feature bit is set in the output ELF file. When set this declares that the ELF file supports the Guarded Control Stack (GCS) security feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, wording improved
lld/docs/ld.lld.1
Outdated
.It Cm gcs Ns = Ns Ar [implicit|never|always] | ||
Specify how the GCS bit is set. A set GCS bit indicates that the object file supports the Guarded Control Stack security feature. | ||
.Cm implicit | ||
is the default, where the GCS bit is inferred from the input objects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From looking at noseparate-code above. I suggest
(default) feature bit is inferred from the input object marking.
I've suggested feature bit rather than GCS bit as we've used the full GNU_PROPERTY_AARCH64_FEATURE_1_GCS feature bit
above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, wording improved
lld/docs/ld.lld.1
Outdated
.Cm implicit | ||
is the default, where the GCS bit is inferred from the input objects. | ||
.Cm never | ||
disables the GCS bit regardless of input markings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest
clears the feature bit regardless of input object marking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, wording improved
Comments handled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nearly there. I think if we can use the same wording as on bti-report and cet-report then we're good.
lld/docs/ld.lld.1
Outdated
.It Cm gcs-report Ns = Ns Ar [none|warning|error] | ||
Specify how to report missing GNU_PROPERTY_AARCH64_FEATURE_1_GCS property. GNU_PROPERTY_AARCH64_FEATURE_1_GCS indicates object file support for the Guarded Control Stack security feature. | ||
.Cm none | ||
is the (default) linker will not report the missing property otherwise, missing property will be reported as a warning or an error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for being a pain. A couple of nits on this line:
I meant (default) at the start of the line. However, looking at bti-report and cet-report above it is probably better to follow the convention and use "is the default," . It was probably that originally; sorry for the noise.
The comma after otherwise looks to be in the wrong position [1]. I think we should follow the wording of bit-report and cet-report.
is the default, linker will not report the missing property otherwise will be reported as a warning or an error.
[1] If I were rewriting with a comma I'd say "report the missing propery, otherwise it will be reported as a warning or an error." However best to stay consistent with the existing wording.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, also for gcs
using (default)
Comments handled (indicating default option and alignment with other report options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks for the updates. Please leave a few days before merging to give @MaskRay a chance to comment.
@llvm/pr-subscribers-lld Author: SivanShani-Arm (sivan-shani) ChangesAdd documentation for the -z gcs= option to the LLD man page. This flag controls how the GCS bit is set in the output:
Clarifies behavior for users and aligns the man page with existing functionality. Full diff: https://github.com/llvm/llvm-project/pull/146522.diff 1 Files Affected:
diff --git a/lld/docs/ld.lld.1 b/lld/docs/ld.lld.1
index 7edc522b4f6a4..49efc1269e7f2 100644
--- a/lld/docs/ld.lld.1
+++ b/lld/docs/ld.lld.1
@@ -798,6 +798,11 @@ Specify how to report the missing GNU_PROPERTY_X86_FEATURE_1_IBT or GNU_PROPERTY
.Cm none
is the default, linker will not report the missing property otherwise will be reported as a warning or an error.
.Pp
+.It Cm gcs-report Ns = Ns Ar [none|warning|error]
+Specify how to report missing GNU_PROPERTY_AARCH64_FEATURE_1_GCS property. GNU_PROPERTY_AARCH64_FEATURE_1_GCS indicates object file support for the Guarded Control Stack security feature.
+.Cm none
+is the default, linker will not report the missing property otherwise will be reported as a warning or an error.
+.Pp
.It Cm dynamic-undefined-weak
Make undefined weak symbols dynamic when the dynamic symbol table is present, if they are referenced from
relocatable object files and not forced local by symbol visibility or versioning. Do not make them dynamic when
@@ -953,6 +958,15 @@ disallows overlap.
.It Cm shstk
x86 only, use shadow stack.
.Pp
+.It Cm gcs Ns = Ns Ar [implicit|never|always]
+Specifies how the GNU_PROPERTY_AARCH64_FEATURE_1_GCS bit is set in the output ELF file. When set, it indicates support for the Guarded Control Stack (GCS) security feature.
+.Cm implicit
+(default) GCS feature bit is inferred from the input object marking.
+.Cm never
+clears the GCS feature bit regardless of input object markings.
+.Cm always
+set the GCS feature bit regardless of input object markings.
+.Pp
.It Cm stack-size Ns = Ns Ar size
Set the main thread's stack size to
.Ar size .
|
lld/docs/ld.lld.1
Outdated
@@ -798,6 +798,11 @@ Specify how to report the missing GNU_PROPERTY_X86_FEATURE_1_IBT or GNU_PROPERTY | |||
.Cm none | |||
is the default, linker will not report the missing property otherwise will be reported as a warning or an error. | |||
.Pp | |||
.It Cm gcs-report Ns = Ns Ar [none|warning|error] | |||
Specify how to report missing GNU_PROPERTY_AARCH64_FEATURE_1_GCS property. GNU_PROPERTY_AARCH64_FEATURE_1_GCS indicates object file support for the Guarded Control Stack security feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GNU_PROPERTY_AARCH64_FEATURE_1_*
properties need to use .Cm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, handled.
lld/docs/ld.lld.1
Outdated
@@ -953,6 +958,15 @@ disallows overlap. | |||
.It Cm shstk | |||
x86 only, use shadow stack. | |||
.Pp | |||
.It Cm gcs Ns = Ns Ar [implicit|never|always] | |||
Specifies how the GNU_PROPERTY_AARCH64_FEATURE_1_GCS bit is set in the output ELF file. When set, it indicates support for the Guarded Control Stack (GCS) security feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use .Cm GNU_PROPERTY_AARCH64_FEATURE_1_GCS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, handled.
lld/docs/ld.lld.1
Outdated
.It Cm gcs Ns = Ns Ar [implicit|never|always] | ||
Specifies how the GNU_PROPERTY_AARCH64_FEATURE_1_GCS bit is set in the output ELF file. When set, it indicates support for the Guarded Control Stack (GCS) security feature. | ||
.Cm implicit | ||
(default) GCS feature bit is inferred from the input object marking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
input object marking
is vague. I'd emphasize relocatable files. Perhaps something like
When all relocatable files contain a .note.gnu.property section containing the GNU_PROPERTY_AARCH64_FEATURE_1_GCS bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, handled.
Comments handled (Add .Cm, use 'relocatable files') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let's see what other reviewers says.
lld/docs/ld.lld.1
Outdated
.Cm implicit | ||
is the default, where the GCS bit is inferred from the input objects. | ||
is the (default) GCS feature bit is inferred from the input object marking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reads as "is the GCS feature bit is inferred... "
is the (default) GCS feature bit is inferred from the input object marking. | |
is the default. GCS feature bit is inferred from the input object marking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Above is outdated, I believe that new version does not suffer from same ambiguity.
lld/docs/ld.lld.1
Outdated
.Cm always | ||
enables the GCS bit regardless of input markings. | ||
set the GCS feature bit regardless of input object markings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set the GCS feature bit regardless of input object markings. | |
sets the GCS feature bit regardless of input object markings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, changed set->sets
lld/docs/ld.lld.1
Outdated
@@ -801,7 +801,7 @@ is the default, linker will not report the missing property otherwise will be re | |||
.It Cm gcs-report Ns = Ns Ar [none|warning|error] | |||
Specify how to report missing GNU_PROPERTY_AARCH64_FEATURE_1_GCS property. GNU_PROPERTY_AARCH64_FEATURE_1_GCS indicates object file support for the Guarded Control Stack security feature. | |||
.Cm none | |||
is the default, linker will not report the missing property otherwise will be reported as a warning or an error. | |||
is the (default) linker will not report the missing property otherwise, missing property will be reported as a warning or an error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without the comma the sentence is strange to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we might want to keep the wording as is, same as it is for
bti-report
cet-report
Which are the same flag type and has the exact same wording.
.Cm GNU_PROPERTY_AARCH64_FEATURE_1_GCS | ||
bit is set in the output ELF file. When set, it indicates support for the Guarded Control Stack (GCS) security feature. | ||
.Cm implicit | ||
(default) The GCS feature bit is set if all input relocatable files have a .note.gnu.property section containing the GNU_PROPERTY_AARCH64_FEATURE_1_GCS bit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.note.gnu.property also needs Cm
[Apologies, Will be away for 5 weeks, response will be delayed] |
Add documentation for the -z gcs= option to the LLD man page. This flag controls how the GCS bit is set in the output:
Clarifies behavior for users and aligns the man page with existing functionality.