[Committed][BFD, LD v2] aarch64: Add support for GCS in AArch64 linker.
Srinath Parvathaneni
srinath.parvathaneni@arm.com
Tue Jan 30 09:13:44 GMT 2024
Hi Nick,
On 1/5/24 10:57, Nick Clifton wrote:
> Hi Srinath,
>
>> This patch will not be committed to development branch until the
>> Kernel GCS
>> ABI changes are approved and committed.
>>
>> Ok to commit this patch to Arm vendor branch based on binutils-master ?
>
> Yes. (Since the branch is ARM specific, you do not really need to check
> with us first, but the thought is appreciated).
Thanks Nick, I made below proposed changes in v2 patch, created
"users/ARM/gcs-binutils-gdb-master" branch and committed the patch
(attached) to this branch.
Regards,
Srinath.
> Looking over the patch I did see one thing which niggled:
>
> + else if (strncmp (optarg, "experimental-gcs-report", 23) == 0)
> + {
> + if (strlen (optarg) == 23 || strcmp (optarg + 23, "=none") == 0)
> + gcs_report = GCS_NONE;
> + else if (strcmp (optarg + 23, "=warning") == 0)
> + gcs_report = GCS_WARN;
> + else if (strcmp (optarg + 23, "=error") == 0)
> + gcs_report = GCS_ERROR;
> + else
> + einfo (_("%P: error: unrecognized: `%s'\''\n"), optarg);
> + }
> + else if (strncmp (optarg, "experimental-gcs", 16) == 0)
> + {
> + if (strlen (optarg) == 16 || strcmp (optarg + 16, "=always") == 0)
> + gcs_type = GCS_ALWAYS;
> + else if (strcmp (optarg + 16, "=never") == 0)
> + gcs_type = GCS_NEVER;
> + else if (strcmp (optarg + 16, "=implicit") == 0)
> + gcs_type = GCS_IMPLICIT;
> + else
> + einfo (_("%P: error: unrecognized: `%s'\''\n"), optarg);
> + }
>
> I do not like the use of magic numbers (23 and 16 in the fragment above).
> Whilst it is fairly obvious where they come from, it is entirely possible
> that a future coder might change one of the strings and not change (all
> of) the places where the string's length is used. So instead I would
> suggest using constants instead, eg:
>
> static const char * egr = "experimental-gcs-report";
> const int egr_len = strlen (egr);
>
> else if (strncmp (optarg, egr, egr_len) == 0)
> {
> if (strlen (optarg) == egr_len || strcmp (optarg + egr_len,
> "=none") == 0)
> gcs_report = GCS_NONE;
> else if (strcmp (optarg + egr_len, "=warning") == 0)
> gcs_report = GCS_WARN;
> else if (strcmp (optarg + egr_len, "=error") == 0)
> gcs_report = GCS_ERROR;
> else
> einfo (_("%P: error: unrecognized: `%s'\''\n"), optarg);
> }
>
> Cheers
> Nick
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcs_linker.patch
Type: text/x-patch
Size: 35720 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20240130/ea4ae38f/attachment-0001.bin>
More information about the Binutils
mailing list