[cfe-commits] r157483 - in /cfe/trunk: lib/CodeGen/CGCall.cpp test/CodeGen/alloc_size.c

[Douglas Gregor]

On May 25, 2012, at 3:23 PM, Nuno Lopes <nunoplopes at sapo.pt> wrote:

> There has been some discussion on both the bounds checking and the alloc_size proposals on the various mailing lists (dev & commits) for the past month.
> Yes, the design is not finished. But I've been making changes to the implementation to accommodate the feedback.

When there are legitimate concerns about the design and implementation of a feature, we prefer that you work on your patches locally and continue to discuss the design on the mailing lists. When the community has reached a general consensus that this is, in fact, the right way to go, the patches can go into the tree.

	- Doug

> Nuno
> 
> 
> Citando Chandler Carruth <chandlerc at google.com>:
> 
>> Nuno, this patch didn't get reviewed before commit. The LLVM review is
>> ongoing, and seems to indicate the solution is not yet ready. Folks have
>> also raised serious concerns about the implementation strategy here.
>> 
>> While supporting the alloc_size attribute at least enough to parse it and
>> reject invalid formulations seems good for GCC compatibility, adding code
>> generation and LLVM support to it is adding a significant amount of
>> maintenance for this feature, and there has been no discussion or general
>> consensus that this functionality's design is the right long-term solution
>> for Clang/LLVM. In fact, there was no discussion at all until Kostya
>> started his email thread, which I don't think you've responded to yet.
>> 
>> I would like to suggest you not continue committing (especially without
>> review) patches toward a system that hasn't been designed and supported by
>> the larger community. I think you really need to take time to engage the
>> rest of the open source group in your efforts, and figure out what the
>> correct design and implementation strategy is here.
>> 
>> I've CC'ed Chris and Doug as they may feel differently. =] I would be very
>> interested in their feedback at least about what is the best process.
>> -Chandler
>> 
>> On Fri, May 25, 2012 at 10:04 AM, Nuno Lopes <nunoplopes at sapo.pt> wrote:
>> 
>>> Author: nlopes
>>> Date: Fri May 25 12:04:42 2012
>>> New Revision: 157483
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=157483&view=rev
>>> Log:
>>> add CodeGen support for the alloc_size attribute
>>> 
>>> Added:
>>>   cfe/trunk/test/CodeGen/alloc_size.c
>>> Modified:
>>>   cfe/trunk/lib/CodeGen/CGCall.cpp
>>> 
>>> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=157483&r1=157482&r2=157483&view=diff
>>> 
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Fri May 25 12:04:42 2012
>>> @@ -2087,6 +2087,23 @@
>>>  CS.setAttributes(Attrs);
>>>  CS.setCallingConv(static_cast<llvm::CallingConv::ID>(CallingConv));
>>> 
>>> +  // add metadata for __attribute__((alloc_size(foo)))
>>> +  if (TargetDecl) {
>>> +    if (const AllocSizeAttr* Attr = TargetDecl->getAttr<AllocSizeAttr>())
>>> {
>>> +      std::vector<llvm::Value*> Args;
>>> +      llvm::IntegerType *Ty =
>>> llvm::IntegerType::getInt32Ty(getLLVMContext());
>>> +      bool isMethod = isa<CXXMethodDecl>(TargetDecl);
>>> +
>>> +      for (AllocSizeAttr::args_iterator I = Attr->args_begin(),
>>> +           E = Attr->args_end(); I != E; ++I) {
>>> +        Args.push_back(llvm::ConstantInt::get(Ty, *I + isMethod));
>>> +      }
>>> +
>>> +      llvm::MDNode *MD = llvm::MDNode::get(getLLVMContext(), Args);
>>> +      CS.getInstruction()->setMetadata("alloc_size", MD);
>>> +    }
>>> +  }
>>> +
>>>  // In ObjC ARC mode with no ObjC ARC exception safety, tell the ARC
>>>  // optimizer it can aggressively ignore unwind edges.
>>>  if (CGM.getLangOpts().ObjCAutoRefCount)
>>> 
>>> Added: cfe/trunk/test/CodeGen/alloc_size.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/alloc_size.c?rev=157483&view=auto
>>> 
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGen/alloc_size.c (added)
>>> +++ cfe/trunk/test/CodeGen/alloc_size.c Fri May 25 12:04:42 2012
>>> @@ -0,0 +1,11 @@
>>> +// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
>>> +
>>> +void *my_recalloc(void *, unsigned, unsigned)
>>> __attribute__((alloc_size(2,3)));
>>> +
>>> +// CHECK: @f
>>> +void* f() {
>>> +  // CHECK: call i8* @my_recalloc{{.*}}, !alloc_size !0
>>> +  return my_recalloc(0, 11, 27);
>>> +}
>>> +
>>> +// CHECK: !0 = metadata !{i32 1, i32 2}
>>>