[foreign] RFR: add test for C stdlib

[Maurizio Cimadamore]

Thanks for the comments.

Well spotted on the strcat issue.

On the remaining test I'll do some work to strengthen them, but there's 
a fine line here between testing the binder and testing the underlying 
library that I don't think we wanna cross.

So, for instance, I agree that printf should be tweaked so that it could 
be fooled less easily - but for things such as qsort, I think that 
feeding random permutations of values into it is a stress test for the 
underlying library, rather than for the binder.

As for setErrno()/etc, as much as you dislike the convention, this is a 
test :-) - and the related comment on inferring getter name is a design 
question, not a test one, so I won't address it here. Here I'll just say 
that the scheme you propose:

int errno() //getter
void errno(int i) //setter

while unambiguous, is so distant from common programming idiom that I 
don't think it would be useful. For this specific reason, the status quo 
is to not infer: if you want getter/setter/pointer-er, you need layout 
annotations - so that you can pick your preferred style. But as I said, 
this has little to do with adding a test :-)


Again, in strlen and strcmp the goal is to validate that the binder can 
pack pointers and dereference them accurately, not to test that the 
strcat/strlen/strcmp themselves are bug free (they should, I hope!). 
That's why I was fine with more lenient testing.

In other words, I recommend to look at this not in the way we would look 
at normally if we were to write unit tests for the functions themselves, 
but rather, trying to think about thorny cases for the binder. For 
instance, in the printf case, there are two cases that the binder needs 
to work through (i) empty variadic list, (ii) non-empty variadic list. 
Under (ii) we could refine more by trying to stress the various argument 
types being passed to the binder, to see if the binder can infer layouts 
accordingly. Etc.

So, the focus is on the binder - from there we walk back to a Java-like 
test; that said, some of the comments you made make perfect sense, and 
I'll rectify the test to take them into account.

Thanks
Maurizio



On 11/06/18 20:09, John Rose wrote:
> On Jun 11, 2018, at 11:02 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>> Hi,
>> this patch adds a slightly more comprehensive test for C's stdlib featuring a wider array of libraries than those tested in the past. The test is meant to stress almost all of the binder features, struct integration, callbacks, symbolic layout resolution, variadic calls, etc.
> Some suggestions about the tests:
>
> fopen returns a value; that should be tested also.
> The path handed to fopen needs to be even less likely IMO.
> Suggest:  "/tmp/java/StdLibTest/nosuchdir/nosuchfile"
>
> Given that qsort has a one-line test, I suggest handing it more
> input arrays, such as all permutations in length 0..5, plus one
> very long randomly shuffled input (with a reproducibly seeded
> RNG).
>
> See https://en.wikipedia.org/wiki/Heap%27s_algorithm for
> simple code enumerate permutations.
>
> The test for time() has potential reproducibility problems also.
> It would be better to have localtime and LDT work on a fixed
> instant (or series of instants) than on the current instant.
> Sorry to be picky; I know time testing is a time sink.
>
> The rand() test shouldn't spuriously fail ever, and even if it
> could 100 tries is enough for now.
>
> The "setErrno" name hurts my eyes.  Camel-case is not as
> normative for C as it is for Java.  Not a big deal, unless we
> are starting to make rules for naming setters, in which case
> we should talk more.
>
> (Also, do you need a "get" annotation for errno if the function
> is named "errno"?  Won't the binder figure it out?  This relates
> to the question of defaulting the setter also; I think it should be
> the same name, overloaded to take an argument.  That makes
> the fewest assumptions about name transformations.)
>
> For the printf test, I suggest giving the three operands somewhat
> varying lengths, and to adjust the format so that the test can
> detect a bug which would reverse the arguments.  Something
> like this:
>     printf("hello(%c,%d,%03d)\n", 9, 33, 126); // \t,33,126
>     printf("hello(%c,%d,%03d)\n", 33, 126, 9); // !,126,009
>     printf("hello(%c,%d,%03d)\n", 126, 9, 33); // ~,9,033
> and:
>     printf("len %d: %*d", 1, 1, 9);
>     printf("len %d: %*d", 2, 2, 9);
>     printf("len %d: %*d", 10, 10, 999);
>
> The problem here, of course, is using the return value to discriminate
> between differing outputs.  A sprintf test would be more direct, since
> the output string could be tested immediately.
>
> The strlen and strcmp tests seem OK, even though they aren't very
> exhaustive.  What could go wrong?  :-)  If I got around to it, I'd
> consider strengthening them by exercising them with various
> combinations of decimal numerals, which are easy to generate
> in Java code.
>
> The strcat test, on the other hand, is wrong.  It has a buffer overrun
> inside it.  It needs to allocate a buffer, fill it with the first string
> using strcpy or strcat, and then the second string can be added
> using strcat.
>
> BTW, this very common bug with buffer overrun is one where we can
> add some value in the Panama runtime, by supplying simple checks to
> test that the buffer's neighboring bytes have not been scribbled when
> the Scope deallocates it.  Likewise, the Scope can deallocate it by
> putting it on a "do not use" list, which can be periodically tested for
> post-free scribbling.  We could also do more elaborate, expensive,
> and comprehensive tripwires with memory protection hacks, if the
> platform supports them.  With HotSpot we get such tripwires when
> we build the debug-mode JVM; with Panama such tripwires could
> be enabled dynamically with a system property.
>
> —John