[PATCH] Allow for the use of StringRef command line options instead of cl::opt<std::string>

Reid Kleckner rnk at google.com
Fri Oct 4 07:18:30 PDT 2013

On Thu, Oct 3, 2013 at 6:45 PM, Puyan Lotfi <plotfi at apple.com> wrote:

> Also agreed. I prefer the simpler solution myself. As far as I can tell,
> some of the cases involving UAF bugs would be averted if
> cl::opt<std::string> were used (as in the case of parse environment
> options). There’s no evidence thus far that suggests existing code would
> encounter UAF bugs.

OK, but many CommandLine users (Mono, previously I know we wanted this for
Unladen Swallow) are trying to set flags in LLVM libraries.  You are
changing LLVM to use cl::opt<StringRef> instead of cl::opt<std::string>.
 So, it seems to me that there really are bugs.


> Long term, we'll probably want to store these global options on the
>> LLVMContext, and then we can kill off this string persisting code.
> Any reason not to do that now, if that's the preference? (I'm not
> suggesting changing the opt APIs from static globals to accessors on
> LLVMContext immediately (I guess that's what you were talking about) but if
> LLVMContext has the desired lifetime we could just wrap the
> ParseCommandLineOptions in an LLVMContext entry point that persists the
> string first, then calls the parse routine, for example)

There's no reason not to do it now.  I just don't like asking for lots of
yak shaving work in code review.  Maybe it's the right thing though.  :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131004/01ae6f1f/attachment.html>

More information about the llvm-commits mailing list