[Xenomai-help] Segmentation fault in rt_printf print thread

[Xenomai-help] Segmentation fault in rt_printf print thread

[Christoph Permes]

Hi, 

I'm using rt_printf for debug output. When running tests for 20 hours or
longer, sometimes I get a segmentation fault.

The backtrace shows:
#0  0xb7d008d8 in fputs () from /lib/libc.so.6
#1  0xb7f26f61 in print_buffers () from /usr/xenomai/lib/librtdk.so.0
#2  0xb7f26fb3 in printer_loop () from /usr/xenomai/lib/librtdk.so.0
#3  0xb7c92f3b in start_thread () from /lib/libpthread.so.0
#4  0xb7d6fb6e in clone () from /lib/libc.so.6

It seems to me that there is a bug in the print thread.

My program runs on x86 using the following software versions:
Kernel 2.6.29.5
Xenomai 2.4.9
I-Pipe patch: adeos-ipipe-2.6.29.5-x86-2.4-02.patch


Thanks, 
Christoph

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Gilles Chanteperdrix]

Christoph Permes wrote:
> Hi, 
> 
> I'm using rt_printf for debug output. When running tests for 20 hours or
> longer, sometimes I get a segmentation fault.
> 
> The backtrace shows:
> #0  0xb7d008d8 in fputs () from /lib/libc.so.6
> #1  0xb7f26f61 in print_buffers () from /usr/xenomai/lib/librtdk.so.0
> #2  0xb7f26fb3 in printer_loop () from /usr/xenomai/lib/librtdk.so.0
> #3  0xb7c92f3b in start_thread () from /lib/libpthread.so.0
> #4  0xb7d6fb6e in clone () from /lib/libc.so.6
> 
> It seems to me that there is a bug in the print thread.
> 
> My program runs on x86 using the following software versions:
> Kernel 2.6.29.5
> Xenomai 2.4.9
> I-Pipe patch: adeos-ipipe-2.6.29.5-x86-2.4-02.patch

Could you try the following patch?

diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
index 0615247..a0aeec3 100644
--- a/src/rtdk/rt_print.c
+++ b/src/rtdk/rt_print.c
@@ -422,6 +422,7 @@ void __rt_print_init(void)
        pthread_attr_t thattr;
        const char *value_str;
        unsigned long long period;
+       unsigned stksize;

        first_buffer = NULL;
        seq_no = 0;
@@ -457,6 +458,9 @@ void __rt_print_init(void)
        pthread_cond_init(&printer_wakeup, NULL);

        pthread_attr_init(&thattr);
-       pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
+       stksize = 32768;
+       if (stksize < PTHREAD_STACK_MIN)
+               stksize = PTHREAD_STACK_MIN;
+       pthread_attr_setstacksize(&thattr, stksize);
        pthread_create(&printer_thread, &thattr, printer_loop, NULL);
 }


-- 
                                          Gilles

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Christoph Permes]

Am Mittwoch, den 26.08.2009, 10:12 +0200 schrieb Gilles Chanteperdrix:
> Christoph Permes wrote:
> > Hi, 
> > 
> > I'm using rt_printf for debug output. When running tests for 20 hours or
> > longer, sometimes I get a segmentation fault.
> > 
> > The backtrace shows:
> > #0  0xb7d008d8 in fputs () from /lib/libc.so.6
> > #1  0xb7f26f61 in print_buffers () from /usr/xenomai/lib/librtdk.so.0
> > #2  0xb7f26fb3 in printer_loop () from /usr/xenomai/lib/librtdk.so.0
> > #3  0xb7c92f3b in start_thread () from /lib/libpthread.so.0
> > #4  0xb7d6fb6e in clone () from /lib/libc.so.6
> > 
> > It seems to me that there is a bug in the print thread.
> > 
> > My program runs on x86 using the following software versions:
> > Kernel 2.6.29.5
> > Xenomai 2.4.9
> > I-Pipe patch: adeos-ipipe-2.6.29.5-x86-2.4-02.patch
> 
> Could you try the following patch?
> 
> diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
> index 0615247..a0aeec3 100644
> --- a/src/rtdk/rt_print.c
> +++ b/src/rtdk/rt_print.c
> @@ -422,6 +422,7 @@ void __rt_print_init(void)
>         pthread_attr_t thattr;
>         const char *value_str;
>         unsigned long long period;
> +       unsigned stksize;
> 
>         first_buffer = NULL;
>         seq_no = 0;
> @@ -457,6 +458,9 @@ void __rt_print_init(void)
>         pthread_cond_init(&printer_wakeup, NULL);
> 
>         pthread_attr_init(&thattr);
> -       pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> +       stksize = 32768;
> +       if (stksize < PTHREAD_STACK_MIN)
> +               stksize = PTHREAD_STACK_MIN;
> +       pthread_attr_setstacksize(&thattr, stksize);
>         pthread_create(&printer_thread, &thattr, printer_loop, NULL);
>  }
> 

I have tried the patch but it crashed again with the same backtrace.

Christoph

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Jan Kiszka]

Christoph Permes wrote:
> Am Mittwoch, den 26.08.2009, 10:12 +0200 schrieb Gilles Chanteperdrix:
>> Christoph Permes wrote:
>>> Hi, 
>>>
>>> I'm using rt_printf for debug output. When running tests for 20 hours or
>>> longer, sometimes I get a segmentation fault.
>>>
>>> The backtrace shows:
>>> #0  0xb7d008d8 in fputs () from /lib/libc.so.6
>>> #1  0xb7f26f61 in print_buffers () from /usr/xenomai/lib/librtdk.so.0
>>> #2  0xb7f26fb3 in printer_loop () from /usr/xenomai/lib/librtdk.so.0
>>> #3  0xb7c92f3b in start_thread () from /lib/libpthread.so.0
>>> #4  0xb7d6fb6e in clone () from /lib/libc.so.6
>>>
>>> It seems to me that there is a bug in the print thread.
>>>
>>> My program runs on x86 using the following software versions:
>>> Kernel 2.6.29.5
>>> Xenomai 2.4.9
>>> I-Pipe patch: adeos-ipipe-2.6.29.5-x86-2.4-02.patch
>> Could you try the following patch?
>>
>> diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
>> index 0615247..a0aeec3 100644
>> --- a/src/rtdk/rt_print.c
>> +++ b/src/rtdk/rt_print.c
>> @@ -422,6 +422,7 @@ void __rt_print_init(void)
>>         pthread_attr_t thattr;
>>         const char *value_str;
>>         unsigned long long period;
>> +       unsigned stksize;
>>
>>         first_buffer = NULL;
>>         seq_no = 0;
>> @@ -457,6 +458,9 @@ void __rt_print_init(void)
>>         pthread_cond_init(&printer_wakeup, NULL);
>>
>>         pthread_attr_init(&thattr);
>> -       pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
>> +       stksize = 32768;
>> +       if (stksize < PTHREAD_STACK_MIN)
>> +               stksize = PTHREAD_STACK_MIN;
>> +       pthread_attr_setstacksize(&thattr, stksize);
>>         pthread_create(&printer_thread, &thattr, printer_loop, NULL);
>>  }
>>
> 
> I have tried the patch but it crashed again with the same backtrace.

