MaggieYi added a comment.
Thanks @simon_tatham and @MaskRay for the quick code review.
When I work on this issue, I want to add an error for both clang and clang
-cc1.
`-mexecute-only` is an ARM-only compiler option. The clang Driver will convert
`-mexecute-only` to `-target-feature +execute-only` in the clang cc1 command.
To pass the `execute-only` option, the clang command is: `clang
-target=armv6t2-eabi -mexecute-only …`. The Arm clang cc1 command:
`clang -cc1 -triple armv6t2-unknown-unknown-eabi -target-feature
+execute-only…`.
If I move the change to the code in Driver/ToolChains/Arch/ARM.cpp, the error
will only deal with the `-mexecute-only` option, but not handle the
`-target-feature +execute-only` in the `clang -cc1` command.
As @MaskRay commented that we don't perform rigid error checking for cc1. In
this case, we could handle the error in the Driver/ToolChains/Arch/ARM.cpp.
However, the target-specific predicate function (named isExecuteOnlyTarget)
allows any other targets that support execute-only could get the effect by
modifying just the `isExecuteOnlyTarget` function. Therefore, I have modified
the `isExecuteOnlyTarget` function to only deal with the `-mexecute-only`
option. I have also added a function (named ARM::supportedExecuteOnly) to avoid
the duplicated code.
================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:326
def err_unsupported_abi_for_opt : Error<"'%0' can only be used with the '%1'
ABI">;
+def err_unsupported_opt_for_execute_only_target
+ : Error<"unsupported option '%0' for the execute only target '%1'">;
----------------
MaskRay wrote:
> We don't need this diagnostic as a common kind (we only use it in driver).
>
> I think we can reuse `err_drv_argument_not_allowed_with` . Though for PS5 you
> will get `... allowed with '-mexecute-only'` even if the user doesn't specify
> `-mexecute-only`, but I hope it is fine.
Since `err_drv_argument_not_allowed_with` is an ARM-only option, We cannot
reuse it.
================
Comment at: clang/lib/Driver/SanitizerArgs.cpp:406
+ if (SanitizerMask KindsToDiagnose =
+ Add & NotAllowedWithExecuteOnly & ~DiagnosedKinds) {
+ if (DiagnoseErrors)
----------------
MaskRay wrote:
> I think it's clear not not to add the variable `NotAllowedWithExecuteOnly`.
>
> Currently, I need to check the definition of `NotAllowedWithExecuteOnly` to
> understand that this comment does what it says. For now, just encoding
> `Function` here is clearer.
`NotAllowedWithExecuteOnly` is modified to include the `SanitizerKind::KCFI`,
therefore I keep it in the code.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158614/new/
https://reviews.llvm.org/D158614
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
