[LTP] [PATCH v2] Refactor fork05 using new LTP API

[Andrea Cervesato]

Hi!

On 3/13/24 12:49, Cyril Hrubis wrote:
> Hi!
>> + * On Friday, May 2, 2003 at 09:47:00AM MST, Ulrich Drepper wrote:
>> + * >Robert Williamson wrote:
>> + * >
>> + * >>   I'm getting a SIGSEGV with one of our tests, fork05.c, that apparently
>> + * >> you wrote (attached below).  The test passes on my 2.5.68 machine running
>> + * >> SuSE 8.0 (glibc 2.2.5 and Linuxthreads), however it segmentation faults on
>> + * >> RedHat 9 running 2.5.68.  The test seems to "break" when it attempts to run
>> + * >> the assembly code....could you take a look at it?
>> + * >
>> + * >There is no need to look at it, I know it cannot work anymore on recent
>> + * >systems.  Either change all uses of %gs to %fs or skip the entire patch
>> + * >if %gs has a nonzero value.
>> + * >
>> + * >- --
>> + * >- --------------.                        ,-.            444 Castro Street
>> + * >Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
>> + * >Red Hat         `--' drepper at redhat.com `---------------------------
>>    *
>> + * On Sat, Aug 12, 2000 at 12:47:31PM -0700, Ulrich Drepper wrote:
>> + * > Ever since the %gs handling was fixed in the 2.3.99 series the
>> + * > appended test program worked.  Now with 2.4.0-test6 it's not working
>> + * > again.  Looking briefly over the patch from test5 to test6 I haven't
>> + * > seen an immediate candidate for the breakage.  It could be missing
>> + * > propagation of the LDT to the new process (and therefore an invalid
>> + * > segment descriptor) or simply clearing %gs.
>> + * >
>> + * > Anyway, this is what you should see and what you get with test5:
>> + * >
>> + * > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + * > a = 42
>> + * > %gs = 0x0007
>> + * > %gs = 0x0007
>> + * > a = 99
>> + * > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + * >
>> + * > This is what you get with test6:
>> + * >
>> + * > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + * > a = 42
>> + * > %gs = 0x0007
>> + * > %gs = 0x0000
>> + * > <SEGFAULT>
>> + * > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + * >
>> + * > If somebody is actually creating a test suite for the kernel, please
>> + * > add this program.  It's mostly self-contained.  The correct handling
>> + * > of %gs is really important since glibc 2.2 will make heavy use of it.
>> + * >
>> + * > - --
>> + * > - ---------------.                          ,-.   1325 Chesapeake Terrace
>> + * > Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
>> + * > Red Hat          `--' drepper at redhat.com   `------------------------
>> + * >
>> + * > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>    */
> Uff, this renders very ugly in the documentation. Can we at least drop
> the email signatures and rewrite the text a bit?
Yes it's horrible, but I didn't know what to do with this. I will try to 
do my best..
>> -#include <stdio.h>
>> -#include <fcntl.h>
>> -#include <unistd.h>
>> -#include <stdlib.h>
>> -#include <sys/wait.h>
>> +#include <asm/ldt.h>
>>   #include "lapi/syscalls.h"
>> -#include "test.h"
>> -
>> -char *TCID = "fork05";
>> -
>> -static char *environ_list[] = { "TERM", "NoTSetzWq", "TESTPROG" };
>> -
>> -#define NUMBER_OF_ENVIRON (sizeof(environ_list)/sizeof(char *))
>> -int TST_TOTAL = NUMBER_OF_ENVIRON;
>> +#include "tst_test.h"
>>   
>>   #if defined(linux) && defined(__i386__)
>>   
>> -struct modify_ldt_ldt_s {
>> -	unsigned int entry_number;
>> -	unsigned long int base_addr;
>> -	unsigned int limit;
>> -	unsigned int seg_32bit:1;
>> -	unsigned int contents:2;
>> -	unsigned int read_exec_only:1;
>> -	unsigned int limit_in_pages:1;
>> -	unsigned int seg_not_present:1;
>> -	unsigned int useable:1;
>> -	unsigned int empty:25;
>> -};
>> -
>> -static int a = 42;
>> -
>> -static void modify_ldt(int func, struct modify_ldt_ldt_s *ptr, int bytecount)
>> +static void run(void)
>>   {
>> -	tst_syscall(__NR_modify_ldt, func, ptr, bytecount);
>> -}
>> -
>> -int main(void)
>> -{
>> -	struct modify_ldt_ldt_s ldt0;
>> +	struct user_desc ldt0;
>> +	int base_addr = 42;
>>   	int lo;
>> -	pid_t pid;
>> -	int res;
>>   
>>   	ldt0.entry_number = 0;
>> -	ldt0.base_addr = (long)&a;
>> +	ldt0.base_addr = (long)&base_addr;
>>   	ldt0.limit = 4;
>>   	ldt0.seg_32bit = 1;
>>   	ldt0.contents = 0;
>> @@ -154,49 +90,34 @@ int main(void)
>>   	ldt0.useable = 1;
>>   	ldt0.empty = 0;
>>   
>> -	modify_ldt(1, &ldt0, sizeof(ldt0));
>> +	tst_syscall(__NR_modify_ldt, 1, &ldt0, sizeof(ldt0));
>>   
>>   	asm volatile ("movw %w0, %%fs"::"q" (7));
>> -
>>   	asm volatile ("movl %%fs:0, %0":"=r" (lo));
>> -	tst_resm(TINFO, "a = %d", lo);
>> +	tst_res(TINFO, "a = %d", lo);
>>   
>>   	asm volatile ("pushl %%fs; popl %0":"=q" (lo));
>> -	tst_resm(TINFO, "%%fs = %#06hx", lo);
>> +	tst_res(TINFO, "%%fs = %#06hx", lo);
>>   
>>   	asm volatile ("movl %0, %%fs:0"::"r" (99));
>>   
>> -	pid = fork();
>> -
>> -	if (pid == 0) {
>> +	if (!SAFE_FORK()) {
>>   		asm volatile ("pushl %%fs; popl %0":"=q" (lo));
>> -		tst_resm(TINFO, "%%fs = %#06hx", lo);
>> +		tst_res(TINFO, "%%fs = %#06hx", lo);
>>   
>>   		asm volatile ("movl %%fs:0, %0":"=r" (lo));
>> -		tst_resm(TINFO, "a = %d", lo);
>> +		tst_res(TINFO, "a = %d", lo);
>> +
>> +		TST_EXP_EQ_LI(lo, 99);
>>   
>> -		if (lo != 99)
>> -			tst_resm(TFAIL, "Test failed");
>> -		else
>> -			tst_resm(TPASS, "Test passed");
> Huh, why have you dropped the only TPASS/TFAIL in the test?
>
We have TST_EXP_EQ_LI()
>>   		exit(lo != 99);
> 	This should be just exit(0).
>
>> -	} else {
>> -		waitpid(pid, &res, 0);
>>   	}
>> -
>> -	return WIFSIGNALED(res);
>
> I guess that we can do waitpid() for the process and fail the test if we
> get SIGSEGV here as well.
>
>
>>   }
>>   
>> -#else /* if defined(linux) && defined(__i386__) */
>> -
>> -int main(void)
>> -{
>> -	tst_resm(TINFO, "%%fs test only for ix86");
>> -
>> -	/*
>> -	 * should be successful on all non-ix86 platforms.
>> -	 */
>> -	tst_exit();
>> -}
>> +static struct tst_test test = {
>> +	.run_all = run
>> +};
>>   
>> -#endif /* if defined(linux) && defined(__i386__) */
>> +#else /* defined(linux) && defined(__i386__) */
>> +	TST_TEST_TCONF("Test only supports linux 32 bits");
>> +#endif
>> -- 
>> 2.35.3
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp

Andrea