[cfe-commits] configure flag --with-default-sysroot to initialize SysRoot

[Sebastian Pop]

Hi,

could you please review the two patches adding the configure flag
--with-default-sysroot?

I addressed all the comments from the previous review by Rafael.

Thanks,
Sebastian
--
Qualcomm Innovation Center, Inc is a member of Code Aurora Forum


---------- Forwarded message ----------
From:  <spop at codeaurora.org>
Date: Fri, Apr 6, 2012 at 3:22 PM
Subject: Re: [cfe-commits] Use GCC_INSTALL_PREFIX to initialize SysRoot
To: Rafael Espíndola <rafael.espindola at gmail.com>
Cc: cfe-commits at cs.uiuc.edu


Hi Rafael,

See attached the patches with the changes that you suggested.
Tested on amd64-linux.  Ok to commit?

Thanks,
Sebastian
--
Qualcomm Innovation Center, Inc is a member of Code Aurora Forum

>>> +    TranslatedArgs(_TranslatedArgs), Redirects(0),
>>> SysRoot(D.SysRoot)
>>>
>>> Is this value of SysRoot ever used? It should not, right? It is
>>> probably better to initialize SysRoot with an invalid value instead of
>>> D.SysRoot.
>>
>> I'm not sure I understand your comment here.
>>
>> We can also implement getSysRoot by returning the sysroot of the driver.
>
> I am suggesting initializing with something like
>
> ... SysRoot(".....")
>
> to make sure we find any code using the SysRoot before the following code
> runs.
>
>>>  if (SysRoot != "") {
>>> +    llvm::SmallString<128> Prefix(SysRoot);
>>> +    if (Prefix.back() == '/') {
>>> +      llvm::sys::path::remove_filename(Prefix); // remove the /
>>> +      SysRoot = Prefix.str();
>>> +    }
>>> +  }
>>>
>>> This points a StringRef to a value that is destroyed at the closing
>>> brace.
>>
>> Right, I have not understood your remark last time.
>> As the original code was not removing the trailing /,
>> I removed that code.
>
> I think you have another use after free bug in
>
> +    CmdArgs.push_back(sysroot.str().c_str());
>
> since this creates a std::string and takes a pointer to the inner c
> string. You probably have to use C.getArgs().MakeArgString.
>
>> Sebastian
>> --
>> Qualcomm Innovation Center, Inc is a member of Code Aurora Forum
>
> Cheers,
> Rafael
>
> p.s.: you dropped cfe-commits in the reply.
>

_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-add-configure-flag-with-default-sysroot.patch
Type: text/x-diff
Size: 3680 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120411/8fe99a2b/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-use-DEFAULT_SYSROOT.patch
Type: text/x-diff
Size: 5898 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120411/8fe99a2b/attachment-0001.patch>