pthreadpool cfi-icall check failure for XNNPACK function pointers casting

[huningxin]

This issue was raised when integrating XNNPACK and pthreadpool into Chromium (CL-4608520) where Chromium has cfi (control-flow-integrity) enabled build that also checks any cfi-icall (indirect call) issues.

For example, the error message could be:

$ out/cfi/components_unittests --gtest_filter=TFLiteModelExecutorTest.ExecuteWithLoadedModel
IMPORTANT DEBUGGING NOTE: batches of tests are run inside their
own process. For debugging a test inside a debugger, use the
--gtest_filter=<your_test_name> flag along with
--single-process-tests.
Using sharding settings from environment. This is shard 0/1
Using 1 parallel jobs.
Note: Google Test filter = TFLiteModelExecutorTest.ExecuteWithLoadedModel
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from TFLiteModelExecutorTest
[ RUN      ] TFLiteModelExecutorTest.ExecuteWithLoadedModel
INFO: Created TensorFlow Lite XNNPACK delegate for CPU.
../../third_party/pthreadpool/src/src/portable-api.c:1465:5: runtime error: control flow integrity check for type 'void (void *, unsigned long, unsigned long, unsigned long, unsigned long)' failed during indirect function call
../../third_party/xnnpack/src/src/operator-run.c:468: note: xnn_compute_igemm defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../third_party/pthreadpool/src/src/portable-api.c:1465:5 in 
[1/1] TFLiteModelExecutorTest.ExecuteWithLoadedModel (CRASHED)
1 test crashed:
    TFLiteModelExecutorTest.ExecuteWithLoadedModel (../../components/optimization_guide/core/tflite_model_executor_unittest.cc:255)
Tests took 4 seconds.

The root cause is that XNNPACK casts xnn_compute_igemm() to pthreadpool's pthreadpool_task_2d_tile_2d_t type:

void xnn_compute_igemm(
    const struct igemm_context context[restrict XNN_MIN_ELEMENTS(1)],
    size_t mr_block_start,
    size_t nr_block_start,
    size_t mr_block_size,
    size_t nr_block_size)

typedef void (*pthreadpool_task_2d_tile_2d_t)(void*, size_t, size_t, size_t, size_t);

The cfi-icall check at pthreadpool's pthreadpool_parallelize_2d_tile_2d would fail because it expects the first argument is void* but it was const struct igemm_context context[restrict XNN_MIN_ELEMENTS(1)].

[Maratyszcza]

Right, XNNPack is being slightly loose with pointers, assuming any pointer is equivalent to void*. No plans to change that.

[huningxin]

Thanks for the clarification. Chromium merged CL-4608520 that would ignore cfi-icall check warnings for pthreadpool.

[davidben]

Being loose with pointers isn't allowed in C/C++. Calling a function pointer through the wrong type is undefined behavior, and the compiler is allowed to generate code as if it doesn't happen. This isn't just a CFI issue but a rule across the entire language.

Either the original functions should be fixed to have the void* type, or pthreadpool should be passed thin wrappers with the correct type.