Author: philippe
Date: Sat Nov 1 22:00:50 2014
New Revision: 14683
Log:
fix 338995 shmat with hugepages (SHM_HUGETLB) fails with EINVAL
Bug is not really fixed, instead the SHM_HUGETLB flag is ignored.
Note that it is not straightforward to properly fix this,
as this implies either to learn aspacemgr what huge pages are.
Also, the trick used in the fix for 333051 cannot be used easily,
because the SHM_HUGETLB flag is given in shmget, while the mmap
is done in shmat.
So, the easiest is to just ignore the SHM_HUGETLB flag.
SHM_HUGETLB is supposed to only give a performance impact.
Ignoring it should be benign.
Theoretically, the caller might expect a sucessful shmget(SHM_HUGETLB)+shmat
to give pages aligned on e.g. 1MB.
In this case, bad luck, the program will misbehave under valgrind.
To warn of this, a warning is given (once) when SHM_HUGETLB is seen.
The map_unmap.c test has been restructured somewaht to allow
TEST_SHM_HUGETLB to be tested independently (or not) of the TEST_MAP_HUGETLB.
Note also that by default, testing MAP_HUGETLB and SHM_HUGETLB
is disabled as usually, huge pages are not enabled.
Modified:
trunk/NEWS
trunk/coregrind/m_syswrap/syswrap-darwin.c
trunk/coregrind/m_syswrap/syswrap-linux.c
trunk/include/vki/vki-linux.h
trunk/none/tests/map_unmap.c
Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Sat Nov 1 22:00:50 2014
@@ -31,6 +31,7 @@
335440 arm64: ld1 (single structure) is not implemented
335713 arm64: unhanded instruction: prfm (immediate)
338731 ppc: Fix testuite build for toolchains not supporting -maltivec
+338995 shmat with hugepages (SHM_HUGETLB) fails with EINVAL
339020 ppc64: memcheck/tests/ppc64/power_ISA2_05 failing in nightly build
339156 gdbsrv not called for fatal signal
339442 Fix testsuite build failure on OS X 10.9
Modified: trunk/coregrind/m_syswrap/syswrap-darwin.c
==============================================================================
--- trunk/coregrind/m_syswrap/syswrap-darwin.c (original)
+++ trunk/coregrind/m_syswrap/syswrap-darwin.c Sat Nov 1 22:00:50 2014
@@ -2310,6 +2310,15 @@
{
PRINT("shmget ( %ld, %ld, %ld )",ARG1,ARG2,ARG3);
PRE_REG_READ3(long, "shmget", vki_key_t, key, vki_size_t, size, int, shmflg);
+ if (ARG3 & VKI_SHM_HUGETLB) {
+ static Bool warning_given = False;
+ ARG3 &= ~VKI_SHM_HUGETLB;
+ if (!warning_given) {
+ warning_given = True;
+ VG_(umsg)(
+ "WARNING: valgrind ignores shmget(shmflg) SHM_HUGETLB\n");
+ }
+ }
}
PRE(shm_open)
Modified: trunk/coregrind/m_syswrap/syswrap-linux.c
==============================================================================
--- trunk/coregrind/m_syswrap/syswrap-linux.c (original)
+++ trunk/coregrind/m_syswrap/syswrap-linux.c Sat Nov 1 22:00:50 2014
@@ -3593,6 +3593,15 @@
case VKI_SHMGET:
PRE_REG_READ4(int, "ipc",
vki_uint, call, int, first, int, second, int, third);
+ if (ARG4 & VKI_SHM_HUGETLB) {
+ static Bool warning_given = False;
+ ARG4 &= ~VKI_SHM_HUGETLB;
+ if (!warning_given) {
+ warning_given = True;
+ VG_(umsg)(
+ "WARNING: valgrind ignores shmget(shmflg) SHM_HUGETLB\n");
+ }
+ }
break;
case VKI_SHMCTL: /* IPCOP_shmctl */
PRE_REG_READ5(int, "ipc",
@@ -3795,6 +3804,15 @@
{
PRINT("sys_shmget ( %ld, %ld, %ld )",ARG1,ARG2,ARG3);
PRE_REG_READ3(long, "shmget", vki_key_t, key, vki_size_t, size, int, shmflg);
+ if (ARG3 & VKI_SHM_HUGETLB) {
+ static Bool warning_given = False;
+ ARG3 &= ~VKI_SHM_HUGETLB;
+ if (!warning_given) {
+ warning_given = True;
+ VG_(umsg)(
+ "WARNING: valgrind ignores shmget(shmflg) SHM_HUGETLB\n");
+ }
+ }
}
PRE(wrap_sys_shmat)
Modified: trunk/include/vki/vki-linux.h
==============================================================================
--- trunk/include/vki/vki-linux.h (original)
+++ trunk/include/vki/vki-linux.h Sat Nov 1 22:00:50 2014
@@ -1160,6 +1160,9 @@
#define VKI_IPC_64 0x0100 /* New version (support 32-bit UIDs, bigger
message sizes, etc. */
+// From /usr/include/bits/shm.h
+# define VKI_SHM_HUGETLB 04000
+
//----------------------------------------------------------------------
// From linux-2.6.8.1/include/linux/sem.h
Modified: trunk/none/tests/map_unmap.c
==============================================================================
--- trunk/none/tests/map_unmap.c (original)
+++ trunk/none/tests/map_unmap.c Sat Nov 1 22:00:50 2014
@@ -10,7 +10,7 @@
echo 20 > /proc/sys/vm/nr_hugepages
Once this is done, uncomment the below, and recompile.
*/
-// #define TEST_MAP_HUGETLB 1
+//#define TEST_MAP_HUGETLB 1
/* Similarly, testing SHM_HUGETLB huge pages is disabled by default.
To have shmget/shmat big pages working, do (as root)
@@ -18,13 +18,12 @@
where 500 is the groupid of the user that runs this test
Once this is done, uncomment the below, and recompile.
*/
-// #define TEST_SHM_HUGETLB 1
+//#define TEST_SHM_HUGETLB 1
-#ifdef TEST_MAP_HUGETLB
-#include <sys/ipc.h>
-#include <sys/shm.h>
-#include <sys/stat.h>
+// Size to use for huge pages
+#define HUGESZ (4 * 1024 * 1024)
+#ifdef TEST_MAP_HUGETLB
/* Ensure this compiles on pre 2.6 systems, or on glibc missing MAP_HUGETLB */
#ifndef MAP_HUGETLB
/* The below works for me on an f12/x86 linux */
@@ -34,6 +33,9 @@
#endif /* TEST_MAP_HUGETLB */
#ifdef TEST_SHM_HUGETLB
+#include <sys/ipc.h>
+#include <sys/shm.h>
+#include <sys/stat.h>
#ifndef SHM_HUGETLB
#define SHM_HUGETLB 04000
#endif
@@ -114,7 +116,6 @@
}
#ifdef TEST_MAP_HUGETLB
-#define HUGESZ (4 * 1024 * 1024)
{
void *expect3;
expect3 = domap(HUGESZ, MAP_HUGETLB);
|
|
From: Rhys K. <rhy...@gm...> - 2014-11-03 14:42:12
|
Hi list,
In the changeset from r14683, I'm fairly sure the HUGETLB option as a
shmflg to shmget() is a Linux 2.6+ only flag.
i.e. the changes to trunk/coregrind/m_syswrap/syswrap-darwin.c can be reversed.
The patch to the darwin file is currently causing compiler errors on
that platform.
On 2 November 2014 09:00, <sv...@va...> wrote:
> Author: philippe
> Date: Sat Nov 1 22:00:50 2014
> New Revision: 14683
>
> Log:
> fix 338995 shmat with hugepages (SHM_HUGETLB) fails with EINVAL
>
> Bug is not really fixed, instead the SHM_HUGETLB flag is ignored.
> Note that it is not straightforward to properly fix this,
> as this implies either to learn aspacemgr what huge pages are.
> Also, the trick used in the fix for 333051 cannot be used easily,
> because the SHM_HUGETLB flag is given in shmget, while the mmap
> is done in shmat.
>
> So, the easiest is to just ignore the SHM_HUGETLB flag.
>
> SHM_HUGETLB is supposed to only give a performance impact.
> Ignoring it should be benign.
> Theoretically, the caller might expect a sucessful shmget(SHM_HUGETLB)+shmat
> to give pages aligned on e.g. 1MB.
> In this case, bad luck, the program will misbehave under valgrind.
> To warn of this, a warning is given (once) when SHM_HUGETLB is seen.
>
> The map_unmap.c test has been restructured somewaht to allow
> TEST_SHM_HUGETLB to be tested independently (or not) of the TEST_MAP_HUGETLB.
>
> Note also that by default, testing MAP_HUGETLB and SHM_HUGETLB
> is disabled as usually, huge pages are not enabled.
>
>
>
> Modified:
> trunk/NEWS
> trunk/coregrind/m_syswrap/syswrap-darwin.c
> trunk/coregrind/m_syswrap/syswrap-linux.c
> trunk/include/vki/vki-linux.h
> trunk/none/tests/map_unmap.c
>
> Modified: trunk/NEWS
> ==============================================================================
> --- trunk/NEWS (original)
> +++ trunk/NEWS Sat Nov 1 22:00:50 2014
> @@ -31,6 +31,7 @@
> 335440 arm64: ld1 (single structure) is not implemented
> 335713 arm64: unhanded instruction: prfm (immediate)
> 338731 ppc: Fix testuite build for toolchains not supporting -maltivec
> +338995 shmat with hugepages (SHM_HUGETLB) fails with EINVAL
> 339020 ppc64: memcheck/tests/ppc64/power_ISA2_05 failing in nightly build
> 339156 gdbsrv not called for fatal signal
> 339442 Fix testsuite build failure on OS X 10.9
>
> Modified: trunk/coregrind/m_syswrap/syswrap-darwin.c
> ==============================================================================
> --- trunk/coregrind/m_syswrap/syswrap-darwin.c (original)
> +++ trunk/coregrind/m_syswrap/syswrap-darwin.c Sat Nov 1 22:00:50 2014
> @@ -2310,6 +2310,15 @@
> {
> PRINT("shmget ( %ld, %ld, %ld )",ARG1,ARG2,ARG3);
> PRE_REG_READ3(long, "shmget", vki_key_t, key, vki_size_t, size, int, shmflg);
> + if (ARG3 & VKI_SHM_HUGETLB) {
> + static Bool warning_given = False;
> + ARG3 &= ~VKI_SHM_HUGETLB;
> + if (!warning_given) {
> + warning_given = True;
> + VG_(umsg)(
> + "WARNING: valgrind ignores shmget(shmflg) SHM_HUGETLB\n");
> + }
> + }
> }
>
> PRE(shm_open)
>
> Modified: trunk/coregrind/m_syswrap/syswrap-linux.c
> ==============================================================================
> --- trunk/coregrind/m_syswrap/syswrap-linux.c (original)
> +++ trunk/coregrind/m_syswrap/syswrap-linux.c Sat Nov 1 22:00:50 2014
> @@ -3593,6 +3593,15 @@
> case VKI_SHMGET:
> PRE_REG_READ4(int, "ipc",
> vki_uint, call, int, first, int, second, int, third);
> + if (ARG4 & VKI_SHM_HUGETLB) {
> + static Bool warning_given = False;
> + ARG4 &= ~VKI_SHM_HUGETLB;
> + if (!warning_given) {
> + warning_given = True;
> + VG_(umsg)(
> + "WARNING: valgrind ignores shmget(shmflg) SHM_HUGETLB\n");
> + }
> + }
> break;
> case VKI_SHMCTL: /* IPCOP_shmctl */
> PRE_REG_READ5(int, "ipc",
> @@ -3795,6 +3804,15 @@
> {
> PRINT("sys_shmget ( %ld, %ld, %ld )",ARG1,ARG2,ARG3);
> PRE_REG_READ3(long, "shmget", vki_key_t, key, vki_size_t, size, int, shmflg);
> + if (ARG3 & VKI_SHM_HUGETLB) {
> + static Bool warning_given = False;
> + ARG3 &= ~VKI_SHM_HUGETLB;
> + if (!warning_given) {
> + warning_given = True;
> + VG_(umsg)(
> + "WARNING: valgrind ignores shmget(shmflg) SHM_HUGETLB\n");
> + }
> + }
> }
>
> PRE(wrap_sys_shmat)
>
> Modified: trunk/include/vki/vki-linux.h
> ==============================================================================
> --- trunk/include/vki/vki-linux.h (original)
> +++ trunk/include/vki/vki-linux.h Sat Nov 1 22:00:50 2014
> @@ -1160,6 +1160,9 @@
>
> #define VKI_IPC_64 0x0100 /* New version (support 32-bit UIDs, bigger
> message sizes, etc. */
> +// From /usr/include/bits/shm.h
> +# define VKI_SHM_HUGETLB 04000
> +
>
> //----------------------------------------------------------------------
> // From linux-2.6.8.1/include/linux/sem.h
>
> Modified: trunk/none/tests/map_unmap.c
> ==============================================================================
> --- trunk/none/tests/map_unmap.c (original)
> +++ trunk/none/tests/map_unmap.c Sat Nov 1 22:00:50 2014
> @@ -10,7 +10,7 @@
> echo 20 > /proc/sys/vm/nr_hugepages
> Once this is done, uncomment the below, and recompile.
> */
> -// #define TEST_MAP_HUGETLB 1
> +//#define TEST_MAP_HUGETLB 1
>
> /* Similarly, testing SHM_HUGETLB huge pages is disabled by default.
> To have shmget/shmat big pages working, do (as root)
> @@ -18,13 +18,12 @@
> where 500 is the groupid of the user that runs this test
> Once this is done, uncomment the below, and recompile.
> */
> -// #define TEST_SHM_HUGETLB 1
> +//#define TEST_SHM_HUGETLB 1
>
> -#ifdef TEST_MAP_HUGETLB
> -#include <sys/ipc.h>
> -#include <sys/shm.h>
> -#include <sys/stat.h>
> +// Size to use for huge pages
> +#define HUGESZ (4 * 1024 * 1024)
>
> +#ifdef TEST_MAP_HUGETLB
> /* Ensure this compiles on pre 2.6 systems, or on glibc missing MAP_HUGETLB */
> #ifndef MAP_HUGETLB
> /* The below works for me on an f12/x86 linux */
> @@ -34,6 +33,9 @@
> #endif /* TEST_MAP_HUGETLB */
>
> #ifdef TEST_SHM_HUGETLB
> +#include <sys/ipc.h>
> +#include <sys/shm.h>
> +#include <sys/stat.h>
> #ifndef SHM_HUGETLB
> #define SHM_HUGETLB 04000
> #endif
> @@ -114,7 +116,6 @@
> }
>
> #ifdef TEST_MAP_HUGETLB
> -#define HUGESZ (4 * 1024 * 1024)
> {
> void *expect3;
> expect3 = domap(HUGESZ, MAP_HUGETLB);
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
|
|
From: Philippe W. <phi...@sk...> - 2014-11-04 13:37:50
|
On Tue, 2014-11-04 at 01:42 +1100, Rhys Kidd wrote: > Hi list, > > In the changeset from r14683, I'm fairly sure the HUGETLB option as a > shmflg to shmget() is a Linux 2.6+ only flag. > i.e. the changes to trunk/coregrind/m_syswrap/syswrap-darwin.c can be reversed. > > The patch to the darwin file is currently causing compiler errors on > that platform. Sorry for the breakage, having no access to a darwin platform, I cannot check it compiles. The shmget darwin man pages found on the net are not very precise e.g. no description of flags. So, it looks reasonable to revert the change for darwin. What is the compilation error ? Thanks Philippe |
|
From: Rhys K. <rhy...@gm...> - 2014-11-04 22:00:52
|
Thanks,
The compiler error is a basic:
gcc -DHAVE_CONFIG_H -I. -I.. -I.. -I../include -I../VEX/pub
-I../VEX/pub -DVGA_amd64=1 -DVGO_darwin=1 -DVGP_amd64_darwin=1
-DVGPV_amd64_darwin_vanilla=1 -I../coregrind
-DVG_LIBDIR="\"/usr/local/lib/valgrind"\"
-DVG_PLATFORM="\"amd64-darwin\"" -arch x86_64 -O2 -g -Wall
-Wcast-align -Wmissing-prototypes -Wshadow -Wpointer-arith
-Wstrict-prototypes -Wmissing-declarations -Wno-format-zero-length
-Wno-tautological-compare -fno-strict-aliasing -fno-builtin
-mmacosx-version-min=10.5 -fno-stack-protector -Wno-long-long
-Wno-tautological-compare -Wno-cast-align -Wno-self-assign
-Wwrite-strings -Wcast-qual -fno-stack-protector -MT
m_syswrap/libcoregrind_amd64_darwin_a-syswrap-darwin.o -MD -MP -MF
m_syswrap/.deps/libcoregrind_amd64_darwin_a-syswrap-darwin.Tpo -c -o
m_syswrap/libcoregrind_amd64_darwin_a-syswrap-darwin.o `test -f
'm_syswrap/syswrap-darwin.c' || echo './'`m_syswrap/syswrap-darwin.c
m_syswrap/syswrap-darwin.c:2313:15: error: use of undeclared
identifier 'VKI_SHM_HUGETLB'
if (ARG3 & VKI_SHM_HUGETLB) {
^
m_syswrap/syswrap-darwin.c:2315:16: error: use of undeclared
identifier 'VKI_SHM_HUGETLB'
ARG3 &= ~VKI_SHM_HUGETLB;
^
2 errors generated.
On 5 November 2014 00:37, Philippe Waroquiers
<phi...@sk...> wrote:
> On Tue, 2014-11-04 at 01:42 +1100, Rhys Kidd wrote:
>> Hi list,
>>
>> In the changeset from r14683, I'm fairly sure the HUGETLB option as a
>> shmflg to shmget() is a Linux 2.6+ only flag.
>
>> i.e. the changes to trunk/coregrind/m_syswrap/syswrap-darwin.c can be reversed.
>>
>> The patch to the darwin file is currently causing compiler errors on
>> that platform.
> Sorry for the breakage, having no access to a darwin platform,
> I cannot check it compiles.
> The shmget darwin man pages found on the net are not very precise
> e.g. no description of flags.
> So, it looks reasonable to revert the change for darwin.
>
> What is the compilation error ?
>
> Thanks
>
> Philippe
>
|
|
From: Julian S. <js...@ac...> - 2014-11-05 15:56:06
|
> m_syswrap/syswrap-darwin.c:2315:16: error: use of undeclared > identifier 'VKI_SHM_HUGETLB' > ARG3 &= ~VKI_SHM_HUGETLB; Per discussion + Rhys' suggestion, I backed out the darwin-specific part in r14692. J |