Opened 4 years ago

Last modified 5 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: GHCi crash 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

Attachments (3)

Change History (22)

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 4 years ago by igloo

  • Description modified (diff)

comment:3 Changed 4 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 3 years ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:6 Changed 2 years 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 2 years 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 2 years 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 2 years ago by rwbarton

  • Status changed from new to patch

comment:10 Changed 2 years 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 2 years 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 2 years 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 23 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 23 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 19 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 13 months ago by thoughtpolice

  • Milestone changed from 7.6.2 to 7.10.1

Moving to 7.10.1.

comment:17 Changed 8 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 7 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.

comment:19 Changed 5 months ago by George

  • Type of failure changed from None/Unknown to GHCi crash
Note: See TracTickets for help on using tickets.