================
@@ -3699,13 +3699,17 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc,
StringRef AttrStr) {
<< Unsupported << None << CurFeature << Target;
}
+ if (ParsedAttrs.BranchProtection.empty()) {
+ if (!ParsedAttrs.SignReturnAddrHardening.empty())
+ Diag(LiteralLoc, diag::warn_attribute_harden_pac_ret_requires_pac_ret);
+ return false;
----------------
atrosinenko wrote:
Oh, this should probably use `return`:
```suggestion
if (!ParsedAttrs.SignReturnAddrHardening.empty())
return Diag(LiteralLoc,
diag::warn_attribute_harden_pac_ret_requires_pac_ret);
return false;
```
As far as I can see, `SemaBase::Diag` returns `Sema::SemaDiagnosticBuilder`
which is converted to boolean `true` iff the diagnostic is "immediate" (which
is always the case except for some CUDA code). The only caller of
`Sema::checkTargetAttr` is static
[`handleTargetAttr`](https://github.com/llvm/llvm-project/blob/33cc998157406abe5c1319e1532bd401dd0aafb6/clang/lib/Sema/SemaDeclAttr.cpp#L3741)
function in the same file that uses the returned value to decide whether to
skip the `target("...")` attribute.
Consider the following example:
```cpp
int f1() { return 1; }
__attribute__((target("branch-protection=bti")))
int f2() { return 1; }
__attribute__((target("cpu=neoverse-v2,branch-protection=pac-ret,harden-pac-ret=load-return-address")))
int f3() { return 1; }
__attribute__((target("cpu=neoverse-v2,branch-protection=bti,harden-pac-ret=load-return-address")))
int f4() { return 1; }
__attribute__((target("cpu=neoverse-v2,harden-pac-ret=load-return-address")))
int f5() { return 1; }
```
Here, `f1` should have a short list of attributes at the LLVM IR level, `f2`
should have the same attributes plus `"branch-target-enforcement"`. `f3` should
have pac-ret and pac-ret hardening enabled and have a long list of supported
extensions in its `"target-features"="..."` attribute. Considering `f4` and
`f5`, according to the warning text, I would expect them to have the same set
of function attributes as `f0` has (because of `'target' attribute ignored`).
This is actually the case for `f4`, but not for `f5`, even though the same
warning is emitted for both
```
/temp/test.c:9:23: warning: 'harden-pac-ret' attribute requires
'branch-protection=pac-ret'; 'target' attribute ignored [-Wignored-attributes]
9 |
__attribute__((target("cpu=neoverse-v2,branch-protection=bti,harden-pac-ret=load-return-address")))
| ^
/temp/test.c:12:23: warning: 'harden-pac-ret' attribute requires
'branch-protection=pac-ret'; 'target' attribute ignored [-Wignored-attributes]
12 |
__attribute__((target("cpu=neoverse-v2,harden-pac-ret=load-return-address")))
| ^
```
PS: This probably requires adding another test case, as the existing ones pass
whether `return` is inserted or not.
https://github.com/llvm/llvm-project/pull/176171
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
