search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.92k stars 548 forks source link

BBC: Blead Breaks SMUELLER/XS-TCC-0.05.tar.gz #19440

Open eserte opened 2 years ago

eserte commented 2 years ago

Since perl 5.35.4 there are no pass reports for this module, see http://matrix.cpantesters.org/?dist=XS-TCC-0.05;reports=1#sl=4,1

There seem to be different types of test failures, here are some different-looking fail reports:

Also (report not yet on cpantesters) probably only on older systems (e.g. debian:jessie) it fails like this (same system with perl 5.34.0 or older is fine):

tcc: error: invalid option -- '-std=gnu99' at t/10_lowlevel.t line 40.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 7.
t/10_lowlevel.t .. 
Dubious, test returned 255 (wstat 65280, 0xff00)
All 7 subtests passed

@tsee: FYI

jkeenan commented 2 years ago

On 2/19/22 06:00, Slaven Rezić wrote:

Since perl 5.35.4 there are no pass reports for this module, see http://matrix.cpantesters.org/?dist=XS-TCC-0.05;reports=1#sl=4,1 http://matrix.cpantesters.org/?dist=XS-TCC-0.05;reports=1#sl=4,1

There seem to be different types of test failures, here are some different-looking fail reports:

The failure below is not new. It has been showing up on some, but not all, smokers since 5.25.7.

Testing this will require the Alien::TinyCC module -- presumably to get the TinyCC C compiler.

Steffen includes this thoughtful comment in the DESCRIPTION section of the documentation:

"Before you consider adopting this module, please have a look at the C::Blocks module. C::Blocks is a more powerful, more lovingly maintained piece of software. Due to current (as of early 2017) scarcity of spare time on my part, you can't expect to get a lot of support for XS::TCC from me. This being said, XS::TCC should work reliably at least on reasonably standard Linux systems. --Steffen"

jkeenan commented 2 years ago

As @eserte noted, XS-TCC has displayed several different kinds of errors in CPANtesters results. On a Linux machine with the following characteristics:

$ uname -mrs
Linux 5.4.0-100-generic x86_64

... I have only been able to reproduce one of those kinds of errors when trying to install XS::TCC against an unthreaded build of Perl 5 blead. That error shows up like this:

$ bleadprove -vb t/30_inline.t 
t/30_inline.t .. 
ok 1 - Alive
...
ok 10 - Alive
Missing right curly or square bracket at (eval 22) line 1, within string
syntax error at (eval 22) line 1, at EOF
Use of uninitialized value $out in concatenation (.) or string at /home/jkeenan/.cpan/build/XS-TCC-0.05-0/blib/lib/XS/TCC.pm line 428.
ok 11 - Alive after compilation
not ok 12 - simple function with pTHX works

#   Failed test 'simple function with pTHX works'
#   at t/30_inline.t line 96.
#          got: undef
#     expected: '6'
ok 13 - Alive
...

I believe the relevant code in XS-TCC's t/30_inline.t is this:

tcc_inline
  #warn_code => 1,
  q{ 
    SV *
    foo3(pTHX_ SV *sv)
    {
      return sv_2mortal(newSViv(SvIV(sv)+1));
    }
  };

pass("Alive after compilation");

is(foo3(5), 6, "simple function with pTHX works"); # <-- test which failed

Based on a study of more than a dozen CPANtesters results from @eserte and @andk, I was able to construct the following bisection command:

perl Porting/bisect.pl \
--start=a9a1cd1d6ae3cd249b78a78a8a41cb041d93c0d0 \
--end=2c205b5406a70a5753a289ca1b05dace7c12727a \
--module=XS::TCC

... which pointed to the following commit:

f0abb35079fb598ed88ef367d405fdd698a736d6 is the first bad commit
commit f0abb35079fb598ed88ef367d405fdd698a736d6
Author: Tony Cook <tony@develop-help.com>
Date:   Mon Sep 6 14:29:45 2021 +1000
Commit:     Tony Cook <tony@develop-help.com>
CommitDate: Mon Sep 13 10:05:02 2021 +1000

    test and fix using T_SV as an OUTPUT parameter

 ext/XS-Typemap/Typemap.pm  | 1 +
 ext/XS-Typemap/Typemap.xs  | 7 +++++++
 ext/XS-Typemap/t/Typemap.t | 6 +++++-
 lib/ExtUtils/typemap       | 2 +-
 4 files changed, 14 insertions(+), 2 deletions(-)
