[cfe-dev] Bug in clang parsing pointer to an array in function parameter list

[Justin Handville]

With the following patch, I can now compile and execute this source file:

int puts(char*);

int main(int argc, char* argv[])
{
  char* foo = argv[0];

  puts(foo);

  return 0;
}

Here's the patch.  I'm curious to see if this will work in all cases.
I am still adding test cases to the build, so it is a bit
experimental:

ndex: CodeGen/CodeGenFunction.h
===================================================================
--- CodeGen/CodeGenFunction.h   (revision 41790)
+++ CodeGen/CodeGenFunction.h   (working copy)
@@ -237,6 +237,11 @@
   };
   llvm::SmallVector<BreakContinue, 8> BreakContinueStack;

+  static bool
+  isCompatibleDeclPointerType(
+    const llvm::Type* Arg,
+    const llvm::Type* DeclPtr);
+
 public:
   CodeGenFunction(CodeGenModule &cgm);

Index: CodeGen/CGDecl.cpp
===================================================================
--- CodeGen/CGDecl.cpp  (revision 41790)
+++ CodeGen/CGDecl.cpp  (working copy)
@@ -96,11 +96,69 @@
   }
 }

+bool
+CodeGenFunction::isCompatibleDeclPointerType(
+  const llvm::Type* ArgType,
+  const llvm::Type* DeclType)
+{
+  if(
+      ArgType->getTypeID() != llvm::Type::PointerTyID
+   || DeclType->getTypeID() != llvm::Type::PointerTyID)
+  {
+    return false;
+  }
+
+  while(
+      ArgType->getNumContainedTypes() == 1
+   && DeclType->getNumContainedTypes() == 1)
+  {
+    const llvm::Type* ArgContainedType = ArgType->getContainedType(0);
+    const llvm::Type* DeclContainedType = DeclType->getContainedType(0);
+
+    //if the two contained types point to the same
+    //location, then these pointer types are compatible
+    if(
+      ArgContainedType == DeclContainedType)
+    {
+      return true;
+    }
+    //is the pointer indirection level different?
+    else if(
+      ArgContainedType->getNumContainedTypes() !=
+        DeclContainedType->getNumContainedTypes())
+    {
+      break;
+    }
+    //are these compatible pointer / array types?
+    else if(
+        llvm::SequentialType::classof(ArgContainedType)
+     && llvm::SequentialType::classof(DeclContainedType))
+    {
+      if( ArgContainedType->getNumContainedTypes() != 1 )
+      {
+        //FIXME will this ever happen with arrays / pointers?
+        break;
+      }
+
+      ArgType = ArgContainedType;
+      DeclType = DeclContainedType;
+    }
+    else
+    {
+      //one or more of these is something other than an array / pointer
+      break;
+    }
+  }
+
+  return false;
+}
+
 /// Emit an alloca for the specified parameter and set up LocalDeclMap.
 void CodeGenFunction::EmitParmDecl(const ParmVarDecl &D, llvm::Value *Arg) {
   QualType Ty = D.getCanonicalType();

   llvm::Value *DeclPtr;
+
   if (!Ty->isConstantSizeType(getContext())) {
     // Variable sized values always are passed by-reference.
     DeclPtr = Arg;
@@ -111,7 +169,19 @@
       // TODO: Alignment
       DeclPtr = new llvm::AllocaInst(LTy, 0, std::string(D.getName())+".addr",
                                      AllocaInsertPt);
-
+
+      //do we need to perform an implicit conversion?
+      if( isCompatibleDeclPointerType(Arg->getType(), DeclPtr->getType()) )
+      {
+        llvm::Value* CastPtr =
+          Builder.CreateBitCast(
+            Arg,
+            LTy,
+            (std::string(D.getName()) + ".implconv").c_str());
+
+        Arg = CastPtr;
+      }
+
       // Store the initial value into the alloca.
       Builder.CreateStore(Arg, DeclPtr);
     } else {

On 9/9/07, Justin Handville <uniheliodem at gmail.com> wrote:
> Perhaps the better question to ask is where should this transform occur?
>
> On 9/9/07, Justin Handville <uniheliodem at gmail.com> wrote:
> > The following code snippet generates an assert when compiled with
> > clang -emit-llvm
> >
> > int main(int argc, char* argv[])
> > {
> >   return 0;
> > }
> >
> > Assert:
> >
> > Assertion failed: (getOperand(0)->getType() ==
> > cast<PointerType>(getOperand(1)->getType())->getElementType() && "Ptr
> > must be a pointer to Val type!"), function AssertOK, file
> > Instructions.cpp, line 796.
> >
> > The problem here is at line 116 of CGDecl.cpp:
> >
> >     if (LTy->isFirstClassType()) {
> >       // TODO: Alignment
> >       DeclPtr = new llvm::AllocaInst(LTy, 0, std::string(D.getName())+".addr",
> >                                      AllocaInsertPt);
> >
> >       // Store the initial value into the alloca.
> >       Builder.CreateStore(Arg, DeclPtr);  //attempting to store
> > char*[] as char**
> >
> > The problem is that llvm treats char*[] and char** as two distinct
> > types as far as I can tell.  EmitParmDecl should convert the type to a
> > char** before calling CreateStore, since this is how the argument
> > would be passed in C ABI.
> >
> > I assume that a transform step should be added before the CreateStore
> > that could normalize the argument to match the declptr.  I assume a
> > good approach would be to descend through both Arg and DeclPtr,
> > transforming Arg to match DeclPtr where compatible, or raising an
> > appropriate diagnostic error if such a transformation is not possible.
> >
> > If I can get some approval that this route would work in this case,
> > then I will go ahead and submit the patch.
> >
> > Thanks,
> > Justin
> >
>