Assertion failure with debugging perl

[eserte]

t/accuracy.t fails, maybe only if perl was compiled with debugging:

PERL_DL_NONLAZY=1 /opt/perl-5.18.1d/bin/perl5.18.1 "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
perl5.18.1: ConstantTime.xs:38: XS_String__Compare__ConstantTime_equals: Assertion `PL_valid_types_PVX[((svtype)((_svcur)->sv_flags & 0xff)) & 0xf] || ((svtype)((_svcur)->sv_flags & 0xff)) == SVt_REGEXP' failed.
t/accuracy.t ..
Failed 9/18 subtests

More information:

• statistical analysis: http://analysis.cpantesters.org/solved?distv=String-Compare-ConstantTime-0.311#conf%3Aoptimize
• test reports overview: http://matrix.cpantesters.org/?dist=String-Compare-ConstantTime%200.311

[hoytech]

Hi sorry, didn't notice this until now. To be honest I have no idea on this one, I don't think I'm doing anything that crazy in the XS... :)

[paultcochrane]

I've been able to reproduce this like so:

perlbrew install -v 5.20.3 --as=5.20.3d -DEBUGGING=both
perlbrew use 5.20.3d
cpanm --installdeps .
perl Makefile.PL
make
make test

Which gives this output:

Running Mkbootstrap for String::Compare::ConstantTime ()                                                  
chmod 644 ConstantTime.bs                                                                                 
PERL_DL_NONLAZY=1 /home/cochrane/perl5/perlbrew/perls/5.20.3d/bin/perl "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
t/99-kwalitee.t .. skipped: these tests are for release candidate testing                                 
t/accuracy.t ..... 1/18 perl: ConstantTime.xs:41: XS_String__Compare__ConstantTime_equals: Assertion `PL_valid_types_PVX[SvTYPE(_svcur) & SVt_MASK] || SvTYPE(_svcur) == SVt_REGEXP' failed.
t/accuracy.t ..... Failed 9/18 subtests                                                                   
t/magic.t ........ ok
t/stats.t ........ skipped: Need to figure out a way to portable collect granular timing data -- see git history
                          
Test Summary Report                                       
-------------------                                                                           
t/accuracy.t   (Wstat: 6 Tests: 9 Failed: 0)
  Non-zero wait status: 6                                   
  Parse errors: Bad plan.  You planned 18 tests but ran 9.
Files=4, Tests=11,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.05 cusr  0.00 csys =  0.07 CPU)
Result: FAIL           
Failed 1/4 test programs. 0/11 subtests failed.
Makefile:1016: recipe for target 'test_dynamic' failed
make: *** [test_dynamic] Error 255

And you're right, i don't think you're doing anything that crazy in the XS... Perhaps the above steps can help you debug the issue further. By guess is that by solving this issue, many of the test failures on CPAN Testers will also be solved.

[paultcochrane]

My XS isn't that great, however, would replacing SvCUR() with SvLEN() do the trick? The tests definitely pass when I make this change, even with debugging mode on.

[hoytech]

Hmm perhaps, I think that should work.. Let me do a little investigation it's been a while since I was reading perlguts so can't remember the difference between them offhand. :)

[paultcochrane]

A cursory read-through of perlapi seems to suggest that SvCUR() is the better choice. Difficult to say conclusively though...

[paultcochrane]

It seems I spoke too soon: sv_len_utf8() seems to be the right choice.

"""
Returns the number of characters in the string in an SV, counting wide UTF-8 bytes as a single character. Handles magic and type coercion.
"""

Swapping SvCUR() for sv_len_utf8() gets the tests to pass on a Perl built with debugging symbols on.

[hoytech]

Hmmm well I believe we would like the actual byte count, no? It's just occurred to me we should have some tests where one of the strings has the utf8 flag set and the other doesn't. I haven't fully considered if there are any information leaks if we handle encodings in this way, so it would be preferable to just deal with byte counts I believe...

[hoytech]

@paultcochrane - Now that you've added the tests in #16 do any of these patches work?

[paultcochrane]

You're right, one should be comparing bytes, however Perl is converting the string to an internal format anyway, so I'm guessing that's why sv_len_utf8() is working. It's hard to know if there's any information lost though (I've not got much experience in this area), althought it's also hard to think of an attack vector where one could use a carefully crafted string with wide characters which then would cause a timing violation such that one could get information about correct/incorrect elements of an attempted password. I do think that one should be very careful though, see e.g. the note about sv_len_utf8 and sv_len in perl5180delta. It is odd, however, that SvCUR doesn't work in debug mode... Reading the perl source didn't give any real hints as to why this might be.

[paultcochrane]

Oh, was writing the above comment (and reading the perl source) and didn't see your above comment. Yes, the patches with sv_len_utf8 still work, however I've only tested with Perl 5.24.3. I think it's probably best to go back and check from Perl 5.8 just to be certain.

[hoytech]

Cool, thanks for letting me know. My main concern was if sv_len_utf8 was returning code-points instead of bytes.

Unfortunately it's been a while since I've looked into all the perl guts stuff so I'm not sure what the SvCUR problem in debug mode is about... I believe in basically all my other modules I use SvCUR and haven't had any problems that I know about.

[paultcochrane]

Which modules are those? I can try them out with debugging turned on and see if the problem is also lurking there. If I'm lucky I might get more insight into the problem.

[paultcochrane]

I've tested Perls 5.8 .. 5.26 both with and without debugging enabled. The code using SvCUR fails on the debugging versions and passes on the non-debugging versions; by making the change to using sv_len_utf8 then the tests pass on all versions. I've got a patch in the pipeline which will runs the test suite on all these versions on Travis. If you want this, I can submit it as a proper pull request.

[hoytech]

For example I do it in Session::Token:

https://github.com/hoytech/Session-Token/blob/master/Token.xs#L71

Sure I'll take it as a PR, it's probably the right thing -- I may hold off on merging it if that's OK, I just want to try to understand the issue a bit better. Thanks for everything you're doing on this issue though!