From d2cc1576d6f2cbf1d782cf48f5adfc71aeb3415b Mon Sep 17 00:00:00 2001 From: David Teller Date: Wed, 26 Nov 2025 12:10:55 +0100 Subject: [PATCH 1/6] [Feature] We should be able to walk through the operands of a call with metadata arguments - resolves #633 --- src/values/basic_value_use.rs | 4 +- src/values/enums.rs | 4 +- src/values/instruction_value.rs | 27 ++++++++----- tests/all/test_instruction_values.rs | 59 +++++++++++++++++++++++++++- 4 files changed, 80 insertions(+), 14 deletions(-) diff --git a/src/values/basic_value_use.rs b/src/values/basic_value_use.rs index ce77b521638..05566113ddf 100644 --- a/src/values/basic_value_use.rs +++ b/src/values/basic_value_use.rs @@ -4,7 +4,7 @@ use llvm_sys::prelude::LLVMUseRef; use std::marker::PhantomData; use crate::basic_block::BasicBlock; -use crate::values::{AnyValueEnum, BasicValueEnum}; +use crate::values::{AnyValueEnum, BasicValueEnum, MetadataValue}; /// Either [BasicValueEnum] or [BasicBlock]. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -13,6 +13,8 @@ pub enum Operand<'ctx> { Value(BasicValueEnum<'ctx>), /// Represents a [BasicBlock]. Block(BasicBlock<'ctx>), + /// Represents a [MetadataValue]. + Metadata(MetadataValue<'ctx>), } impl<'ctx> Operand<'ctx> { diff --git a/src/values/enums.rs b/src/values/enums.rs index 4234ca1ee2d..bb9d8733b47 100644 --- a/src/values/enums.rs +++ b/src/values/enums.rs @@ -118,7 +118,7 @@ impl<'ctx> AnyValueEnum<'ctx> { AnyValueEnum::InstructionValue(InstructionValue::new(value)) }, LLVMTypeKind::LLVMMetadataTypeKind => panic!("Metadata values are not supported as AnyValue's."), - _ => panic!("The given type is not supported."), + other => panic!("The given type is not supported: {:?}", other), } } @@ -292,7 +292,7 @@ impl<'ctx> BasicValueEnum<'ctx> { LLVMTypeKind::LLVMScalableVectorTypeKind => { BasicValueEnum::ScalableVectorValue(ScalableVectorValue::new(value)) }, - _ => unreachable!("The given type is not a basic type."), + other => panic!("The given type is not a basic type: {:?}", other), } } diff --git a/src/values/instruction_value.rs b/src/values/instruction_value.rs index d0a12a334da..f0505f88aa9 100644 --- a/src/values/instruction_value.rs +++ b/src/values/instruction_value.rs @@ -3,17 +3,17 @@ use llvm_sys::core::LLVMGetGEPSourceElementType; use llvm_sys::core::{ LLVMGetAlignment, LLVMGetAllocatedType, LLVMGetFCmpPredicate, LLVMGetICmpPredicate, LLVMGetIndices, LLVMGetInstructionOpcode, LLVMGetInstructionParent, LLVMGetMetadata, LLVMGetNextInstruction, LLVMGetNumIndices, - LLVMGetNumOperands, LLVMGetOperand, LLVMGetOperandUse, LLVMGetPreviousInstruction, LLVMGetVolatile, - LLVMHasMetadata, LLVMInstructionClone, LLVMInstructionEraseFromParent, LLVMInstructionRemoveFromParent, - LLVMIsAAllocaInst, LLVMIsABasicBlock, LLVMIsAGetElementPtrInst, LLVMIsALoadInst, LLVMIsAStoreInst, - LLVMIsATerminatorInst, LLVMIsConditional, LLVMIsTailCall, LLVMSetAlignment, LLVMSetMetadata, LLVMSetOperand, - LLVMSetVolatile, LLVMValueAsBasicBlock, + LLVMGetNumOperands, LLVMGetOperand, LLVMGetOperandUse, LLVMGetPreviousInstruction, LLVMGetTypeKind, + LLVMGetVolatile, LLVMHasMetadata, LLVMInstructionClone, LLVMInstructionEraseFromParent, + LLVMInstructionRemoveFromParent, LLVMIsAAllocaInst, LLVMIsABasicBlock, LLVMIsAGetElementPtrInst, LLVMIsALoadInst, + LLVMIsAStoreInst, LLVMIsATerminatorInst, LLVMIsAValueAsMetadata, LLVMIsConditional, LLVMIsTailCall, + LLVMSetAlignment, LLVMSetMetadata, LLVMSetOperand, LLVMSetVolatile, LLVMTypeOf, LLVMValueAsBasicBlock, }; #[llvm_versions(10..)] use llvm_sys::core::{LLVMGetAtomicRMWBinOp, LLVMIsAAtomicCmpXchgInst, LLVMIsAAtomicRMWInst}; use llvm_sys::core::{LLVMGetOrdering, LLVMSetOrdering}; use llvm_sys::prelude::LLVMValueRef; -use llvm_sys::LLVMOpcode; +use llvm_sys::{LLVMOpcode, LLVMTypeKind}; use std::{ffi::CStr, fmt, fmt::Display}; @@ -684,14 +684,21 @@ impl<'ctx> InstructionValue<'ctx> { } let is_basic_block = unsafe { !LLVMIsABasicBlock(operand).is_null() }; - if is_basic_block { let bb = unsafe { BasicBlock::new(LLVMValueAsBasicBlock(operand)) }; - Some(Operand::Block(bb.expect("BasicBlock should always be valid"))) - } else { - Some(Operand::Value(unsafe { BasicValueEnum::new(operand) })) + return Some(Operand::Block(bb.expect("BasicBlock should always be valid"))); + } + + if unsafe { LLVMGetTypeKind(LLVMTypeOf(operand)) == LLVMTypeKind::LLVMMetadataTypeKind } { + // We may be dealing with null metadata, which would break memory invariants. + if LLVMIsAValueAsMetadata(operand).is_null() { + return None; + } + return Some(Operand::Metadata(MetadataValue::new(operand))); } + + Some(Operand::Value(unsafe { BasicValueEnum::new(operand) })) } /// Get an instruction value operand iterator. diff --git a/tests/all/test_instruction_values.rs b/tests/all/test_instruction_values.rs index d62d7840ae9..9b37e883cbd 100644 --- a/tests/all/test_instruction_values.rs +++ b/tests/all/test_instruction_values.rs @@ -1,11 +1,12 @@ use inkwell::context::Context; +use inkwell::debug_info::AsDIScope; #[cfg(not(feature = "typed-pointers"))] use inkwell::types::AnyType; use inkwell::types::{AnyTypeEnum, BasicType}; use inkwell::values::{BasicValue, CallSiteValue, InstructionOpcode::*}; #[llvm_versions(10..)] use inkwell::AtomicRMWBinOp; -use inkwell::{AddressSpace, AtomicOrdering, FloatPredicate, IntPredicate}; +use inkwell::{debug_info, AddressSpace, AtomicOrdering, FloatPredicate, IntPredicate}; #[test] #[ignore] @@ -661,6 +662,62 @@ fn test_metadata_kinds() { ]); } +#[test] +fn test_metadata_as_operand() { + // clang can introduce instructions such as + // call void @llvm.dbg.declare(metadata i32* %2, metadata !16, metadata !DIExpression()), !dbg !17 + // we want to make sure that looking at the operands works. + + let context = Context::create(); + let module = context.create_module("testing"); + + let void_type = context.void_type(); + let i32_type = context.i32_type(); + let fn_type = void_type.fn_type(&[], false); + let function = module.add_function("fun", fn_type, None); + let block = context.append_basic_block(function, "block"); + + let (debug_info_builder, compile_unit) = module.create_debug_info_builder( + true, + debug_info::DWARFSourceLanguage::C, + "test.ll", + "/tmp", + "test", + false, + "", + 0, + "split", + debug_info::DWARFEmissionKind::Full, + 0, + false, + false, + "/", + "18.1", + ); + + let address_space = AddressSpace::default(); + let debug_loc = debug_info_builder.create_debug_location(&context, 5, 32, compile_unit.as_debug_info_scope(), None); + #[allow(deprecated)] + let i32_ptr = i32_type.ptr_type(address_space).const_null(); + let di_i32_type = debug_info_builder.create_basic_type("i32", 32, 0, 0).unwrap(); + let var_info = debug_info_builder.create_auto_variable( + compile_unit.as_debug_info_scope(), + "var", + compile_unit.get_file(), + 42, + di_i32_type.as_type(), + false, + 0, + 32, + ); + let instruction = debug_info_builder.insert_declare_at_end(i32_ptr, Some(var_info), None, debug_loc, block); + assert_eq!(instruction.get_num_operands(), 4); + assert!(instruction.get_operand(0).is_some()); + assert!(instruction.get_operand(1).is_none()); + assert!(instruction.get_operand(2).is_none()); + assert!(instruction.get_operand(3).is_some()); +} + #[test] fn test_find_instruction_with_name() { use inkwell::context::Context; From 4ef5413767ab69a9f8e6283a00d8a5be014a1a2d Mon Sep 17 00:00:00 2001 From: David Teller Date: Fri, 28 Nov 2025 12:35:50 +0100 Subject: [PATCH 2/6] [QoL] Metadata shorthands methods for Operand --- src/values/basic_value_use.rs | 52 +++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/src/values/basic_value_use.rs b/src/values/basic_value_use.rs index 05566113ddf..342fb1e2a70 100644 --- a/src/values/basic_value_use.rs +++ b/src/values/basic_value_use.rs @@ -32,6 +32,13 @@ impl<'ctx> Operand<'ctx> { matches!(self, Self::Block(_)) } + /// Determines if the [Operand] is a [Metadata]. + #[inline] + #[must_use] + pub fn is_metadata(self) -> bool { + matches!(self, Self::Metadata(_)) + } + /// If the [Operand] is a [BasicValueEnum], map it into [Option::Some]. #[inline] #[must_use] @@ -52,6 +59,16 @@ impl<'ctx> Operand<'ctx> { } } + /// If the [Operand] is a [Metadata], map it into [Option::Some]. + #[inline] + #[must_use] + pub fn metadata(self) -> Option> { + match self { + Self::Metadata(md) => Some(md), + _ => None, + } + } + /// Expect [BasicValueEnum], panic with the message if it is not. #[inline] #[must_use] @@ -74,12 +91,27 @@ impl<'ctx> Operand<'ctx> { } } + /// Expect [Metadata], panic with the message if it is not. + #[inline] + #[must_use] + #[track_caller] + pub fn expect_metadata(self, msg: &str) -> MetadataValue<'ctx> { + match self { + Self::Metadata(md) => md, + _ => panic!("{msg}"), + } + } + /// Unwrap [BasicValueEnum]. Will panic if it is not. #[inline] #[must_use] #[track_caller] pub fn unwrap_value(self) -> BasicValueEnum<'ctx> { - self.expect_value("Called unwrap_value() on UsedValue::Block.") + match self { + Self::Value(value) => value, + Self::Block(_) => panic!("Called unwrap_value() on UsedValue::Block."), + Self::Metadata(_) => panic!("Called unwrap_value() on UsedValue::Metadata."), + } } /// Unwrap [BasicBlock]. Will panic if it is not. @@ -87,7 +119,23 @@ impl<'ctx> Operand<'ctx> { #[must_use] #[track_caller] pub fn unwrap_block(self) -> BasicBlock<'ctx> { - self.expect_block("Called unwrap_block() on UsedValue::Value.") + match self { + Self::Value(_) => panic!("Called unwrap_value() on UsedValue::Value."), + Self::Block(block) => block, + Self::Metadata(_) => panic!("Called unwrap_value() on UsedValue::Metadata."), + } + } + + /// Unwrap [Metadata]. Will panic if it is not. + #[inline] + #[must_use] + #[track_caller] + pub fn unwrap_metadata(self) -> MetadataValue<'ctx> { + match self { + Self::Value(_) => panic!("Called unwrap_value() on UsedValue::Value."), + Self::Block(_) => panic!("Called unwrap_value() on UsedValue::Block."), + Self::Metadata(md) => md, + } } } From c2c8b0b0739ddcf50343ffefe3ea07604c43f889 Mon Sep 17 00:00:00 2001 From: David Teller Date: Fri, 28 Nov 2025 22:46:28 +0100 Subject: [PATCH 3/6] Fixes, as requested --- src/values/basic_value_use.rs | 12 ++++++------ src/values/instruction_value.rs | 25 +++++++++++++++++++++---- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/values/basic_value_use.rs b/src/values/basic_value_use.rs index 342fb1e2a70..7007e00488f 100644 --- a/src/values/basic_value_use.rs +++ b/src/values/basic_value_use.rs @@ -109,8 +109,8 @@ impl<'ctx> Operand<'ctx> { pub fn unwrap_value(self) -> BasicValueEnum<'ctx> { match self { Self::Value(value) => value, - Self::Block(_) => panic!("Called unwrap_value() on UsedValue::Block."), - Self::Metadata(_) => panic!("Called unwrap_value() on UsedValue::Metadata."), + Self::Block(_) => panic!("Called unwrap_value() on Operand::Block."), + Self::Metadata(_) => panic!("Called unwrap_value() on Operand::Metadata."), } } @@ -120,9 +120,9 @@ impl<'ctx> Operand<'ctx> { #[track_caller] pub fn unwrap_block(self) -> BasicBlock<'ctx> { match self { - Self::Value(_) => panic!("Called unwrap_value() on UsedValue::Value."), + Self::Value(_) => panic!("Called unwrap_value() on Operand::Value."), Self::Block(block) => block, - Self::Metadata(_) => panic!("Called unwrap_value() on UsedValue::Metadata."), + Self::Metadata(_) => panic!("Called unwrap_value() on Operand::Metadata."), } } @@ -132,8 +132,8 @@ impl<'ctx> Operand<'ctx> { #[track_caller] pub fn unwrap_metadata(self) -> MetadataValue<'ctx> { match self { - Self::Value(_) => panic!("Called unwrap_value() on UsedValue::Value."), - Self::Block(_) => panic!("Called unwrap_value() on UsedValue::Block."), + Self::Value(_) => panic!("Called unwrap_value() on Operand::Value."), + Self::Block(_) => panic!("Called unwrap_value() on Operand::Block."), Self::Metadata(md) => md, } } diff --git a/src/values/instruction_value.rs b/src/values/instruction_value.rs index f0505f88aa9..38a78a78975 100644 --- a/src/values/instruction_value.rs +++ b/src/values/instruction_value.rs @@ -6,11 +6,13 @@ use llvm_sys::core::{ LLVMGetNumOperands, LLVMGetOperand, LLVMGetOperandUse, LLVMGetPreviousInstruction, LLVMGetTypeKind, LLVMGetVolatile, LLVMHasMetadata, LLVMInstructionClone, LLVMInstructionEraseFromParent, LLVMInstructionRemoveFromParent, LLVMIsAAllocaInst, LLVMIsABasicBlock, LLVMIsAGetElementPtrInst, LLVMIsALoadInst, - LLVMIsAStoreInst, LLVMIsATerminatorInst, LLVMIsAValueAsMetadata, LLVMIsConditional, LLVMIsTailCall, + LLVMIsAStoreInst, LLVMIsATerminatorInst, LLVMIsConditional, LLVMIsTailCall, LLVMSetAlignment, LLVMSetMetadata, LLVMSetOperand, LLVMSetVolatile, LLVMTypeOf, LLVMValueAsBasicBlock, }; #[llvm_versions(10..)] use llvm_sys::core::{LLVMGetAtomicRMWBinOp, LLVMIsAAtomicCmpXchgInst, LLVMIsAAtomicRMWInst}; +#[llvm_versions(17..)] +use llvm_sys::core::LLVMIsAValueAsMetadata; use llvm_sys::core::{LLVMGetOrdering, LLVMSetOrdering}; use llvm_sys::prelude::LLVMValueRef; use llvm_sys::{LLVMOpcode, LLVMTypeKind}; @@ -690,16 +692,31 @@ impl<'ctx> InstructionValue<'ctx> { return Some(Operand::Block(bb.expect("BasicBlock should always be valid"))); } + if let Some(metadata) = self.try_ingest_metadata(operand) { + return Some(Operand::Metadata(metadata)); + } + + + Some(Operand::Value(unsafe { BasicValueEnum::new(operand) })) + } + + #[llvm_versions(17..)] // LLVMIsAValueAsMetadata was introduced un 17.0. + unsafe fn try_ingest_metadata(self, operand: LLVMValueRef) -> Option> { + assert!(!operand.is_null()); if unsafe { LLVMGetTypeKind(LLVMTypeOf(operand)) == LLVMTypeKind::LLVMMetadataTypeKind } { // We may be dealing with null metadata, which would break memory invariants. if LLVMIsAValueAsMetadata(operand).is_null() { return None; } - return Some(Operand::Metadata(MetadataValue::new(operand))); + return Some(MetadataValue::new(operand)); } - - Some(Operand::Value(unsafe { BasicValueEnum::new(operand) })) + None } + #[llvm_versions(..17)] // LLVMIsAValueAsMetadata was introduced un 17.0. + unsafe fn try_ingest_metadata(self, operand: LLVMValueRef) -> Option> { + None + } + /// Get an instruction value operand iterator. pub fn get_operands(self) -> OperandIter<'ctx> { From fe2b6f5ee372864c68425ee019b7de0a2b012c97 Mon Sep 17 00:00:00 2001 From: David Teller Date: Sat, 29 Nov 2025 09:01:15 +0100 Subject: [PATCH 4/6] Clarifying message --- src/values/basic_value_use.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/values/basic_value_use.rs b/src/values/basic_value_use.rs index 7007e00488f..6960ff36746 100644 --- a/src/values/basic_value_use.rs +++ b/src/values/basic_value_use.rs @@ -6,7 +6,9 @@ use std::marker::PhantomData; use crate::basic_block::BasicBlock; use crate::values::{AnyValueEnum, BasicValueEnum, MetadataValue}; -/// Either [BasicValueEnum] or [BasicBlock]. +/// Either [BasicValueEnum], a [BasicBlock] or a [Metadata]. +/// +/// Note that [Metadata] variants are only constructed for LLVM 17+. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum Operand<'ctx> { /// Represents a [BasicValueEnum]. From d3b8bba74edfd8f1bd7706a08725ec773d89eab2 Mon Sep 17 00:00:00 2001 From: David Teller Date: Sat, 29 Nov 2025 10:37:43 +0100 Subject: [PATCH 5/6] Oops, the llvm version fix was broken, now it's unbroken --- src/values/instruction_value.rs | 32 +++++++++++++++++----------- tests/all/test_instruction_values.rs | 1 + 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/values/instruction_value.rs b/src/values/instruction_value.rs index 38a78a78975..adf459cab1b 100644 --- a/src/values/instruction_value.rs +++ b/src/values/instruction_value.rs @@ -3,19 +3,21 @@ use llvm_sys::core::LLVMGetGEPSourceElementType; use llvm_sys::core::{ LLVMGetAlignment, LLVMGetAllocatedType, LLVMGetFCmpPredicate, LLVMGetICmpPredicate, LLVMGetIndices, LLVMGetInstructionOpcode, LLVMGetInstructionParent, LLVMGetMetadata, LLVMGetNextInstruction, LLVMGetNumIndices, - LLVMGetNumOperands, LLVMGetOperand, LLVMGetOperandUse, LLVMGetPreviousInstruction, LLVMGetTypeKind, + LLVMGetNumOperands, LLVMGetOperand, LLVMGetOperandUse, LLVMGetPreviousInstruction, LLVMGetVolatile, LLVMHasMetadata, LLVMInstructionClone, LLVMInstructionEraseFromParent, LLVMInstructionRemoveFromParent, LLVMIsAAllocaInst, LLVMIsABasicBlock, LLVMIsAGetElementPtrInst, LLVMIsALoadInst, LLVMIsAStoreInst, LLVMIsATerminatorInst, LLVMIsConditional, LLVMIsTailCall, - LLVMSetAlignment, LLVMSetMetadata, LLVMSetOperand, LLVMSetVolatile, LLVMTypeOf, LLVMValueAsBasicBlock, + LLVMSetAlignment, LLVMSetMetadata, LLVMSetOperand, LLVMSetVolatile, LLVMValueAsBasicBlock, }; #[llvm_versions(10..)] use llvm_sys::core::{LLVMGetAtomicRMWBinOp, LLVMIsAAtomicCmpXchgInst, LLVMIsAAtomicRMWInst}; #[llvm_versions(17..)] -use llvm_sys::core::LLVMIsAValueAsMetadata; +use llvm_sys::core::{LLVMGetTypeKind, LLVMIsAValueAsMetadata, LLVMTypeOf}; use llvm_sys::core::{LLVMGetOrdering, LLVMSetOrdering}; use llvm_sys::prelude::LLVMValueRef; -use llvm_sys::{LLVMOpcode, LLVMTypeKind}; +use llvm_sys::LLVMOpcode; +#[llvm_versions(17..)] +use llvm_sys::LLVMTypeKind; use std::{ffi::CStr, fmt, fmt::Display}; @@ -692,29 +694,33 @@ impl<'ctx> InstructionValue<'ctx> { return Some(Operand::Block(bb.expect("BasicBlock should always be valid"))); } - if let Some(metadata) = self.try_ingest_metadata(operand) { - return Some(Operand::Metadata(metadata)); + match self.try_ingest_metadata(operand) { + // This is indeed metadata. + Ok(Some(metadata)) => return Some(Operand::Metadata(metadata)), + // This is null metadata, which we can't convert to an operand. + Err(_) => return None, + // Not metadata at all. + Ok(None) => {} } - Some(Operand::Value(unsafe { BasicValueEnum::new(operand) })) } #[llvm_versions(17..)] // LLVMIsAValueAsMetadata was introduced un 17.0. - unsafe fn try_ingest_metadata(self, operand: LLVMValueRef) -> Option> { + unsafe fn try_ingest_metadata(self, operand: LLVMValueRef) -> Result>, ()> { assert!(!operand.is_null()); if unsafe { LLVMGetTypeKind(LLVMTypeOf(operand)) == LLVMTypeKind::LLVMMetadataTypeKind } { // We may be dealing with null metadata, which would break memory invariants. if LLVMIsAValueAsMetadata(operand).is_null() { - return None; + return Err(()); } - return Some(MetadataValue::new(operand)); + return Ok(Some(MetadataValue::new(operand))); } - None + Ok(None) } #[llvm_versions(..17)] // LLVMIsAValueAsMetadata was introduced un 17.0. - unsafe fn try_ingest_metadata(self, operand: LLVMValueRef) -> Option> { - None + unsafe fn try_ingest_metadata(self, _: LLVMValueRef) -> Result>, ()> { + Ok(None) } diff --git a/tests/all/test_instruction_values.rs b/tests/all/test_instruction_values.rs index 9b37e883cbd..73ce4d8502c 100644 --- a/tests/all/test_instruction_values.rs +++ b/tests/all/test_instruction_values.rs @@ -662,6 +662,7 @@ fn test_metadata_kinds() { ]); } +#[llvm_versions(17..)] #[test] fn test_metadata_as_operand() { // clang can introduce instructions such as From 2ca4cd77c6d27d9a2e85a4e649abdb5d7f9f2b44 Mon Sep 17 00:00:00 2001 From: David Teller Date: Sun, 30 Nov 2025 15:25:39 +0100 Subject: [PATCH 6/6] Reworking PR --- src/values/basic_value_use.rs | 2 -- src/values/enums.rs | 1 + src/values/instruction_value.rs | 36 ++++------------------------ src/values/metadata_value.rs | 10 ++++++-- tests/all/test_instruction_values.rs | 12 ++++++---- tests/all/test_values.rs | 18 +++++++++++--- 6 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/values/basic_value_use.rs b/src/values/basic_value_use.rs index 6960ff36746..41d4a84e7f4 100644 --- a/src/values/basic_value_use.rs +++ b/src/values/basic_value_use.rs @@ -7,8 +7,6 @@ use crate::basic_block::BasicBlock; use crate::values::{AnyValueEnum, BasicValueEnum, MetadataValue}; /// Either [BasicValueEnum], a [BasicBlock] or a [Metadata]. -/// -/// Note that [Metadata] variants are only constructed for LLVM 17+. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum Operand<'ctx> { /// Represents a [BasicValueEnum]. diff --git a/src/values/enums.rs b/src/values/enums.rs index bb9d8733b47..f8fcbcc1b75 100644 --- a/src/values/enums.rs +++ b/src/values/enums.rs @@ -461,6 +461,7 @@ impl<'ctx> AggregateValueEnum<'ctx> { impl<'ctx> BasicMetadataValueEnum<'ctx> { pub(crate) unsafe fn new(value: LLVMValueRef) -> Self { + assert!(!value.is_null()); match LLVMGetTypeKind(LLVMTypeOf(value)) { LLVMTypeKind::LLVMFloatTypeKind | LLVMTypeKind::LLVMFP128TypeKind diff --git a/src/values/instruction_value.rs b/src/values/instruction_value.rs index adf459cab1b..0f8d2a6865d 100644 --- a/src/values/instruction_value.rs +++ b/src/values/instruction_value.rs @@ -3,20 +3,17 @@ use llvm_sys::core::LLVMGetGEPSourceElementType; use llvm_sys::core::{ LLVMGetAlignment, LLVMGetAllocatedType, LLVMGetFCmpPredicate, LLVMGetICmpPredicate, LLVMGetIndices, LLVMGetInstructionOpcode, LLVMGetInstructionParent, LLVMGetMetadata, LLVMGetNextInstruction, LLVMGetNumIndices, - LLVMGetNumOperands, LLVMGetOperand, LLVMGetOperandUse, LLVMGetPreviousInstruction, + LLVMGetNumOperands, LLVMGetOperand, LLVMGetOperandUse, LLVMGetPreviousInstruction, LLVMGetTypeKind, LLVMGetVolatile, LLVMHasMetadata, LLVMInstructionClone, LLVMInstructionEraseFromParent, LLVMInstructionRemoveFromParent, LLVMIsAAllocaInst, LLVMIsABasicBlock, LLVMIsAGetElementPtrInst, LLVMIsALoadInst, - LLVMIsAStoreInst, LLVMIsATerminatorInst, LLVMIsConditional, LLVMIsTailCall, - LLVMSetAlignment, LLVMSetMetadata, LLVMSetOperand, LLVMSetVolatile, LLVMValueAsBasicBlock, + LLVMIsAStoreInst, LLVMIsATerminatorInst, LLVMIsConditional, LLVMIsTailCall, LLVMSetAlignment, LLVMSetMetadata, + LLVMSetOperand, LLVMSetVolatile, LLVMTypeOf, LLVMValueAsBasicBlock, }; #[llvm_versions(10..)] use llvm_sys::core::{LLVMGetAtomicRMWBinOp, LLVMIsAAtomicCmpXchgInst, LLVMIsAAtomicRMWInst}; -#[llvm_versions(17..)] -use llvm_sys::core::{LLVMGetTypeKind, LLVMIsAValueAsMetadata, LLVMTypeOf}; use llvm_sys::core::{LLVMGetOrdering, LLVMSetOrdering}; use llvm_sys::prelude::LLVMValueRef; use llvm_sys::LLVMOpcode; -#[llvm_versions(17..)] use llvm_sys::LLVMTypeKind; use std::{ffi::CStr, fmt, fmt::Display}; @@ -694,36 +691,13 @@ impl<'ctx> InstructionValue<'ctx> { return Some(Operand::Block(bb.expect("BasicBlock should always be valid"))); } - match self.try_ingest_metadata(operand) { - // This is indeed metadata. - Ok(Some(metadata)) => return Some(Operand::Metadata(metadata)), - // This is null metadata, which we can't convert to an operand. - Err(_) => return None, - // Not metadata at all. - Ok(None) => {} + if unsafe { LLVMGetTypeKind(LLVMTypeOf(operand)) == LLVMTypeKind::LLVMMetadataTypeKind } { + return Some(Operand::Metadata(MetadataValue::new(operand))); } Some(Operand::Value(unsafe { BasicValueEnum::new(operand) })) } - #[llvm_versions(17..)] // LLVMIsAValueAsMetadata was introduced un 17.0. - unsafe fn try_ingest_metadata(self, operand: LLVMValueRef) -> Result>, ()> { - assert!(!operand.is_null()); - if unsafe { LLVMGetTypeKind(LLVMTypeOf(operand)) == LLVMTypeKind::LLVMMetadataTypeKind } { - // We may be dealing with null metadata, which would break memory invariants. - if LLVMIsAValueAsMetadata(operand).is_null() { - return Err(()); - } - return Ok(Some(MetadataValue::new(operand))); - } - Ok(None) - } - #[llvm_versions(..17)] // LLVMIsAValueAsMetadata was introduced un 17.0. - unsafe fn try_ingest_metadata(self, _: LLVMValueRef) -> Result>, ()> { - Ok(None) - } - - /// Get an instruction value operand iterator. pub fn get_operands(self) -> OperandIter<'ctx> { OperandIter { diff --git a/src/values/metadata_value.rs b/src/values/metadata_value.rs index 48b37c9579e..e490b9c4c26 100644 --- a/src/values/metadata_value.rs +++ b/src/values/metadata_value.rs @@ -101,7 +101,7 @@ impl<'ctx> MetadataValue<'ctx> { // SubTypes: Node only one day // REVIEW: BasicMetadataValueEnum only if you can put metadata in metadata... - pub fn get_node_values(self) -> Vec> { + pub fn get_node_values(self) -> Vec>> { if self.is_string() { return Vec::new(); } @@ -117,7 +117,13 @@ impl<'ctx> MetadataValue<'ctx> { }; vec.iter() - .map(|val| unsafe { BasicMetadataValueEnum::new(*val) }) + .map(|val| { + if val.is_null() { + None + } else { + Some(unsafe { BasicMetadataValueEnum::new(*val) }) + } + }) .collect() } diff --git a/tests/all/test_instruction_values.rs b/tests/all/test_instruction_values.rs index 73ce4d8502c..dafabc2fe5e 100644 --- a/tests/all/test_instruction_values.rs +++ b/tests/all/test_instruction_values.rs @@ -662,7 +662,6 @@ fn test_metadata_kinds() { ]); } -#[llvm_versions(17..)] #[test] fn test_metadata_as_operand() { // clang can introduce instructions such as @@ -713,10 +712,13 @@ fn test_metadata_as_operand() { ); let instruction = debug_info_builder.insert_declare_at_end(i32_ptr, Some(var_info), None, debug_loc, block); assert_eq!(instruction.get_num_operands(), 4); - assert!(instruction.get_operand(0).is_some()); - assert!(instruction.get_operand(1).is_none()); - assert!(instruction.get_operand(2).is_none()); - assert!(instruction.get_operand(3).is_some()); + assert_eq!(instruction.get_operands().count(), 4); + for (i, operand) in instruction.get_operands().enumerate() { + assert!(operand.is_some()); + // A cheap (and certainly insufficient) test that we can walk through + // the operand without segfaulting. + eprintln!("operand {i} is {:?}", operand); + } } #[test] diff --git a/tests/all/test_values.rs b/tests/all/test_values.rs index 568d852357b..427513f3e3a 100644 --- a/tests/all/test_values.rs +++ b/tests/all/test_values.rs @@ -917,7 +917,11 @@ fn test_metadata() { let md_node_child = context.metadata_node(&[bool_val.into(), f32_val.into()]); let md_node = context.metadata_node(&[bool_val.into(), f32_val.into(), md_string.into(), md_node_child.into()]); - let node_values = md_node.get_node_values(); + let node_values: Vec<_> = md_node + .get_node_values() + .iter() + .map(|v| v.expect("Node value should not have been none")) + .collect(); assert_eq!(md_node.get_string_value(), None); assert_eq!(node_values.len(), 4); @@ -938,7 +942,11 @@ fn test_metadata() { assert_eq!(global_md.len(), 1); - let md = global_md[0].get_node_values(); + let md: Vec<_> = md_node + .get_node_values() + .iter() + .map(|v| v.expect("Node value should not have been none")) + .collect(); assert_eq!(md.len(), 4); assert_eq!(md[0].into_int_value(), bool_val); @@ -992,7 +1000,11 @@ fn test_metadata() { assert!(ret_instr.has_metadata()); assert!(ret_instr.get_metadata(1).is_none()); - let md_node_values = ret_instr.get_metadata(2).unwrap().get_node_values(); + let md_node_values: Vec<_> = md_node + .get_node_values() + .iter() + .map(|v| v.expect("Node value should not have been none")) + .collect(); assert_eq!(md_node_values.len(), 1); assert_eq!(