bisect run success
That took 3048 seconds.

I was able to confirm this result by building unthreaded perls at the preceding commit, installing XS::TCC successfully, then building at the cited commit and having a test failure when trying to install XS::TCC.

I suspect I may get different errors when installing against a threaded build, but I haven't attempted that yet.

@tonycoz, can you take a look?

jkeenan commented 2 years ago

I suspect I may get different errors when installing against a threaded build, but I haven't attempted that yet.

And that has proved to be the case. Building a threaded perl at blead and attempting to install XS::TCC via cpanm, I get:

$ thrprove -vb t/*.t
t/10_lowlevel.t .. 
ok 1 - Alive
ok 2 - An object of class 'XS::TCC::TCCState' isa 'XS::TCC::TCCState'
ok 3 - Alive
ok 4
ok 5 - Real code example compiles
ok 6
ok 7 - An object of class 'XS::TCC::TCCSymbol' isa 'XS::TCC::TCCSymbol'
tcc: error: invalid option -- '-std=gnu99' at t/10_lowlevel.t line 40.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 7.
Dubious, test returned 255 (wstat 65280, 0xff00)
All 7 subtests passed 
t/20_parser.t .... 
ok 1 - Parsing no code yields empty but valid result
ok 2 - parsing basic function
ok 3 - parsing basic function
1..3
ok
t/30_inline.t .... 
ok 1 - Alive
ok 2 - Alive after compilation
Dubious, test returned 1 (wstat 256, 0x100)
All 2 subtests passed 

Test Summary Report
-------------------
t/10_lowlevel.t (Wstat: 65280 Tests: 7 Failed: 0)
  Non-zero exit status: 255
  Parse errors: No plan found in TAP output
t/30_inline.t  (Wstat: 256 Tests: 2 Failed: 0)
  Non-zero exit status: 1
  Parse errors: No plan found in TAP output
Files=3, Tests=12,  1 wallclock secs ( 0.04 usr  0.01 sys +  0.41 cusr  0.08 csys =  0.54 CPU)
Result: FAIL

@tonycoz, can you take a look?

jkeenan commented 2 years ago

I suspect I may get different errors when installing against a threaded build, but I haven't attempted that yet.

And that has proved to be the case. Building a threaded perl at blead and attempting to install XS::TCC via cpanm, I get:

$ thrprove -vb t/*.t
t/10_lowlevel.t .. 
ok 1 - Alive
ok 2 - An object of class 'XS::TCC::TCCState' isa 'XS::TCC::TCCState'
ok 3 - Alive
ok 4
ok 5 - Real code example compiles
ok 6
ok 7 - An object of class 'XS::TCC::TCCSymbol' isa 'XS::TCC::TCCSymbol'
tcc: error: invalid option -- '-std=gnu99' at t/10_lowlevel.t line 40.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 7.
Dubious, test returned 255 (wstat 65280, 0xff00)
All 7 subtests passed 
t/20_parser.t .... 
ok 1 - Parsing no code yields empty but valid result
ok 2 - parsing basic function
ok 3 - parsing basic function
1..3
ok
t/30_inline.t .... 
ok 1 - Alive
ok 2 - Alive after compilation
Dubious, test returned 1 (wstat 256, 0x100)
All 2 subtests passed 

Test Summary Report
-------------------
t/10_lowlevel.t (Wstat: 65280 Tests: 7 Failed: 0)
  Non-zero exit status: 255
  Parse errors: No plan found in TAP output
t/30_inline.t  (Wstat: 256 Tests: 2 Failed: 0)
  Non-zero exit status: 1
  Parse errors: No plan found in TAP output
Files=3, Tests=12,  1 wallclock secs ( 0.04 usr  0.01 sys +  0.41 cusr  0.08 csys =  0.54 CPU)
Result: FAIL

Bisecting with the following invocation:

perl Porting/bisect.pl \
-Duseithreads \
--start=e19f9232db \
--end=170944218d \
--module=XS::TCC

... we find that building XS::TCC against threaded perls first encounters test failures in this commit:

3683a9ee5ad61f8cb5eac30e61b4936bd0445bdb is the first bad commit
commit 3683a9ee5ad61f8cb5eac30e61b4936bd0445bdb
Author: Nicholas Clark <nick@ccl4.org>
Date:   Sun Jun 20 17:24:15 2021 +0000
Commit:     Tony Cook <tony@develop-help.com>
CommitDate: Tue Sep 7 15:46:16 2021 +1000

    If we have thread local storage, use it instead of posix_getspecific().

    Declare and use a variable PL_current_context to store a pointer to the
    thread's interpreter struct. Use this to implement implicit context undef
    ITHREADS, instead of a call to posix_getspecific().

    For normal threaded code, this will eliminate function calls. For threaded
    code in shared objects (the typical configuration for Linux distribution
    builds) there still needs to be a function call to get the pointer to
    thread local variables, but there will only be one per function, not one
    per read of the variable.

 globals.c  |  8 ++++++++
 makedef.pl |  3 +++
 thread.h   | 32 ++++++++++++++++++++++++++------
 3 files changed, 37 insertions(+), 6 deletions(-)
bisect run success
That took 3478 seconds.

@nwc10, @tonycoz, can you take a look?

Thank you very much. Jim Keenan

nwc10 commented 2 years ago

OK, it's easy, and it's complex.

If I do this, then the immediate problem related to commit 3683a9ee5ad61f8cb5eac30e61b4936bd0445bdb goes:

diff --git a/thread.h b/thread.h
index dd98a43b6d..33c20994b2 100644
--- a/thread.h
+++ b/thread.h
@@ -372,7 +372,7 @@
 #    define PTHREAD_GETSPECIFIC(key) pthread_getspecific(key)
 #endif

-#if defined(PERL_THREAD_LOCAL) && !defined(PERL_GET_CONTEXT) && !defined(PERL_SET_CONTEXT) && !defined(__cplusplus)
+#if defined(PERL_THREAD_LOCAL) && !defined(PERL_GET_CONTEXT) && !defined(PERL_SET_CONTEXT) && !defined(__cplusplus) && !defined(__TINYC__)
 /* Use C11 thread-local storage, where possible.
  * Frustratingly we can't use it for C++ extensions, C++ and C disagree on the
  * syntax used for thread local storage, meaning that the working token that

tcc defines __TINYC__

At which point I still get test failures in XS::TCC, but the same threaded/unthreaded:

t/10_lowlevel.t .. 1/? tcc: error: undefined symbol '__builtin_expect' at t/10_lowlevel.t line 80.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 2 just after 8.
t/10_lowlevel.t .. Dubious, test returned 2 (wstat 512, 0x200)
All 8 subtests passed
t/20_parser.t .... ok
t/30_inline.t .... 1/? tcc: error: undefined symbol '__builtin_expect' at t/30_inline.t line 24.
t/30_inline.t .... All 3 subtests passed

This, I infer, is because we (now) use one of our macros LIKELY or UNLIKELY in an inline function, and this expands to __builtin_expect, which is gcc-specific

So I guess we can then whack the next mole by doing something similar for LIKELY and UNLIKELY

But this doesn't scale. Does it need to?

The bigger problem here is that perl builds on the assumption that it probes the system for what works, uses newer features where available for better performance, but falls back and still works when the compiler/C library etc does not. (And the relevant information for the system is encoded in Config for future reference)

But all that is undermined when perl is configured using one C compiler (and the system's default compiler), but then extensions attempt to build using a different C compiler which does not implement all features. And these features aren't really things we can detect in C headers. These extensions are making assumptions - that they can ignore our value in $Config{cc} but take all the specific flags in $Config{ccflags}. Until recently this mix-and-match approach has worked, but it seems to be coming unstuck now.

I don't want to say "but to support this existing module unchanged, we can't use these newer features" as that penalises (likely literally) 99% of our userbase for the benefit of a single module on CPAN without a release in 5 years.

If it's only the PERL_THREAD_LOCAL definition, LIKELY and UNLIKELY, and it's only tcc, then working around it in 3 places in our header files probably is the least worse plan. It doesn't generalise, but it seems that attempting generalise it to a hypothetical third compiler that we can't even test against would be over engineering.

So I'm not sure what the best solution is.

jkeenan commented 2 years ago

[snip]

The bigger problem here is that perl builds on the assumption that it probes the system for what works, uses newer features where available for better performance, but falls back and still works when the compiler/C library etc does not. (And the relevant information for the system is encoded in Config for future reference)

But all that is undermined when perl is configured using one C compiler (and the system's default compiler), but then extensions attempt to build using a different C compiler which does not implement all features. And these features aren't really things we can detect in C headers. These extensions are making assumptions - that they can ignore our value in $Config{cc} but take all the specific flags in $Config{ccflags}. Until recently this mix-and-match approach has worked, but it seems to be coming unstuck now.

I don't want to say "but to support this existing module unchanged, we can't use these newer features" as that penalises (likely literally) 99% of our userbase for the benefit of a single module on CPAN without a release in 5 years.

If it's only the PERL_THREAD_LOCAL definition, LIKELY and UNLIKELY, and it's only tcc, then working around it in 3 places in our header files probably is the least worse plan. It doesn't generalise, but it seems that attempting generalise it to a hypothetical third compiler that we can't even test against would be over engineering.

So I'm not sure what the best solution is.

Given that, as I noted earlier, upstream author @tsee has already quasi-deprecated this module, we could bring this problem to his attention and (1) ask him if he wants to provide a workaround; (2) if not, ask him to formally deprecate the module. That would give us some time to figure out whether this affects other CPAN or darkpan code and decide on a better policy.

@tonycoz, do you have advice on this?

hvds commented 2 years ago

The bigger problem here is that perl builds on the assumption that it probes the system for what works, uses newer features where available for better performance, but falls back and still works when the compiler/C library etc does not. (And the relevant information for the system is encoded in Config for future reference)

But all that is undermined when perl is configured using one C compiler (and the system's default compiler), but then extensions attempt to build using a different C compiler which does not implement all features. And these features aren't really things we can detect in C headers. These extensions are making assumptions - that they can ignore our value in $Config{cc} but take all the specific flags in $Config{ccflags}. Until recently this mix-and-match approach has worked, but it seems to be coming unstuck now.

You say "these extensions": are you aware of other modules doing this sort of thing?

It seems clear from what you found that the problem here lies in the module rather than in perl. By the sound of it, to get a properly working version of this module the first job to get would be to do an actual port of perl to tinycc (and it isn't obvious that anyone is motivated to do that), and then for the module's build process to essentially run the full configure process with tcc, so that it has a shadow Config.pm available to it.

hvds commented 2 years ago

You say "these extensions": are you aware of other modules doing this sort of thing?

Ah, I see that the aforementioned C::Blocks is also using tcc. I note though that for now at least it still passes its tests.

nwc10 commented 2 years ago

actual port of perl to tinycc

So, on dromedary.p5h.org I built tinycc 0.9.27 (which is the last proper release), and tried to build blead. It fails to build locale.c because of this chain of bad assumptions

perl.h has

#if (! defined(__IBMC__) || __IBMC__ >= 1210)                               \
 && ((   defined(static_assert) && (   defined(_ISOC11_SOURCE)              \
                                    || (__STDC_VERSION__ - 0) >= 201101L))  \
     || (defined(__cplusplus) && __cplusplus >= 201103L))
/* XXX static_assert is a macro defined in <assert.h> in C11 or a compiler
   builtin in C++11.  But IBM XL C V11 does not support _Static_assert, no
   matter what <assert.h> says.
*/
#  define STATIC_ASSERT_DECL(COND) static_assert(COND, #COND)

