[Snippets] Fixed set for Windows #17859
Conversation
|
/azp run openvino-onnxruntime-lin |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run openvino-ngraph-onnx-lin |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -106,7 +106,8 @@ void LinearIR::LoopManager::get_io_loop_ports(LinearIR::constExprIt loop_begin_p | |||
| const auto& expr = *expr_it; | |||
| for (size_t i = 0; i < expr->get_input_count(); ++i) { | |||
| const auto in_port = expr->get_input_port(i); | |||
| const auto& parent_expr = in_port.get_connected_ports().begin()->get_expr(); | |||
| const auto source_port = *in_port.get_connected_ports().begin(); | |||
| const auto& parent_expr = source_port.get_expr(); | |||
There was a problem hiding this comment.
Please describe in details why original code led to segfault on Windows?
There was a problem hiding this comment.
Honestly, I'm faced with this problem several times on ngraph (get_output_target_inputs has the same behavior) 😄
Let's take a look at previous 109 line:
get_connected_ports() returns sets of ExpressionPort instances (the method creates new set and returns it).
Then we take iterator on first element using begin() (this element is definitely in the set).
And we call get_expr from this first element and takes reference of shared pointer. But the set of connected ports is destroyed since it's local variable and isn't saved in this code frame. Thus, reference of shared pointer refs to destroyed memory as I understand. And we get corrupted data:
But as I said, this element is definitely in the set:
Sure, there is another solution. Save just the copy of shared pointer, not reference:
| const auto& parent_expr = source_port.get_expr(); | |
| const auto parent_expr = in_port.get_connected_ports().begin()->get_expr(); |
for every taste 🙂
There was a problem hiding this comment.
Thank you for the explanation. On my personal taste I would prefer one line solution.
There was a problem hiding this comment.
As you say 😁
Done
a-sidorova
force-pushed
the
fix/snippets/set_iterator
branch
from
4d9bebf to
7089efe
Compare
Details: