Dynamic subgraph accesses elements of empty vector

[leezu]

Description

Dynamic subgraph code accesses elements of empty vector, causing MXNet to abort.
The PR only passed CI because of the outdated toolchain used on CI. CI with updated toolchain (ie #17984) catches the bug.

Error Message

/usr/include/c++/7/bits/stl_vector.h:815: std::vector<_Tp, _Alloc>::const_reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) const [with _Tp = mxnet:: Resource; _Alloc = std::allocator<mxnet::Resource>; std::vector<_Tp, _Alloc>::const_reference = const mxnet::Resource&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Asser tion '__builtin_expect(__n < this->size(), true)' failed.

To Reproduce

Compile MXNet with gcc7+ and _GLIBCXX_ASSERTIONS, for example via
CC=gcc-7 CXX=g++-7 cmake -GNinja -DUSE_CUDA=0 -DCMAKE_BUILD_TYPE=RelWithDebInfo ..; ninja.

Run python3 -m nose --verbose ../tests/python/unittest/test_extensions.py -m test_subgraph.
This will raise the error above.

OR apply the following simple patch

diff --git a/src/c_api/c_api.cc b/src/c_api/c_api.cc
index 949a59406..2c17320f0 100644
--- a/src/c_api/c_api.cc
+++ b/src/c_api/c_api.cc
@@ -187,7 +187,7 @@ void CustomFComputeDispatcher(const std::string op_name,
   }

   // get memory resource and mxnet backend streams
-  const Resource &resource = ctx.requested[0];
+  const Resource &resource = ctx.requested.at(0);
   mshadow::Stream<mxnet::cpu> *cpu_stream = ctx.get_stream<mxnet::cpu>();
   mshadow::Stream<mxnet::gpu> *gpu_stream = ctx.get_stream<mxnet::gpu>();

and MXNet will always crash with vector: :_M_range_check: __n (which is 0) >= this->size() (which is 0) during the dynamic subgraph test.

cc @samskalicky @rondogency

[samskalicky]

Hi @leezu, @rondogency and I have discussed unifying the resource requests for custom ops and custom subgraph ops and have made the change in #17762 to remove DefaultSubgraphOpResourceRequest. This change will fix the issue such that ctx.requested will always have at least one element.

But @rondogency being the overly cautious expert that he is also changed it to ctx.requested.at(0) and added CHECK(ctx.requested.size() >= 2) too.

Thanks for investigating this issue!

[leezu]

Fixed by #17762. Thanks @rondogency and @samskalicky for the quick fix.