olista01 added inline comments.
================
Comment at: lib/CodeGen/CGDeclCXX.cpp:364
if (RASignKind != CodeGenOptions::SignReturnAddressScope::None)
+ {
Fn->addFnAttr("sign-return-address",
----------------
LLVM style has the opening brace on the same line as the if. There's a
git-clang-format script in the clang repo which can fix up style issues like
this for a whole patch, changing touching lines you haven't changes.
================
Comment at: lib/CodeGen/CGDeclCXX.cpp:374
+ : "b_key");
+ }
+
----------------
I think we need to handle branch-target-enforcement here too.
================
Comment at: lib/CodeGen/TargetInfo.cpp:4982
+ if (Kind != CodeGenOptions::SignReturnAddressScope::None)
+ {
+ Fn->addFnAttr("sign-return-address",
----------------
Style nit: brace on same line as if.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:1433
+static std::tuple<StringRef, StringRef, bool>
+ParseAArch64BranchProtection(const Driver &D, const ArgList &Args, const Arg
*A) {
----------------
Please add a comment about what the parts of the return value are.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:1440
+ StringRef Value = A->getValue();
+ // This maps onto -mbranch-protection=<scope>+<key>
+
----------------
I'm not sure what this is trying to say. Is it referring to the "standard" case
below?
================
Comment at: lib/Driver/ToolChains/Clang.cpp:1448
+ } else if (!Value.equals("none")) {
+ SmallVector<StringRef, 3> BranchProtection;
+ StringRef(A->getValue()).split(BranchProtection, '+');
----------------
I'd make this 4, because that's the longest sensible argument
("pac-ret+leaf+b-key+bti").
================
Comment at: lib/Driver/ToolChains/Clang.cpp:1460
+ Scope = "non-leaf";
+ while (++Protection != BranchProtection.end()) {
+ if (Protection->equals("leaf"))
----------------
Add a comment about the fact that "leaf" and "b-key" must follow "pac-ret", to
explain why we don't just parse this in one loop.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:1543
+ if (A->getOption().matches(options::OPT_msign_return_address_EQ))
+ {
+ Scope = A->getValue();
----------------
Nit: brace on same line as if.
================
Comment at: test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp:7
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ALL
--check-prefix=CHECK-A-KEY
struct Foo {
----------------
This should also test branch target enforcement (it's also missing from the
code).
https://reviews.llvm.org/D51429
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
