[llvm] r223574 - IR: Disallow function-local metadata attachments

Duncan P. N. Exon Smith dexonsmith at apple.com
Sat Dec 6 10:20:20 PST 2014


> On 2014 Dec 6, at 08:49, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Fri, Dec 5, 2014 at 6:29 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com <mailto:dexonsmith at apple.com>> wrote:
> Author: dexonsmith
> Date: Fri Dec  5 20:29:44 2014
> New Revision: 223574
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=223574&view=rev <http://llvm.org/viewvc/llvm-project?rev=223574&view=rev>
> Log:
> IR: Disallow function-local metadata attachments
> 
> Metadata attachments to instructions cannot be function-local.
> 
> This is part of PR21532.
> 
> Modified:
>     llvm/trunk/lib/AsmParser/LLParser.cpp
>     llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
>     llvm/trunk/lib/IR/Metadata.cpp
>     llvm/trunk/test/Feature/metadata.ll
>     llvm/trunk/test/Linker/metadata-a.ll
>     llvm/trunk/test/Linker/metadata-b.ll
> 
> Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=223574&r1=223573&r2=223574&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=223574&r1=223573&r2=223574&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
> +++ llvm/trunk/lib/AsmParser/LLParser.cpp Fri Dec  5 20:29:44 2014
> @@ -62,6 +62,8 @@ bool LLParser::ValidateEndOfModule() {
>              NumberedMetadata[SlotNo] == nullptr)
>            return Error(MDList[i].Loc, "use of undefined metadata '!" +
>                         Twine(SlotNo) + "'");
> +        assert(!NumberedMetadata[SlotNo]->isFunctionLocal() &&
> +               "Unexpected function-local metadata");
>          Inst->setMetadata(MDList[i].MDKind, NumberedMetadata[SlotNo]);
>        }
>      }
> @@ -1529,6 +1531,8 @@ bool LLParser::ParseInstructionMetadata(
>        if (ParseMetadataListValue(ID, PFS))
>          return true;
>        assert(ID.Kind == ValID::t_MDNode);
> +      if (ID.MDNodeVal->isFunctionLocal())
> +        return TokError("unexpected function-local metadata");
>        Inst->setMetadata(MDK, ID.MDNodeVal);
>      } else {
>        unsigned NodeID = 0;
> 
> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=223574&r1=223573&r2=223574&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=223574&r1=223573&r2=223574&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Fri Dec  5 20:29:44 2014
> @@ -2359,8 +2359,12 @@ std::error_code BitcodeReader::ParseMeta
>            MDKindMap.find(Kind);
>          if (I == MDKindMap.end())
>            return Error(BitcodeError::InvalidID);
> -        Value *Node = MDValueList.getValueFwdRef(Record[i+1]);
> -        Inst->setMetadata(I->second, cast<MDNode>(Node));
> +        MDNode *Node = cast<MDNode>(MDValueList.getValueFwdRef(Record[i+1]));
> +        if (Node->isFunctionLocal())
> +          // Drop the attachment.  This used to be legal, but there's no
> +          // upgrade path.
> +          break;
> +        Inst->setMetadata(I->second, Node);
>          if (I->second == LLVMContext::MD_tbaa)
>            InstsWithTBAATag.push_back(Inst);
>        }
> 
> Modified: llvm/trunk/lib/IR/Metadata.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=223574&r1=223573&r2=223574&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=223574&r1=223573&r2=223574&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/IR/Metadata.cpp (original)
> +++ llvm/trunk/lib/IR/Metadata.cpp Fri Dec  5 20:29:44 2014
> @@ -639,6 +639,8 @@ void Instruction::setMetadata(unsigned K
> 
>    // Handle the case when we're adding/updating metadata on an instruction.
>    if (Node) {
> +    assert(!Node->isFunctionLocal() &&
> +           "Function-local metadata cannot be attached to instructions");
>      LLVMContextImpl::MDMapTy &Info = getContext().pImpl->MetadataStore[this];
>      assert(!Info.empty() == hasMetadataHashEntry() &&
>             "HasMetadata bit is wonked");
> 
> Modified: llvm/trunk/test/Feature/metadata.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Feature/metadata.ll?rev=223574&r1=223573&r2=223574&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Feature/metadata.ll?rev=223574&r1=223573&r2=223574&view=diff>
> ==============================================================================
> --- llvm/trunk/test/Feature/metadata.ll (original)
> +++ llvm/trunk/test/Feature/metadata.ll Fri Dec  5 20:29:44 2014
> @@ -5,7 +5,7 @@ define void @foo(i32 %x) {
>    call void @llvm.zonk(metadata !1, i64 0, metadata !1)
>    store i32 0, i32* null, !whatever !0, !whatever_else !{}, !more !{metadata !"hello"}
>    store i32 0, i32* null, !whatever !{metadata !"hello", metadata !1, metadata !{}, metadata !2}
> -  ret void, !whatever !{i32 %x}
> 
> Should we have a test case that includes function local metadata and tests that it gracefully fails to be parsed? 

Of course.  Thanks for catching that.  I'll get that done soon.

And the same thing for complicated function-local metadata.

> +  ret void
>  }
> 
>  declare void @llvm.zonk(metadata, i64, metadata) nounwind readnone
> 
> Modified: llvm/trunk/test/Linker/metadata-a.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/metadata-a.ll?rev=223574&r1=223573&r2=223574&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/metadata-a.ll?rev=223574&r1=223573&r2=223574&view=diff>
> ==============================================================================
> --- llvm/trunk/test/Linker/metadata-a.ll (original)
> +++ llvm/trunk/test/Linker/metadata-a.ll Fri Dec  5 20:29:44 2014
> @@ -1,15 +1,15 @@
>  ; RUN: llvm-link %s %p/metadata-b.ll -S -o - | FileCheck %s
> 
>  ; CHECK: define void @foo(i32 %a)
> -; CHECK: ret void, !attach !0, !also !{i32 %a}
> +; CHECK: ret void, !attach !0
>  ; CHECK: define void @goo(i32 %b)
> -; CHECK: ret void, !attach !1, !and !{i32 %b}
> +; CHECK: ret void, !attach !1
>  ; CHECK: !0 = metadata !{i32 524334, void (i32)* @foo}
>  ; CHECK: !1 = metadata !{i32 524334, void (i32)* @goo}
> 
>  define void @foo(i32 %a) nounwind {
>  entry:
> -  ret void, !attach !0, !also !{ i32 %a }
> +  ret void, !attach !0
>  }
> 
>  !0 = metadata !{i32 524334, void (i32)* @foo}
> 
> Modified: llvm/trunk/test/Linker/metadata-b.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/metadata-b.ll?rev=223574&r1=223573&r2=223574&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/metadata-b.ll?rev=223574&r1=223573&r2=223574&view=diff>
> ==============================================================================
> --- llvm/trunk/test/Linker/metadata-b.ll (original)
> +++ llvm/trunk/test/Linker/metadata-b.ll Fri Dec  5 20:29:44 2014
> @@ -3,7 +3,7 @@
> 
>  define void @goo(i32 %b) nounwind {
>  entry:
> -  ret void, !attach !0, !and !{ i32 %b }
> +  ret void, !attach !0
>  }
> 
>  !0 = metadata !{i32 524334, void (i32)* @goo}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>




More information about the llvm-commits mailing list