December 2020 Archives

I'm Making Headway Now

Last January there was a post on reddit which claimed that my module JSON::Parse was not only failing some of the JSON Test Suite tests, but also crashing on one of them. Anyway I should have got around to doing something about it sooner, but here are my conclusions.

First of all there was a crash on one of the files, which went something like this: [{"":[{"":[{"", repeated about 100,000 times. (The actual file is here if you really want to see it.) Investigating it using a LInode, I found that after 80,000 open brackets the stack was overflowing, causing the crash to occur. If I added a printf in the midst of my code the printf would cause the stack overflow, so it wasn't actually due to my code but just because the stack size seems to be quite small on Linux.

There are various things one could do to tackle this, but it does seem a bit unlikely that anyone would want to have that many open brackets, so what I did as a strategy was to add a "max_depth" of parsing after which it would stop. I thought 10,000 open { and [ would be enough for anyone, and it would satisfy the people who want to run the JSON Test Suite tests, but I also added an option for the user to alter the max depth and get the max depth as well.

After finishing that I started to wonder what other modules were doing that they had passed this test. A quick glance at the Cpanel::JSON::XS documentation and it seems that those people had the same idea, but their maximum depth is a measly 512 as opposed to my whopping 10,000. I'm not sure which is better or worse. ๐Ÿ˜•

There were a couple of other tests in the JSON Test Suite which I failed, which were tests about Unicode; the JSON Test Suite thinks that it is compulsory for JSON parsers to pass through Unicode non-characters, UTF-8 representing surrogate pairs, and so on. I'm not really sure why they think so, there is a bit of a discussion on the github but that doesn't strike me as being very convincing. Frankly I think JSON::Parse is much more useful tool for users if it rejects the non-characters and other crap, ๐Ÿ’ฉ so I would need to see much stronger justification than I have done to alter it in the way that JSON Test Suite suggests. emoticon.png

In other news, I've also made it possible for the module to be installed from github, so it should now be possible to start using continuous integration. I've already got Travis working, and I hope to add some more useful things soon.

Misusing newSVpv

I managed to cause another set of obscure bugs by misusing newSVpv. The goal of this code is to split the RGB and the alpha (transparent) part of a PNG image, for the use of the PDF::Builder module on CPAN.

Here the SV in newSVpv is "scalar value", and the "pv" means "string". My code says this:

sv = newSVpv ("", len);

and it causes the crash on Solaris and other operating systems because the above statement is a bad idea. What happens is that Perl copies "len" bytes from my string, where "len" might be a very large number, for a large PNG image, but I've given it the string "" to copy from. So Perl tries to copy from uninitialised, or possibly even inaccessible, parts of the computer's memory. I found out my error only after almost giving up, by using valgrind to look for memory errors.

The correct version of this is

 sv = newSV (sv_len);
 SvPOK_on (sv);
 SvCUR_set (sv, sv_len);

We make a new SV, then we tell Perl that our new SV is meant to be a string using SvPOK_on, then we tell Perl the length of our string is sv_len with SvCUR_set.

Here is the manual page from Perl 5.32 (perldoc perlapi) for newSVpv:

**newSVpv** Creates a new SV and copies a string (which may contain "NUL" ("\0") characters) into it. The reference count for the SV is set to 1. If "len" is zero, Perl will compute the length using "strlen()", (which means if you use this option, that "s" can't have embedded "NUL" characters and has to have a terminating "NUL" byte).

It doesn't really say that it is going to copy len bytes of the string, but I suppose that is obvious in retrospect since it does say that the string may contain NUL characters, so the only test it could be making for the end of the string would be the number of bytes.

This function can cause reliability issues if you are likely to pass in empty strings that are not null terminated, because it will run strlen on the string and potentially run past valid memory.

This seems to be warning about what happened to me in Image::PNG::Libpng version 0.55, but what is an "empty string" that is "not null terminated"? An empty string to me is "", which is automatically NUL terminated. `

Using "newSVpvn" is a safer alternative for non "NUL" terminated strings. For string literals use "newSVpvs" instead. This function will work fine for "NUL" terminated strings, but if you want to avoid the if statement on whether to call "strlen" use "newSVpvn" instead (calling "strlen" yourself). SV* newSVpv(const char *const s, const STRLEN len)

I don't really see why that would be safer. Here is the documentation for newSVpvn:

newSVpvn Creates a new SV and copies a string into it, which may contain "NUL" characters ("\0") and other binary data. The reference count for the SV is set to 1. Note that if "len" is zero, Perl will create a zero length (Perl) string. You are responsible for ensuring that the source buffer is at least "len" bytes long. If the "buffer" argument is NULL the new SV will be undefined. SV* newSVpvn(const char *const buffer, const STRLEN len)

It seems to be the same thing except that it doesn't look for the NUL (the zero byte) but insists on having the string length, so the only time it would be "safer" would be if len was accidentally set to zero. So what it meant by "an empty string which is not null (NUL) terminated" was the case where the user put the length as zero, but also passed a pointer which is not NULL, but points to some random place in memory where there is no NUL (zero, '\0') byte to be found, and then newSVpv went on a wild goose chase over random bytes of memory looking for the end of the string. I wonder if that has ever actually happened to anyone, or was it just a possibility that the authors were worried about?

Anyway that doesn't cover dummies like me who send in NUL-terminated strings, but lie about their length to try to get extra memory allocated, but I may have been the first person ever to abuse newSVpv in this way.

Also, wouldn't it be clearer to say that it creates a new SV of length len+1 and copies len bytes of a string into it, if the string (buffer) is not NULL, or no bytes if it is NULL.

It would also be nice if it could have been explained a bit better what to do with the return value of newSV. My initial problem, the reason I ended up abusing newSVpv, was because I couldn't work out how to tell Perl that the thing I'd created with newSV was a string. The answer to this turned out to be SvPOK_on, which is documented, but I found in fact by rooting through the Perl source code trying to work out what SvPOK was checking for. The documentation of newSV in perlguts says this about it:

In the unlikely case of a SV requiring more complex initialization, you can create an empty SV with newSV(len). If len is 0 an empty SV of type NULL is returned, else an SV of type PV is returned with len + 1 (for the NUL) bytes of storage allocated, accessible via SvPVX. In both cases the SV has the undef value.

I don't think my case is particularly unlikely, it seems fairly normal to want to get some memory to write into before actually retrieving the "string" to write, for example in my case I am calling a libpng routine to get the "string" to write into the buffer, so I want to allocate the buffer before retrieving. If I allocate the memory myself then use newSVpv on my allocated memory, there is an unnecessary copy of data, so it seems sensible to just write into the buffer of the SV.

It wasn't very clear to me what an SV of type PV with the undef value was, or how I could turn my SV with the undef value into something which Perl recognised as a string. My own preference would be for the documentation to go into more practical details of how to accomplish tasks.

Alles in Ordnung

Perl returns its hash values in a random order. Since 5.14 or so, the random order changes every time. So if you loop over your hash values, you get a different ordering each time.

for my $k (keys %hash) { }

No problem you say, I'll use sort to order my keys.

for my $k (sort keys %hash) { }

But what if you want to use a non-default order, like case-insensitive? Easy-peasy you claim.

for my $k (sort {uc ($a) cmp uc ($b)} keys %hash) { }

Now here's my problem. I'm using XS to loop through the hash, and I want to sort the keys in the hash according to the user's preference.

Well, there must be a way to sort things with the Perl API mustn't there? Well, there is something called sortsv_flags which wants a user-defined sorting routine of the type SVCOMPARE_t. So if I stuff the hash keys into an array of pointers, then I can sort them using Perl's default sorting if I use Per's default sorting routine, which is called Perl_sv_cmp. So that is that "sorted".

However, what if I want to allow a user-defined ordering in my XS program? I had a look with Google and with grep cpan but I couldn't find a lot of information, or an example of an XS module which does this. Writing a callback to call the Perl function from C is documented in perlcall, but it's not at all clear to me how I can set the user-defined function within my SVCOMPARE_t function, since there seems to be no way to pass my object into that.

The way I would do this in C is by using qsort_r, which allows me to pass a pointer to a void * which I can fill with anything I want to. The easiest way to go seems to be to use that, but qsort_r is another can of worms, since it is not standardised.

qsort_r started out on BSD, then Gnu implemented it, but Gnu had a bright idea of putting the arguments in the opposite order to BSD. Then Bill Gates came along and thought he would implement it too for his "Windows" operating system, but he decided to call it qsort_s, but with some of the arguments in the same order as Gnu, and some of the arguments in the same order as BSD. ๐Ÿ‘ทโ€

Some bright spark figured this all out though and made some macros, which you can find on github, which do it all for you and give you a universal qsort_r-like function. That would be great, except the macros don't actually work on ๐Ÿ“ Strawberry Perl, since that looks too much like a Gnu environment, and changing the macros so that they give the correct definition of qsort_s, I then found out that some versions of Windows don't even seem to have the qsort_s function.

Thankfully the ๐Ÿ‘‘ Regents of the University of California ๐Ÿ‘‘ have made their operating system open source, so what I did in the end is just to copy the BSD qsort_r source code into my module, give it a different name, and voila, I now have reliable user-defined sorting in the module, and it seems to work OK so far.

But is this really the best possible solution? Is there not some kind of clever trick that one can use to access the Perl sorting with a user-defined function from XS, and get all the $a and $b stuff too?

The Persuaders!

The Persuaders! is a particularly silly 1970s action TV series which lasted for only one season. Tony Curtis plays Danny Wilde to Roger Moore's Lord Brett Sinclair. Tony is the funny guy and Roger is the straight man. After the Persuaders, Roger Moore went on to become James Bond, and Tony Curtis went on to date Debee Ashby.


Although it wasn't a hit in America, The Persuaders actually became more popular in translation than in the original. One of the running gags is American Danny (Tony Curtis) making fun of snooty, toffee-nosed, upper-crust "your lordship" Roger Moore. But since the translated versions were for people who didn't know anything about British or American people, the translators invented new dialogue unrelated to the original, which ended up being more entertaining for the audience than the English dialogue. Which has nothing to do with Perl but here we go.

When I made a libpng module I wanted it to be compatible with libpng as far as possible. I usually use British spellings like "colour" and "grey" but libpng insists on "color" and "gray". (Oddly enough the PNG specification on the web uses "colour" and "grey" though.) So the documentation and examples were fully compatible with Roger Moore and his Aston Martin, but all the code was Tony Curtis-style "color" and "gray". Anyway after a while I have to say it was starting to get old writing colour then color then gray then grey.


It wouldn't feel right to change the libpng names, besides which it would break compatibility for users, so I decided it would be easier to just use the libpng-style American spellings everywhere. Around version 0.49 I unified everything to "color" and "gray" in the code, the variables, the documentation, and the examples. I even threw in a "behavior" for good measure in the documentation, although I've just realised I also have a "behaviour" in there too.

I've been tempted to go back to British spellings in the documentation, but I have the thought that some day someone else who might take over maintenance of the module from me, or people who don't speak English as a native language, and are confused by the spelling variation, might be glad that I used consistent spellings.

Anyway, as Basil Brush might say, that seems like a persuasive argument to me.

Drawing a blank with XS

I spent quite a lot of time trying to work out what this error message meant:

Error: Unterminated '#if/#ifdef/#ifndef' in Libpng.xs, line 1328

The first problem here is that line 1328 is the end of the file, so that wasn't a big help.

After spending a lot of time counting #if and #endif statements in the file over and over again, in the end I had the bright idea of looking at the actual XS output, and managed to find the problem. Apologies for quoting it in full here but I can't think of a good way to truncate it:

#if 0
#define XSubPPtmpAAAD 1

XS_EUPXS(XS_Image__PNG__Libpng_set_crc_action); /* prototype to pass -Wmissing-prototypes */Image__PNG__Libpng_set_crc_action)
    dVAR; dXSARGS;
    if (items != 3)
       croak_xs_usage(cv,  "Png, crit_action, ancil_action");
        Image__PNG__Libpng      Png;
        int     crit_action = (int)SvIV(ST(1))
        int     ancil_action = (int)SvIV(ST(2))

        if (SvROK(ST(0)) && sv_derived_from(ST(0), "Image::PNG::Libpng")) {

            IV tmp = SvIV((SV*)SvRV(ST(0)));
            Png = INT2PTR(Image__PNG__Libpng,tmp);
            Perl_croak_nocontext("%s: %s is not of type %s",
                        "Png", "Image::PNG::Libpng")
#line 905 "Libpng.xs"
        png_set_crc_action (Png->png, crit_action, ancil_action);
#endif /* 0 */
#line 2836 "Libpng.c"

Can you work it out?

It turns out that XS processes

#if 0

perl_png_set_crc_action  (Png, crit_action, ancil_action);
    Image::PNG::Libpng Png;
    int crit_action;
    int ancil_action;
    png_set_crc_action (Png->png, crit_action, ancil_action);

#endif /* 0 */

completely differently from

#if 0

perl_png_set_crc_action  (Png, crit_action, ancil_action);
    Image::PNG::Libpng Png;
    int crit_action;
    int ancil_action;
    png_set_crc_action (Png->png, crit_action, ancil_action);
#endif /* 0 */

In the former case, the #endif goes after the function, and in the latter case it goes into the function body before the end of the function.

Bill & Ted's Bogus Journey

Most operating systems have a version of libpng, the library for reading and writing the PNG (portable network graphics) image format on them. Unfortunately, though, the libpng is often fairly old.

I wrote a CPAN module which links against libpng, but then trying to get the module tested with CPAN testers, a lot of bugs would happen. It was frustrating because I couldn't work out what was going wrong with the tests unless I could find out what version of libpng was installed on the testing machine.

The solution I came up with in the end was a bogus test file which merely prints the libpng version as its skip_all message. This turned out to be quite effective in working out what is going wrong as various improvements to my PNG module turn out to trip bugs in older versions of libpng.

About Ben Bullock

user-pic Perl user since about 2006, I have also released some CPAN modules.