[PATCH] Allow for the use of StringRef command line options instead of cl::opt<std::string>
plotfi at apple.com
Fri Oct 4 10:42:49 PDT 2013
On Oct 4, 2013, at 7:18 AM, Reid Kleckner <rnk at google.com> wrote:
> 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. :)
Took a look at the mono code again. Yep, I think there’s going to be problems there without a std::string backing things.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits