New daily reports

New daily reports

[Luka Marčetić]

The report will look better next time, this time I
     * finished the template for the generator
Tomorrow I should:
     *finish the generator itself, generate the first test collection 
(task nr. 7, was it?)
Weather got pretty bad here, so I better scoot off.
Luka

Re: New daily reports

[Solar Designer]

On Thu, Aug 04, 2011 at 12:14:39AM +0200, Luka Mar??eti?? wrote:
> The report will look better next time, this time I
>     * finished the template for the generator
> Tomorrow I should:
>     *finish the generator itself, generate the first test collection 
> (task nr. 7, was it?)

Since the generator approach and details may change a lot based on
Rich's feedback, wouldn't it be better to temporarily switch to a task
not requiring the generator?  Are there any pending tests that you'd
choose to implement straight in C anyway?

Alexander

Re: New daily reports

[Luka Marčetić]

On 08/04/2011 12:46 AM, Solar Designer wrote:
> On Thu, Aug 04, 2011 at 12:14:39AM +0200, Luka Mar??eti?? wrote:
>> The report will look better next time, this time I
>>      * finished the template for the generator
>> Tomorrow I should:
>>      *finish the generator itself, generate the first test collection
>> (task nr. 7, was it?)
> Since the generator approach and details may change a lot based on
> Rich's feedback, wouldn't it be better to temporarily switch to a task
> not requiring the generator?  Are there any pending tests that you'd
> choose to implement straight in C anyway?
>
> Alexander
I could temporarily focus on "9. Signals" if we don't settle the 
generator issue.
Luka.

Re: New daily reports

[Solar Designer]

Luka, Rich -

On Thu, Aug 04, 2011 at 12:51:30PM +0200, Luka Mar??eti?? wrote:
> I could temporarily focus on "9. Signals" if we don't settle the 
> generator issue.

Sounds fine to me, while we wait for Rich's comments.

