AaronBallman wrote: > > Thank you for your patience! > > In general, I think adding a new qualifier to Clang needs to come with > > significant justification for the implementation and maintenance costs. In > > this specific case, those costs are somewhat amortized because we already > > have the `__ptrauth` qualifier and this piggybacks off that work. But I > > don't think the benefits justify the costs: if the user writes `__ptrauth` > > on an integer type, we can determine whether that type is "good enough" or > > not and issue a diagnostic for invalid or suspicious uses, same as if the > > user spelled it `__ptrauth_restricted_intptr`. > > It was a very 50/50 decision to have a different spelling at the time, so I'm > fine with dropping this, I just need to do some work to determine how to do > the feature checks nicely.
Excellent, thank you! > > The implementation already has to update typechecking in various places > > regardless of which way we spell the qualifiers, so there should not be too > > much extra implementation burden from only using one qualifier. > > The implementation actually becomes simpler - it removes a bunch of work for > debugging, and the type checking itself is just "__ptrauth only applies to > pointers and __ptrauth_restricted_intptr only applies to integers". > > > So on balance, I think we should only upstream the single qualifier and use > > a header file + feature tests for compatibility with older compilers. Is > > that something you can get behind? (You're still the expert in this space, > > so if I'm trivializing concerns, please speak up!) > > @AaronBallman we might need a feature check -- maybe (please help with what > it should be called, I'm terrible at naming) `__ptrauth_can_haz_intptrs`? > It's a little weird as I think support for the ptrauth qualifier has not yet > been in any official clang release, so from an end user's point of view > `__ptrauth` will always have worked on pointer sized integers. It's possible > that we might actually be able to manage by assuming that the lack of the > `__ptrauth_restricted_intptr` qualifier means `__ptrauth` works on pointers > and pointer sized integers but it will require some checking first. > > In the mean time I'm going to update this PR to remove the new spelling and > related shenanigans, and then update tests. Would it make sense to add the feature macro to your downstream instead? e.g., downstream supports `__has_feature(ptrauth_restricted_intptr)` and upstream would return `false` for it. https://github.com/llvm/llvm-project/pull/137580 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
