Prevent nullptr dereferences in C API for counting attributes.

[maleadt]

Summary

If the function or instruction doesn't have any attributes, AttributeSetNode::get returns a null pointer.

[maleadt]

Fixed similar issue with the AttributeRef getters.

[deadalnix]

tools/llvm-c-test/echo.cpp

382

Why is this necessary ?

817

dito

[maleadt]

tools/llvm-c-test/echo.cpp

382

It isn't, I added it as a means of a test, as it segfaults without the proposed nullptr checks.
Or where should I put tests for the C API (first time contributor here, forgive my ignorance)?

[deadalnix]

tools/llvm-c-test/echo.cpp

382

Yes but on the other hand, it doesn't test this case for all the functions inside the test. I think this is the wrong tradeof.

However, that'd be great to add tests for these function somewhere else.

[maleadt]

Move tests in their own attributes unit test file.

[maleadt]

Format the new tests.

[deadalnix]

tools/llvm-c-test/main.c

91–94 ↗

(On Diff #77990)

I would create just one, but if you think that's better that way, I'm good with it.

[maleadt]

tools/llvm-c-test/main.c

91–94 ↗

(On Diff #77990)

No real preference at my side. I modeled it after the metadata tests, which also seemed pretty fine grained.

[maleadt]

I don't have commit access, could somebody land this? Thanks!

[deadalnix]

Ok, let me merge this.

lib/IR/Core.cpp

 
 void LLVMAddAttributeAtIndex(LLVMValueRef F, LLVMAttributeIndex Idx,
                              LLVMAttributeRef A) {
   unwrap<Function>(F)->addAttribute(Idx, unwrap(A));
 }
 
 unsigned LLVMGetAttributeCountAtIndex(LLVMValueRef F, LLVMAttributeIndex Idx) {
   auto *ASN = AttributeSetNode::get(unwrap<Function>(F)->getAttributes(), Idx);
+  if (!ASN)
+    return 0;
   return ASN->getNumAttributes();
 }
 
 void LLVMGetAttributesAtIndex(LLVMValueRef F, LLVMAttributeIndex Idx,
                               LLVMAttributeRef *Attrs) {
   auto *ASN = AttributeSetNode::get(unwrap<Function>(F)->getAttributes(), Idx);
+  if (!ASN)
+    return;
   for (auto A: make_range(ASN->begin(), ASN->end()))
     *Attrs++ = wrap(A);
 }
 
 LLVMAttributeRef LLVMGetEnumAttributeAtIndex(LLVMValueRef F,
                                              LLVMAttributeIndex Idx,
                                              unsigned KindID) {
   return wrap(unwrap<Function>(F)->getAttribute(Idx,
void LLVMAddCallSiteAttribute(LLVMValueRef C, LLVMAttributeIndex Idx,
                               LLVMAttributeRef A) {
   CallSite(unwrap<Instruction>(C)).addAttribute(Idx, unwrap(A));
 }
 
 unsigned LLVMGetCallSiteAttributeCount(LLVMValueRef C,
                                        LLVMAttributeIndex Idx) {
   auto CS = CallSite(unwrap<Instruction>(C));
   auto *ASN = AttributeSetNode::get(CS.getAttributes(), Idx);
+  if (!ASN)
+    return 0;
   return ASN->getNumAttributes();
 }
 
 void LLVMGetCallSiteAttributes(LLVMValueRef C, LLVMAttributeIndex Idx,
                                LLVMAttributeRef *Attrs) {
   auto CS = CallSite(unwrap<Instruction>(C));
   auto *ASN = AttributeSetNode::get(CS.getAttributes(), Idx);
+  if (!ASN)
+    return;
   for (auto A: make_range(ASN->begin(), ASN->end()))
     *Attrs++ = wrap(A);
 }
 
 LLVMAttributeRef LLVMGetCallSiteEnumAttribute(LLVMValueRef C,
                                               LLVMAttributeIndex Idx,
                                               unsigned KindID) {
   return wrap(CallSite(unwrap<Instruction>(C))

tools/llvm-c-test/echo.cpp

LLVMValueRef CloneValue(LLVMValueRef Src) {
     LLVMDisposeBuilder(Builder);
     return Dst;
   }
 
   void CloneAttrs(LLVMValueRef Src, LLVMValueRef Dst) {
     auto Ctx = LLVMGetModuleContext(M);
     int ArgCount = LLVMGetNumArgOperands(Src);
     for (int i = LLVMAttributeReturnIndex; i <= ArgCount; i++) {
+      if (LLVMGetCallSiteAttributeCount(Src, i) > 0) {
         for (unsigned k = 0, e = LLVMGetLastEnumAttributeKind(); k < e; ++k) {
           if (auto SrcA = LLVMGetCallSiteEnumAttribute(Src, i, k)) {
             auto Val = LLVMGetEnumAttributeValue(SrcA);
             auto A = LLVMCreateEnumAttribute(Ctx, k, Val);
             LLVMAddCallSiteAttribute(Dst, i, A);
           }
         }
       }
     }
+  }
 
   LLVMValueRef CloneInstruction(LLVMValueRef Src, LLVMBuilderRef Builder) {
     check_value_kind(Src, LLVMInstructionValueKind);
     if (!LLVMIsAInstruction(Src))
       report_fatal_error("Expected an instruction");
 
     const char *Name = LLVMGetValueName(Src);
 
while (true) {
     if (LLVMGetNamedFunction(M, Name))
       report_fatal_error("Function already cloned");
     auto Ty = LLVMGetElementType(TypeCloner(M).Clone(Cur));
     auto F = LLVMAddFunction(M, Name, Ty);
 
     // Copy attributes
     for (int i = LLVMAttributeFunctionIndex, c = LLVMCountParams(F);
          i <= c; ++i) {
+      if (LLVMGetAttributeCountAtIndex(Cur, i) > 0) {
         for (unsigned k = 0, e = LLVMGetLastEnumAttributeKind(); k < e; ++k) {
           if (auto SrcA = LLVMGetEnumAttributeAtIndex(Cur, i, k)) {
             auto Val = LLVMGetEnumAttributeValue(SrcA);
             auto DstA = LLVMCreateEnumAttribute(Ctx, k, Val);
             LLVMAddAttributeAtIndex(F, i, DstA);
           }
         }
       }
+    }
 
     Next = LLVMGetNextFunction(Cur);
     if (Next == nullptr) {
       if (Cur != End)
         report_fatal_error("Last function does not match End");
       break;
     }