[llvm-commits] [llvm] r160751 - in /llvm/trunk: include/llvm/Analysis/MemoryBuiltins.h lib/Analysis/MemoryBuiltins.cpp lib/Transforms/Instrumentation/BoundsChecking.cpp test/Instrumentation/BoundsChecking/strings.ll

[John Criswell]

On 7/27/12 3:39 AM, Chandler Carruth wrote:
> Sorry it's taken me a bit to circle back to this problem...
>
> On Wed, Jul 25, 2012 at 2:34 PM, Nuno Lopes <nunoplopes at sapo.pt 
> <mailto:nunoplopes at sapo.pt>> wrote:
>
>     Quoting Benjamin Kramer <benny.kra at gmail.com
>     <mailto:benny.kra at gmail.com>>:
>
>     > On 25.07.2012, at 22:24, Nuno Lopes <nunoplopes at sapo.pt
>     <mailto:nunoplopes at sapo.pt>> wrote:
>     >
>     >> Quoting Duncan Sands <baldrick at free.fr <mailto:baldrick at free.fr>>:
>     >>
>     >>> Hi Nuno,
>     >>>
>     >>>> original commit msg:
>     >>>> MemoryBuiltins: add support to determine the size of strdup'ed
>     >>>> non-constant strings
>     >>>
>     >>> the dragonegg bots don't use cmake but they broke too:
>     >>>
>     >>> MemoryBuiltins.cpp:(.text+0x40dc): undefined reference to
>     >>> `llvm::EmitStrNLen(llvm::Value*, llvm::Value*,
>     llvm::IRBuilder<true,
>     >>> llvm::ConstantFolder, llvm::IRBuilderDefaultInserter<true> >&,
>     >>> llvm::TargetData
>     >>> const*, llvm::TargetLibraryInfo const*)'
>     >>> MemoryBuiltins.cpp:(.text+0x4207): undefined reference to
>     >>> `llvm::EmitStrLen(llvm::Value*, llvm::IRBuilder<true,
>     llvm::ConstantFolder,
>     >>> llvm::IRBuilderDefaultInserter<true> >&, llvm::TargetData const*,
>     >>> llvm::TargetLibraryInfo const*)'
>     >>
>     >> Interesting.. I guess I need to try to build it on another machine.
>     >
>     > Isn't using lib/Transforms/Utils from lib/Analysis a layering
>     > violation (cyclic dependency)?
>
>     Well, I really need to access some functions from
>     lib/Transforms/Utils. Is there any dependency of lib/Transforms/Utils
>     on lib/Analysis?
>
>
> No, you simply cannot depend on lib/Transforms code from within 
> lib/Analysis. That is a complete violation of the layering design here.
>
>     I really don't have a clue on how to solve this problem.. :)
>
>
> I think the fundamental problem is that ObjectSizeOffsetEvaluator 
> isn't an analysis, and shouldn't be in lib/Analysis at all. From its 
> description:
>
> "Evaluate the size and offset of an object pointed [to] by a value*. 
> May create code to compute the result at run-time.". An Analysis pass 
> shouldn't create code. The only consumer of this interface is 
> lib/Transforms/Instrumentation/BoundsChecking.cpp, I think this class 
> should move to an anonymous namespace within that instrumentation 
> pass. If we want to hoist it, it could grow to live in 
> lib/Transforms/Utils/ObjectSize.cpp or even 
> lib/Transforms/Utils/MemoryBuiltins.cpp. I would only move it out of 
> the anonymous namespace if you have a pretty strong idea that others 
> will want to use the logic. I would only move it to a memory builtin 
> utility file if you think its likely we'll want more general memory 
> builtin transform utilities in the future.

As an FYI, SAFECode will probably use the logic the next time we update 
to LLVM mainline.

-- John T.

>
> Can I make at least the move to the anonymous namespace? I'd like to 
> get the layering issue fixed.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120727/dd42cab6/attachment.html>