[PATCH] D90868: [IR] Define @llvm.ptrauth intrinsics.

John McCall via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 26 14:20:53 PDT 2021


rjmccall added inline comments.


================
Comment at: llvm/docs/PointerAuth.md:135
+If ``value`` does not have a correct signature for ``key`` and ``extra data``,
+the behavior is undefined.
+
----------------
ab wrote:
> rjmccall wrote:
> > ab wrote:
> > > kristof.beyls wrote:
> > > > I'm not entirely sure if we'd like to say behavior is undefined when the signature isn't valid.
> > > > If the "undefined" here means the same as "undefined behavior" in C/C++, wouldn't that allow the compiler to assume that it could never happen that the signature isn't valid, and e.g. optimize away signature checks based on that.
> > > > I assume that kind of behavior may be mostly theoretical at the moment, but it probably would be better to not enable elimination of signature checks by the compiler, even if only theoretical.
> > > > That being said, I'm not sure what the correct description should be for the behavior. For now, I can't come up with anything better than:
> > > > 
> > > > ```
> > > > If ``value`` does not have a correct signature for ``key`` and ``extra data``,
> > > > the behavior is a target-specific side effect.
> > > > ```
> > > That's an interesting question.  Since this was written, we've converged to making it a hard requirement for auth/resign operations to trap.  In our implementation, there's currently still an escape hatch (via cl::opt and attribute) to disable that, but we're planning to get rid of that.  Whether we want that upstream or not is an open question (really for all of you): it can helps with bringup, but with FPAC maybe we're better off always forcing the pre-FPAC codegen to have the SW check/trap.
> > > 
> > > Regarding:
> > > > If the "undefined" here means the same as "undefined behavior" in C/C++, wouldn't that allow the compiler to assume that it could never happen that the signature isn't valid, and e.g. optimize away signature checks based on that.
> > > 
> > > yes, it does allow that, and our implementation does optimize away auth/resigns with unused results.
> > > Which brings up an interesting point: since we trap in the auth intrinsic itself, using it to check the signature is at best unergonomic (you'd have to do catch the trap somehow).  So, for cases where the intention is only to check the signature of a pointer without using it, we've been using a different pattern: strip the original pointer, sign it with the expected scheme, and compare that with the original pointer.  That's a little obscure (and has some hardening ramifications), so we've considered adding an intrinsic that does that.  But it hasn't been a common pattern.
> > > 
> > > So, concretely, I'd be in favor of explicitly defining this (and `resign`) as trapping.  "target-specific side effect" sounds reasonable and avoids over-specifying it as well, though I don't have a specific worry in mind with describing the trap.
> > I think it would be good to reserve the right to not trap if the result is unused, but otherwise, yes, I agree that this should not have undefined behavior on invalidly-signed inputs.
> I mentioned that they are still defined to be side-effect-free (matching their IntrNoMem definition in Intrinsics.td) and can be eliminated.  How does that sound?
SGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90868/new/

https://reviews.llvm.org/D90868



More information about the llvm-commits mailing list