r253410 - Fix for use-after-free which caused test failure in cuda-detect.cu.

[David Blaikie via cfe-commits]

On Tue, Nov 17, 2015 at 5:35 PM, Artem Belevich <tra at google.com> wrote:

> getLibDeviceFile only used once per device-side compilation in driver, so
> returning the string should be OK here.
>
> BTW, would copy elision kick in when I use std::string Path =
> getLibDeviceFile("foo"); ?
>

Yeah - if the caller needs a copy anyway (if they're going to build a new
string from it - appending things, etc) then, yeah, the API choice won't
impact performance much/at all.

- Dave


>
> --Artem
>
> On Tue, Nov 17, 2015 at 5:12 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Tue, Nov 17, 2015 at 4:37 PM, Artem Belevich via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: tra
>>> Date: Tue Nov 17 18:37:41 2015
>>> New Revision: 253410
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=253410&view=rev
>>> Log:
>>> Fix for use-after-free which caused test failure in cuda-detect.cu.
>>>
>>> Return std::string itself instead StringRef to a temporary std::string.
>>>
>>> Modified:
>>>     cfe/trunk/lib/Driver/ToolChains.h
>>>
>>> Modified: cfe/trunk/lib/Driver/ToolChains.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=253410&r1=253409&r2=253410&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Driver/ToolChains.h (original)
>>> +++ cfe/trunk/lib/Driver/ToolChains.h Tue Nov 17 18:37:41 2015
>>> @@ -186,7 +186,7 @@ protected:
>>>      /// \brief Get the detected Cuda device library path.
>>>      StringRef getLibDevicePath() const { return CudaLibDevicePath; }
>>>      /// \brief Get libdevice file for given architecture
>>> -    StringRef getLibDeviceFile(StringRef Gpu) const {
>>> +    std::string getLibDeviceFile(StringRef Gpu) const {
>>>        return CudaLibDeviceMap.lookup(Gpu);
>>>
>>
>> You could implement this as:
>>
>> auto I = CudaLibDeviceMap.find(Gpu);
>> return I != CudaLibDeviceMap.end() ? I->second : "";
>>
>> returning StringRef
>>
>> Or, if you know the element will always be in the collection, you could
>> just assert that and "return CudaLibDeviceMap[Gpu]; - and you could return
>> const std::string& there, or StringRef, whichever.
>>
>> (both/any of these options would avoid needing to allocate a new string
>> to every caller)
>>
>>
>>>      }
>>>    };
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
>
> --
> --Artem Belevich
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151117/dd79e7ce/attachment-0001.html>