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

[Puyan Lotfi]

Hi

As requested, I have ran the address sanitizer tests. I ran them many times, and could not find any reproducible failures.
I did notice some intermittent failures that came and went depending on what revision I had gotten from SVN.

Due to concern for use-after-free I have included a patch to clang that allows strings passed to ParseCommandLineOptions() from the CreateTargetMachine() function to persist (this was the only location in LLVM or clang where I found ParseCommandLineOptions being called with automatic memory); what it does is strdups the args for -debug-pass, -limit-float-precision as well as backend args and stores the result to a global vector of StringRefs. This vector is only populated for unique strings to avoid leaking repeatedly.

The other three patches include changes to the CommandLine library to enable StringRef (which I already sent out), changes to places using cl::opt<std::string> to use StringRef (also already sent out), and also the requested changes to the CommandLine documentation.

I think this should cover all the basis. I’ll check in if this looks ok to you all.

Thanks

-Puyan





On Sep 23, 2013, at 10:43 AM, Reid Kleckner <rnk at google.com> wrote:

> I worry that this patch creates a significant risk for use-after-free bugs.  Can you:
> 
> 1. Explain how string use-after-free is supposed to be avoided?
> 2. Update the docs if ParseCommandLine options is no longer going to save copies of strings.
> 3. Make sure the internal tests pass with an AddressSanitizer build, because I see things like ParseEnvironmentOptions which look broken with this change.
> 
> 
> On Mon, Sep 23, 2013 at 10:00 AM, Chris Lattner <clattner at apple.com> wrote:
> 
> On Sep 19, 2013, at 3:01 PM, Puyan Lotfi <plotfi at apple.com> wrote:
> 
> > Hi All
> >
> > I’ve improved this StringRef patch and I’d like some more feedback and hopefully a submission.
> > My changes to the original patch thus far have been:
> >
> > - I added changes to CreateTargerMachine() in clang to have the SmallVector passed in from the caller.
> > - I found more places where cl::opt<str::string> was used, and I changed them to use cl::opt<StringRef>.
> >
> > I’ve attached a patch that should apply cleanly with an llvm checkout that includes clang checked out into the tools directory.
> 
> This looks great to me.  When you commit it, please split it into two pieces though:
> 
> 1) The part that adds cl compatibility with StringRef.
> 2) The part that switches lots of cl::opt<> to use StringRef instead of std::string.
> 
> Thanks Puyan,
> 
> -Chris
> _______________________________________________
> 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/20130930/81422b9c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clangWorkaroundParseCommandLineOptions.diff
Type: application/octet-stream
Size: 2023 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130930/81422b9c/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130930/81422b9c/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cloptStringRef.diff
Type: application/octet-stream
Size: 45922 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130930/81422b9c/attachment-0001.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130930/81422b9c/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: CommandLine.diff
Type: application/octet-stream
Size: 3945 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130930/81422b9c/attachment-0002.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130930/81422b9c/attachment-0003.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: CommandLineDocs.diff
Type: application/octet-stream
Size: 2641 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130930/81422b9c/attachment-0003.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130930/81422b9c/attachment-0004.html>