Could you try to find out, what memory access is precisely causing the
SEGV? Maybe even what variables of the rt_printf code have invalid
states? Or do you have a portable test case for us to reproduce the issue?

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Christoph Permes]

Hi,

I have examined the print_buffers() function and my core dump:

#1  0xb8087f61 in print_buffers () at rt_print.c:380
380				fprintf(head->dest, "%s", head->text);
(gdb) print head->dest
$1 = (FILE *) 0x445b205d
(gdb) print head->text
$2 = "_"
(gdb) print (char*)head
$3 = 0x8ea5a02 "] [D] [V_MainClampToteRequest   ] ===> [1]\n"
(gdb) print buffer->read_pos
$4 = 3482
(gdb) print *(char*)buffer->ring@domain.hid
$5 = "... [67046.506] [CONTROL] [1104820] [\000] [D] [V_MainClamp"

As the above output shows the head pointer points to a wrong memory
address, the head->dest FILE pointer results from some text written to
the buffer.

buffer = get_next_buffer();
if (!buffer)
        break;

read_pos = buffer->read_pos;
head = buffer->ring + read_pos;
len = strlen(head->text);

if (len) {
        /* Print out non-empty entry and proceed */
        fprintf(head->dest, "%s", head->text); // ==> SEGV
        read_pos += sizeof(*head) + len;
} else {
        /* Emptry entries mark the wrap-around */
        read_pos = 0;
}

Obviously the value of buffer->read_pos is not correct or the buffer
pointer returned by get_next_buffer() points to a wrong address.

Christoph

Am Donnerstag, den 27.08.2009, 10:24 +0200 schrieb Jan Kiszka:
> Christoph Permes wrote:
> > Am Mittwoch, den 26.08.2009, 10:12 +0200 schrieb Gilles Chanteperdrix:
> >> Christoph Permes wrote:
> >>> Hi, 
> >>>
> >>> I'm using rt_printf for debug output. When running tests for 20 hours or
> >>> longer, sometimes I get a segmentation fault.
> >>>
> >>> The backtrace shows:
> >>> #0  0xb7d008d8 in fputs () from /lib/libc.so.6
> >>> #1  0xb7f26f61 in print_buffers () from /usr/xenomai/lib/librtdk.so.0
> >>> #2  0xb7f26fb3 in printer_loop () from /usr/xenomai/lib/librtdk.so.0
> >>> #3  0xb7c92f3b in start_thread () from /lib/libpthread.so.0
> >>> #4  0xb7d6fb6e in clone () from /lib/libc.so.6
> >>>
> >>> It seems to me that there is a bug in the print thread.
> >>>
> >>> My program runs on x86 using the following software versions:
> >>> Kernel 2.6.29.5
> >>> Xenomai 2.4.9
> >>> I-Pipe patch: adeos-ipipe-2.6.29.5-x86-2.4-02.patch
> >> Could you try the following patch?
> >>
> >> diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
> >> index 0615247..a0aeec3 100644
> >> --- a/src/rtdk/rt_print.c
> >> +++ b/src/rtdk/rt_print.c
> >> @@ -422,6 +422,7 @@ void __rt_print_init(void)
> >>         pthread_attr_t thattr;
> >>         const char *value_str;
> >>         unsigned long long period;
> >> +       unsigned stksize;
> >>
> >>         first_buffer = NULL;
> >>         seq_no = 0;
> >> @@ -457,6 +458,9 @@ void __rt_print_init(void)
> >>         pthread_cond_init(&printer_wakeup, NULL);
> >>
> >>         pthread_attr_init(&thattr);
> >> -       pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
> >> +       stksize = 32768;
> >> +       if (stksize < PTHREAD_STACK_MIN)
> >> +               stksize = PTHREAD_STACK_MIN;
> >> +       pthread_attr_setstacksize(&thattr, stksize);
> >>         pthread_create(&printer_thread, &thattr, printer_loop, NULL);
> >>  }
> >>
> > 
> > I have tried the patch but it crashed again with the same backtrace.
> 
> Could you try to find out, what memory access is precisely causing the
> SEGV? Maybe even what variables of the rt_printf code have invalid
> states? Or do you have a portable test case for us to reproduce the issue?
> 
> Thanks,
> Jan
> 

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Jan Kiszka]

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

Christoph Permes wrote:
> Hi,
> 
> I have examined the print_buffers() function and my core dump:
> 
> #1  0xb8087f61 in print_buffers () at rt_print.c:380
> 380				fprintf(head->dest, "%s", head->text);
> (gdb) print head->dest
> $1 = (FILE *) 0x445b205d
> (gdb) print head->text
> $2 = "_"
> (gdb) print (char*)head
> $3 = 0x8ea5a02 "] [D] [V_MainClampToteRequest   ] ===> [1]\n"
> (gdb) print buffer->read_pos
> $4 = 3482
> (gdb) print *(char*)buffer->ring@domain.hid
> $5 = "... [67046.506] [CONTROL] [1104820] [\000] [D] [V_MainClamp"
> 
> As the above output shows the head pointer points to a wrong memory
> address, the head->dest FILE pointer results from some text written to
> the buffer.
> 
> buffer = get_next_buffer();
> if (!buffer)
>         break;
> 
> read_pos = buffer->read_pos;
> head = buffer->ring + read_pos;
> len = strlen(head->text);
> 
> if (len) {
>         /* Print out non-empty entry and proceed */
>         fprintf(head->dest, "%s", head->text); // ==> SEGV
>         read_pos += sizeof(*head) + len;
> } else {
>         /* Emptry entries mark the wrap-around */
>         read_pos = 0;
> }
> 
> Obviously the value of buffer->read_pos is not correct or the buffer
> pointer returned by get_next_buffer() points to a wrong address.

Hmm, strange. Code meditation didn't help, so I need to keep you busy
with testing. Could you try this instrumentation? It should choke if the
rt_vfprintf actually overwrites already written data.

Thanks,
Jan

diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
index 0615247..bcd8c88 100644
--- a/src/rtdk/rt_print.c
+++ b/src/rtdk/rt_print.c
@@ -16,6 +16,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA.
  */
 