and static_assert is defined, so our code assumes that it exists.

in turn /usr/include/assert.h has

#if defined __USE_ISOC11 && !defined __cplusplus
/* Static assertion.  Requires support in the compiler.  */
# undef static_assert
# define static_assert _Static_assert
#endif

in turn, /usr/include/feature.h has

/* If _GNU_SOURCE was defined by the user, turn on all the other features.  */
#ifdef _GNU_SOURCE
# undef  _ISOC95_SOURCE
# define _ISOC95_SOURCE 1
# undef  _ISOC99_SOURCE
# define _ISOC99_SOURCE 1
# undef  _ISOC11_SOURCE
# define _ISOC11_SOURCE 1

and in turn, config.h has

/* HAS_GNULIBC:
 *      This symbol, if defined, indicates to the C program that
 *      the GNU C library is being used.  A better check is to use
 *      the __GLIBC__ and __GLIBC_MINOR__ symbols supplied with glibc.
 */
#define HAS_GNULIBC     /**/
#if defined(HAS_GNULIBC) && !defined(_GNU_SOURCE)
#   define _GNU_SOURCE
#endif

and

/* HAS_MEMMEM:
 *      This symbol, if defined, indicates that the memmem routine is
 *      available to return a pointer to the start of the first occurrence
 *      of a substring in a memory area (or NULL if not found).
 *      In glibc, memmem is a GNU extension.  The function is visible in
 *      libc, but the prototype is only visible if _GNU_SOURCE is #defined.
 *      Thus we only define this if both the prototype and symbol are found.
 */
