Solaris 11.1 changes building of code past the point of __NORETURN
- by alanc
While Solaris 11.1 was under development, we started seeing some errors in
the builds of the upstream X.Org git master sources, such as:
"Display.c", line 65: Function has no return statement : x_io_error_handler
"hostx.c", line 341: Function has no return statement : x_io_error_handler
from functions that were defined to match a specific callback definition
that declared them as returning an int if they did return, but these were
calling exit() instead of returning so hadn't listed a return value.
These had been generating warnings for years which we'd been ignoring, but
X.Org has made enough progress in cleaning up code for compiler warnings
and static analysis issues lately, that the community turned up the default
error levels, including the gcc flag -Werror=return-type and the
equivalent Solaris Studio cc flags -v -errwarn=E_FUNC_HAS_NO_RETURN_STMT,
so now these became errors that stopped the build.
Yet on Solaris, gcc built this code fine, while Studio errored out.
Investigation showed this was due to the Solaris headers, which during
Solaris 10 development added a number of annotations to the headers when
gcc was being used for the amd64 kernel bringup before the Studio amd64
port was ready. Since Studio did not support the inline form of these
annotations at the time, but instead used #pragma for them,
the definitions were only present for gcc.
To resolve this, I fixed both sides of the problem, so that it would work
for building new X.Org sources on older Solaris releases or with older
Studio compilers, as well as fixing the general problem before it broke
more software building on Solaris.
To the X.Org sources, I added the traditional Studio
#pragma does_not_return to recognize that functions like exit()
don't ever return, in patches such as this Xserver patch. Adding a dummy return statement was
ruled out as that introduced unreachable code errors from compilers and
analyzers that correctly realized you couldn't reach that code after a
return statement.
And on the Solaris 11.1 side, I updated the annotation definitions in
<sys/ccompile.h> to enable for Studio 12.0 and later compilers
the annotations already existing in a number of system headers for functions
like exit() and abort(). If you look in that file you'll
see the annotations we currently use, though the forms there haven't gone
through review to become a Committed interface, so may change in the future.
Actually getting this integrated into Solaris though took a bit more work
than just editing one header file. Our ELF binary build comparison tool,
wsdiff,
actually showed a large number of differences in the resulting binaries due
to the compiler using this information for branch prediction, code path
analysis, and other possible optimizations, so after comparing enough of the
disassembly output to be comfortable with the changes, we also made sure to
get this in early enough in the release cycle so that it would get plenty of
test exposure before the release.
It also required updating quite a bit of code to avoid introducing new lint
or compiler warnings or errors, and people building applications on top of
Solaris 11.1 and later may need to make similar changes if they want to keep
their build logs similarly clean.
Previously, if you had a function that was declared with a non-void return
type, lint and cc would warn if you didn't return a value, even if you called
a function like exit() or panic() that ended execution.
For instance:
#include <stdlib.h>
int
callback(int status)
{
if (status == 0)
return status;
exit(status);
}
would previously require a never executed return 0; after the
exit() to avoid lint warning "function falls off bottom without
returning value".
Now the compiler & lint will both issue "statement not reached"
warnings for a return 0; after the final exit(),
allowing (or in some cases, requiring) it to be removed. However, if
there is no return statement anywhere in the function, lint will warn
that you've declared a function returning a value that never does so,
suggesting you can declare it as void. Unfortunately, if your
function signature is required to match a certain form, such as in a
callback, you not be able to do so, and will need to add a
/* LINTED */ to the end of the function.
If you need your code to build on both a newer and an older release, then you
will either need to #ifdef these unreachable statements, or, to keep
your sources common across releases, add to your sources the corresponding
#pragma recognized by both current and older compiler versions, such as:
#pragma does_not_return(exit)
#pragma does_not_return(panic)
Hopefully this little extra work is paid for by the compilers & code
analyzers being able to better understand your code paths, giving you
better optimizations and more accurate errors & warning messages.
