[cfe-dev] Bug in clang parsing pointer to an array in function parameter list
Justin Handville
uniheliodem at gmail.com
Sun Sep 9 15:59:06 PDT 2007
Ah. Then I'll check in the front end. I thought that the char*[] in
the parameter list looked a little funny, but I thought maybe that
might be intended.
Without the patch, the code asserts, since it is trying to store a
char*[] into a char** location. It wouldn't be difficult to add the
check to the appropriate place in the front end to change the argument
declaration to a char**, as llvm-gcc does.
On 9/9/07, Steve Naroff <snaroff at apple.com> wrote:
>
> On Sep 9, 2007, at 3:32 PM, Justin Handville wrote:
>
> > 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;
> > }
> >
>
> Justin,
>
> I'm a bit confused. The front-end (Sema::ParseParamDeclarator)
> already converts argv from "char *[]" to "char **".
>
> See the "DeclRefExpr" node below (on line 6).
>
> I don't know why the code generator would need to fiddle with this.
>
> Chris will need to advise...
>
> snaroff
>
> [dylan:~/llvm/tools/clang] admin% ../../Debug/bin/clang justin.c -
> parse-ast-dump
>
> int puts(char *);
>
> int main(int argc, char *argv[])
> (CompoundStmt 0x3105ed0 <justin.c:5:1, line:11:1>
> (DeclStmt 0x3105bf0 <:0:0>
> 0x3105dd0 "char *foo =
> (ArraySubscriptExpr 0x3105db0 <justin.c:6:15, col:21> 'char *'
> (DeclRefExpr 0x3105d70 <col:15> 'char **' Decl='argv'
> 0x3105d40)
> (IntegerLiteral 0x3105d90 <col:20> 'int' 0))")
> (CallExpr 0x3105e70 <line:8:3, col:11> 'int'
> (ImplicitCastExpr 0x3105e60 <col:3> 'int (*)(char *)'
> (DeclRefExpr 0x3105e00 <col:3> 'int (char *)' Decl='puts'
> 0x3105c20))
> (DeclRefExpr 0x3105e20 <col:8> 'char *' Decl='foo' 0x3105dd0))
> (ReturnStmt 0x3105ec0 <line:10:3, col:10>
> (IntegerLiteral 0x3105ea0 <col:10> 'int' 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
> >>>
> >>
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
More information about the cfe-dev
mailing list