Skip to content

Conversation

@xdoardo
Copy link
Collaborator

@xdoardo xdoardo commented Nov 24, 2025

Depending on the alignment of the types of sealed globals, the known-bits optimisation pass may infer that the lower bits of a sealed pointer to those globals are zeroed, and optimise away logical computations such as

%0 = call i32 @llvm.cheri.cap.address.get.i32(ptr addrspace(200) @secondHeap) ; @secondHeap = linkonce_odr dso_local addrspace(200) global ... section ".sealed_objects", comdat, align 8 #0
store i32 %0, ptr addrspace(200) %addr, align 4
%2 = load i32, ptr addrspace(200) %addr, align 4
%and = and i32 %2, 7 ; -> statically optimised to 0

This PR makes Clang generate a call to the llvm.launder intrinsic to obtain a new pointer value that carries fresh invariant group information, specifically when lowering the __builtin_cheri_address_get intrinsic when the argument is a sealed pointer.

This PR also adds a simple test in the more general test on sealed global values, in order to keep this behaviour stable.

@xdoardo xdoardo requested a review from resistor November 24, 2025 15:42
@resistor
Copy link
Collaborator

After thinking on this a bit, I think we should add a new intrinsic (something like @llvm.launder.alignment()) rather than reusing @llvm.launder.invariant.group. That intrinsic happens to work for the moment, but it's not defined to block alignment inference, and I suspect that it would not be acceptable to upstream to land that usage.

For reference, here's the code that defines @llvm.launder.invariant.group:

def int_launder_invariant_group : DefaultAttrsIntrinsic<[llvm_anyptr_ty],

And here is where it gets codegen'd:
case Intrinsic::launder_invariant_group:

We should be able to simply create another intrinsic that is handled exactly like it in those two locations, and we'll be set.

@xdoardo
Copy link
Collaborator Author

xdoardo commented Nov 25, 2025

feaed6c adds the new intrinsic. That commit also changes the previous use of the other launder intrinsic to use this new one and fixes the test I patched in the previous commit.

The addition of the new intrinsic in the ValueTracking pass is so that nonnull qualifiers are kept between the argument passed to the new intrinsic and the result.

I haven't added any documentation to the new intrinsic to the .td file because I guess that adding a new intrinsic should mean adding more documentation than that, which will come in another commit, as soon as I figure out where the documentation should be.

@resistor
Copy link
Collaborator

The addition of the new intrinsic in the ValueTracking pass is so that nonnull qualifiers are kept between the argument passed to the new intrinsic and the result.

Is it the non-nullness inference correct if we're stashing information in the low bits?

@resistor
Copy link
Collaborator

If we're confident in the nonnullness answer, the rest of this looks fine to me.

@xdoardo
Copy link
Collaborator Author

xdoardo commented Nov 26, 2025

I think it makes sense to keep the annotation if the object we want to observe the address of has the annotation. In the case shown in the example, the object is a global so I think LLVM automatically knows that the pointer must be non null. Adding values to the low bits should not invalidate this assumption, even more so if we take into account that, as said in the langref (no link to that qualifier in particular), nonnull does not imply dereferenceable.

Of course, we shouldn't add nonnull when we don't know that the object given to the intrinsic is non null, and this is respected by this patch. For example:

int unknown_sealed(void *__sealed_capability obj) {
  return __builtin_cheri_address_get(obj);
}

is translated to

; Function Attrs: minsize mustprogress nofree nosync nounwind optsize willreturn memory(inaccessiblemem: readwrite)
define dso_local i32 @unknown_sealed(ptr addrspace(200) noundef readnone %obj) local_unnamed_addr addrspace(200) #5 {
entry:
  %0 = tail call ptr addrspace(200) @llvm.launder.alignment.p200(ptr addrspace(200) %obj)
  %1 = tail call i32 @llvm.cheri.cap.address.get.i32(ptr addrspace(200) %0)
  ret i32

Let me know if I am missing something.

@xdoardo xdoardo force-pushed the launder-sealed-globals branch from b68b76d to 8ca4c09 Compare November 26, 2025 08:50
…it to lower __builtin_cheri_address_get on sealed pointers
@xdoardo xdoardo force-pushed the launder-sealed-globals branch from 8ca4c09 to 249baf9 Compare November 26, 2025 14:36
@xdoardo xdoardo merged commit 9ac3c7c into CHERIoT-Platform:cheriot Nov 26, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants