open_posix_testsuite/mmap24-2: Relax condition a bit

PATCH New Hongzhi.Song 4 participants 9 messages Oct 11, 2018 – Oct 24, 2018

lore

Share & tools

lore

Hongzhi.Song

author

2018-10-11 02:58 UTC #0 lore raw

PATCH 1/1 open_posix_testsuite/mmap24-2: Relax condition a bit

Mips will return EINVAL instead of ENOMEM as expected if the range [addr + len) exceeds TASK_SIZE.

Linux kernel code: arch/mips/mm/mmap.c

if (flags & MAP_FIXED) {
    /* Even MAP_FIXED mappings must reside within TASK_SIZE */
    if (TASK_SIZE - len < addr)
        return -EINVAL;

Relax the condition and accept both ENOMEM and EINVAL as expected outcome.

Signed-off-by Hongzhi.Song <[email protected]>

---
 .../open_posix_testsuite/conformance/interfaces/mmap/24-2.c    | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

testcases/open_posix_testsuite/conformance/interfaces/mmap/24-2.c +5 -5

index de51d43..810e5c8 100644
@@ -7,7 +7,7 @@
  * source tree.
  *
  * The mmap() function shall fail if:
- * [ENOMEM] MAP_FIXED was specified,
+ * [ENOMEM or EINVAL] MAP_FIXED was specified,
  * and the range [addr,addr+len) exceeds that allowed
  * for the address space of a process; or, if MAP_FIXED was not specified and
  * there is insufficient room in the address space to effect the mapping.

@@ -15,7 +15,7 @@
  * Test Step:
  * 1. Map a shared memory object, with size exceeding the value get from
  *    rlim_cur of resource RLIMIT_AS, setting MAP_FIXED;
- * 3. Should get ENOMEM.
+ * 3. Should get ENOMEM or EINVAL.
  */
 
 #define _XOPEN_SOURCE 600

@@ -93,8 +93,8 @@ int main(void)
 	       (unsigned long)len);
 	pa = mmap(addr, len, PROT_READ | PROT_WRITE, MAP_FIXED | MAP_SHARED, fd,
 		  0);
-	if (pa == MAP_FAILED && errno == ENOMEM) {
-		printf("Got ENOMEM: %s\nTest PASSED\n", strerror(errno));
+	if (pa == MAP_FAILED && (errno == ENOMEM || errno == EINVAL)) {
+		printf("Got ENOMEM or EINVAL: %s\nTest PASSED\n", strerror(errno));
 		exit(PTS_PASS);
 	}

@@ -103,6 +103,6 @@ int main(void)
 	else
 		munmap(pa, len);
 	close(fd);
-	printf("Test Fail: Did not get ENOMEM as expected\n");
+	printf("Test Failed: Did not get ENOMEM or EINVAL as expected\n");
 	return PTS_FAIL;
 }

Signature · 1 line

2.8.1

Petr Vorel

2018-10-12 16:20 UTC #1 lore raw

PATCH 1/1 open_posix_testsuite/mmap24-2: Relax condition a bit

Hi Hongzhi,

Quoted 9 lines

Mips will return EINVAL instead of ENOMEM as expected if the range [addr + len) exceeds TASK_SIZE.
Linux kernel code: arch/mips/mm/mmap.c
if (flags & MAP_FIXED) {
    /* Even MAP_FIXED mappings must reside within TASK_SIZE */
    if (TASK_SIZE - len < addr)
        return -EINVAL;

Quoted 4 lines

Relax the condition and accept both ENOMEM and EINVAL as expected outcome.
Signed-off-by Hongzhi.Song <[email protected]>

Reviewed-by Petr Vorel <[email protected]>

Thanks for your patch, LGTM.

Kind regards,
Petr

Hongzhi

2018-10-16 01:37 UTC #2 lore raw

PATCH 1/1 open_posix_testsuite/mmap24-2: Relax condition a bit

In reply to Hongzhi.Song

ping.

--Hongzhi

Quoted 58 lines · Hongzhi.Song

Mips will return EINVAL instead of ENOMEM as expected if the range [addr + len) exceeds TASK_SIZE.

Linux kernel code: arch/mips/mm/mmap.c

if (flags & MAP_FIXED) {
     /* Even MAP_FIXED mappings must reside within TASK_SIZE */
     if (TASK_SIZE - len < addr)
         return -EINVAL;

Relax the condition and accept both ENOMEM and EINVAL as expected outcome.

Signed-off-by Hongzhi.Song <[email protected]>

---
  .../open_posix_testsuite/conformance/interfaces/mmap/24-2.c    | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

testcases/open_posix_testsuite/conformance/interfaces/mmap/24-2.c +5 -5

index de51d43..810e5c8 100644
@@ -7,7 +7,7 @@
   * source tree.
   *
   * The mmap() function shall fail if:
- * [ENOMEM] MAP_FIXED was specified,
+ * [ENOMEM or EINVAL] MAP_FIXED was specified,
   * and the range [addr,addr+len) exceeds that allowed
   * for the address space of a process; or, if MAP_FIXED was not specified and
   * there is insufficient room in the address space to effect the mapping.

@@ -15,7 +15,7 @@
   * Test Step:
   * 1. Map a shared memory object, with size exceeding the value get from
   *    rlim_cur of resource RLIMIT_AS, setting MAP_FIXED;
- * 3. Should get ENOMEM.
+ * 3. Should get ENOMEM or EINVAL.
   */
 
  #define _XOPEN_SOURCE 600

@@ -93,8 +93,8 @@ int main(void)
  	       (unsigned long)len);
  	pa = mmap(addr, len, PROT_READ | PROT_WRITE, MAP_FIXED | MAP_SHARED, fd,
  		  0);
-	if (pa == MAP_FAILED && errno == ENOMEM) {
-		printf("Got ENOMEM: %s\nTest PASSED\n", strerror(errno));
+	if (pa == MAP_FAILED && (errno == ENOMEM || errno == EINVAL)) {
+		printf("Got ENOMEM or EINVAL: %s\nTest PASSED\n", strerror(errno));
  		exit(PTS_PASS);
  	}

@@ -103,6 +103,6 @@ int main(void)
  	else
  		munmap(pa, len);
  	close(fd);
-	printf("Test Fail: Did not get ENOMEM as expected\n");
+	printf("Test Failed: Did not get ENOMEM or EINVAL as expected\n");
  	return PTS_FAIL;
  }

Cyril Hrubis

2018-10-16 13:02 UTC #3 lore raw

PATCH 1/1 open_posix_testsuite/mmap24-2: Relax condition a bit

In reply to Hongzhi.Song

Hi!

Quoted 8 lines

Linux kernel code: arch/mips/mm/mmap.c
if (flags & MAP_FIXED) {
    /* Even MAP_FIXED mappings must reside within TASK_SIZE */
    if (TASK_SIZE - len < addr)
        return -EINVAL;
Relax the condition and accept both ENOMEM and EINVAL as expected outcome.

Hmm, it's always complicated with these POSIX tests, since we have to follow the POSIX specification there.

And the POSIX clearly says that mmap() shall fail with ENOMEM if MAP_FIXED was specified, and the range [addr,addr+len) exceeds that allowed for the address space of a process. So I guess that the best solution here would be limiting the address space with rlimit, so that we actually happen to hit the ENOMEM instead of EINVAL.

At least we cannot say that we passed the test with EINVAL because the assertion we tried to test wasn't triggered.

Signature · 2 lines

Cyril Hrubis
[email protected]

Hongzhi

2018-10-17 05:52 UTC #4 lore raw

PATCH 1/1 open_posix_testsuite/mmap24-2: Relax condition a bit

Quoted 9 lines · Cyril Hrubis

Hi!
Quoted 8 lines
Linux kernel code: arch/mips/mm/mmap.c
if (flags & MAP_FIXED) {
     /* Even MAP_FIXED mappings must reside within TASK_SIZE */
     if (TASK_SIZE - len < addr)
         return -EINVAL;
Relax the condition and accept both ENOMEM and EINVAL as expected outcome.

Quoted 8 lines

Hmm, it's always complicated with these POSIX tests, since we have to follow the POSIX specification there.
And the POSIX clearly says that mmap() shall fail with ENOMEM if MAP_FIXED was specified, and the range [addr,addr+len) exceeds that allowed for the address space of a process. So I guess that the best solution here would be limiting the address space with rlimit, so that we actually happen to hit the ENOMEM instead of EINVAL.

I just referred to follow commit which has similar issue.

[commit id: e7bab61882847]
syscalls/mmap15: relax condition a bit

High address is arch specific, and it also occasionally changes as can be se总共可用的内存大小的最en in history of this test.

    Relax the co总共可用的内存大小的最ndition and accept both ENOMEM and
EINVAL
    as expected outcome.
    总共可用的内存大小的最
    Fixes: #390

Mips returns EINVAL and never returns ENOMEM if [addr+len]
exceeds task size. I think it's has no relation with rlimit.

--Hongzhi

Quoted 4 lines

At least we cannot say that we passed the test with EINVAL because the assertion we tried to test wasn't triggered.

Cyril Hrubis

2018-10-17 08:39 UTC #5 lore raw

PATCH 1/1 open_posix_testsuite/mmap24-2: Relax condition a bit

Hi!

Quoted 13 lines

Quoted 8 lines
Hmm, it's always complicated with these POSIX tests, since we have to follow the POSIX specification there.
And the POSIX clearly says that mmap() shall fail with ENOMEM if MAP_FIXED was specified, and the range [addr,addr+len) exceeds that allowed for the address space of a process. So I guess that the best solution here would be limiting the address space with rlimit, so that we actually happen to hit the ENOMEM instead of EINVAL.
I just referred to follow commit which has similar issue.
?????? [commit id: e7bab61882847]
?????? syscalls/mmap15: relax condition a bit

Quoted 8 lines

?????? High address is arch specific, and it also occasionally changes ?????? as can be se?????????????????????????????????en in history of this test.
?????? Relax the co?????????????????????????????????ndition and accept both ENOMEM and
EINVAL
?????? as expected outcome.
?????? ?????????????????????????????????
?????? Fixes: #390

The difference is that the mmap15 is a Linux specific test, we can accept any behavior we think is reasonable.

Quoted 2 lines

Mips returns EINVAL and never returns ENOMEM if [addr+len]
??exceeds task size. I think it's has no relation with rlimit.

While the POSIX tests are written to test the conformity of the implementation, we have to stick to POSIX there and actually it seems to require that ENOMEM has to be returned if [addr,addr+len) exceeds that allowed for the address space of a process, so I guess that mips is not strictly conforming to POSIX here, however that is not a reason to change the test. And in this case it's not a big deal and we can possilby just ignore the failure in the same way we do for a few rwlock tests where Linux behavior differs from the one mandated by POSIX.

Signature · 2 lines

Cyril Hrubis
[email protected]

Hongzhi

2018-10-17 09:43 UTC #6 lore raw

PATCH 1/1 open_posix_testsuite/mmap24-2: Relax condition a bit

Quoted 25 lines · Cyril Hrubis

Hi!
Quoted 13 lines
Quoted 8 lines
Hmm, it's always complicated with these POSIX tests, since we have to follow the POSIX specification there.
And the POSIX clearly says that mmap() shall fail with ENOMEM if MAP_FIXED was specified, and the range [addr,addr+len) exceeds that allowed for the address space of a process. So I guess that the best solution here would be limiting the address space with rlimit, so that we actually happen to hit the ENOMEM instead of EINVAL.
I just referred to follow commit which has similar issue.
?????? [commit id: e7bab61882847]
?????? syscalls/mmap15: relax condition a bit
Quoted 8 lines
?????? High address is arch specific, and it also occasionally changes ?????? as can be se?????????????????????????????????en in history of this test.
?????? Relax the co?????????????????????????????????ndition and accept both ENOMEM and
EINVAL
?????? as expected outcome.
?????? ?????????????????????????????????
?????? Fixes: #390
The difference is that the mmap15 is a Linux specific test, we can accept any behavior we think is reasonable.

Quoted 8 lines

Quoted 2 lines
Mips returns EINVAL and never returns ENOMEM if [addr+len]
??exceeds task size. I think it's has no relation with rlimit.
While the POSIX tests are written to test the conformity of the implementation, we have to stick to POSIX there and actually it seems to require that ENOMEM has to be returned if [addr,addr+len) exceeds that allowed for the address space of a process, so I guess that mips is not strictly conforming to POSIX here, however that is not a reason to change the test.

How about to skip mips arch by using "ifdef __mips__" ? Otherwise the test will failed on mips.

--Hongzhi

Quoted 4 lines

And in this case it's not a big deal and we can
possilby just ignore the failure in the same way we do for a few rwlock tests where Linux behavior differs from the one mandated by POSIX.

Cyril Hrubis

2018-10-17 10:51 UTC #7 lore raw

PATCH 1/1 open_posix_testsuite/mmap24-2: Relax condition a bit

Hi!

Quoted 2 lines

How about to skip mips arch by using "ifdef __mips__" ? Otherwise the test will failed on mips.

There are other POSIX-like systems that runs on mips that may need to run the test, the best course of action would be skipping the test in the testrunner that you use to run the tests.

That gives a question, how do you run these testcases? Do you run these by typing in the 'make test'?

Signature · 2 lines

Cyril Hrubis
[email protected]

Hongzhi

2018-10-24 02:29 UTC #8 lore raw

PATCH 1/1 open_posix_testsuite/mmap24-2: Relax condition a bit

In reply to Cyril Hrubis

Quoted 25 lines · Cyril Hrubis

Hi!
Quoted 13 lines
Quoted 8 lines
Hmm, it's always complicated with these POSIX tests, since we have to follow the POSIX specification there.
And the POSIX clearly says that mmap() shall fail with ENOMEM if MAP_FIXED was specified, and the range [addr,addr+len) exceeds that allowed for the address space of a process. So I guess that the best solution here would be limiting the address space with rlimit, so that we actually happen to hit the ENOMEM instead of EINVAL.
I just referred to follow commit which has similar issue.
?????? [commit id: e7bab61882847]
?????? syscalls/mmap15: relax condition a bit
Quoted 8 lines
?????? High address is arch specific, and it also occasionally changes ?????? as can be se?????????????????????????????????en in history of this test.
?????? Relax the co?????????????????????????????????ndition and accept both ENOMEM and
EINVAL
?????? as expected outcome.
?????? ?????????????????????????????????
?????? Fixes: #390
The difference is that the mmap15 is a Linux specific test, we can accept any behavior we think is reasonable.

Quoted 7 lines

Quoted 2 lines
Mips returns EINVAL and never returns ENOMEM if [addr+len]
??exceeds task size. I think it's has no relation with rlimit.
While the POSIX tests are written to test the conformity of the implementation, we have to stick to POSIX there and actually it seems to require that ENOMEM has to be returned if [addr,addr+len) exceeds that allowed for the address space of a process, so I guess that mips is not strictly conforming to POSIX here,

Hi Cyril,

The POSIX specification says:
"If MAP_FIXED is set, mmap() may return MAP_FAILED and set errno to [EINVAL]."
[http://pubs.opengroup.org/onlinepubs/9699919799/functions/mmap.html]

So I think the mips kernel remains POSIX compliant.

I hope to send V2 with commit log explaining this clearly. With the patch, the testcase will support more Arches.

--Hongzhi

Quoted 5 lines

however that is not a reason to
change the test. And in this case it's not a big deal and we can possilby just ignore the failure in the same way we do for a few rwlock tests where Linux behavior differs from the one mandated by POSIX.