[BUGFIX] Reject duplicate input names in C Predict API

[tunalloc]

Description

Avoid a potential uninitialized read when using the C Predict API with symbols where the same input name occurs on more than one node. Other parts of MXNet appear to treat this as an error, so this change makes the C API reject it with an error too.

Since the C Predict API has been removed in MXNet v2 this PR is for merging into the 1.x branch.

Checklist

Essentials

• PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
• Changes are complete (i.e. I finished coding on this PR)
• All changes have test coverage
• Code is well-documented

I could not find any existing tests for the C Predict API. Please let me know if there's documentation that should be updated.

Changes

• Reject symbols where a name occurs more than once in the C Predict API

Comments

It's possible to cause an uninitialized read in the C Predict API. If I construct a symbol and parameters file using the Python API as follows:

import mxnet
net = mxnet.symbol.Variable('a') + mxnet.symbol.Variable('a')
mxnet.nd.save('foo-params', {})
net.save('foo-symbol.json')

Then substitute the contents of the generated files into this C++ code. This causes an uninitialized read and fails the final assertion.

#include <mxnet/c_predict_api.h>
#include <string>
#include <cassert>

namespace {

const char* SYMBOL_JSON = "{\"nodes\":[{\"op\":\"null\",\"name\":\"a\",\"inputs\":[]},{\"op\":\"null\",\"name\":\"a\",\"inputs\":[]},{\"op\":\"elemwise_add\",\"name\":\"_plus0\",\"inputs\":[[0,0,0],[1,0,0]]}],\"arg_nodes\":[0,1],\"node_row_ptr\":[0,1,2,3],\"heads\":[[2,0,0]],\"attrs\":{\"mxnet_version\":[\"int\",10901]}}"; 
const char* PARAMS_BYTES = "\x12\x01\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0\x0";
const std::size_t PARAMS_SIZE = 32;

const char* INPUT_KEYS[] = { "a" };
const std::size_t INPUT_KEYS_SIZE = 1;

const uint32_t INPUT_SHAPE_INDPTR[] = { 0, 1 };
const uint32_t INPUT_SHAPE_DATA[] = { 1 };

}

int main(int, char**) {
  PredictorHandle handle = nullptr;  

  int res = MXPredCreate(
    SYMBOL_JSON,
    static_cast<const void*>(PARAMS_BYTES),
    PARAMS_SIZE,
    1,
    0,
    INPUT_KEYS_SIZE,
    INPUT_KEYS,
    INPUT_SHAPE_INDPTR,
    INPUT_SHAPE_DATA,
    &handle);
  assert(res == 0);

  static const float INPUT_DATA[] = { 5. };
  res = MXPredSetInput(handle, "a", INPUT_DATA, 1);
  assert(res == 0);

  res = MXPredForward(handle);
  assert(res == 0);

  float output = 0.0;
  res = MXPredGetOutput(handle, 0, &output, 1);
  assert(res == 0);

  assert(output == 10.0);
  
  return 0;
}

If instead I generate the symbol as follows the test passes:

import mxnet
var = mxnet.symbol.Variable('a')
net =  var + var
mxnet.nd.save('foo-params', {})
net.save('foo-symbol.json')

I believe the API should reject symbols where an input name appears more than once. This seems to happen in other parts of the code. For example the following code fails with an assertion error:

import mxnet
net = mxnet.symbol.Variable('a') + mxnet.symbol.Variable('a')
out = net.eval(ctx = mxnet.cpu(), a = mxnet.nd.array([[5.]]))

This change makes the C Predict API reject such symbols. With this change the above C++ code fails on the first assertion.

[mxnet-bot]

Hey @tunalloc , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

• To trigger all jobs: @mxnet-bot run ci [all]
• To trigger specific jobs: @mxnet-bot run ci [job1, job2]

---

CI supported jobs: [clang, edge, centos-cpu, centos-gpu, website, windows-cpu, miscellaneous, unix-cpu, unix-gpu, sanity, windows-gpu]

---

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

[waytrue17]

@szha @leezu would you take a look?

[tunalloc]

Apologies for the build failure. Updated the PR. The first version had been prepared on an earlier branch. It should be working now.

[tunalloc]

Looks like just unix-gpu is failing now. The failures look likely to be unrelated to this change.

[leezu]

@mxnet-bot run ci [unix-gpu]

[mxnet-bot]

Jenkins CI successfully triggered : [unix-gpu]