Opened 4 years ago

Last modified 2 months ago

#5188 infoneeded bug

Runtime error when allocating lots of memory

Reported by: knyblad Owned by:
Priority: low Milestone: 7.12.1
Component: Runtime System Version: 6.12.1
Keywords: Cc: hvr, simonmar
Operating System: Linux Architecture: x86
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description (last modified by igloo)

In GHCI execute

let powers = 2:map (2^) powers
powers

When calculating the fourth element of the list, you get the following error message:

<interactive>: internal error: getMBlock: mmap: Invalid argument
    (GHC version 6.12.1 for i386_unknown_linux)
    Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug

If you have trouble reproducing the bug, try substituting an other number for 2 in the code, e.g. I have also got that error message when I defined powers as:

let powers = 4:map (4^) powers

and

let powers = 13:map (13^) powers

Change History (21)

comment:1 Changed 4 years ago by simonmar

  • Milestone set to 7.2.1

Here's the relevant bit of code:

    if (ret == (void *)-1) {
	if (errno == ENOMEM || 
	    (errno == EINVAL && sizeof(void*)==4 && size >= 0xc0000000)) {
	    // If we request more than 3Gig, then we get EINVAL
	    // instead of ENOMEM (at least on Linux).
	    errorBelch("out of memory (requested %lu bytes)", size);
	    stg_exit(EXIT_FAILURE);
	} else {
	    barf("getMBlock: mmap: %s", strerror(errno));
	}
    }

Perhaps we're getting EINVAL under different circumstances. I didn't manage to reproduce it (yet).

comment:2 Changed 3 years ago by igloo

  • Description modified (diff)

comment:3 Changed 3 years ago by igloo

  • Milestone changed from 7.2.1 to 7.4.1

comment:4 Changed 3 years ago by igloo

  • Milestone changed from 7.4.1 to 7.6.1
  • Priority changed from normal to low

comment:5 Changed 2 years ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:6 Changed 18 months ago by rwbarton

  • difficulty set to Unknown

I was able to reproduce this (Linux 2.6.32-5-686, GHC 7.4.2). Here is the offending mmap call, from strace:

mmap2(0x8d900000, 0, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = -1 EINVAL (Invalid argument)

... which is certainly invalid (from the mmap man page: "EINVAL (since Linux 2.6.12) length was 0.")

I'll try to figure out why it is mmapping 0 bytes. Could be an integer overflow, when size is computed in osGetMBlocks...

comment:7 Changed 18 months ago by rwbarton

So I investigated further with gdb, and while I no longer have the exact numbers, gmp was indeed asking for an allocation of more than 4095 MB, resulting in osGetMBlocks(4096) and an integer overflow when computing size resulting in size = 0.

The fix should be to detect when the computation of size will overflow a size_t, and report an out-of-memory error if this occurs.

comment:8 Changed 18 months ago by rwbarton

The first attached patch checks for integer overflow in the computation of size in osGetMBlocks. With the patch the example program now exits with this message:

pow: out of memory (requested 4096 MBlocks)

The second and third patches are extra checks for overflow when converting between integer types in my_mmap and osGetMBlocks. AFAIK it *should* always be the case that W_ >= nat and that size_t >= W_ on platforms supported by GHC; but on any system where that is the case, the C compiler can eliminate the tests anyways, so I recommend including both tests.

Oops, I see there is a missing % in the middle patch.

comment:9 Changed 18 months ago by rwbarton

  • Status changed from new to patch

comment:10 Changed 18 months ago by Austin Seipp <aseipp@…>

In 48865521de6638240819b3979edbb3d33401dc8e/ghc:

Check for integer overflow in osGetMBlocks

Fixes Trac #5188.

Signed-off-by: Austin Seipp <[email protected]>

comment:11 Changed 18 months ago by thoughtpolice

I reverted 48865521 (and its kin) because unfortunately it causes a validate failure on amd64/Linux, where the conditional in patch #2 is always false, meaning we will always calculate size = n * MBLOCK_SIZE.

I'll look into this shortly.

comment:12 Changed 18 months ago by rwbarton

Right, the issue is that on amd64/Linux, sizeof(nat) is 4 and sizeof(W_) is 8, so overflow can never occur, which gcc warns about, and validate builds with -Wextra -Werror. Annoying.

Austin and I discussed this on IRC and here are a couple possible workarounds we came up with.

  1. Wrap the test in an #if that tests sizeof(nat) and sizeof(W_) to determine whether overflow is possible. Portable, but fiddly.
  1. Disable gcc warnings using #pragma GCC diagnostic pragmas around the test, themselves conditionalized to gcc. Less fiddly, but GHC doesn't currently use this method to disable any warnings and I somewhat hesitate to be the first. I also haven't tested that this really works.

comment:13 Changed 18 months ago by simonmar

I'm fairly sure that casting -1 to an unsigned type (W_) is undefined in C, because the value is outside the range of the type. Why not use HS_WORD_MAX?

comment:14 Changed 18 months ago by rwbarton

Casting to an unsigned integral type is defined to have "modulo 2n" semantics by C99 §6.3.1.3, so casting -1 to any unsigned integral type will result in the largest representable value of that type. But HS_WORD_MAX is okay too, and probably easier to read; I just didn't think to look for such a thing in HsFFI.h. (I find the proliferation of identical types StgWord, W_, HsWord a little confusing, but I'll get over it.)

comment:15 Changed 14 months ago by thoughtpolice

  • Cc hvr added
  • Status changed from patch to infoneeded

Reid, any word here - would you like you tweak this patch?

comment:16 Changed 8 months ago by thoughtpolice

  • Milestone changed from 7.6.2 to 7.10.1

Moving to 7.10.1.

comment:17 Changed 3 months ago by thomie

  • Cc simonmar added
  • Component changed from GHCi to Runtime System

In commit 29ee739ec8937501171426ef845279ec307b18b8:

Author: Austin Seipp <>
Date:   Thu Aug 29 17:30:16 2013 -0500

    Revert "Check for integer overflow in osGetMBlocks"
    
    This reverts commit 48865521de6638240819b3979edbb3d33401dc8e.
    
    Signed-off-by: Austin Seipp <[email protected]>

comment:18 Changed 2 months ago by thoughtpolice

  • Milestone changed from 7.10.1 to 7.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

Note: See TracTickets for help on using tickets.