I am tempted to propose something not on the original list, such as
testing of ctype macros and locales, or looking for functions that make
variable size allocations on the stack (may be tricked into overwriting
another thread's stack or the heap), but I realize that you have more
than enough tasks already.

So let it be signals for now.

Thanks,

Alexander

Re: New daily reports

[Luka Marčetić]

On 08/04/2011 01:54 PM, Solar Designer wrote:
> I am tempted to propose something not on the original list, such as
> testing of ctype macros and locales, or looking for functions that make
> variable size allocations on the stack (may be tricked into overwriting
> another thread's stack or the heap), but I realize that you have more
> than enough tasks already.

Wouldn't (parts of) those be tasks 0 and 7 respectively (coincidentally, 
the two tasks that were next on my to-do list after the generator)?

Luka

Re: New daily reports

[Solar Designer]

On Thu, Aug 04, 2011 at 02:01:09PM +0200, Luka Mar??eti?? wrote:
> On 08/04/2011 01:54 PM, Solar Designer wrote:
> >I am tempted to propose something not on the original list, such as
> >testing of ctype macros and locales, or looking for functions that make
> >variable size allocations on the stack (may be tricked into overwriting
> >another thread's stack or the heap), but I realize that you have more
> >than enough tasks already.
> 
> Wouldn't (parts of) those be tasks 0 and 7 respectively (coincidentally, 
> the two tasks that were next on my to-do list after the generator)?

I'm afraid that testing of ctype macros and locales for proper operation
and for some peculiar properties (behavior on negative ints, etc.) is
not on the list currently on the wiki, even though I would have liked
such testing to be done eventually.

Task 0 ("Base definition tests") would include making sure that ctype
macros are defined correctly, but not that they work correctly.  Or at
least that's how I read Rich's description of this task.  Rich?

Task 7 ("Functions which manipulate temp copies of an argument string")
would in fact cover the alloca() issue I referred to above.  In glibc,
crypt() and crypt_r() suffer from this problem when the password string
is not 32- or 64-bit aligned (depending on hash type) and the salt
string requests MD5-crypt or one of the SHA-crypt flavors.  (The copying
is performed to have the data aligned for word-sized accesses by the
crypto code.  So it is skipped when the string happens to be already
aligned.)

Alexander

Re: New daily reports - started pthread_eintr.c

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

On 08/04/2011 12:51 PM, Luka Marčetić wrote:
> On 08/04/2011 12:46 AM, Solar Designer wrote:
>> Are there any pending tests that you'd
>> choose to implement straight in C anyway?
>>
>> Alexander
> I could temporarily focus on "9. Signals" if we don't settle the 
> generator issue.
> Luka.
>

I tried to do the above, but got pretty stuck, so I'll have to finish it 
tomorrow unfortunately.
Accordingly, priorities are:
     *finish pthread_eintr.c
     *decide about the generator, take another task
Attached is the diff form of the above report.

Talk to you soon.
Luka.


[-- Attachment #2: to-report --]
[-- Type: text/plain, Size: 323 bytes --]

 a.out            |binary
 alloc.c          |    6 -
 alloc.c~         |    8 -
 pthread_eintr.c  |  135 +++++++++++++++++++++++++++++
 pthread_eintr.c~ |  135 +++++++++++++++++++++++++++++
 string.c~        |  255 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 530 insertions(+), 9 deletions(-)

Re: New daily reports - started pthread_eintr.c

[Solar Designer]

On Fri, Aug 05, 2011 at 02:02:38AM +0200, Luka Mar??eti?? wrote:
> On 08/04/2011 12:51 PM, Luka Mar??eti?? wrote:
> >I could temporarily focus on "9. Signals" if we don't settle the 
> >generator issue.
> 
> I tried to do the above, but got pretty stuck, so I'll have to finish it 
> tomorrow unfortunately.

If you need help, please let Rich know - describe the reason why you
feel stuck and ask him for advice explicitly.

> Accordingly, priorities are:
>     *finish pthread_eintr.c
>     *decide about the generator, take another task
> Attached is the diff form of the above report.

Thanks!  So, 135 lines of code written in pthread_eintr.c.

>  pthread_eintr.c  |  135 +++++++++++++++++++++++++++++

Alexander

Re: New daily reports - debugging alloc.c et al

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

On 08/05/2011 02:02 AM, Luka Marčetić wrote:
> I tried to do the above, but got pretty stuck, so I'll have to finish 
> it tomorrow unfortunately.
> Accordingly, priorities are:
>     *finish pthread_eintr.c
>     *decide about the generator, take another task
> Attached is the diff form of the above report.
>
> Talk to you soon.
> Luka.

Hello
The day started with Szabolcs suggesting I use valgrind to debug 
alloc.c. A very good idea, one which changed my priorities. Rich 
explained to me how memory management works behind the scenes (an 
explanation I very much needed), and suggested an mmap-based method to 
employ to find bugs. I've been changing the prog until now (6am here), 
but the method of allocation that I've been recommended either fails to 
fulfill its promise, or I'm blind to my own bug.
Here's a minimal test case:
http://paste.debian.net/125242/
A quick explanation: Alloc.c needs to squat most of process' virtual 
memory. I employ a kind of binary search to find the greatest size that 
can be allocated, but then it turns out, mmap and malloc are able to 
continue allocating memory still. Not sure why.

Priorities:
* finish this test collection for good (need your replies for that)
* work on pthread_eintr.c

That's it for today.
Luka

P.S. Lost diff history, attached is repo vs local /tests diff, come 
commits not included

[-- Attachment #2: to-report --]
[-- Type: text/plain, Size: 529 bytes --]

 a.out               |binary
 alloc               |binary
 alloc.c             |  177 ++++++++++++++--------
 alloc.c.older       |  358 +++++++++++++++++++++++++++++++++++++++++++++
 alloc.c~            |  406 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/common.h     |    1 
 common/common.h.gch |binary
 common/e_name.c~    |   32 ++++
 common/testgen.c    |   66 ++++++++
 common/testgen.c~   |   66 ++++++++
 pthread_eintr.c     |  129 ++++++++++++++++
 11 files changed, 1170 insertions(+), 65 deletions(-)

Re: New daily reports - debugging alloc.c et al

[Szabolcs Nagy]

* Luka Mar??eti?? <paxcoder@gmail.com> [2011-08-06 06:40:12 +0200]:
> here), but the method of allocation that I've been recommended
> either fails to fulfill its promise, or I'm blind to my own bug.
> Here's a minimal test case:
> http://paste.debian.net/125242/
> A quick explanation: Alloc.c needs to squat most of process' virtual
> memory. I employ a kind of binary search to find the greatest size
> that can be allocated, but then it turns out, mmap and malloc are
> able to continue allocating memory still. Not sure why.
> 

it works here as expected

the first loop finds the largest area that can be mmaped
(about 2.9G here)

then the second loop finds extra mmapable smaller regions
(this is slow here as there are about 70000 extra mmappable
pages, it would be better to do it in bigger chunks like:

    /*tries to mmap even more now:*/
    printf("Number of *additional* bytes mmap'd:\n");
    most=0;
    for(j=1<<20; j; j/=2) {
        printf("%8zu ", j);
        for(i=0; mmap(NULL, j*PAGE_SIZE, PROT_NONE, MAP_PRIVATE, fd, 0) != MAP_FAILED; ++i)
            printf(".");
        most += i*j*PAGE_SIZE;
        printf("\n");
    }
    printf("%zu\n", most);
)

after this the last loop with malloc immediately
fails (there is nothing to mmap anymore, but maybe
if the allocator already has some area reserved
then a few mallocs can succeed)

Re: New daily reports - debugging alloc.c et al

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2011-08-06 13:15:05 +0200]:
> pages, it would be better to do it in bigger chunks like:
> 
>     /*tries to mmap even more now:*/
>     printf("Number of *additional* bytes mmap'd:\n");
>     most=0;
>     for(j=1<<20; j; j/=2) {
>         printf("%8zu ", j);
>         for(i=0; mmap(NULL, j*PAGE_SIZE, PROT_NONE, MAP_PRIVATE, fd, 0) != MAP_FAILED; ++i)

hm j=1<<20 is not the right choice, 1<<16 should be ok

> after this the last loop with malloc immediately
> fails (there is nothing to mmap anymore, but maybe
> if the allocator already has some area reserved
> then a few mallocs can succeed)

it seems if i add a small malloc at the begining of main
then the last loop can still allocate with glibc
(here glibc can allocate 32 pages worth of extra space)

interestingly /proc/self/smaps does not seem to
list all the successful mmaps.. i wonder why but
i did not investigate it further

Re: New daily reports - debugging alloc.c et al

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]

* Szabolcs Nagy <nsz@port70.net> [2011-08-06 13:50:15 +0200]:
> interestingly /proc/self/smaps does not seem to
> list all the successful mmaps.. i wonder why but
this is false, i failed to read smaps entirely

as discussed on irc here is a code that can
fill up the vm quickly and painlessly

[-- Attachment #2: vmfill.c --]
[-- Type: text/x-csrc, Size: 1453 bytes --]

#include <stdlib.h>
#include <stdint.h>
#include <stdio.h>
#include <sys/mman.h>
#include <fcntl.h>
#include <unistd.h>

#ifndef PAGE_SIZE
	#define PAGE_SIZE sysconf(_SC_PAGE_SIZE)
#endif

static void printvm() {
	int fd = open("/proc/self/smaps", O_RDONLY);
	char buf[4096];
	int n;

	printf("/proc/self/smaps:\n");
	fflush(stdout);
	while ((n = read(fd, buf, sizeof buf)) > 0)
		write(1, buf, n);
	close(fd);
}

static size_t mmapmax(int fd, void **p) {
	size_t i,j,n=0;

	for (i=j=SIZE_MAX/2; i>0; i/=2) {
		if ((*p=mmap(NULL, j, PROT_NONE, MAP_PRIVATE, fd, 0)) == MAP_FAILED)
			j -= i/2;
		else {
			n = j;
			munmap(*p, n);
			j += i/2;
		}
	}
	if (n && (*p=mmap(NULL, n, PROT_NONE, MAP_PRIVATE, fd, 0)) == MAP_FAILED) {
		fprintf(stderr, "failed to mmap the same amount again.\n");
		exit(1);
	}
	return n;
}

int main(int argc, char *argv[]) {
	const int fd = open("/dev/zero", O_RDWR);
	void *p[100];
	size_t n[100];
	size_t sum = 0;
	int i;
	int vmmaps = 0;

	if (argc == 2 && argv[1][0]=='-' && argv[1][1]=='v')
		vmmaps = 1;

	for (i=0; i<sizeof p/sizeof *p; i++) {
		n[i] = mmapmax(fd, p+i);
		if (!n[i])
			break;
		sum += n[i];
		printf("%d %16zu B %012zx-%012zx\n", i, n[i], (size_t)p[i], (size_t)p[i]+n[i]);
	}
	printf("mmaped %zu B in %d blocks.\n", sum, i);

	printf("try malloc:");
	for(i=0; i < 1000 && malloc(PAGE_SIZE) != NULL; i++)
		printf(".");
	printf(" %zu B\n", i*(size_t)PAGE_SIZE);

	if (vmmaps)
		printvm();
	return 0;
}

Re: New daily reports - debugging alloc.c et al

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2011-08-06 16:34:33 +0200]:
> 	for (i=j=SIZE_MAX/2; i>0; i/=2) {
> 		if ((*p=mmap(NULL, j, PROT_NONE, MAP_PRIVATE, fd, 0)) == MAP_FAILED)
> 			j -= i/2;
> 		else {

i've just realized that

  for (i=j=SIZE_MAX/2+1; i>=PAGE_SIZE; i/=2) {

is better

(if j is not initialized to a power of 2 then i/=2
does not do proper binary search)

Re: New daily reports - debugging alloc.c still

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

What the title says.

Priorities:
* figure out how to continue writing pthread_eintr.c so that it works 
regarless of the nr of cores, write as many of function tests as possible
* apply different tools on setuid.c to remove the hang, employ the 
strategy from point one here as well
(in both cases, the program is trying to interrupt a child's function call)
* go through alloc.c again, to hopefully get an idea of why there are 
"unexpected features".

P.S. Attached is my to-report | grep -v musl #because i've recompiled musl

[-- Attachment #2: to-report-grep --]
[-- Type: text/plain, Size: 626 bytes --]

 repo/tests/a.out                                                        |binary
 tests/a.out                                                             |binary
 tests/alloc.c                                                           |  176 
 tests/alloc.c~                                                          |  177 
 tests/common/e_name.c                                                   |   43 
 tests/common/e_name.c~                                                  |   43 
 tests/core                                                              |binary
 2042 files changed, 14491 insertions(+), 170 deletions(-)

Re: New daily reports - debugging alloc.c still

[Solar Designer]

On Sun, Aug 07, 2011 at 04:41:33AM +0200, Luka Mar??eti?? wrote:
>  2042 files changed, 14491 insertions(+), 170 deletions(-)

Please be careful to include only source files in your diffstat's.  This
one is nearly useful for seeing how many lines of code you actually
changed.  Thanks!

Alexander

Re: New daily reports - debugging alloc.c still

[Rich Felker]

On Sun, Aug 07, 2011 at 04:41:33AM +0200, Luka Marčetić wrote:
> What the title says.
> 
> Priorities:
> * figure out how to continue writing pthread_eintr.c so that it
> works regarless of the nr of cores, write as many of function tests
> as possible

It would really help if I could see your progress on this. I suspect
you're over-thinking it. For any pthread function that will block/wait
(e.g. to obtain a lock), hitting it with a signal and checking for
EINTR is not a difficult race condition. It's like hunting turtles
with a sniper rifle. The target thread is just sitting there waiting
for you to signal it.

I would do something like this:

1. Tell the target thread to make the call that will block.

2. Sleep for a fraction of a second to give it time to wake up and
make the call.

3. Send the signal.

4. Sleep again for a fraction of a second to give it time to get
interrupted, if it's going to.

5. Do whatever's needed to unblock the call (e.g. if the thread is
blocked on pthread_mutex_lock, unlock the mutex it's trying to lock).

6. Check the return value of the call to see whether it was 0 or EINTR
(or possibly something else).

For calls which don't block, it's a lot harder to test and you may
need a race approach, but I would consider them very low priority for
testing, since a good implementation won't do anything that would
return EINTR here.

> * apply different tools on setuid.c to remove the hang, employ the
> strategy from point one here as well
> (in both cases, the program is trying to interrupt a child's function call)

Would you like me to send you the setuid test I have working on my
system? It might need some tweaking to hit the race on single-core
machines but you're welcome to use it for ideas or as a starting
point.

> * go through alloc.c again, to hopefully get an idea of why there
> are "unexpected features".

By the way, buf.c is crashing for me in free(), which means something
clobbered the heap. I think we discussed this before: allocating just
the right amount of memory and waiting for a crash is not a valid
test, because you've trashed the heap state at that point, and it
might or might not be detectable due to UB. If you want to look for
crashes you could follow the string functions test approach (mmap at
least 2 pages, with a non-accessible page at the end) but I think it
would probably work just as well just to allocate buffers that are a
plenty large, pre-fill the buffer with a pattern, and make sure the
pattern has not been clobbered past the zone the function was
permitted to write to.

(Note that the reason we needed the mmap thing for string functions
was that we were not just testing for out-of-bounds writes but also
out-of-bounds reads.)


Rich

Re: New daily reports - debugging alloc.c still

[Luka Marčetić]

Hey,

On 08/07/2011 09:32 AM, Rich Felker wrote:
> On Sun, Aug 07, 2011 at 04:41:33AM +0200, Luka Marčetić wrote:
>> What the title says.
>>
>> Priorities:
>> * figure out how to continue writing pthread_eintr.c so that it
>> works regarless of the nr of cores, write as many of function tests
>> as possible
> It would really help if I could see your progress on this.

Sorry, read it late, and then when I started responding, I had a need to 
at least fix some stuff I left hanging the last time I worked on 
pthread_eintr. I'll commit pthread_eintr.c shortly, but I should say 
that I've found a way to do it introducing usleep() into that while loop 
we talked about on IRC (the one that sends signals in order to try to 
get pthread_* to exit). I'll commit the collection containing a single 
test (pthread_create) shortly.

> I would do something like this:
>
> 1. Tell the target thread to make the call that will block.

I'll ask you about those functions that can be blocked in IRC (I don't 
believe pthread_create is one of them).

> 2. Sleep for a fraction of a second to give it time to wake up and
> make the call.

Speaking of which: Do you know what would cause the compiler to warn me 
about usleep()? I compile with a C99 flag and those two POSIX/XOPEN 
flags from the Makefile. It is in SUSv4...

> [...]

> For calls which don't block, it's a lot harder to test and you may
> need a race approach, but I would consider them very low priority for
> testing, since a good implementation won't do anything that would
> return EINTR here.

I can do those that can block/wait first, then the rest. But anyway, I 
think the method I employed for pthread_create should work for 
non-blocking ones.

> Would you like me to send you the setuid test I have working on my
> system? It might need some tweaking to hit the race on single-core
> machines but you're welcome to use it for ideas or as a starting
> point.

Nah, I'll fix it eventually. Thanks.

[...] (I'll need to clarify some things from the rest with you before I 
can respond)

Later,
Luka

P.S. I'll also commit the newer alloc.c, though it'll spit out random 
stuff as it is now.

Re: New daily reports - buf.c

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 252 bytes --]

Rich brought clobbering to my attention, as well as uncertainty of 
SIGSEGV in the current approach. I'm writing wrapper functions and a few 
other ones to fix the test.
Luka

Manually cleaned-up diff for monday only (though that's relative) attached.

[-- Attachment #2: to-report --]
[-- Type: text/plain, Size: 81 bytes --]

 tests/buf.c       |  504 ++++++++++++++++++++++++++++++++++++++++++------------

Re: New daily reports - nothing

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

Nothing to report for today, I'm afraid.
Not much time left either. I hoped you'd suggest what to do, but I had 
to spend time fixing stuff anyway. And since I still do, my priorities are:

- Fix and commit buf.c *
- Fix other tests that are still broken (setuid, alloc)
- Complete the started ones (pthread_eintr)

* I don't know what's up with this: http://codepad.org/Ysn2roYC.
Thanks.
Luka
<http://codepad.org/Ysn2roYC>

[-- Attachment #2: Type: text/html, Size: 818 bytes --]

Re: New daily reports - nothing

[Rich Felker]

On Wed, Aug 10, 2011 at 03:34:24AM +0200, Luka Marčetić wrote:
> Nothing to report for today, I'm afraid.
> Not much time left either. I hoped you'd suggest what to do, but I
> had to spend time fixing stuff anyway. And since I still do, my
> priorities are:
> 
> - Fix and commit buf.c *
> - Fix other tests that are still broken (setuid, alloc)
> - Complete the started ones (pthread_eintr)
> 
> * I don't know what's up with this: http://codepad.org/Ysn2roYC.

See http://pubs.opengroup.org/onlinepubs/9699919799/functions/mprotect.html

  The mprotect() function shall change the access protections to be
  that specified by prot for those whole pages containing any part of
  the address space of the process starting at address addr and
  continuing for len bytes.

There is no way to protect individual bytes. You could arrange for the
first byte past the end of the buffer to be protected by starting the
buffer "len" bytes before a page boundary, but if your goal is to
detect invalid writes rather than invalid reads, it's just as good to
allocate an overly-long buffer, fill the space that should not be used
with known content, and check whether that content has been clobbered
after the call. This avoids requiring any hacks with signal handlers
for SIGSEGV.

Especially as you're nearing the deadline, I'd like to ask you to
please listen when I make recommendations like this. Sure learning
about mprotect is educational, but in terms of getting stuff done, if
you'd taken my advice several days (a week now?) back about how to
check for writes past the end of the buffer, you would have been able
to spend your time today getting something done rather than wondering
why mprotect wasn't doing what you wanted....

Rich

Re: New daily reports - nothing

[Solar Designer]

Luka, Rich -

On Wed, Aug 10, 2011 at 03:34:24AM +0200, Luka Mar??eti?? wrote:
> Nothing to report for today, I'm afraid.

Thank you for posting a report anyway.  It helps.

> Not much time left either. I hoped you'd suggest what to do,

Rich - please suggest which of the remaining tasks Luka should focus on.
Maybe you already did on IRC or elsewhere, but I haven't seen that on
the list.

> but I had 
> to spend time fixing stuff anyway. And since I still do, my priorities are:
> 
> - Fix and commit buf.c *
> - Fix other tests that are still broken (setuid, alloc)
> - Complete the started ones (pthread_eintr)
> 
> * I don't know what's up with this: http://codepad.org/Ysn2roYC.

I took a look.  You're trying to mmap() and mprotect() other than a
multiple of page size.  This can't possibly work as your code expects it
to.  You need to round up your allocation size to the page size, and map
two entire unreadable pages (not size_t quantities) before and after it.
This might not satisfy your needs, though - I don't know what the
purpose of this malloc_bounded() was exactly.

The segfault occurs because your first mprotect() call makes the entire
page starting at stp inaccessible.  Not just a size_t quantity, because
it can't possibly do that.  The second mprotect() call is invoked on a
base address that is not page-aligned, so it fails with EINVAL (at least
on my system).  This is easily seen with strace (you could see it too),
and it'd be a good idea to check the return values in your code.

I hope this helps.

Thanks,

Alexander

P.S. Your use of "Re:" on ever-changing Subjects remains completely
ridiculous.  As explained before, this has nothing to do with threading,
but it breaks the convention of "Re:" being followed by the previous
message's Subject (that you're replying to).  For your use, it's best
not to include "Re:".

Re: New daily reports - nothing

[Solar Designer]

Luka -

On Wed, Aug 10, 2011 at 03:34:24AM +0200, Luka Mar??eti?? wrote:
> * I don't know what's up with this: http://codepad.org/Ysn2roYC.

Oh, one more detail: I think you're over-using those pastebin-like
services.  The short piece of source code and the segfault message would
be better posted right to this list.

I am pointing these things out because part of the objective of GSoC is
for you to learn to participate in a project.

Alexander

Re: New daily reports - nothing

[Rich Felker]

On Wed, Aug 10, 2011 at 06:07:25AM +0400, Solar Designer wrote:
> Luka -
> 
> On Wed, Aug 10, 2011 at 03:34:24AM +0200, Luka Mar??eti?? wrote:
> > * I don't know what's up with this: http://codepad.org/Ysn2roYC.
> 
> Oh, one more detail: I think you're over-using those pastebin-like
> services.  The short piece of source code and the segfault message would
> be better posted right to this list.

I agree. Pastebins are for sharing short snippets on IRC. They're
really inappropriate for mailing lists because they provide little/no
guarantee the URL and the contents it refers to will be
valid/preserved long-term and they make it impossible for someone
reading the archives later to tell what was being discussed.

> I am pointing these things out because part of the objective of GSoC is
> for you to learn to participate in a project.

*nod*

Rich

Re: New daily reports - nothing

[Rich Felker]

On Wed, Aug 10, 2011 at 03:34:24AM +0200, Luka Marčetić wrote:
> Nothing to report for today, I'm afraid.
> Not much time left either. I hoped you'd suggest what to do, but I
> had to spend time fixing stuff anyway. And since I still do, my
> priorities are:
> 
> - Fix and commit buf.c *
> - Fix other tests that are still broken (setuid, alloc)
> - Complete the started ones (pthread_eintr)

As I see it, what you've got (in varying stages of completeness) is
categories 1, 2, 3, 6, 8, and 9, from the project description here:

http://openwall.info/wiki/musl/unit-tests

Here are the things I would like you to focus on right now, roughly in
order of priority:

1. Fixing issues with test validity, like the buffer overflow/heap
corruption issues that make it impossible to actually detect failure.
This is a must.

2. Documenting the tests you have: what assertions they test. As
examples, "memcpy does not read past the end of the source buffer", or
"pthread_mutex_lock does not return EINTR when a signal is handled
while waiting for the lock". Along with that, a description of what
conditions the test covers (since for most of the tests, there's
theoretically a near-infinite set of possible inputs, and you can only
test a "representative" subset).

(I know you already have pretty good comments in the code, but what
I'm talking about is higher-level documentation, whether in comments
or separate from the source, about the larger purpose of the code and
what each test is checking.)

3. Cleaning up the build system and source to make sure it builds
without modification (except perhaps CFLAGS tweaks) on fairly recent
glibc and musl version.

4. Finish testing additional areas in the categories you're already
working on.

5. Test categories 4 and 5. I think it would be nice to pull in some
existing third-party (e.g. GNU) tests for these, but clean them up (as
in the project description) to avoid checking for GNU-specific stuff
and not to bail out as soon as the first test fails.

One thing I'd like you to drop for now is working on the setuid test.
It's been a time sink, and based on the work and discussion we already
did (which were very valuable in themselves), I have a working test
for it. You're welcome to incorporate that in cluts (preferably after
GSoC). I know this is kinda frustrating, but we really don't have time
for you to keep trying to fix it alongside all the other work that
remains to be done.

Rich

Re: New daily reports - nothing

[Luka Marčetić]

On 08/10/2011 04:02 AM, Solar Designer wrote:
> P.S. Your use of "Re:" on ever-changing Subjects remains completely
> ridiculous.  As explained before, this has nothing to do with threading,
> but it breaks the convention of "Re:" being followed by the previous
> message's Subject (that you're replying to).  For your use, it's best
> not to include "Re:".

My bad, I should stay within the same thread though (regardless of 
loosing the "Re:" and changing the subject), right?
Thanks

Re: New daily reports - nothing

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]

On 08/10/2011 03:38 AM, Rich Felker wrote:

(Thanks for explaining mprotect first of all)

> Especially as you're nearing the deadline, I'd like to ask you to
> please listen when I make recommendations like this. Sure learning
> about mprotect is educational, but in terms of getting stuff done, if
> you'd taken my advice several days (a week now?) back about how to
> check for writes past the end of the buffer, you would have been able
> to spend your time today getting something done rather than wondering
> why mprotect wasn't doing what you wanted....
>
> Rich

The worst thing is, I already do check that: I write '\r' in the last 
byte of the buffer, and then call the function saying the buffer is 
size-1 long (so it shows if it gets overwritten). It doesn't even make 
sense to test for reading/writing beyond size+1, except to test for 
implementation lunacy. I have no idea why I did that anymore. I 
should've just removed sigset altogether. I didn't need mprotect, nor 
wrappers... :-(
P.S. Final buf.c thus won't look like the attached file. In it, 
alloc_bounded() is the broken one - a fixed version may appear somewhere 
else in the future though.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: buf.c --]
[-- Type: text/x-csrc; name="buf.c", Size: 20092 bytes --]

#include <errno.h>
#include <signal.h>
#include <setjmp.h>
#include <sys/mman.h>
#include <fcntl.h>
#include "common/common.h"
#include <string.h>   //strerror_r
#include <limits.h>   //PATH_MAX
#include <stdlib.h>   //mbstowcs
#include <stdio.h>    //getdelim, snprintf
#include <unistd.h>   //confstr, getcwd, gethostname, readlink
#include <iconv.h>    //iconv
#include <time.h>     //time, gmtime
#include <monetary.h> //strfmon

/*
 * Copyright (c) 2011 Luka Marčetić<paxcoder@gmail.com>
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted.
 *
 * There's ABSOLUTELY NO WARRANTY, express or implied.
 */

/**
 ** \file
 ** Tests functions for writing beyond string lenght and errno's they set
 ** tests: confstr, getcwd, getdelim, gethostname, iconv, mbstowcs, snprintf,
 **        readlink, strfmon, strftime, wcstombs, ttyname_r, strerror_r
 ** depends: malloc, sigaction, strlen, mkstemp, fdopen,fclose,fileno, strptime
 **          iconv_open,iconv_close, free, setjmp, longjmp, open,close,
 **          mmap,mprotect,munmap
 **/

jmp_buf env;
struct sigaction act, oldact[2]; //set by main, used in wrappers:

void bridge_sig_jmp(int sig);
static void* malloc_bounded(size_t size);
static int   free_bounded(void* vp);
static int wrap_confstr(int name, char *buf, size_t len);
static int wrap_getcwd(char *buf, size_t size);
static int wrap_getdelim(char **lineptr, size_t *n, int delimiter,FILE *stream);
static int wrap_gethostname(char *name, size_t namelen);
static int wrap_mbstowcs(wchar_t *pwcs, char *s, size_t n);
static int wrap_snprintf(char *s, size_t n, char *format/*, ...*/);
static int wrap_readlink(char *path, char *buf, size_t bufsize);
static int wrap_strfmon(char *s, size_t maxsize, char *format/*,...*/, float F);
static int wrap_strftime(char *s,size_t maxsize,char *format,struct tm *timeptr);
static int wrap_wcstombs(char *s, wchar_t *pwcs, size_t n);
static int wrap_strerror_r(int errnum, char *strerrbuf, size_t buflen);
static int wrap_ttyname_r(int fildes, char *name, size_t namesize);
static int wrap_iconv(iconv_t cd, char **inbuf,  size_t *inbytesleft,
                                  char **outbuf, size_t *outbytesleft);

//još malloc wrapper thingy----
int main()
{
    iconv_t cd;
    wchar_t *ws;
    size_t size;
    struct tm tm;
    FILE *stream = NULL;
    char *s, *fun, *tmp, filename[] = "/tmp/clutsXXXXXX";
    int fd, sig, function, wrong, failed, err, err_expected;
    int ffun[] = {2, 3,8}; //functions that depend on the temporary file(two do)
    
    //GLOBAL:
    act=(struct sigaction){.sa_handler=bridge_sig_jmp, .sa_flags=SA_NODEFER};

    failed = 0;
    function = 1;
    do {
        err = wrong = 0;
        s = fun = tmp = NULL;
        ws = NULL;
        stream = NULL;
        if (seq_has(function, ffun) && (fd=mkstemp(filename)) != -1) {
            if ((stream=fdopen(fd, "w+b")) == NULL)
                wrong = -1;
        }
        switch(function) {
            case 1:
                fun = sreturnf("confstr(_CS_PATH, s, sizeof(s)-1)");
                size = confstr(_CS_PATH, NULL, 0);
                if (size < 2) {
                    wrong = -2;
                    break;
                }
                s = malloc_bounded(size);
                s[size-1] = '\r';
                
                
                if ((err=wrap_confstr(_CS_PATH, s, size-1)) == -1)
                    wrong = 1;
                else
                    /*if ((err))
                        wrong = 5;
                    else */if (s[size-1] != '\r')
                        wrong = 2;
            break;
            case 2:
                fun = sreturnf("getcwd(s, sizeof(s)-1)");
                s = malloc(PATH_MAX);
                size = strlen(getcwd(s, PATH_MAX))+1;
                free(s); s = NULL;
                if (size < 2) {
                    wrong = -2;
                    break;
                }
                s = malloc_bounded(size);
                s[size-1] = '\r';
                
                if ((err=wrap_getcwd(s, size-1)) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
                else if (err != (err_expected = ERANGE))
                    wrong = 3;
            break;
            case 3:
                //same for getline
                fun = sreturnf("getdelim(&s, &<strlen(s)>, '\\n', stream)");
                if (stream == NULL)
                    break;
                fwrite("123\n", (size = 4), 1, stream);
                s = malloc_bounded(size);
                s[size-1] = '\r';
                
                if((err=wrap_getdelim(&s,(size_t[]){size-1},'\n',stream)) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
            break;
            case 4:
                fun = sreturnf("gethostname(s, sizeof(s)-1)");
                s = malloc(HOST_NAME_MAX);
                gethostname(s, HOST_NAME_MAX);
                size = strlen(s) + 1;
                free(s); s = NULL;
                if (size < 2) {
                    wrong = -2;
                    break;
                }
                s = malloc_bounded(size);
                s[size-1] = '\r';
                
                if ((err=wrap_gethostname(s, size-1)) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
                else if (err != (err_expected = ERANGE))
                    wrong = 3;
            break;
            case 5:
                fun = sreturnf("iconv(&<\"abcd\">, &<4>, &s, &<3>)");
                s = malloc_bounded((size = 4));
                s[size-1] = '\r';
                
                cd = iconv_open("", "");
                err = wrap_iconv(
                        cd, 
                        (char **)(char *[]){"abcd"},
                        (size_t *)(size_t []){size},
                        (char **)(char *[]){s},
                        (size_t *)(size_t []){size-1}
                );
                iconv_close(cd);
                if (err == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
                else if (err != (err_expected = E2BIG))
                    wrong = 3;
            break;
            case 6:
                fun = sreturnf("mbstowcs(ws, \"abc\", 3)");
                size = 4;
                ws = malloc_bounded(size * sizeof(wchar_t));
                ws[size-1] = L'\r';
                
                if ((err=wrap_mbstowcs(ws, "abcd", size-1)) == -1)
                    wrong = 1;
                else if (ws[size-1] != L'\r')
                    wrong = 2;
                else if (err != (err_expected = E2BIG))
                    wrong = 3;
            break;
            case 7:
                fun = sreturnf("snprintf(s, 3, \"abc\")");
                s = malloc_bounded((size = 4));
                s[size-1] = '\r';
                
                if ((err=wrap_snprintf(s, size-1, "abc")) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
            break;
            case 8:
                fun=sreturnf("readlink(\"%s.sym\", s, sizeof(s)-1)", filename);
                tmp = sreturnf("%s.sym", filename);
                if (stream == NULL)
                    break;
                else if (symlink(filename, tmp)) {
                    wrong = 4;
                    break;
                }
                s = malloc(PATH_MAX);
                size = readlink(tmp, s, PATH_MAX);
                free(s); s = NULL;
                if (size < 2) {
                    wrong = -2;
                    break;
                }
                s = malloc_bounded(size);
                s[size-1] = '\r';
                
                if ((err=wrap_readlink(tmp, s, size-1)) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
            break;
            case 9:
                fun = sreturnf("strerror_r(1, s, sizeof(s)-1)");
                s = malloc(80);
                strerror_r(1, s, 80);
                size = strlen(s);
                free(s); s = NULL;
                if (size < 2) {
                    wrong = -2;
                    break;
                }
                s = malloc_bounded(size);
                s[size-1] = '\r';
                
                if ((err = wrap_strerror_r(1, s, size-1)) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
                else if (err != (err_expected = ERANGE))
                    wrong = 3;
            break;
            case 10:
                fun = sreturnf("strfmon(s, sizeof(s)-1, \"%%!i\", 123.0)");
                s = malloc(80);
                size = strfmon(s, 80, "%!i", 123.0) + 1;
                free(s); s = NULL;
                if (size < 2) {
                    wrong = -2;
                    break;
                }
                s = malloc_bounded(size);
                s[size-1] = '\r';
                
                if ((err=wrap_strfmon(s, size-1, "%!i", 123.0)) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
                else if (err != (err_expected = E2BIG))
                    wrong = 3;
            break;
            case 11:
                fun = sreturnf("strftime(s, sizeof(s)-1, \"%Y\", tm)");
                s = malloc(80);
                tm = *gmtime((time_t []){time(NULL)});
                strftime(s, 80, "%Y", &tm);
                size = strlen(s) + 1;
                free(s); s = NULL;
                if (size < 2) {
                    wrong = -2;
                    break;
                }
                s = malloc_bounded(size);
                s[size-1] = '\r';
                
                if ((err=wrap_strftime(s, size-1, "%Y", &tm)) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
            break;
            case 12:
                fun = sreturnf("ttyname_r(STDERR_FILENO, s, sizeof(s)-1)");
                s = malloc(TTY_NAME_MAX);
                err = ttyname_r(STDERR_FILENO, s, TTY_NAME_MAX);
                size = strlen(s) + 1;
                free(s); s = NULL;
                if (size < 2) {
                    wrong = -2;
                    break;
                }
                s = malloc_bounded(size);
                s[size-1] = '\r';
                
                if ((err = wrap_ttyname_r(STDERR_FILENO, s, size-1)) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
                else if (err != (err_expected = ERANGE))
                    wrong = 3;
            break;
            case 13:
                fun = sreturnf("wcstombs(s, L\"abc\" 3)");
                s = malloc_bounded((size = 4));
                s[size-1] = '\r';
                
                if ((err=wrap_wcstombs(s, L"abcd", size-1)) == -1)
                    wrong = 1;
                else if (s[size-1] != '\r')
                    wrong = 2;
            break;
            default:
                function = 0; //exit while
            break;
        }
        if(((sig=free_bounded(ws)) || (sig=free_bounded(s))) && !wrong)
            wrong = 6;

        if (wrong) {
            ++failed;
            if (wrong == -2)
                fprintf(
                    stderr,
                    "%s could not be tested, because a preceeding call to the"
                    " same function returned length (of s) lesser than 2\n",
                    fun
                );
            else if (wrong == -1)
                fprintf(stderr, "%s is unable to run, opening \"%s\" failed\n",
                        fun, filename);
            else if (wrong == 1)
                fprintf(stderr, "%s caused a SIGSEGV!\n", fun);
            else if (wrong == 2)
                fprintf(stderr, "%s wrote too many bytes to the buffer\n",fun);
            else if (wrong == 3) {
                free(tmp);
                fprintf(stderr, "%s failed to set errno=%s",
                        fun, (tmp = e_name(err_expected)));
                free(tmp);
                fprintf(stderr, " (it is %s)\n", (tmp = e_name(err)));
            }
            else if (wrong == 4)
                fprintf(stderr,
                        "%s was not called,"
                        " symlink(\"%s\", \"%s.sym\") failed\n",
                        fun, filename, filename);
            else if (wrong == 5) {
                free(tmp);
                fprintf(stderr, "%s set errno = %s\n", fun, (tmp=e_name(err)));
            }
            else if (wrong == 6)
                fprintf(
                    stderr,
                    "%s caused a signal %d to be received. A pointer passed to"
                    " the function might be errorneously redirected\n",
                    fun, sig
                );
        }
        free(fun);
        free(tmp);
        if (stream != NULL && seq_has(function, ffun))
            fclose(stream);
    } while(function++);
    
    return failed;
}

///A bridge function for sigaction(), jumps via longjmp() back to a setjmp()
void bridge_sig_jmp(int sig)
{
    longjmp(env, sig);
    return;
}

/**
 ** A quasi-implementation of malloc(), which allocates 2 additional words, one
 ** before the memory segment of the requested size, one after. It protects
 ** those words against reading/writing, so accessing them raises a SIGSEGV.
 ** \param size length of the memory segment to be allocated for reading/writing
 ** \returns a pointer to the allocated memory, NULL on failure
 ** \warning Needs to be freed by free_bounded(), not the regular free()
 **/
static void* malloc_bounded(size_t size) {
    int fd = open("/dev/zero", O_RDWR);
    size_t *stp = NULL;
    
    if (fd != -1) {
        size   +=  2 * sizeof(size_t);  //+boundry words
        stp    =   mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
        if (stp != MAP_FAILED) {
            *stp   =   size;                //write metadata to the first word
            //seal both ends:
            mprotect(stp,                                sizeof(size_t), PROT_NONE);
            mprotect((((char*)stp)+size)-sizeof(size_t), sizeof(size_t), PROT_NONE);
            ++stp;
        }
        else
            stp = NULL;
        close(fd);
    }
    return (void *)stp;
}
/**
 ** A quasi-implementation of free, which uses the metadata hidden in boundry
 ** words (allocated by malloc_bounded()) to unmap space.
 ** \param vp the pointer returned by malloc_bounded()
 ** \returns a caught signal on failure
 **/
static int free_bounded(void* vp) {
    size_t *stp = ((size_t *)vp)-1;
    int sig = 0;
    
    if(vp != NULL) {
        sigaction(SIGABRT, &act, &oldact[0]);
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!(sig = setjmp(env))) {
            mprotect(stp, sizeof(size_t), PROT_READ);
            munmap(stp, *stp);
        }   
        sigaction(SIGSEGV, &oldact[0], NULL);
        sigaction(SIGABRT, &oldact[0], NULL);
    }
    return sig;
}

///< Wrappers start here: They take the same~ arguments as the functions that
///< they call, but catch SIGSEGVs, and return error codes on failure
static int wrap_confstr(int name, char *buf, size_t len)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (confstr(name, buf, len)  ==  0)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_getcwd(char *buf, size_t size)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (getcwd(buf, size)  ==  NULL)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_getdelim(char **lineptr, size_t *n, int delimiter, FILE *stream)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (getdelim(lineptr, n, delimiter, stream)  ==  -1)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_gethostname(char *name, size_t namelen)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (gethostname(name, namelen)  ==  -1)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_iconv(iconv_t cd, char **inbuf,  size_t *inbytesleft,
                                  char **outbuf, size_t *outbytesleft)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (iconv(cd, inbuf, inbytesleft, outbuf, outbytesleft)==(size_t)-1)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_mbstowcs(wchar_t *pwcs, char *s, size_t n)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (mbstowcs(pwcs, s, n)  ==  (size_t)-1)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_snprintf(char *s, size_t n, char *format/*, ...*/)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (snprintf(s, n, format)  <  0)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_readlink(char *path, char *buf, size_t bufsize)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (readlink(path, buf, bufsize)  ==  -1)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_strfmon(char *s, size_t maxsize, char *format/*, ...*/, float F)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (strfmon(s, maxsize, format, F)  ==  -1)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_strftime(char *s,size_t maxsize,char *format,struct tm *timeptr)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (strftime(s, maxsize, format, timeptr)  ==  0)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_wcstombs(char *s, wchar_t *pwcs, size_t n)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env)) {
            if (wcstombs(s, pwcs, n)  ==  (size_t)-1)
                err = errno;
        }else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_strerror_r(int errnum, char *strerrbuf, size_t buflen)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env))
            err = strerror_r(errnum, strerrbuf, buflen);
        else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}
static int wrap_ttyname_r(int fildes, char *name, size_t namesize)
{
        int err = 0;
        sigaction(SIGSEGV, &act, &oldact[0]);
        if(!setjmp(env))
            err = ttyname_r(fildes, name, namesize);
        else
            err = -1;
        sigaction(SIGSEGV, &oldact[0], NULL);
        return err;
}

Re: New daily reports - nothing

[Solar Designer]

Luka,

On Wed, Aug 10, 2011 at 01:23:06PM +0200, Luka Mar??eti?? wrote:
> On 08/10/2011 04:02 AM, Solar Designer wrote:
> >P.S. Your use of "Re:" on ever-changing Subjects remains completely
> >ridiculous.  As explained before, this has nothing to do with threading,
> >but it breaks the convention of "Re:" being followed by the previous
> >message's Subject (that you're replying to).  For your use, it's best
> >not to include "Re:".
> 
> My bad, I should stay within the same thread though (regardless of 
> loosing the "Re:" and changing the subject), right?

Not necessarily.  If I were you, I'd be starting a new thread with each
daily report.  Then possible comments on your daily report, such as from
Rich, would be in the thread with that daily report.  But the next daily
report will start a new thread and have its comments, if any, in that
new thread.  Otherwise, you have a thread that spans multiple months and
multiple topics around cluts, with the only thing in common being that
it is about daily reports for cluts.

Another thing you could improve is always mention cluts in the Subject
of your daily reports because this mailing list is not limited to cluts -
in fact, it is primarily about musl rather than cluts.  Something like
"cluts report for 8/10 - implemented this and that" (with 8/10 being
today's date) will work better.

Thanks,

Alexander

Re: New daily reports - nothing

[Luka Marčetić]

On 08/10/2011 06:59 AM, Rich Felker wrote:
> Here are the things I would like you to focus on right now, roughly in
> order of priority:
>
> 1. Fixing issues with test validity, like the buffer overflow/heap
> corruption issues that make it impossible to actually detect failure.
> This is a must.

Of course.

> 2. Documenting the tests you have: what assertions they test. As
> examples, "memcpy does not read past the end of the source buffer", or
> "pthread_mutex_lock does not return EINTR when a signal is handled
> while waiting for the lock". Along with that, a description of what
> conditions the test covers (since for most of the tests, there's
> theoretically a near-infinite set of possible inputs, and you can only
> test a "representative" subset).
> (I know you already have pretty good comments in the code, but what
> I'm talking about is higher-level documentation, whether in comments
> or separate from the source, about the larger purpose of the code and
> what each test is checking.)

This is from buf.c:

/**
  ** \file
  ** Tests functions for writing beyond string lenght and errno's they set
  ** tests: confstr, getcwd, getdelim, gethostname, iconv, mbstowcs, 
snprintf,
  **        readlink, strfmon, strftime, wcstombs, ttyname_r, strerror_r
     [...]
  **/

If I wrote similar descriptions for all the tests, would that do? It 
says what the test collection tests, and for which functions. I reckon 
that, for example, the fact that `confstr` is tested using _CS_PATH 
should stay in the source code. Esp. for things like numeric.c where 
test data is huge.

> 3. Cleaning up the build system and source to make sure it builds
> without modification (except perhaps CFLAGS tweaks) on fairly recent
> glibc and musl version.

I'll make sure it builds with the newest musl as well. If you have some 
more specific instructions, let me know. Or if there's something I miss, 
please tell me. Thanks

> 4. Finish testing additional areas in the categories you're already
> working on.

I suppose you mean finish adding remaining tests to pthread_eintr.c. Or 
is there something I missed in collections that I only call broken, 
implying they would be done when fixed?

> 5. Test categories 4 and 5. I think it would be nice to pull in some
> existing third-party (e.g. GNU) tests for these, but clean them up (as
> in the project description) to avoid checking for GNU-specific stuff
> and not to bail out as soon as the first test fails.

So huge format string for snprintf, and weeding out glibc-specific tests 
from autoconf tests? Where can I get the latter? Do I rewrite them, or 
just try to incorporate into cluts (I'm guessing they're GPL, so...).

> One thing I'd like you to drop for now is working on the setuid test.
> It's been a time sink, and based on the work and discussion we already
> did (which were very valuable in themselves), I have a working test
> for it. You're welcome to incorporate that in cluts (preferably after
> GSoC). I know this is kinda frustrating, but we really don't have time
> for you to keep trying to fix it alongside all the other work that
> remains to be done.

Ok, I'll do that.
Thanks for the instructions, Rich.
Luka

Re: New daily reports - nothing

[Luka Marčetić]

On 08/10/2011 01:56 PM, Solar Designer wrote:
> If I were you, I'd be starting a new thread with each
> daily report.
> Something like
> "cluts report for 8/10 - implemented this and that" (with 8/10 being
> today's date) will work better.

I'll do that. Thanks.
I forgetting to add cluts into the subject mix. Rich, sry about that.
Luka

Re: New daily reports - nothing

[Luka Marčetić]

On 08/10/2011 06:59 AM, Rich Felker wrote:
> 3. Cleaning up the build system and source to make sure it builds
> without modification (except perhaps CFLAGS tweaks) on fairly recent
> glibc and musl version.

Wait. I've removed sqrtl dependency from alloc.c (I think I committed 
that, not sure), but does this mean you also want me to comment out 
monetary.h header, and its function's test from buf.c?

Re: New daily reports - nothing

[Rich Felker]

On Wed, Aug 10, 2011 at 02:44:36PM +0200, Luka Marčetić wrote:
> On 08/10/2011 06:59 AM, Rich Felker wrote:
> >3. Cleaning up the build system and source to make sure it builds
> >without modification (except perhaps CFLAGS tweaks) on fairly recent
> >glibc and musl version.
> 
> Wait. I've removed sqrtl dependency from alloc.c (I think I

This kind of thing (not making tests depend on completely unrelated
functionality) is the most important since it also relates to
correctness (we don't want bugs in functionality X to impact the
results of tests for functionality Y).

> committed that, not sure), but does this mean you also want me to
> comment out monetary.h header, and its function's test from buf.c?

I hadn't thought too much about that. Ideally musl would have just had
strfmon (or at least a stub for it) since a long time ago, but I never
ran into anything that needed it.

I'm not sure what the best thing for cluts to do at this stage is. In
the long term, I'd like to have either a configure script or something
in the build system whereby tests that can't build are automatically
excluded (without excluding other related tests that could build) but
for now it might be the best to have a simple #ifdef so this test
could be disabled from the makefile settings.

Rich

Re: New daily reports - nothing

[Luka Marčetić]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

On 08/10/2011 04:25 PM, Rich Felker wrote:
>> Wait. I've removed sqrtl dependency from alloc.c (I think I
> This kind of thing (not making tests depend on completely unrelated
> functionality) is the most important since it also relates to
> correctness (we don't want bugs in functionality X to impact the
> results of tests for functionality Y).

I thought I used sqrtl there rather cunningly there: Now it just does 
calloc(8, BIG/8+1), but then what it did was ~calloc(√BIG, √BIG). My 
reasoning was: If BIG/8 was judged to be too long by the implementation 
(like yours), the former would fails by default, whereas in the latter 
case, it still only fails if the implementation mishandles products of 
the multiplication. I'm aware of the problem of dependencies though - 
that's why I'm documenting them.

>> committed that, not sure), but does this mean you also want me to
>> comment out monetary.h header, and its function's test from buf.c?
> [...] configure script [...]
> for now it might be the best to have a simple #ifdef so this test
> could be disabled from the makefile settings.

K
Luka

[-- Attachment #2: Type: text/html, Size: 2992 bytes --]

Re: New daily reports - nothing

[Rich Felker]

On Wed, Aug 10, 2011 at 07:21:59PM +0200, Luka Marčetić wrote:
> On 08/10/2011 04:25 PM, Rich Felker wrote:
> >>Wait. I've removed sqrtl dependency from alloc.c (I think I
> >This kind of thing (not making tests depend on completely unrelated
> >functionality) is the most important since it also relates to
> >correctness (we don't want bugs in functionality X to impact the
> >results of tests for functionality Y).
> 
> I thought I used sqrtl there rather cunningly there: Now it just
> does calloc(8, BIG/8+1), but then what it did was ~calloc(√BIG,
> √BIG). My reasoning was: If BIG/8 was judged to be too long by the

What about SIZE_MAX>>(4*sizeof(size_t))?

Rich

Re: New daily reports - nothing

[Rich Felker]

On Wed, Aug 10, 2011 at 08:23:37PM +0200, Luka Marčetić wrote:
> On 08/10/2011 07:33 PM, Rich Felker wrote:
> >What about SIZE_MAX>>(4*sizeof(size_t))?
> 
> Never heard of it. What about PTRDIFF_MAX though?

My point was that SIZE_MAX>>(4*sizeof(size_t)) is an easy way to get
the square root of SIZE_MAX without long double arithmetic. PTRDIFF_T
is rather irrelevant to testing calloc overflows, where the whole
point is that A*B overflows SIZE_MAX and results in a small integer
value where the allocation will certainly succeed.

Rich

Re: New daily reports - nothing

[Luka Marčetić]

On 08/10/2011 07:33 PM, Rich Felker wrote:
> What about SIZE_MAX>>(4*sizeof(size_t))?

Never heard of it. What about PTRDIFF_MAX though?

Re: New daily reports - nothing

[Rich Felker]

On Wed, Aug 10, 2011 at 08:34:46PM +0200, Luka Marčetić wrote:
> On 08/10/2011 08:21 PM, Rich Felker wrote:
> >On Wed, Aug 10, 2011 at 08:23:37PM +0200, Luka Marčetić wrote:
> >>On 08/10/2011 07:33 PM, Rich Felker wrote:
> >>>What about SIZE_MAX>>(4*sizeof(size_t))?
> >>Never heard of it. What about PTRDIFF_MAX though?
> >My point was that SIZE_MAX>>(4*sizeof(size_t)) is an easy way to get
> >the square root of SIZE_MAX without long double arithmetic.
> 
> Doesn't seem that easy to me.

It is. That's the beauty of logarithms.

> >PTRDIFF_T
> >is rather irrelevant to testing calloc overflows, where the whole
> >point is that A*B overflows SIZE_MAX and results in a small integer
> >value where the allocation will certainly succeed.
> 
> The task says I should test for PTRDIFF_T. So i tried to make a
> general function.

Testing if malloc allocates memory larger than PTRDIFF_MAX is
independent of testing if calloc is subject to multiplication overflow
vulnerabilities.

Rich

Re: New daily reports - nothing

[Luka Marčetić]

On 08/10/2011 08:21 PM, Rich Felker wrote:
> On Wed, Aug 10, 2011 at 08:23:37PM +0200, Luka Marčetić wrote:
>> On 08/10/2011 07:33 PM, Rich Felker wrote:
>>> What about SIZE_MAX>>(4*sizeof(size_t))?
>> Never heard of it. What about PTRDIFF_MAX though?
> My point was that SIZE_MAX>>(4*sizeof(size_t)) is an easy way to get
> the square root of SIZE_MAX without long double arithmetic.

Doesn't seem that easy to me.

> PTRDIFF_T
> is rather irrelevant to testing calloc overflows, where the whole
> point is that A*B overflows SIZE_MAX and results in a small integer
> value where the allocation will certainly succeed.

The task says I should test for PTRDIFF_T. So i tried to make a general 
function.

Re: New daily reports - nothing

[Rich Felker]

On Wed, Aug 10, 2011 at 12:59:33AM -0400, Rich Felker wrote:
> As I see it, what you've got (in varying stages of completeness) is
> categories 1, 2, 3, 6, 8, and 9, from the project description here:
> 
> http://openwall.info/wiki/musl/unit-tests
> 
> Here are the things I would like you to focus on right now, roughly in
> order of priority:
> [...]
> 1. Fixing issues with test validity, like the buffer overflow/heap
> [...]
> 2. Documenting the tests you have: what assertions they test. As
> [...]
> 3. Cleaning up the build system and source to make sure it builds
> [...]
> 4. Finish testing additional areas in the categories you're already
> [...]
> 5. Test categories 4 and 5. I think it would be nice to pull in some

Luka, could you please reply with an estimate on how many hours of
work you think each of these areas will take you and an intended
schedule for how and when you plan to accomplish them?

Rich

Re: New daily reports - nothing

[Luka Marčetić]

On 08/14/2011 10:00 PM, Rich Felker wrote:
> On Wed, Aug 10, 2011 at 12:59:33AM -0400, Rich Felker wrote:
>> As I see it, what you've got (in varying stages of completeness) is
>> categories 1, 2, 3, 6, 8, and 9, from the project description here:
>>
>> http://openwall.info/wiki/musl/unit-tests
>>
>> Here are the things I would like you to focus on right now, roughly in
>> order of priority:
>> [...]
>> 1. Fixing issues with test validity, like the buffer overflow/heap
>> [...]
>> 2. Documenting the tests you have: what assertions they test. As
>> [...]
>> 3. Cleaning up the build system and source to make sure it builds
>> [...]
>> 4. Finish testing additional areas in the categories you're already
>> [...]
>> 5. Test categories 4 and 5. I think it would be nice to pull in some
> Luka, could you please reply with an estimate on how many hours of
> work you think each of these areas will take you and an intended
> schedule for how and when you plan to accomplish them?
>
> Rich

Hello.
Work required for the above:
1. Depends on what the issues really are. Eg. valgrind is clean on 
alloc.c, but it still fails for musl on 64-bit, because I get nothing 
allocated following vm_limit. Explicitly putting malloc() after it 
works, but doing it via the function fails. It's really hard to debug 
child processes (which is how a lot of collections are written), I 
should perhaps try Alexander's method. But I really can't say how much 
it'll take me - depends on inspiration and help i get from others. It's 
been time consuming up until now, but I think I've polished it enough so 
I just need to spot this one thing I'm missing and then correction won't 
be a problem.
2. I've updated pthread_eintr.c. If there's something you want me to 
expound on, let me know. I can try and generate doxygen docs now. Not 
sure if we need more user documentation, but I could add a few more 
sentences to README just so that people know what to expect when they 
run ./cluts etc. (a few hours to setup, and update docs if there are errors)
3. I can make buf.c collection test whether MUSL is defined just as I've 
recently have done for pthread_eintr.c. I'd appreciate if someone could 
tell me how to add `install-musl` which should add an additional 
-DMUSL=1 flag. (this and testing it shouldn't take more than a couple 
hours).
4. and 5. Check IRC please about pthread_eintr.c. Also, I'd like to 
finish string.c before starting new collections if that's fine with me. 
Based on past experience, I believe that rewriting string.c will take me 
about three days of intensive work. I can't give you my estimate for 
tests 4 and 5 (sprintf and stdio tests), because other than the 
"example" given in 4, I don't know what else will need to be tested, and 
I don't know how exactly autoconf looks like.

Schedule for the above:
I can do 2 and 3 today, then start working on (either part of nr 5 or) 
string.c, and after that, come back to 1. Now I can't commit after 
Saturday, but I can still do nr 4, and after that 5 when I have time 
(college starts, so maybe not right away).
When I'm done with 2 and 3 today, I can take a look at autoconf, and 
perhaps give you an estimate for that.
If you have other suggestions, please let me know soon.
Luka