[cfe-commits] r115049 - in /cfe/trunk: include/clang/Basic/Version.h lib/Basic/Makefile lib/Basic/Version.cpp

[Daniel Dunbar]

On Wed, Sep 29, 2010 at 11:17 AM, Benjamin Kramer
<benny.kra at googlemail.com> wrote:
>
> On 29.09.2010, at 19:57, Daniel Dunbar wrote:
>
>> Author: ddunbar
>> Date: Wed Sep 29 12:57:10 2010
>> New Revision: 115049
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=115049&view=rev
>> Log:
>> Basic: Add support for git svn to get the repo version in clang executable,
>> patch by Jonathan Mulder!
>>
>> Modified:
>>    cfe/trunk/include/clang/Basic/Version.h
>>    cfe/trunk/lib/Basic/Makefile
>>    cfe/trunk/lib/Basic/Version.cpp
>>
>> Modified: cfe/trunk/lib/Basic/Version.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Version.cpp?rev=115049&r1=115048&r2=115049&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Basic/Version.cpp (original)
>> +++ cfe/trunk/lib/Basic/Version.cpp Wed Sep 29 12:57:10 2010
>> @@ -21,26 +21,27 @@
>> namespace clang {
>>
>> llvm::StringRef getClangRepositoryPath() {
>> -  static const char URL[] = "$URL$";
>> -  const char *URLEnd = URL + strlen(URL);
>> +#ifdef SVN_REPOSITORY
>> +  if (SVN_REPOSITORY[0] != '\0') {
>> +    static const char URL[] = SVN_REPOSITORY;
>> +    const char *URLEnd = URL + strlen(URL) - 1;
>> +
>> +    // Strip off version from a build from an integration branch.
>> +    const char *End = strstr(URL, "/src/tools/clang");
>> +    if (End)
>> +      URLEnd = End;
>> +
>> +    const char *Begin = strstr(URL, "cfe/");
>> +    if (Begin)
>> +      return llvm::StringRef(Begin + 4, URLEnd - Begin - 4);
>>
>> -  const char *End = strstr(URL, "/lib/Basic");
>> -  if (End)
>> -    URLEnd = End;
>> -
>> -  // Strip off version from a build from an integration branch.
>> -  End = strstr(URL, "/src/tools/clang");
>> -  if (End)
>> -    URLEnd = End;
>> -
>> -  const char *Begin = strstr(URL, "cfe/");
>> -  if (Begin)
>> -    return llvm::StringRef(Begin + 4, URLEnd - Begin - 4);
>> -
>> -  return llvm::StringRef(URL, URLEnd - URL);
>> +    return llvm::StringRef(URL, URLEnd - URL);
>> +  }
>> +#endif
>> +  return "";
>> }
>>
>> -std::string getClangRevision() {
>> +llvm::StringRef getClangRevision() {
>
> We can't return a string temporary as a StringRef safely.

Silly me. I spot checked the code for this in getClangRepositoryPath,
which is safe, and almost changed it anyway. Then I forgot to check
getClangRevision.

Will fix.

 - Daniel

>> #ifdef SVN_REVISION
>>   if (SVN_REVISION[0] != '\0') {
>>     std::string revision;
>> @@ -55,10 +56,15 @@
>> std::string getClangFullRepositoryVersion() {
>>   std::string buf;
>>   llvm::raw_string_ostream OS(buf);
>> -  OS << getClangRepositoryPath();
>> -  const std::string &Revision = getClangRevision();
>> -  if (!Revision.empty())
>> -    OS << ' ' << Revision;
>> +  const llvm::StringRef &Path = getClangRepositoryPath();
>> +  const llvm::StringRef &Revision = getClangRevision();
>
> Use after free due to the string problem above.
>
>> +  if (!Path.empty())
>> +    OS << Path;
>> +  if (!Revision.empty()) {
>> +    if (!Path.empty())
>> +      OS << ' ';
>> +    OS << Revision;
>> +  }
>>   return OS.str();
>> }
>