[llvm] r307085 - Fix dangling StringRefs found by clang-tidy misc-dangling-handle check.

[Alexander Kornienko via llvm-commits]

On Mon, Jul 10, 2017 at 7:15 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Wed, Jul 5, 2017 at 8:02 AM Alexander Kornienko via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: alexfh
>> Date: Tue Jul  4 08:13:02 2017
>> New Revision: 307085
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=307085&view=rev
>> Log:
>> Fix dangling StringRefs found by clang-tidy misc-dangling-handle check.
>>
>> Modified:
>>     llvm/trunk/tools/llvm-lto/llvm-lto.cpp
>>     llvm/trunk/tools/llvm-readobj/COFFDumper.cpp
>>
>> Modified: llvm/trunk/tools/llvm-lto/llvm-lto.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-
>> lto/llvm-lto.cpp?rev=307085&r1=307084&r2=307085&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/tools/llvm-lto/llvm-lto.cpp (original)
>> +++ llvm/trunk/tools/llvm-lto/llvm-lto.cpp Tue Jul  4 08:13:02 2017
>> @@ -383,7 +383,7 @@ loadAllFilesForIndex(const ModuleSummary
>>
>>    for (auto &ModPath : Index.modulePaths()) {
>>      const auto &Filename = ModPath.first();
>> -    auto CurrentActivity = "loading file '" + Filename + "'";
>> +    std::string CurrentActivity = ("loading file '" + Filename +
>> "'").str();
>>      auto InputOrErr = MemoryBuffer::getFile(Filename);
>>      error(InputOrErr, "error " + CurrentActivity);
>>      InputBuffers.push_back(std::move(*InputOrErr));
>> @@ -475,7 +475,7 @@ private:
>>      std::vector<std::unique_ptr<MemoryBuffer>> InputBuffers;
>>      for (unsigned i = 0; i < InputFilenames.size(); ++i) {
>>        auto &Filename = InputFilenames[i];
>> -      StringRef CurrentActivity = "loading file '" + Filename + "'";
>> +      std::string CurrentActivity = "loading file '" + Filename + "'";
>>        auto InputOrErr = MemoryBuffer::getFile(Filename);
>>        error(InputOrErr, "error " + CurrentActivity);
>>        InputBuffers.push_back(std::move(*InputOrErr));
>> @@ -710,7 +710,7 @@ private:
>>      std::vector<std::unique_ptr<MemoryBuffer>> InputBuffers;
>>      for (unsigned i = 0; i < InputFilenames.size(); ++i) {
>>        auto &Filename = InputFilenames[i];
>> -      StringRef CurrentActivity = "loading file '" + Filename + "'";
>> +      std::string CurrentActivity = "loading file '" + Filename + "'";
>>        auto InputOrErr = MemoryBuffer::getFile(Filename);
>>        error(InputOrErr, "error " + CurrentActivity);
>>        InputBuffers.push_back(std::move(*InputOrErr));
>>
>> Modified: llvm/trunk/tools/llvm-readobj/COFFDumper.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-
>> readobj/COFFDumper.cpp?rev=307085&r1=307084&r2=307085&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/tools/llvm-readobj/COFFDumper.cpp (original)
>> +++ llvm/trunk/tools/llvm-readobj/COFFDumper.cpp Tue Jul  4 08:13:02 2017
>> @@ -1637,7 +1637,11 @@ static StringRef getBaseRelocTypeName(ui
>>    case COFF::IMAGE_REL_BASED_HIGHADJ: return "HIGHADJ";
>>    case COFF::IMAGE_REL_BASED_ARM_MOV32T: return "ARM_MOV32(T)";
>>    case COFF::IMAGE_REL_BASED_DIR64: return "DIR64";
>> -  default: return "unknown (" + llvm::utostr(Type) + ")";
>> +  default: {
>> +    static std::string Result;
>> +    Result = "unknown (" + llvm::utostr(Type) + ")";
>> +    return Result;
>>
>
> This looks concerning/wrong. Now two callers tot his API could interfere
> with each other (the returned StringRef is only valid until the function is
> called again)
>
> Perhaps this function should return std::string instead.
>

The motivation for this solution is to keep the cheap path cheap (and the
expensive one isn't going to happen under normal circumstances anyway).
I've inspected the only caller, and it's not going to cause such issues.
However, I should have probably added a FIXME about this or somehow draw
the attention of someone who cares about this code to fix it properly.


>
>
>> +  }
>>    }
>>  }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170711/d9724067/attachment.html>