dnn: Fix dangling pointers returned by GetLayerNames#927
dnn: Fix dangling pointers returned by GetLayerNames#927deradiri wants to merge 2 commits intohybridgroup:devfrom
GetLayerNames#927Conversation
|
Yes, I see how the previous implementation was lacking. I just wonder however how this test here has been passing? Line 59 in 5bde20d |
|
@deadprogram Good question... I guess maybe it's that the the caffe backend keeps the layer names around for longer/permanently? I discovered this issue using openvino, will see if I can add a test case. Is there some procedure for adding models as test artifacts? |
5bde20d to
891e7a4
Compare
'GetLayerNames' returns an array of 'char' pointers to cstrings in a 'vector<string>'; unfortunately, once the vector is out of scope, the strings are destroyed. 'GetLayerNames' callers are then left with dangling pointers. This change fixes the problem by expanding the 'strs' buffer returned by 'GetLayerNames' and copying the vector's cstrings into it.
@deadprogram It turns out this test passes by pure luck, I ran it a few times and logged all the layers. Below is a diff between two runs: The layer name at index 1 seems to always be intact, so rather than add a new model I'll update the test to check a set of layers distributed throughout the array. Diff of runs with and without the fix in this PR is below: |
891e7a4 to
f776143
Compare
f776143 to
b55ebc1
Compare


GetLayerNamesreturns an array ofcharpointers tocstringsin avector<string>; unfortunately, once the vector is out of scope, the strings are destroyed.GetLayerNamescallers are then left with dangling pointers.This change fixes the problem by expanding the
strsbuffer returned byGetLayerNamesand copying the vector's cstrings into it.Testing:
See this comment
Here's an example of the bug in action:
Before fix:
$ go test -tags customenv,static Layer 0: @� Layer 1: 0� ...After fix:
$ go test -tags customenv,static Layer 0: detector/yolo-v3-tiny/Conv_12/BiasAdd/YoloRegion Layer 1: detector/yolo-v3-tiny/Conv_9/BiasAdd/YoloRegion ...