[omniORB] Add [[noreturn]] attribute to functions throwing exceptions

[Michał Liszcz]

Hi,

I'm working on integration of clang-analyzer (https://clang-analyzer.llvm.org/)
with cppTango (https://gitlab.com/tango-controls/cppTango) which uses omniORB.

We are seeing some false positives coming from omniORB headers (mostly from
seqTemplatedecls.h and stringtypes.h). The analyzer takes impossible execution
path because it does not have access to definitions of throwing functions like
_CORBA_new_operator_return_null. For instance we see this warning:

/usr/include/omniORB4/stringtypes.h:730:13: warning: Array access
(from variable 'newdata') results in a null pointer dereference
[core.NullDereference]
        newdata[i] = pd_data[i];

The warning is coming from below code, where analyzer assumes that allocbuf
returns null but it does not know that _CORBA_new_operator_return_null always
throws and instead continues the analysis.

    char** newdata = allocbuf(newmax);
    if (!newdata) {
      _CORBA_new_operator_return_null();
      // never reach here
    }
    for (unsigned long i=0; i < pd_len; i++) {
      if (pd_rel) {
    newdata[i] = pd_data[i];

Here is the link showing control flow:
https://tango-controls.gitlab.io/-/cppTango/-/jobs/1055130028/artifacts/clang-analyzer-results/scan-build-2021-02-25-09-32-01-442292-q6upph97/report-w_attribute.cpp-copybuffer-11-1.html#EndPath

What I propose to do is to explicitly indicate that these functions are not
going to return but will throw an exception by putting [[noreturn]] attribute
in definitions in CORBA_basetypes.h:

[[noreturn]] extern void _CORBA_new_operator_return_null();
[[noreturn]] extern void _CORBA_bound_check_error();
[[noreturn]] extern void _CORBA_marshal_sequence_range_check_error(cdrStream&);
[[noreturn]] extern void _CORBA_invoked_nil_pseudo_ref();
[[noreturn]] extern void _CORBA_invoked_nil_objref();

This is enough to help clang-analyzer to reject impossible paths. I think that
other static analysis tools can also benefit from this extra information. Note
that this attribute is available only since C++11 so probably some check or an
extra flag at autotools level would be needed if support for older compilers
is needed.

Would it be possible to have [[noreturn]] attribute added in omniORB code?
This can not be worked around in any other way.

Another thing worth considering is to replace comments like below one with
assertions (assert(pd_len <= newmax)) that would enforce these invariants and
would also provide some valuable information for static analysis tools.
    // Invariant:  pd_len <= newmax

I can work on a patch if above proposals can be accepted, but I'd need some
guidance.

Thanks,
Michal