#define HAS_MEMMEM              /**/

at which point I concluded that really that version of tinycc isn't really compatible with glibc-aware code.

So I tried the mob branch, which is the development head on https://repo.or.cz/w/tinycc.git

I need to Configure like this:

./Configure -des -Dusedevel -Dcc=tcc -Dccdlflags=-Wl,--export-dynamic

because tinycc doesn't support -Wl,-E (dammit), only the long form. gcc (ie GNU ld) supports both, so one conceivably could switch all Linux to do this, but the relevant code is this in Configure and I'd be loathe to touch it:

case "$ccdlflags" in
    '') case "$osname" in
        *linux*|hpux|gnu*) dflt='-Wl,-E' ;;
        sunos)             dflt='none'   ;;
        *)                 dflt='none'   ;;
    esac ;;
    ' ') dflt='none' ;;
    *)   dflt="$ccdlflags" ;;
esac
rp="Any special flags to pass to $cc to use dynamic linking?"
. ./myread
case "$ans" in
    none) ccdlflags=' ' ;;
    *)    ccdlflags="$ans" ;;
esac
;;

Really, tinycc itself should change to accept -E as a synonym for --export-dynamic

Built like that, the only test failures I see are:

$ ./perl TEST porting/libperl.t ../ext/POSIX/t/fenv.t
t/../ext/POSIX/t/fenv ... #   Failed test 'FLT_ROUNDS under FE_TOWARDZERO'
#   at t/fenv.t line 41.
#          got: 1
#     expected: 0
#   Failed test 'FLT_ROUNDS under FE_UPWARD'
#   at t/fenv.t line 41.
#          got: 1
#     expected: 2
#   Failed test 'FLT_ROUNDS under FE_DOWNWARD'
#   at t/fenv.t line 41.
#          got: 1
#     expected: 3
# Looks like you failed 3 tests of 16.
FAILED at test 7
t/porting/libperl ....... # Failed test 4 - has data const symbols at porting/libperl.t line 323
# Failed test 5 - has PL_no_mem at porting/libperl.t line 324
FAILED at test 4
Failed 2 tests out of 0, 0.00% okay.
        ../ext/POSIX/t/fenv.t
        porting/libperl.t