+#include <assert.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <limits.h>
@@ -37,7 +38,10 @@
 
 #define RT_PRINT_LINE_BREAK		256
 
+#define RT_PRINT_HEAD_MAGIC		0xDEADBEAF
+
 struct entry_head {
+	uint32_t magic;
 	FILE *dest;
 	uint32_t seq_no;
 	char text[1];
@@ -103,6 +107,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 	read_pos = buffer->read_pos;
 	xnarch_read_memory_barrier();
 
+	assert(write_pos == read_pos ||
+	       ((struct entry_head *)buffer->read_pos)->magic ==
+	       RT_PRINT_HEAD_MAGIC);
+
 	/* Is our write limit the end of the ring buffer? */
 	if (write_pos >= read_pos) {
 		/* Keep a savety margin to the end for at least an empty entry */
@@ -114,6 +122,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 		if (len == 0 && read_pos > sizeof(struct entry_head)) {
 			/* Write out empty entry */
 			head = buffer->ring + write_pos;
+			head->magic = RT_PRINT_HEAD_MAGIC;
 			head->seq_no = seq_no;
 			head->text[0] = 0;
 
@@ -136,6 +145,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 
 	res = vsnprintf(head->text, len, format, args);
 
+	assert(write_pos == read_pos ||
+	       ((struct entry_head *)buffer->read_pos)->magic ==
+	       RT_PRINT_HEAD_MAGIC);
+
 	if (res < len) {
 		/* Text was written completely, res contains its length */
 		len = res;
@@ -147,6 +160,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 
 	/* If we were able to write some text, finalise the entry */
 	if (len > 0) {
+		head->magic = RT_PRINT_HEAD_MAGIC;
 		head->seq_no = ++seq_no;
 		head->dest = stream;
 
@@ -159,6 +173,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
 		/* An empty entry marks the wrap-around */
 		head = buffer->ring + write_pos;
+		head->magic = RT_PRINT_HEAD_MAGIC;
 		head->seq_no = seq_no;
 		head->text[0] = 0;
 
@@ -382,6 +397,8 @@ static void print_buffers(void)
 		head = buffer->ring + read_pos;
 		len = strlen(head->text);
 
+		assert(head->magic == RT_PRINT_HEAD_MAGIC);
+
 		if (len) {
 			/* Print out non-empty entry and proceed */
 			fprintf(head->dest, "%s", head->text);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Christoph Permes]

Am Freitag, den 28.08.2009, 09:38 +0200 schrieb Jan Kiszka:
> Hmm, strange. Code meditation didn't help, so I need to keep you busy
> with testing. Could you try this instrumentation? It should choke if the
> rt_vfprintf actually overwrites already written data.
> 
> Thanks,
> Jan
> 
> diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
> index 0615247..bcd8c88 100644
> --- a/src/rtdk/rt_print.c
> +++ b/src/rtdk/rt_print.c
> @@ -16,6 +16,7 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA.
>   */
>  
> +#include <assert.h>
>  #include <errno.h>
>  #include <inttypes.h>
>  #include <limits.h>
> @@ -37,7 +38,10 @@
>  
>  #define RT_PRINT_LINE_BREAK		256
>  
> +#define RT_PRINT_HEAD_MAGIC		0xDEADBEAF
> +
>  struct entry_head {
> +	uint32_t magic;
>  	FILE *dest;
>  	uint32_t seq_no;
>  	char text[1];
> @@ -103,6 +107,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  	read_pos = buffer->read_pos;
>  	xnarch_read_memory_barrier();
>  
> +	assert(write_pos == read_pos ||
> +	       ((struct entry_head *)buffer->read_pos)->magic ==
> +	       RT_PRINT_HEAD_MAGIC);
> +
>  	/* Is our write limit the end of the ring buffer? */
>  	if (write_pos >= read_pos) {
>  		/* Keep a savety margin to the end for at least an empty entry */
> @@ -114,6 +122,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  		if (len == 0 && read_pos > sizeof(struct entry_head)) {
>  			/* Write out empty entry */
>  			head = buffer->ring + write_pos;
> +			head->magic = RT_PRINT_HEAD_MAGIC;
>  			head->seq_no = seq_no;
>  			head->text[0] = 0;
>  
> @@ -136,6 +145,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  
>  	res = vsnprintf(head->text, len, format, args);
>  
> +	assert(write_pos == read_pos ||
> +	       ((struct entry_head *)buffer->read_pos)->magic ==
> +	       RT_PRINT_HEAD_MAGIC);
> +
>  	if (res < len) {
>  		/* Text was written completely, res contains its length */
>  		len = res;
> @@ -147,6 +160,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  
>  	/* If we were able to write some text, finalise the entry */
>  	if (len > 0) {
> +		head->magic = RT_PRINT_HEAD_MAGIC;
>  		head->seq_no = ++seq_no;
>  		head->dest = stream;
>  
> @@ -159,6 +173,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
>  		/* An empty entry marks the wrap-around */
>  		head = buffer->ring + write_pos;
> +		head->magic = RT_PRINT_HEAD_MAGIC;
>  		head->seq_no = seq_no;
>  		head->text[0] = 0;
>  
> @@ -382,6 +397,8 @@ static void print_buffers(void)
>  		head = buffer->ring + read_pos;
>  		len = strlen(head->text);
>  
> +		assert(head->magic == RT_PRINT_HEAD_MAGIC);
> +
>  		if (len) {
>  			/* Print out non-empty entry and proceed */
>  			fprintf(head->dest, "%s", head->text);
> 

Now I get a SEGV at the first line of the first assert statement
(assert(write_pos == read_pos || ) in rt_vfprintf.
This happens at the first rt_printf call, also with the rtprint example
program.

Thanks, 
Christoph

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Jan Kiszka]

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

Christoph Permes wrote:
> Am Freitag, den 28.08.2009, 09:38 +0200 schrieb Jan Kiszka:
>> Hmm, strange. Code meditation didn't help, so I need to keep you busy
>> with testing. Could you try this instrumentation? It should choke if the
>> rt_vfprintf actually overwrites already written data.
>>
>> Thanks,
>> Jan
>>
>> diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
>> index 0615247..bcd8c88 100644
>> --- a/src/rtdk/rt_print.c
>> +++ b/src/rtdk/rt_print.c
>> @@ -16,6 +16,7 @@
>>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA.
>>   */
>>  
>> +#include <assert.h>
>>  #include <errno.h>
>>  #include <inttypes.h>
>>  #include <limits.h>
>> @@ -37,7 +38,10 @@
>>  
>>  #define RT_PRINT_LINE_BREAK		256
>>  
>> +#define RT_PRINT_HEAD_MAGIC		0xDEADBEAF
>> +
>>  struct entry_head {
>> +	uint32_t magic;
>>  	FILE *dest;
>>  	uint32_t seq_no;
>>  	char text[1];
>> @@ -103,6 +107,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>>  	read_pos = buffer->read_pos;
>>  	xnarch_read_memory_barrier();
>>  
>> +	assert(write_pos == read_pos ||
>> +	       ((struct entry_head *)buffer->read_pos)->magic ==
>> +	       RT_PRINT_HEAD_MAGIC);
>> +
>>  	/* Is our write limit the end of the ring buffer? */
>>  	if (write_pos >= read_pos) {
>>  		/* Keep a savety margin to the end for at least an empty entry */
>> @@ -114,6 +122,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>>  		if (len == 0 && read_pos > sizeof(struct entry_head)) {
>>  			/* Write out empty entry */
>>  			head = buffer->ring + write_pos;
>> +			head->magic = RT_PRINT_HEAD_MAGIC;
>>  			head->seq_no = seq_no;
>>  			head->text[0] = 0;
>>  
>> @@ -136,6 +145,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>>  
>>  	res = vsnprintf(head->text, len, format, args);
>>  
>> +	assert(write_pos == read_pos ||
>> +	       ((struct entry_head *)buffer->read_pos)->magic ==
>> +	       RT_PRINT_HEAD_MAGIC);
>> +
>>  	if (res < len) {
>>  		/* Text was written completely, res contains its length */
>>  		len = res;
>> @@ -147,6 +160,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>>  
>>  	/* If we were able to write some text, finalise the entry */
>>  	if (len > 0) {
>> +		head->magic = RT_PRINT_HEAD_MAGIC;
>>  		head->seq_no = ++seq_no;
>>  		head->dest = stream;
>>  
>> @@ -159,6 +173,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>>  	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
>>  		/* An empty entry marks the wrap-around */
>>  		head = buffer->ring + write_pos;
>> +		head->magic = RT_PRINT_HEAD_MAGIC;
>>  		head->seq_no = seq_no;
>>  		head->text[0] = 0;
>>  
>> @@ -382,6 +397,8 @@ static void print_buffers(void)
>>  		head = buffer->ring + read_pos;
>>  		len = strlen(head->text);
>>  
>> +		assert(head->magic == RT_PRINT_HEAD_MAGIC);
>> +
>>  		if (len) {
>>  			/* Print out non-empty entry and proceed */
>>  			fprintf(head->dest, "%s", head->text);
>>
> 
> Now I get a SEGV at the first line of the first assert statement
> (assert(write_pos == read_pos || ) in rt_vfprintf.
> This happens at the first rt_printf call, also with the rtprint example
> program.

Untested stuff as I should already be on the road. Here is another try,
but please don't expect replies from me over this weekend.

Jan

diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
index 0615247..06dabc1 100644
--- a/src/rtdk/rt_print.c
+++ b/src/rtdk/rt_print.c
@@ -16,6 +16,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA.
  */
 
+#include <assert.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <limits.h>
@@ -37,7 +38,10 @@
 
 #define RT_PRINT_LINE_BREAK		256
 
+#define RT_PRINT_HEAD_MAGIC		0xDEADBEAF
+
 struct entry_head {
+	uint32_t magic;
 	FILE *dest;
 	uint32_t seq_no;
 	char text[1];
@@ -103,6 +107,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 	read_pos = buffer->read_pos;
 	xnarch_read_memory_barrier();
 
+	assert(write_pos == read_pos ||
+	       ((struct entry_head *)buffer->ring + buffer->read_pos)->magic ==
+	       RT_PRINT_HEAD_MAGIC);
+
 	/* Is our write limit the end of the ring buffer? */
 	if (write_pos >= read_pos) {
 		/* Keep a savety margin to the end for at least an empty entry */
@@ -114,6 +122,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 		if (len == 0 && read_pos > sizeof(struct entry_head)) {
 			/* Write out empty entry */
 			head = buffer->ring + write_pos;
+			head->magic = RT_PRINT_HEAD_MAGIC;
 			head->seq_no = seq_no;
 			head->text[0] = 0;
 
@@ -136,6 +145,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 
 	res = vsnprintf(head->text, len, format, args);
 
+	assert(write_pos == read_pos ||
+	       ((struct entry_head *)buffer->ring + buffer->read_pos)->magic ==
+	       RT_PRINT_HEAD_MAGIC);
+
 	if (res < len) {
 		/* Text was written completely, res contains its length */
 		len = res;
@@ -147,6 +160,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 
 	/* If we were able to write some text, finalise the entry */
 	if (len > 0) {
+		head->magic = RT_PRINT_HEAD_MAGIC;
 		head->seq_no = ++seq_no;
 		head->dest = stream;
 
@@ -159,6 +173,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
 		/* An empty entry marks the wrap-around */
 		head = buffer->ring + write_pos;
+		head->magic = RT_PRINT_HEAD_MAGIC;
 		head->seq_no = seq_no;
 		head->text[0] = 0;
 
@@ -382,6 +397,8 @@ static void print_buffers(void)
 		head = buffer->ring + read_pos;
 		len = strlen(head->text);
 
+		assert(head->magic == RT_PRINT_HEAD_MAGIC);
+
 		if (len) {
 			/* Print out non-empty entry and proceed */
 			fprintf(head->dest, "%s", head->text);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Christoph Permes]

Am Freitag, den 28.08.2009, 11:07 +0200 schrieb Jan Kiszka:
> Christoph Permes wrote:
> 
> Untested stuff as I should already be on the road. Here is another try,
> but please don't expect replies from me over this weekend.
> 
> Jan
> 
> diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
> index 0615247..06dabc1 100644
> --- a/src/rtdk/rt_print.c
> +++ b/src/rtdk/rt_print.c
> @@ -16,6 +16,7 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA.
>   */
>  
> +#include <assert.h>
>  #include <errno.h>
>  #include <inttypes.h>
>  #include <limits.h>
> @@ -37,7 +38,10 @@
>  
>  #define RT_PRINT_LINE_BREAK		256
>  
> +#define RT_PRINT_HEAD_MAGIC		0xDEADBEAF
> +
>  struct entry_head {
> +	uint32_t magic;
>  	FILE *dest;
>  	uint32_t seq_no;
>  	char text[1];
> @@ -103,6 +107,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  	read_pos = buffer->read_pos;
>  	xnarch_read_memory_barrier();
>  
> +	assert(write_pos == read_pos ||
> +	       ((struct entry_head *)buffer->ring + buffer->read_pos)->magic ==
> +	       RT_PRINT_HEAD_MAGIC);
> +
>  	/* Is our write limit the end of the ring buffer? */
>  	if (write_pos >= read_pos) {
>  		/* Keep a savety margin to the end for at least an empty entry */
> @@ -114,6 +122,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  		if (len == 0 && read_pos > sizeof(struct entry_head)) {
>  			/* Write out empty entry */
>  			head = buffer->ring + write_pos;
> +			head->magic = RT_PRINT_HEAD_MAGIC;
>  			head->seq_no = seq_no;
>  			head->text[0] = 0;
>  
> @@ -136,6 +145,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  
>  	res = vsnprintf(head->text, len, format, args);
>  
> +	assert(write_pos == read_pos ||
> +	       ((struct entry_head *)buffer->ring + buffer->read_pos)->magic ==
> +	       RT_PRINT_HEAD_MAGIC);
> +
>  	if (res < len) {
>  		/* Text was written completely, res contains its length */
>  		len = res;
> @@ -147,6 +160,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  
>  	/* If we were able to write some text, finalise the entry */
>  	if (len > 0) {
> +		head->magic = RT_PRINT_HEAD_MAGIC;
>  		head->seq_no = ++seq_no;
>  		head->dest = stream;
>  
> @@ -159,6 +173,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
>  		/* An empty entry marks the wrap-around */
>  		head = buffer->ring + write_pos;
> +		head->magic = RT_PRINT_HEAD_MAGIC;
>  		head->seq_no = seq_no;
>  		head->text[0] = 0;
>  
> @@ -382,6 +397,8 @@ static void print_buffers(void)
>  		head = buffer->ring + read_pos;
>  		len = strlen(head->text);
>  
> +		assert(head->magic == RT_PRINT_HEAD_MAGIC);
> +
>  		if (len) {
>  			/* Print out non-empty entry and proceed */
>  			fprintf(head->dest, "%s", head->text);
> 

I ran some tests over the weekend and got two more crashs. In both cases
the assert statement in print_buffers() failed.

After taking a closer look on the core file from last week and the new
ones I found out that parts of the printed text were overwritten with
one or more zero characters.
As a result the strlen() statement in print_buffers() does not return
the complete text length and the next try to read data from the buffer
fails as the read position is not correct anymore.

As a first workaround I will try to store the length returned by
vsnprintf() in the entry_head struct and use this value for incrementing
the read position to avoid the SEGV.

Christoph

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Christoph Permes]

Hi, 

I've created a patch that provides a workaround for the segmentation
fault:

--- a/src/rtdk/rt_print.c	2009-05-21 16:34:54.000000000 +0200
+++ b/src/rtdk/rt_print.c	2009-09-04 10:09:19.000000000 +0200
@@ -40,6 +40,7 @@
 struct entry_head {
 	FILE *dest;
 	uint32_t seq_no;
+	uint32_t len;
 	char text[1];
 } __attribute__((packed));
 
@@ -113,6 +114,7 @@
 			/* Write out empty entry */
 			head = buffer->ring + write_pos;
 			head->seq_no = __seq_no;
+			head->len = 0;
 			head->text[0] = 0;
 
 			/* Forward to the ring buffer start */
@@ -146,6 +148,7 @@
 	/* If we were able to write some text, finalise the entry */
 	if (len > 0) {
 		head->seq_no = ++__seq_no;
+		head->len = len;
 		head->dest = stream;
 
 		/* Move forward by text and head length */
@@ -158,6 +161,7 @@
 		/* An empty entry marks the wrap-around */
 		head = buffer->ring + write_pos;
 		head->seq_no = __seq_no;
+		head->len = 0;
 		head->text[0] = 0;
 
 		write_pos = 0;
@@ -373,7 +377,7 @@
 
 		read_pos = buffer->read_pos;
 		head = buffer->ring + read_pos;
-		len = strlen(head->text);
+		len = head->len;
 
 		if (len) {
 			/* Print out non-empty entry and proceed */

With this patch I get no segfaults anymore, but of course it would be
interesting why parts of the ring buffer are overwritten with null
characters.

Christoph


Am Freitag, den 28.08.2009, 11:07 +0200 schrieb Jan Kiszka:
> Christoph Permes wrote:
> > Am Freitag, den 28.08.2009, 09:38 +0200 schrieb Jan Kiszka:
> >> Hmm, strange. Code meditation didn't help, so I need to keep you busy
> >> with testing. Could you try this instrumentation? It should choke if the
> >> rt_vfprintf actually overwrites already written data.
> >>
> >> Thanks,
> >> Jan
> >>
> >> diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
> >> index 0615247..bcd8c88 100644
> >> --- a/src/rtdk/rt_print.c
> >> +++ b/src/rtdk/rt_print.c
> >> @@ -16,6 +16,7 @@
> >>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA.
> >>   */
> >>  
> >> +#include <assert.h>
> >>  #include <errno.h>
> >>  #include <inttypes.h>
> >>  #include <limits.h>
> >> @@ -37,7 +38,10 @@
> >>  
> >>  #define RT_PRINT_LINE_BREAK		256
> >>  
> >> +#define RT_PRINT_HEAD_MAGIC		0xDEADBEAF
> >> +
> >>  struct entry_head {
> >> +	uint32_t magic;
> >>  	FILE *dest;
> >>  	uint32_t seq_no;
> >>  	char text[1];
> >> @@ -103,6 +107,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
> >>  	read_pos = buffer->read_pos;
> >>  	xnarch_read_memory_barrier();
> >>  
> >> +	assert(write_pos == read_pos ||
> >> +	       ((struct entry_head *)buffer->read_pos)->magic ==
> >> +	       RT_PRINT_HEAD_MAGIC);
> >> +
> >>  	/* Is our write limit the end of the ring buffer? */
> >>  	if (write_pos >= read_pos) {
> >>  		/* Keep a savety margin to the end for at least an empty entry */
> >> @@ -114,6 +122,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
> >>  		if (len == 0 && read_pos > sizeof(struct entry_head)) {
> >>  			/* Write out empty entry */
> >>  			head = buffer->ring + write_pos;
> >> +			head->magic = RT_PRINT_HEAD_MAGIC;
> >>  			head->seq_no = seq_no;
> >>  			head->text[0] = 0;
> >>  
> >> @@ -136,6 +145,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
> >>  
> >>  	res = vsnprintf(head->text, len, format, args);
> >>  
> >> +	assert(write_pos == read_pos ||
> >> +	       ((struct entry_head *)buffer->read_pos)->magic ==
> >> +	       RT_PRINT_HEAD_MAGIC);
> >> +
> >>  	if (res < len) {
> >>  		/* Text was written completely, res contains its length */
> >>  		len = res;
> >> @@ -147,6 +160,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
> >>  
> >>  	/* If we were able to write some text, finalise the entry */
> >>  	if (len > 0) {
> >> +		head->magic = RT_PRINT_HEAD_MAGIC;
> >>  		head->seq_no = ++seq_no;
> >>  		head->dest = stream;
> >>  
> >> @@ -159,6 +173,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
> >>  	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
> >>  		/* An empty entry marks the wrap-around */
> >>  		head = buffer->ring + write_pos;
> >> +		head->magic = RT_PRINT_HEAD_MAGIC;
> >>  		head->seq_no = seq_no;
> >>  		head->text[0] = 0;
> >>  
> >> @@ -382,6 +397,8 @@ static void print_buffers(void)
> >>  		head = buffer->ring + read_pos;
> >>  		len = strlen(head->text);
> >>  
> >> +		assert(head->magic == RT_PRINT_HEAD_MAGIC);
> >> +
> >>  		if (len) {
> >>  			/* Print out non-empty entry and proceed */
> >>  			fprintf(head->dest, "%s", head->text);
> >>
> > 
> > Now I get a SEGV at the first line of the first assert statement
> > (assert(write_pos == read_pos || ) in rt_vfprintf.
> > This happens at the first rt_printf call, also with the rtprint example
> > program.
> 
> Untested stuff as I should already be on the road. Here is another try,
> but please don't expect replies from me over this weekend.
> 
> Jan
> 
> diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
> index 0615247..06dabc1 100644
> --- a/src/rtdk/rt_print.c
> +++ b/src/rtdk/rt_print.c
> @@ -16,6 +16,7 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA.
>   */
>  
> +#include <assert.h>
>  #include <errno.h>
>  #include <inttypes.h>
>  #include <limits.h>
> @@ -37,7 +38,10 @@
>  
>  #define RT_PRINT_LINE_BREAK		256
>  
> +#define RT_PRINT_HEAD_MAGIC		0xDEADBEAF
> +
>  struct entry_head {
> +	uint32_t magic;
>  	FILE *dest;
>  	uint32_t seq_no;
>  	char text[1];
> @@ -103,6 +107,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  	read_pos = buffer->read_pos;
>  	xnarch_read_memory_barrier();
>  
> +	assert(write_pos == read_pos ||
> +	       ((struct entry_head *)buffer->ring + buffer->read_pos)->magic ==
> +	       RT_PRINT_HEAD_MAGIC);
> +
>  	/* Is our write limit the end of the ring buffer? */
>  	if (write_pos >= read_pos) {
>  		/* Keep a savety margin to the end for at least an empty entry */
> @@ -114,6 +122,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  		if (len == 0 && read_pos > sizeof(struct entry_head)) {
>  			/* Write out empty entry */
>  			head = buffer->ring + write_pos;
> +			head->magic = RT_PRINT_HEAD_MAGIC;
>  			head->seq_no = seq_no;
>  			head->text[0] = 0;
>  
> @@ -136,6 +145,10 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  
>  	res = vsnprintf(head->text, len, format, args);
>  
> +	assert(write_pos == read_pos ||
> +	       ((struct entry_head *)buffer->ring + buffer->read_pos)->magic ==
> +	       RT_PRINT_HEAD_MAGIC);
> +
>  	if (res < len) {
>  		/* Text was written completely, res contains its length */
>  		len = res;
> @@ -147,6 +160,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  
>  	/* If we were able to write some text, finalise the entry */
>  	if (len > 0) {
> +		head->magic = RT_PRINT_HEAD_MAGIC;
>  		head->seq_no = ++seq_no;
>  		head->dest = stream;
>  
> @@ -159,6 +173,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
>  	    read_pos <= write_pos && read_pos > buffer->size - write_pos) {
>  		/* An empty entry marks the wrap-around */
>  		head = buffer->ring + write_pos;
> +		head->magic = RT_PRINT_HEAD_MAGIC;
>  		head->seq_no = seq_no;
>  		head->text[0] = 0;
>  
> @@ -382,6 +397,8 @@ static void print_buffers(void)
>  		head = buffer->ring + read_pos;
>  		len = strlen(head->text);
>  
> +		assert(head->magic == RT_PRINT_HEAD_MAGIC);
> +
>  		if (len) {
>  			/* Print out non-empty entry and proceed */
>  			fprintf(head->dest, "%s", head->text);
> 

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Jan Kiszka]

Christoph Permes wrote:
> Hi, 
> 
> I've created a patch that provides a workaround for the segmentation
> fault:
> 
> --- a/src/rtdk/rt_print.c	2009-05-21 16:34:54.000000000 +0200
> +++ b/src/rtdk/rt_print.c	2009-09-04 10:09:19.000000000 +0200
> @@ -40,6 +40,7 @@
>  struct entry_head {
>  	FILE *dest;
>  	uint32_t seq_no;
> +	uint32_t len;
>  	char text[1];
>  } __attribute__((packed));
>  
> @@ -113,6 +114,7 @@
>  			/* Write out empty entry */
>  			head = buffer->ring + write_pos;
>  			head->seq_no = __seq_no;
> +			head->len = 0;
>  			head->text[0] = 0;
>  
>  			/* Forward to the ring buffer start */
> @@ -146,6 +148,7 @@
>  	/* If we were able to write some text, finalise the entry */
>  	if (len > 0) {
>  		head->seq_no = ++__seq_no;
> +		head->len = len;
>  		head->dest = stream;
>  
>  		/* Move forward by text and head length */
> @@ -158,6 +161,7 @@
>  		/* An empty entry marks the wrap-around */
>  		head = buffer->ring + write_pos;
>  		head->seq_no = __seq_no;
> +		head->len = 0;
>  		head->text[0] = 0;
>  
>  		write_pos = 0;
> @@ -373,7 +377,7 @@
>  
>  		read_pos = buffer->read_pos;
>  		head = buffer->ring + read_pos;
> -		len = strlen(head->text);
> +		len = head->len;
>  
>  		if (len) {
>  			/* Print out non-empty entry and proceed */
> 
> With this patch I get no segfaults anymore, but of course it would be
> interesting why parts of the ring buffer are overwritten with null
> characters.
> 

Frankly, I'm suspecting some issue in your application here - at least
as long as you do not have some broken-out test case for us...

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Christoph Permes wrote:
>> Hi, 
>>
>> I've created a patch that provides a workaround for the segmentation
>> fault:
>>
>> --- a/src/rtdk/rt_print.c	2009-05-21 16:34:54.000000000 +0200
>> +++ b/src/rtdk/rt_print.c	2009-09-04 10:09:19.000000000 +0200
>> @@ -40,6 +40,7 @@
>>  struct entry_head {
>>  	FILE *dest;
>>  	uint32_t seq_no;
>> +	uint32_t len;
>>  	char text[1];
>>  } __attribute__((packed));
>>  
>> @@ -113,6 +114,7 @@
>>  			/* Write out empty entry */
>>  			head = buffer->ring + write_pos;
>>  			head->seq_no = __seq_no;
>> +			head->len = 0;
>>  			head->text[0] = 0;
>>  
>>  			/* Forward to the ring buffer start */
>> @@ -146,6 +148,7 @@
>>  	/* If we were able to write some text, finalise the entry */
>>  	if (len > 0) {
>>  		head->seq_no = ++__seq_no;
>> +		head->len = len;
>>  		head->dest = stream;
>>  
>>  		/* Move forward by text and head length */
>> @@ -158,6 +161,7 @@
>>  		/* An empty entry marks the wrap-around */
>>  		head = buffer->ring + write_pos;
>>  		head->seq_no = __seq_no;
>> +		head->len = 0;
>>  		head->text[0] = 0;
>>  
>>  		write_pos = 0;
>> @@ -373,7 +377,7 @@
>>  
>>  		read_pos = buffer->read_pos;
>>  		head = buffer->ring + read_pos;
>> -		len = strlen(head->text);
>> +		len = head->len;
>>  
>>  		if (len) {
>>  			/* Print out non-empty entry and proceed */
>>
>> With this patch I get no segfaults anymore, but of course it would be
>> interesting why parts of the ring buffer are overwritten with null
>> characters.
>>
> 
> Frankly, I'm suspecting some issue in your application here - at least
> as long as you do not have some broken-out test case for us...

Would not it be possible to mprotect the rtdk buffer when it is not used
in order to catch any write to it outside of rtdk functions?

-- 
                                          Gilles

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Christoph Permes wrote:
>>> Hi, 
>>>
>>> I've created a patch that provides a workaround for the segmentation
>>> fault:
>>>
>>> --- a/src/rtdk/rt_print.c	2009-05-21 16:34:54.000000000 +0200
>>> +++ b/src/rtdk/rt_print.c	2009-09-04 10:09:19.000000000 +0200
>>> @@ -40,6 +40,7 @@
>>>  struct entry_head {
>>>  	FILE *dest;
>>>  	uint32_t seq_no;
>>> +	uint32_t len;
>>>  	char text[1];
>>>  } __attribute__((packed));
>>>  
>>> @@ -113,6 +114,7 @@
>>>  			/* Write out empty entry */
>>>  			head = buffer->ring + write_pos;
>>>  			head->seq_no = __seq_no;
>>> +			head->len = 0;
>>>  			head->text[0] = 0;
>>>  
>>>  			/* Forward to the ring buffer start */
>>> @@ -146,6 +148,7 @@
>>>  	/* If we were able to write some text, finalise the entry */
>>>  	if (len > 0) {
>>>  		head->seq_no = ++__seq_no;
>>> +		head->len = len;
>>>  		head->dest = stream;
>>>  
>>>  		/* Move forward by text and head length */
>>> @@ -158,6 +161,7 @@
>>>  		/* An empty entry marks the wrap-around */
>>>  		head = buffer->ring + write_pos;
>>>  		head->seq_no = __seq_no;
>>> +		head->len = 0;
>>>  		head->text[0] = 0;
>>>  
>>>  		write_pos = 0;
>>> @@ -373,7 +377,7 @@
>>>  
>>>  		read_pos = buffer->read_pos;
>>>  		head = buffer->ring + read_pos;
>>> -		len = strlen(head->text);
>>> +		len = head->len;
>>>  
>>>  		if (len) {
>>>  			/* Print out non-empty entry and proceed */
>>>
>>> With this patch I get no segfaults anymore, but of course it would be
>>> interesting why parts of the ring buffer are overwritten with null
>>> characters.
>>>
>> Frankly, I'm suspecting some issue in your application here - at least
>> as long as you do not have some broken-out test case for us...
> 
> Would not it be possible to mprotect the rtdk buffer when it is not used
> in order to catch any write to it outside of rtdk functions?
> 

Hmm, good idea, should work. Once set up, only rt_vsnprintf requires
write access to the ring.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Gilles Chanteperdrix]

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

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Christoph Permes wrote:
>>>> Hi, 
>>>>
>>>> I've created a patch that provides a workaround for the segmentation
>>>> fault:
>>>>
>>>> --- a/src/rtdk/rt_print.c	2009-05-21 16:34:54.000000000 +0200
>>>> +++ b/src/rtdk/rt_print.c	2009-09-04 10:09:19.000000000 +0200
>>>> @@ -40,6 +40,7 @@
>>>>  struct entry_head {
>>>>  	FILE *dest;
>>>>  	uint32_t seq_no;
>>>> +	uint32_t len;
>>>>  	char text[1];
>>>>  } __attribute__((packed));
>>>>  
>>>> @@ -113,6 +114,7 @@
>>>>  			/* Write out empty entry */
>>>>  			head = buffer->ring + write_pos;
>>>>  			head->seq_no = __seq_no;
>>>> +			head->len = 0;
>>>>  			head->text[0] = 0;
>>>>  
>>>>  			/* Forward to the ring buffer start */
>>>> @@ -146,6 +148,7 @@
>>>>  	/* If we were able to write some text, finalise the entry */
>>>>  	if (len > 0) {
>>>>  		head->seq_no = ++__seq_no;
>>>> +		head->len = len;
>>>>  		head->dest = stream;
>>>>  
>>>>  		/* Move forward by text and head length */
>>>> @@ -158,6 +161,7 @@
>>>>  		/* An empty entry marks the wrap-around */
>>>>  		head = buffer->ring + write_pos;
>>>>  		head->seq_no = __seq_no;
>>>> +		head->len = 0;
>>>>  		head->text[0] = 0;
>>>>  
>>>>  		write_pos = 0;
>>>> @@ -373,7 +377,7 @@
>>>>  
>>>>  		read_pos = buffer->read_pos;
>>>>  		head = buffer->ring + read_pos;
>>>> -		len = strlen(head->text);
>>>> +		len = head->len;
>>>>  
>>>>  		if (len) {
>>>>  			/* Print out non-empty entry and proceed */
>>>>
>>>> With this patch I get no segfaults anymore, but of course it would be
>>>> interesting why parts of the ring buffer are overwritten with null
>>>> characters.
>>>>
>>> Frankly, I'm suspecting some issue in your application here - at least
>>> as long as you do not have some broken-out test case for us...
>> Would not it be possible to mprotect the rtdk buffer when it is not used
>> in order to catch any write to it outside of rtdk functions?
>>
> 
> Hmm, good idea, should work. Once set up, only rt_vsnprintf requires
> write access to the ring.

Ok. Here it comes. Note however that calling rt_printf with this patch
will cause the caller to switch to secondary mode, but that is OK for
debugging. Also note that I only compile-tested the patch.

-- 
                                          Gilles


[-- Attachment #2: xeno-rtdk-mprotect.diff --]
[-- Type: text/x-patch, Size: 1476 bytes --]

diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
index 10285c0..5e7cda6 100644
--- a/src/rtdk/rt_print.c
+++ b/src/rtdk/rt_print.c
@@ -101,6 +101,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 	read_pos = buffer->read_pos;
 	xnarch_read_memory_barrier();
 
+	mprotect(buffer->ring, buffer->size, PROT_READ | PROT_WRITE);
 	/* Is our write limit the end of the ring buffer? */
 	if (write_pos >= read_pos) {
 		/* Keep a savety margin to the end for at least an empty entry */
@@ -165,6 +166,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 
 	/* All entry data must be written before we can update write_pos */
 	xnarch_write_memory_barrier();
+	mprotect(buffer->ring, buffer->size, PROT_READ);
 
 	buffer->write_pos = write_pos;
 
@@ -221,6 +223,7 @@ int rt_print_init(size_t buffer_size, const char *buffer_name)
 		size = __default_buffer_size;
 	else if (size < RT_PRINT_LINE_BREAK)
 		return EINVAL;
+	size = (size + getpagesize() - 1) & ~(getpagesize() - 1);
 
 	if (buffer) {
 		/* Only set name if buffer size is unchanged or default */
@@ -235,12 +238,11 @@ int rt_print_init(size_t buffer_size, const char *buffer_name)
 	if (!buffer)
 		return ENOMEM;
 
-	buffer->ring = malloc(size);
+	buffer->ring = mmap(NULL, size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE,  -1, 0);
 	if (!buffer->ring) {
 		free(buffer);
 		return ENOMEM;
 	}
-	memset(buffer->ring, 0, size);
 
 	buffer->read_pos  = 0;
 	buffer->write_pos = 0;

Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Gilles Chanteperdrix]

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

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Christoph Permes wrote:
>>>>> Hi, 
>>>>>
>>>>> I've created a patch that provides a workaround for the segmentation
>>>>> fault:
>>>>>
>>>>> --- a/src/rtdk/rt_print.c	2009-05-21 16:34:54.000000000 +0200
>>>>> +++ b/src/rtdk/rt_print.c	2009-09-04 10:09:19.000000000 +0200
>>>>> @@ -40,6 +40,7 @@
>>>>>  struct entry_head {
>>>>>  	FILE *dest;
>>>>>  	uint32_t seq_no;
>>>>> +	uint32_t len;
>>>>>  	char text[1];
>>>>>  } __attribute__((packed));
>>>>>  
>>>>> @@ -113,6 +114,7 @@
>>>>>  			/* Write out empty entry */
>>>>>  			head = buffer->ring + write_pos;
>>>>>  			head->seq_no = __seq_no;
>>>>> +			head->len = 0;
>>>>>  			head->text[0] = 0;
>>>>>  
>>>>>  			/* Forward to the ring buffer start */
>>>>> @@ -146,6 +148,7 @@
>>>>>  	/* If we were able to write some text, finalise the entry */
>>>>>  	if (len > 0) {
>>>>>  		head->seq_no = ++__seq_no;
>>>>> +		head->len = len;
>>>>>  		head->dest = stream;
>>>>>  
>>>>>  		/* Move forward by text and head length */
>>>>> @@ -158,6 +161,7 @@
>>>>>  		/* An empty entry marks the wrap-around */
>>>>>  		head = buffer->ring + write_pos;
>>>>>  		head->seq_no = __seq_no;
>>>>> +		head->len = 0;
>>>>>  		head->text[0] = 0;
>>>>>  
>>>>>  		write_pos = 0;
>>>>> @@ -373,7 +377,7 @@
>>>>>  
>>>>>  		read_pos = buffer->read_pos;
>>>>>  		head = buffer->ring + read_pos;
>>>>> -		len = strlen(head->text);
>>>>> +		len = head->len;
>>>>>  
>>>>>  		if (len) {
>>>>>  			/* Print out non-empty entry and proceed */
>>>>>
>>>>> With this patch I get no segfaults anymore, but of course it would be
>>>>> interesting why parts of the ring buffer are overwritten with null
>>>>> characters.
>>>>>
>>>> Frankly, I'm suspecting some issue in your application here - at least
>>>> as long as you do not have some broken-out test case for us...
>>> Would not it be possible to mprotect the rtdk buffer when it is not used
>>> in order to catch any write to it outside of rtdk functions?
>>>
>> Hmm, good idea, should work. Once set up, only rt_vsnprintf requires
>> write access to the ring.
> 
> Ok. Here it comes. Note however that calling rt_printf with this patch
> will cause the caller to switch to secondary mode, but that is OK for
> debugging. Also note that I only compile-tested the patch.

Except that the first mprotect should go before the read barrier. So, a
better version.

-- 
                                          Gilles


[-- Attachment #2: xeno-rtdk-mprotect.2.diff --]
[-- Type: text/x-patch, Size: 1456 bytes --]

diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
index 0615247..fd752d1 100644
--- a/src/rtdk/rt_print.c
+++ b/src/rtdk/rt_print.c
@@ -101,6 +101,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 	/* Take a snapshot of the ring buffer state */
 	write_pos = buffer->write_pos;
 	read_pos = buffer->read_pos;
+	mprotect(buffer->ring, buffer->size, PROT_READ | PROT_WRITE);
 	xnarch_read_memory_barrier();
 
 	/* Is our write limit the end of the ring buffer? */
@@ -167,6 +168,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 
 	/* All entry data must be written before we can update write_pos */
 	xnarch_write_memory_barrier();
+	mprotect(buffer->ring, buffer->size, PROT_READ);
 
 	buffer->write_pos = write_pos;
 
@@ -223,6 +225,7 @@ int rt_print_init(size_t buffer_size, const char *buffer_name)
 		size = default_buffer_size;
 	else if (size < RT_PRINT_LINE_BREAK)
 		return EINVAL;
+	size = (size + getpagesize() - 1) & ~(getpagesize() - 1);
 
 	if (buffer) {
 		/* Only set name if buffer size is unchanged or default */
@@ -237,12 +240,11 @@ int rt_print_init(size_t buffer_size, const char *buffer_name)
 	if (!buffer)
 		return ENOMEM;
 
-	buffer->ring = malloc(size);
+	buffer->ring = mmap(NULL, size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE,  -1, 0);
 	if (!buffer->ring) {
 		free(buffer);
 		return ENOMEM;
 	}
-	memset(buffer->ring, 0, size);
 
 	buffer->read_pos  = 0;
 	buffer->write_pos = 0;