Erlang/OTP Projects

Documentation

Planet Trapexit

Bookmark

Personal tools

Erlang Mailing Lists

Author

Message

< Erlang patches mailing list ~ Few HiPE patches

Guest

Posted: Wed Jun 23, 2010 5:35 am

Guest

Hi again,

Do you guys have any idea what these are? Do they look reasonable?
Obvious errors?

http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-al?revision=1.2
http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-am?revision=1.2
http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-an?revision=1.2
http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-ao?revision=1.2
http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-ap?revision=1.2

I've contacted the original author but I haven't received any reply yet.
I've no idea why these exist in the NetBSD pkgsrc but I'd like to either
get rid of them or submit them to you guys to be included in the
official code tree.

Martti

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist

Guest

Posted: Wed Jun 23, 2010 5:36 am

Guest

Posted: Wed Jun 23, 2010 6:32 am

Guest

I just found this in our hacks.mk
(http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/hacks.mk?revision=1.2)

###
### XXX Don't let the kernel put the map elsewhere.
###
.if (${OPSYS} == "NetBSD" || \
${OPSYS} == "OpenBSD" || \
${OPSYS} == "FreeBSD" || \
${OPSYS} == "Solaris")
CFLAGS+= -DMMAP_MAP_FIXED
.endif

Any ideas why this is there?

Martti

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist

Guest

Posted: Wed Jun 23, 2010 6:32 am

Guest

Posted: Wed Jun 23, 2010 8:14 am

Guest

I got the following answer from the patch author. The same thing is
valid for NetBSD as well so I'm going to submit a proper patch later...

Martti

If I understand your question, you're asking about MAP_FIXED.
I work on FreeBSD, and those patches reflect the absense of
MMAP_MAP_FIXED:

$ grep -rF MMAP_MAP_FIXED /usr/include
$ uname -mrs
FreeBSD 6.3-STABLE i386

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist

Guest

Posted: Wed Jun 23, 2010 8:14 am

Guest

Posted: Wed Jun 23, 2010 10:29 am

Guest

Martti Kuparinen writes:
> Hi again,
>
> Do you guys have any idea what these are? Do they look reasonable?
> Obvious errors?
>
> http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-al?revision=1.2

For amd64: ignore the availability of MAP_32BIT if the fake MMAP_MAP_FIXED is
defined. Weird.

It also breaks Solaris. In a followup message you mention setting MMAP_MAP_FIXED
outside of the original sources. Although that would seem to unbreak Solaris it's
a totally unacceptable approach: the integrity of the sources must not depend on
external hacks outside of our control.

Why on earth would one want to ignore MAP_32BIT? If it's defined but doesn't
work, then the correct solution is to

#if ...
#undef MAP_32BIT
#endif

somewhere before this code.

> http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-am?revision=1.2

For arm:
1) Use MAP_ANON if MAP_ANONYMOUS isn't defined. This is Ok.
2. Add MAP_FIXED to mmap call if fake MMAP_MAP_FIXED is defined.
With standard-compliant mmap() [ie POSIX or SUS] this would result in an attempt
to mmap() anonymous memory at address zero, which is bogus and won't work.

Just how broken is BSD's mmap()? And have they redefined MAP_FIXED to mean
something completely non-standard?

Strong NAK util proper justification is given.

> http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-an?revision=1.2

For ppc: add MAP_FIXED to mmap() calls under the same conditions as above for arm.
Same breakage, same NAK.

> http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-ao?revision=1.2

For sparc: ditto.
There's a cleanup there to remove some dead x86_64 code: that one is Ok.

> http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-ap?revision=1.2