I conclude that basically there isn't really much "porting" to do, and possibly a bug in tinycc codegen related to obscure floating point.

The config.sh diff, edited to remove obvious s/gcc/tcc/, the enormous number of symbols that only gcc defines, and some non-absolute paths in libpth, is roughly:

-cccdlflags='-fPIC'
-ccdlflags='-Wl,-E'
-ccflags='-std=gnu99 -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
+cccdlflags=' '
+ccdlflags='-Wl,--export-dynamic'
+ccflags='-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'

-d_thread_local='define'
+d_thread_local='undef'

-ld_can_script='define'
-lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'
-ldflags=' -fstack-protector-strong -L/usr/local/lib'
+ld_can_script='undef'
+lddlflags='-shared -O2 -L/usr/local/lib'
+ldflags=' -L/usr/local/lib'

-perl_static_inline='static __inline__'
-perl_thread_local='__thread'
+perl_static_inline='static inline'
+perl_thread_local=''

Which basically seems to come down to the crunch problems the rest of this ticket is seeing - if tcc does the ./Configure probes, not the system's gcc, then config.sh and config.h have different (working for tcc values). But if tcc is invoked just assuming the gcc derived values will work, things fail because tcc doesn't support them.

rjbs commented 2 years ago

I agree with Nicholas's evaluation here. Boiled down, we should not hold back improvements in the programming environment to support this behavior (compile perl and extensions differently) which has never been something really supported.

I am marking this not-a-blocker