<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Oct 4, 2013, at 7:18 AM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">On Thu, Oct 3, 2013 at 6:45 PM, Puyan Lotfi <span dir="ltr"><<a href="mailto:plotfi@apple.com" target="_blank">plotfi@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word">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. </div>
</blockquote><div><br></div><div>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.</div>
<div><br></div><div><br></div><div>David:</div><div><div class="im"><blockquote class="gmail_quote" style="font-family:arial,sans-serif;font-size:13px;margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Long term, we'll probably want to store these global options on the LLVMContext, and then we can kill off this string persisting code.</blockquote>
</blockquote><blockquote class="gmail_quote" style="font-family:arial,sans-serif;font-size:13px;margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
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)</blockquote>
<div><br></div><div>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.  :) </div></div></div></div></div></div>
</blockquote></div><br><div>Took a look at the mono code again. Yep, I think there’s going to be problems there without a std::string backing things. </div></body></html>