For x86 (32-bit): add MAP_FIXED to mmap() call as above. NAK.
There's a similar adjustment wrt MAP_32BIT as in the amd64 patch, but that's
dead code (#ifdef __x86_64__) so it seems unjustified.

I'm amused by the CVS comments about HiPE triggering BSD kernel panics ...

There are other patches in that directory that would seem to range from
obvious (use $(INSTALL_DATA) not $(INSTALL_PROGRAM) for non-executables,
proper ANSI/ISO C in F_PTR/L_PTR typedefs), to reasonable (BSD-specific
snippets for signal handling), to questionable (why do you patch the BEAM
code generator?).

If you want to discuss specific patches, current or future, please post them
(not links to them!) to erlang-patches.

/Mikael

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist

Guest

Posted: Wed Jun 23, 2010 10:29 am

Guest

Posted: Wed Jun 23, 2010 12:42 pm

Guest

On Wed, 2010-06-23 at 12:29 +0200, Mikael Pettersson wrote:

> Why on earth would one want to ignore MAP_32BIT? If it's defined but doesn't

Don't shoot the messenger (=me), I have to discuss with the original
author and ask why he did things the way he did. I'll update the patch
when I have a better solution.

> I'm amused by the CVS comments about HiPE triggering BSD kernel panics ...

I was running bleeding edge, development version of NetBSD and it had a
bug for a short period of time (which is now fixed) and that had nothing
to do with Erlang. The official NetBSD release version did not suffer
from this bug. I admit I should have put a better commit message...

Martti

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist

Guest

Posted: Wed Jun 23, 2010 12:42 pm

Guest

Posted: Thu Jun 24, 2010 5:39 am

Guest

You were absolutely right, those HiPE patches were not needed, I only
left these two. At least NetBSD does not have MAP_ANONYOUS but MAP_ANON
according to mmap(2). Happy with these?

--- erts/emulator/hipe/hipe_arm.c.orig 2010-06-24 07:25:07.000000000
+0300
+++ erts/emulator/hipe/hipe_arm.c 2010-06-24 07:25:43.000000000
+0300
@@ -73,6 +73,10 @@
#define in_area(ptr,start,nbytes) \
((unsigned long)((char*)(ptr) - (char*)(start)) < (nbytes))

+#if !defined(MAP_ANONYMOUS) && defined(MAP_ANON)
+#define MAP_ANONYOUS MAP_ANON
+#endif
+
static void *new_code_mapping(void)
{
return mmap(0, SEGMENT_NRBYTES,

--- erts/emulator/hipe/hipe_sparc.c.orig 2010-06-24
07:26:50.000000000 +0
300
+++ erts/emulator/hipe/hipe_sparc.c 2010-06-24 07:27:25.000000000
+0300
@@ -130,6 +130,10 @@
#define ALLOC_CODE_STATS(X) do{}while(0)
#endif

+#if !defined(MAP_ANONYMOUS) && defined(MAP_ANON)
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+
static void morecore(unsigned int alloc_bytes)
{
unsigned int map_bytes;

> reasonable (BSD-specific snippets for signal handling)

I looked at erts/emulator/hipe/hipe_x86_signal.c again and I have one
question: INIT is defined inside #ifdef...#endif but used without any
#ifdef..#endif check. Why? Without

@@ -326,7 +327,9 @@
struct sigaction sa;
int i;

+#ifndef __NetBSD__
INIT();
+#endif

hipe_sigaltstack_init();

I get an error like this

obj/i386-unknown-netbsdelf5.99.30/opt/smp/hipe_x86_signal.o: In function
`hipe_signal_init':
hipe_x86_signal.c Sad

.text+0x9): undefined reference to `INIT'

> to questionable (why do you patch the BEAM code generator?).

I assume you are talking about the now-removed patch-ah
(http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-ah?revision=1.3) which was

--- lib/compiler/src/v3_codegen.erl.orig 2010-06-11 18:30:11.000000000
+0300
+++ lib/compiler/src/v3_codegen.erl 2010-06-17 10:19:44.000000000 +0300
@@ -1520,6 +1520,8 @@
Sizes = filter(fun({_,{integer,0}}) -> false;
(_) -> true end, Sizes0),
case Sizes of
+ [] ->
+ {bs_init2,[{integer,0}]};
[{1,_}|_] ->
{bs_init_bits,cg_binary_bytes_to_bits(Sizes, [])};
[{8,_}|_] ->

I've removed this now.

Martti

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist

Guest

Posted: Thu Jun 24, 2010 5:39 am

Guest

Guest

Posted: Mon Jun 28, 2010 11:24 am

Guest

Martti Kuparinen writes:
> You were absolutely right, those HiPE patches were not needed, I only
> left these two. At least NetBSD does not have MAP_ANONYOUS but MAP_ANON
> according to mmap(2). Happy with these?
>
>
> --- erts/emulator/hipe/hipe_arm.c.orig 2010-06-24 07:25:07.000000000
> +0300
> +++ erts/emulator/hipe/hipe_arm.c 2010-06-24 07:25:43.000000000
> +0300
> @@ -73,6 +73,10 @@
> #define in_area(ptr,start,nbytes) \
> ((unsigned long)((char*)(ptr) - (char*)(start)) < (nbytes))
>
> +#if !defined(MAP_ANONYMOUS) && defined(MAP_ANON)
> +#define MAP_ANONYOUS MAP_ANON
> +#endif
> +
> static void *new_code_mapping(void)
> {
> return mmap(0, SEGMENT_NRBYTES,
>
>

OK

> --- erts/emulator/hipe/hipe_sparc.c.orig 2010-06-24
> 07:26:50.000000000 +0
> 300
> +++ erts/emulator/hipe/hipe_sparc.c 2010-06-24 07:27:25.000000000
> +0300
> @@ -130,6 +130,10 @@
> #define ALLOC_CODE_STATS(X) do{}while(0)
> #endif
>
> +#if !defined(MAP_ANONYMOUS) && defined(MAP_ANON)
> +#define MAP_ANONYMOUS MAP_ANON
> +#endif
> +
> static void morecore(unsigned int alloc_bytes)
> {
> unsigned int map_bytes;
>
>

OK

> > reasonable (BSD-specific snippets for signal handling)
>
> I looked at erts/emulator/hipe/hipe_x86_signal.c again and I have one
> question: INIT is defined inside #ifdef...#endif but used without any
> #ifdef..#endif check. Why? Without
>
>
> @@ -326,7 +327,9 @@
> struct sigaction sa;
> int i;
>
> +#ifndef __NetBSD__
> INIT();
> +#endif
>
> hipe_sigaltstack_init();
>
>
> I get an error like this
>
>
> obj/i386-unknown-netbsdelf5.99.30/opt/smp/hipe_x86_signal.o: In function
> `hipe_signal_init':
> hipe_x86_signal.c Sad

.text+0x9): undefined reference to `INIT'

That's because the target in question should have selected one of the
#if .. #endif sections for its sigaction() wrapper implementation,
which would also define the INIT() macro. By not selecting any such
section you don't get an INIT() macro definition, but you also don't
get any sigaction() wrapper, which is the whole point of this file.

The sigaction() wrapper to enforce SA_ONSTACK is required on x86/amd64.
If a port to a target omits it then that port is unsafe and unsupported.

> > to questionable (why do you patch the BEAM code generator?).
>
> I assume you are talking about the now-removed patch-ah
> (http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-ah?revision=1.3) which was
>
> --- lib/compiler/src/v3_codegen.erl.orig 2010-06-11 18:30:11.000000000
> +0300
> +++ lib/compiler/src/v3_codegen.erl 2010-06-17 10:19:44.000000000 +0300
> @@ -1520,6 +1520,8 @@
> Sizes = filter(fun({_,{integer,0}}) -> false;
> (_) -> true end, Sizes0),
> case Sizes of
> + [] ->
> + {bs_init2,[{integer,0}]};
> [{1,_}|_] ->
> {bs_init_bits,cg_binary_bytes_to_bits(Sizes, [])};
> [{8,_}|_] ->
>
>
> I've removed this now.

I was. Maybe it is a good change, but we need the original author's
comments about it and a review by the BEAM compiler folks. And it's
not a HiPE patch so it should be handled separately.

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist

Guest

Posted: Mon Jun 28, 2010 1:38 pm

Guest

All times are GMT

View next topic View previous topic Page 1 of 1 Erlang Forum - Trap Exit Forum Index ~ Erlang patches mailing list

This forum is locked: you cannot post, reply to, or edit topics.

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You cannot attach files in this forum
You cannot download files in this forum

Hosted by Erlang Solutions | Powered by phpBB and NoseBleed v1.14

Search

Navigation

Erlang/OTP Projects

Documentation

Planet Trapexit

Bookmark

Personal tools

Erlang Mailing Lists

< Erlang patches mailing list ~ Few HiPE patches