Implement cet-report as supported in binutils.
bti-report has the same behaviour for AArch64-BTI.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I'm in favour of adding the additional flag. It is useful when an effort has been made to get all input files marked, and we want to make sure that the program stays that way. I think that complements the existing force-bti flag which is useful when transitioning a program into compatibility.
One thought is that this could also be useful for ibt. Is there scope for -z fail-if-not=bti and -z fail-if-not=ibt which might make this more extensible for future property flags?
-z force-bti with -Werror has the same behaviour but not always possible to use -Werror and dangerous to rely on it.
GCC/Clang support -Werror. ld.bfd/gold/ld.lld support --fatal-warnings.
Does this have the same semantics as -z cet-report=error? (https://bugs.llvm.org/show_bug.cgi?id=45483)
If yes, we can use a similar tri-state option.
BTW, how do we handle object files created by objcopy -I binary -O elfXXX a.txt a.o or ld -b binary a.txt -o a.o? Such object files don't have .note.gnu.property
It does look very similar, the -z cet-report=error also includes a warning state, whereas -z fail-if-not=bti only has an error state. Will have to let Daniel comment on that.
IIUC this is creating an ELF object from binary or loading a binary directly (ld -b). In those particular cases we would not set the property in the ELF image and BTI would not be enabled, which would mean the program would run but not have the BTI protection. In theory the --force-bti option could be used in that circumstance. I don't have a good suggestion for a more automatic solution right now. My expectation is that incorporating a binary would be rare in a Linux/Android environment.
-z bti-report=[none,warning,error] sounds good to me. I'll add then -z cet-report too.
One small comment on the error message for the command line checking, but otherwise looks good to me.
lld/ELF/Driver.cpp | ||
---|---|---|
1228 | typo: recognized I think it would be worth reporting option.second in some way so that they know that the option was matched, it just had a bad parameter. "-z " + toString(reportArg.first) + "=, parameter " + option.second + " is not recognized" |
lld/ELF/Config.h | ||
---|---|---|
233 | ||
lld/ELF/Driver.cpp | ||
375 | remove braces consider using && | |
981 | ||
1220 | delete blank line | |
1228 | Use Twine to concatenate the message. | |
2137 | ||
2179 | This condition is untested. You may need CHECK-EMPTY: https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-empty-directive | |
2184–2186 | This condition is untested. |
lld/ELF/Config.h | ||
---|---|---|
233 | llvm::StringRef -> StringRef could be applied to the whole file. |
lld/ELF/Config.h | ||
---|---|---|
233 | ||
lld/ELF/Driver.cpp | ||
375 | not addressed | |
1161 | use member initializer to replace assignment here. | |
1228 | The diagnostic is untested. (So when I mentioned -z cet-report=, I did have a plan to implement the x86 part by myself...) | |
lld/test/ELF/i386-feature-cet.s | ||
26 | -o /dev/null if the output is unused | |
lld/test/ELF/x86-64-feature-cet.s | ||
29 | # REPORT_FORCE-EMPTY: | |
34 | Add # CE_REPORT_WARN-EMPTY: below |
Out of town for ~10 days. Sorry for the long delay.
LGTM.
lld/ELF/Config.h | ||
---|---|---|
233 | Seems better to group the two options with other llvm::StringRef (above). | |
lld/ELF/Driver.cpp | ||
981 | ||
1220 | If you swap the two loops you can call split only once for each -z option. for (opt::Arg *arg : args.filtered(OPT_z)) { .. split if (option.first == "bti-report" || option.first == "cet-report") { if (!isValidReportString(option.second)) ... if (option.first == "bti-report") config->zBtiReport = ... else ... } } Perhaps open coding | |
lld/docs/ld.lld.1 | ||
706 | .Cm none is the default, | |
710 | .Cm none is the default, Use man docs/ld.lld.1 to view the file to ensure good formatting. |