samitolvanen added a comment. In D119296#3684022 <https://reviews.llvm.org/D119296#3684022>, @joaomoreira wrote:
> Some minor suggestions/not so relevant: > > - Change the name of CFIType to CFIHash, as it is probably more descriptive. My thinking here was that Type is more generic than a Hash, but I don't really have a strong opinion about this. > - Change the KCFI.* into CFI.* where the passes/structures can be re-used by > a different CFI approach. For example, X86KCFI::emitCheck can be easily > re-used by other CFI schemes. Similarly, the LowerKCFI_CHECK function can > also be easily re-used. Because these other approaches are hypothetical by > now, this is not a big deal... but may be subject to changes in the future, > in case someone comes up with new schemes. Sure, it should be trivial for a future patch that wants to reuse this code to rename the functions and classes as needed. > - Deny the ENDBR64/32 opcodes as valid hashes for the CFIType. There was an > effort in the past to prevent it from being emitted as an immediate, thus, > perhaps, we should also care. Since FineIBT may inherit hashes from KCFI, it > would be great to prevent these from becoming valid indirect targets or > needing to be checked. Good point. It should be quite unlikely that we'll end up randomly generating ENDBR as a hash, but I added a helper to the X86 code to ensure we don't end up emitting these values. > Some relevant optimizations/improvements for future work: > > - When LTO is used, hashes may be suppressed for non-local + non-address > taken functions. > - As suggested by Andy Cooper, add a salt attribute that allows to > differentiate hashes of functions with the same prototype. Agreed, and another request from Peter Z was to add a flag to omit ENDBR from functions with the `!kcfi_type` attribute. I think these can all be separate follow-up patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119296/new/ https://reviews.llvm.org/D119296 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
