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 ↗

(On Diff #77352)

Why is this necessary ?

817 ↗

(On Diff #77352)

dito

[maleadt]

tools/llvm-c-test/echo.cpp

382 ↗

(On Diff #77352)

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 ↗

(On Diff #77352)

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

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

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))

test/Bindings/llvm-c/callsite_attributes.ll

+; RUN: llvm-as %s -o %t.bc
+; RUN: llvm-c-test --test-callsite-attributes < %t.bc
+; This used to segfault
+
+define void @Y() {
+    ret void
+}
+
+define void @X() {
+    call void @X()
+    ret void
+}

test/Bindings/llvm-c/function_attributes.ll

+; RUN: llvm-as %s -o %t.bc
+; RUN: llvm-c-test --test-function-attributes < %t.bc
+; This used to segfault
+
+define void @X() {
+    ret void
+}

tools/llvm-c-test/CMakeLists.txt

if(USE_LLVM_DYLIB)
   set(LLVM_LINK_COMPONENTS)
 endif()
 
 if (LLVM_COMPILER_IS_GCC_COMPATIBLE)
   set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 -Wstrict-prototypes")
 endif ()
 
 add_llvm_tool(llvm-c-test
+  attributes.c
   calc.c
   diagnostic.c
   disassemble.c
   echo.cpp
   helpers.c
   include-all.c
   main.c
   module.c
   metadata.c
   object.c
   targets.c
   )
 
 if(USE_LLVM_DYLIB)
   target_link_libraries(llvm-c-test LLVM)
 endif()

tools/llvm-c-test/attributes.c

+/*===-- attributes.c - tool for testing libLLVM and llvm-c API ------------===*\
+|*                                                                            *|
+|*                     The LLVM Compiler Infrastructure                       *|
+|*                                                                            *|
+|* This file is distributed under the University of Illinois Open Source      *|
+|* License. See LICENSE.TXT for details.                                      *|
+|*                                                                            *|
+|*===----------------------------------------------------------------------===*|
+|*                                                                            *|
+|* This file implements the --test-attributes and --test-callsite-attributes  *|
+|* commands in llvm-c-test.                                                   *|
+|*                                                                            *|
+\*===----------------------------------------------------------------------===*/
+
+#include "llvm-c-test.h"
+
+#include <stdlib.h>
+
+int llvm_test_function_attributes(void) {
+  LLVMEnablePrettyStackTrace();
+
+  LLVMModuleRef M = llvm_load_module(false, true);
+
+  LLVMValueRef F = LLVMGetFirstFunction(M);
+  while (F) {
+    // Read attributes
+    for (int Idx = LLVMAttributeFunctionIndex, ParamCount = LLVMCountParams(F);
+         Idx <= ParamCount; ++Idx) {
+      int AttrCount = LLVMGetAttributeCountAtIndex(F, Idx);
+      LLVMAttributeRef *Attrs = (LLVMAttributeRef*) malloc(AttrCount * sizeof(LLVMAttributeRef));
+      LLVMGetAttributesAtIndex(F, Idx, Attrs);
+      free(Attrs);
+    }
+    F = LLVMGetNextFunction(F);
+  }
+
+  LLVMDisposeModule(M);
+
+  return 0;
+}
+
+int llvm_test_callsite_attributes(void) {
+  LLVMEnablePrettyStackTrace();
+
+  LLVMModuleRef M = llvm_load_module(false, true);
+
+  LLVMValueRef F = LLVMGetFirstFunction(M);
+  while (F) {
+    LLVMBasicBlockRef BB;
+    for (BB = LLVMGetFirstBasicBlock(F); BB;
+         BB = LLVMGetNextBasicBlock(BB)) {
+      LLVMValueRef I;
+      for (I = LLVMGetFirstInstruction(BB); I;
+           I = LLVMGetNextInstruction(I)) {
+        if (LLVMIsACallInst(I)) {
+          // Read attributes
+          for (int Idx = LLVMAttributeFunctionIndex, ParamCount = LLVMCountParams(F);
+               Idx <= ParamCount; ++Idx) {
+            int AttrCount = LLVMGetCallSiteAttributeCount(I, Idx);
+            LLVMAttributeRef *Attrs = (LLVMAttributeRef*) malloc(AttrCount * sizeof(LLVMAttributeRef));
+            LLVMGetCallSiteAttributes(I, Idx, Attrs);
+            free(Attrs);
+          }
+        }
+      }
+    }
+
+    F = LLVMGetNextFunction(F);
+  }
+
+  LLVMDisposeModule(M);
+
+  return 0;
+}

tools/llvm-c-test/llvm-c-test.h

 int llvm_targets_list(void);
 
 // echo.c
 int llvm_echo(void);
 
 // diagnostic.c
 int llvm_test_diagnostic_handler(void);
 
+// attributes.c
+int llvm_test_function_attributes();
+int llvm_test_callsite_attributes();
+
 #ifdef __cplusplus
 }
 #endif /* !defined(__cplusplus) */
 
 #endif

tools/llvm-c-test/main.c

int main(int argc, char **argv) {
   } else if (argc == 2 && !strcmp(argv[1], "--disassemble")) {
     return llvm_disassemble();
   } else if (argc == 2 && !strcmp(argv[1], "--calc")) {
     return llvm_calc();
   } else if (argc == 2 && !strcmp(argv[1], "--add-named-metadata-operand")) {
     return llvm_add_named_metadata_operand();
   } else if (argc == 2 && !strcmp(argv[1], "--set-metadata")) {
     return llvm_set_metadata();
+  } else if (argc == 2 && !strcmp(argv[1], "--test-function-attributes")) {
+    return llvm_test_function_attributes();
+  } else if (argc == 2 && !strcmp(argv[1], "--test-callsite-attributes")) {
+    return llvm_test_callsite_attributes();
   } else if (argc == 2 && !strcmp(argv[1], "--echo")) {
     return llvm_echo();
   } else if (argc == 2 && !strcmp(argv[1], "--test-diagnostic-handler")) {
     return llvm_test_diagnostic_handler();
   } else {
     print_usage();
   }
 
   return 1;
 }