I finally found some time to work on this patch again. Attached is a new
version that adds two things.
First, as Dmitri suggested, I added test cases.
Second, I found a problem with my initial patch that leads to crashes in
situations like the following:
1. __attribute__((annotate("foo"))) void foo();
2. void bar(){foo();}
3. void foo() {}
I'll try to explain what I *think* happens here when my original patch is
applied (note the *think*, I'm by no means a Clang expert so please be gentle
when I get this completely wrong :-))
1. A declaration is created for a function "foo" having an unspecified number
of arguments. This declaration is not emitted but queued for deferred
emission. My patch *did* create an annotation referring to this declaration,
though.
2. The call site triggers emission of the declaration of "foo".
3. Now "foo" has zero arguments and the original declaration is replaced by a
new function. Since the added annotation still refers to the deleted
declaration, this causes a crash.
So, what my patch does is make sure the annotations are only created right
before they are emitted and annotations referring to non-existing values are
ignored.
On Monday 28 January 2013 17:09:41 Job Noorman wrote:
> Hi Dmitri,
>
> Yes, I will add a testcase if someone can confirm the behavior introduced by
> this patch would be accepted. Just to not waste my time writing a testcase
> for a feature that will never make it into Clang :-)
>
> Job
>
> On Monday 28 January 2013 16:59:21 Dmitri Gribenko wrote:
> > On Mon, Jan 28, 2013 at 3:35 PM, Job Noorman <[email protected]> wrote:
> > > Attached is a patch that makes sure that annotate attributes are also
> > > emitted for function declarations. It simply calls
> > > CodeGenModule::AddGlobalAnnotations in
> > > CodeGenModule::SetFunctionAttributes
> > > instead of in CodeGenModule::EmitGlobalFunctionDefinition.
> > >
> > > See
> > > http://lists.cs.uiuc.edu/pipermail/cfe-users/2013-January/000046.html
> > > for a yet unanswered question I asked about this.
> >
> > Hello Job,
> >
> > I don't know the answer for your question, but the patch should
> > include a testcase.
> >
> > Dmitridiff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index efff4b1..70f426a 100644
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -743,6 +743,8 @@ void CodeGenModule::SetFunctionAttributes(GlobalDecl GD,
if (const SectionAttr *SA = FD->getAttr<SectionAttr>())
F->setSection(SA->getName());
+ if (FD->hasAttr<AnnotateAttr>())
+ AddGlobalAnnotations(FD, F);
}
void CodeGenModule::AddUsedGlobal(llvm::GlobalValue *GV) {
@@ -952,9 +954,25 @@ void CodeGenModule::EmitDeferred() {
}
void CodeGenModule::EmitGlobalAnnotations() {
- if (Annotations.empty())
+ if (NeededAnnotations.empty())
return;
+ std::vector<llvm::Constant*> Annotations;
+
+ for (AnnotationMap::const_iterator it = NeededAnnotations.begin(),
+ end = NeededAnnotations.end(); it != end; ++it) {
+ llvm::Value* V = it->second;
+ if (!V)
+ continue;
+ llvm::GlobalValue* GV = cast<llvm::GlobalValue>(V);
+ const ValueDecl* D = it->first;
+ // Get the struct elements for these annotations.
+ for (specific_attr_iterator<AnnotateAttr>
+ ai = D->specific_attr_begin<AnnotateAttr>(),
+ ae = D->specific_attr_end<AnnotateAttr>(); ai != ae; ++ai)
+ Annotations.push_back(EmitAnnotateAttr(GV, *ai, D->getLocation()));
+ }
+
// Create a new global variable for the ConstantStruct in the Module.
llvm::Constant *Array = llvm::ConstantArray::get(llvm::ArrayType::get(
Annotations[0]->getType(), Annotations.size()), Annotations);
@@ -1016,11 +1034,7 @@ llvm::Constant *CodeGenModule::EmitAnnotateAttr(llvm::GlobalValue *GV,
void CodeGenModule::AddGlobalAnnotations(const ValueDecl *D,
llvm::GlobalValue *GV) {
assert(D->hasAttr<AnnotateAttr>() && "no annotate attribute");
- // Get the struct elements for these annotations.
- for (specific_attr_iterator<AnnotateAttr>
- ai = D->specific_attr_begin<AnnotateAttr>(),
- ae = D->specific_attr_end<AnnotateAttr>(); ai != ae; ++ai)
- Annotations.push_back(EmitAnnotateAttr(GV, *ai, D->getLocation()));
+ NeededAnnotations[D] = GV;
}
bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) {
@@ -2077,8 +2091,6 @@ void CodeGenModule::EmitGlobalFunctionDefinition(GlobalDecl GD) {
AddGlobalCtor(Fn, CA->getPriority());
if (const DestructorAttr *DA = D->getAttr<DestructorAttr>())
AddGlobalDtor(Fn, DA->getPriority());
- if (D->hasAttr<AnnotateAttr>())
- AddGlobalAnnotations(D, Fn);
}
void CodeGenModule::EmitAliasDefinition(GlobalDecl GD) {
diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h
index 6e5985d..00272ef 100644
--- a/lib/CodeGen/CodeGenModule.h
+++ b/lib/CodeGen/CodeGenModule.h
@@ -298,7 +298,8 @@ class CodeGenModule : public CodeGenTypeCache {
llvm::BumpPtrAllocator MangledNamesAllocator;
/// Global annotations.
- std::vector<llvm::Constant*> Annotations;
+ typedef std::map<const ValueDecl*, llvm::WeakVH> AnnotationMap;
+ AnnotationMap NeededAnnotations;
/// Map used to get unique annotation strings.
llvm::StringMap<llvm::Constant*> AnnotationStrings;
diff --git a/test/CodeGen/annotations-forward-declaration-crash.c b/test/CodeGen/annotations-forward-declaration-crash.c
new file mode 100644
index 0000000..5279c4b
--- /dev/null
+++ b/test/CodeGen/annotations-forward-declaration-crash.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -o - -emit-llvm | FileCheck %s
+
+// CHECK: private unnamed_addr constant [4 x i8] c"foo\00", section "llvm.metadata"
+// CHECK: @llvm.global.annotations = appending global [1 x { i8*, i8*, i8*, i32 }] [{ i8*, i8*, i8*, i32 } { i8* bitcast (void ()* @foo to i8*), i8* getelementptr inbounds ([4 x i8]* {{.+}}, i32 0, i32 0), i8* getelementptr inbounds ([{{.+}} x i8]* {{.+}}, i32 0, i32 0), i32 8 }], section "llvm.metadata"
+
+__attribute__((annotate("foo"))) void foo();
+void bar(){foo();}
+void foo() {}
diff --git a/test/CodeGen/annotations-forward-declaration.c b/test/CodeGen/annotations-forward-declaration.c
new file mode 100644
index 0000000..509d5b3
--- /dev/null
+++ b/test/CodeGen/annotations-forward-declaration.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -o - -emit-llvm | FileCheck %s
+
+// CHECK: private unnamed_addr constant [4 x i8] c"foo\00", section "llvm.metadata"
+// CHECK: @llvm.global.annotations = appending global [1 x { i8*, i8*, i8*, i32 }] [{ i8*, i8*, i8*, i32 } { i8* bitcast (void ()* @foo to i8*), i8* getelementptr inbounds ([4 x i8]* {{.+}}, i32 0, i32 0), i8* getelementptr inbounds ([{{.+}} x i8]* {{.+}}, i32 0, i32 0), i32 6 }], section "llvm.metadata"
+
+__attribute__((annotate("foo"))) void foo(void);
+void bar(){foo();}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
