Re: [PATCH] non-gcc-specific exception.sh

[Nicolas Iooss <nicolas.iooss@m4x.org>]

On Mon, Oct 7, 2019 at 9:21 PM Michael Shigorin <mike@altlinux.org> wrote:
>
> On Mon, Oct 07, 2019 at 06:27:29PM +0200, Nicolas Iooss wrote:
> > > please find attached the patch to (hopefully) improve
> > > self-surgery script that uses gcc-specific -aux-info now.
> > > Should help clang, icc and the like (in my case there's
> > > no proper gcc port for the target platform just yet).
> > How did you test this patch? On my system (Arch Linux x86-64),
> > I get the following differences in the generated list of
> > functions, for libselinux:
> >
> > +select
> > +pselect
> > -selinuxfs_exists
>
> selinuxfs_exists is declared as just int, not external int, and
> gets filtered out with awk one-liner within the original script.
>
> I've spotted select/pselect and have paid some attention to
> zeroing the diff *but* overlooked the <stdin> filter in the
> script from libselinux-2.9 tagged tree I started looking at.
> Evgeny suggested doing something about the changed source
> marker format either, shame on me for distracting.
>
> > This is because /usr/include/sys/select.h contains "extern int select
> > (int __nfds, fd_set *__restrict __readfds," and "extern int pselect
> > (int __nfds, fd_set *__restrict __readfds,", and because
> > libselinux/include/selinux/selinux.h contains "int
> > selinuxfs_exists(void);" without "extern". Your patch therefore
> > changes things in a way that seems unintended.
>
> Will this one do better?  Looks a bit messy though[1]:
>
> gcc -E -I../include -xc ../include/selinux/selinux.h |
>   awk -F '[ (]' '/^#/{p=0} \
>     /^# .*selinux\/selinux\.h/{p=1} \
>     /^extern int/{if(p==1){print $3}}'
>
> > As the regular expression you sent is quite fragile a possible
> > way of preventing issues such as the differences I found is
> > to try using both methods (-aux-info and -E) in a "make test"
> > target and fail with a fatal error when they do not produce
> > the same output (this fatal error would be caught by a
> > continuous-integration system, which would make the developers
> > aware of something wrong).
>
> Good test for those who have it handy, my primary intent was
> to be able to at least build without (not-yet-ported) gcc.
>
> > Moreover, please send your patch inline, if possible (for example with
> > "git send-email"), and add a "Signed-off-by:" line as documented in
> > https://github.com/SELinuxProject/selinux/blob/master/CONTRIBUTING.md#contributing-code.
>
> Thank you for the review and suggestions, please have a look
> at the proposed handler replacement; if it's ok I'll arrange
> it as a commit and hopefully figure out the test tomorrow,
> getting sleepy by now...

Hello,
I wanted to spend some time testing different approaches before
replying. Your awk command works fine, but I am wondering whether its
complexity is needed at all. For example, why not directly grab the
"extern int" lines in selinux.h (using sed -n s/.../.../p). This would
work well with libselinux and avoid using awk for this simple task,
but not for libsemanage... or it could, with:

    cat ../include/semanage/*.h | \
    sed -n 's/^extern \+int \+\([0-9A-Za-z_]\+\) *(.*$/\1/p'

Actually, the main reason why it is currently not possible to migrate
from "gcc -aux-info and awk" to "cat | sed" is caused by the fact that
the file generated by "gcc -aux-info" adds "extern" before all
function prototypes. Adding "extern" directly to the prototypes of all
exported functions in header files seems to be required in order to
use something else than "gcc -aux-info" to generate the glue code that
handles exception in Python bindings.

Therefore I suggest the following approach:
1. add "extern" to all exported functions in libselinux and
libsemanage public headers (I can send a patch for this tomorrow if
you don't want to do this)
2. change exception.sh to use something else than "gcc -aux-info" to
generate the glue code
3. add something so that "make test" compares the result of "$CC
-aux-info" (if $CC is gcc-compatible) with the new way to find
functions, in order to report differences in the generated glue code.
This could for example consist in adding some code in exception.sh
such that "./exception.sh test" performs this check.

Would this suit you?

By the way, I found out that libsemanage generates glue code for
select() and pselect() too, as it does not filter the output of "gcc
-aux-info". This is a bug that needs to be fixed.

Thanks,
Nicolas