Glob problem with memory

Glob problem with memory

[Felix Rosencrantz]

When attempting to do an "echo *(/)" or use other glob qualifiers,
I've been getting a bus error with this stack trace on Solaris 5.6.
I started seeing this problem in builds sometime after Dec 14th.
I think it is related to the recently made memory changes.
I don't see the problem if I replace the hcalloc on line glob.c:1312
with a zcalloc, so the problem seems to be related to memory allocation.

#0  0x47370 in glob (list=0x17acac, np=0x17acec) at glob.c:1320
#1  0x98608 in globlist (list=0x17acac) at subst.c:222
#2  0x3c8ec in execcmd (cmd=0x17abec, input=0, output=0, how=2, last1=2)
    at exec.c:1815
#3  0x39578 in execpline2 (pline=0x17ac8c, how=2, input=0, output=0, last1=0)
    at exec.c:1062
#4  0x386b8 in execpline (l=0x17abd4, how=2, last1=0) at exec.c:875
#5  0x37fc8 in execlist (list=0x17ac9c, dont_change_job=0, exiting=0)
    at exec.c:744
#6  0x5747c in loop (toplevel=1, justonce=0) at init.c:145
#7  0x23414 in main (argc=2, argv=0xeffffa3c) at ./main.c:89

-FR.


__________________________________________________
Do You Yahoo!?
Talk to your friends online with Yahoo! Messenger.
http://messenger.yahoo.com

Re: Glob problem with memory

[Peter Stephenson]

Felix Rosencrantz wrote:
> When attempting to do an "echo *(/)" or use other glob qualifiers,
> I've been getting a bus error with this stack trace on Solaris 5.6.
> I started seeing this problem in builds sometime after Dec 14th.
> I think it is related to the recently made memory changes.
> I don't see the problem if I replace the hcalloc on line glob.c:1312
> with a zcalloc, so the problem seems to be related to memory allocation.

We should check that the new mmap() memory things in mem.c always return
64-bit aligned pointers where necessary, even on 32-bit machines.  I was
getting just this sort of problem with the old form of the zhalloc()
mechanism when I introduced 64-bit integer support and before I fixed the
alignment.  I'll have a look at this, but Sven might be able to trace it
more effectively when he gets back.  Meanwhile, changing the USE_MMAP
#define in mem.c to #undef USE_MMAP should fix things up for the time
being.

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>

Re: Glob problem with memory

[Peter Stephenson]

Felix Rosencrantz wrote:
> When attempting to do an "echo *(/)" or use other glob qualifiers,
> I've been getting a bus error with this stack trace on Solaris 5.6.

P.S. to my last message:  actually, if Linux is anything to go by, mmap()
should return things in multiples of the page size, in which case there
would be no alignment problems.  But it still seems something of this kind
is the most likely solution, so

1) you can probably see if it is by locating the (single) mmap() call in
mem.c, and adding underneath
  fprintf(stderr, "%p\n", h);
  fflush(stderr);
and looking at the pointers output;

2) I suppose it could be failing to use mmap64() for some reason --- at
least, this is the correct call for Linux with large file support, I don't
know if that's true for Solaris, but it's a good bet.  You could check with
nm whether zsh is linked with that.

In any case, I haven't seen any problems under Linux.
-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>

Re: Glob problem with memory

[jarausch]

On  1 Jan, Peter Stephenson wrote:
> Felix Rosencrantz wrote:
>> When attempting to do an "echo *(/)" or use other glob qualifiers,
>> I've been getting a bus error with this stack trace on Solaris 5.6.
>> I started seeing this problem in builds sometime after Dec 14th.
>> I think it is related to the recently made memory changes.
>> I don't see the problem if I replace the hcalloc on line glob.c:1312
>> with a zcalloc, so the problem seems to be related to memory allocation.
> 
> We should check that the new mmap() memory things in mem.c always return
> 64-bit aligned pointers where necessary, even on 32-bit machines.  I was
> getting just this sort of problem with the old form of the zhalloc()
> mechanism when I introduced 64-bit integer support and before I fixed the
> alignment.  I'll have a look at this, but Sven might be able to trace it
> more effectively when he gets back.  Meanwhile, changing the USE_MMAP
> #define in mem.c to #undef USE_MMAP should fix things up for the time
> being.
> 
Here on my IRIX box (it's 64 bit but I am using -n32 i.e. 32 bit code)
I get the same bus error,
BUT mem.c does NOT use mmap here, since my <sys/mman.h> doesn't define
MAP_ANONYMOUS

So what can we do?
Helmut.

-- 
Helmut Jarausch
Lehrstuhl fuer Numerische Mathematik
Institute of Technology, RWTH Aachen
D 52056 Aachen, Germany

Re: Glob problem with memory

[jarausch]

On  3 Jan, To: pws@pwstephenson.fsnet.co.uk wrote:
> On  1 Jan, Peter Stephenson wrote:
>> Felix Rosencrantz wrote:
>>> When attempting to do an "echo *(/)" or use other glob qualifiers,
>>> I've been getting a bus error with this stack trace on Solaris 5.6.
>>> I started seeing this problem in builds sometime after Dec 14th.
>>> I think it is related to the recently made memory changes.
>>> I don't see the problem if I replace the hcalloc on line glob.c:1312
>>> with a zcalloc, so the problem seems to be related to memory allocation.
>> 
>> We should check that the new mmap() memory things in mem.c always return
>> 64-bit aligned pointers where necessary, even on 32-bit machines.  I was
>> getting just this sort of problem with the old form of the zhalloc()
>> mechanism when I introduced 64-bit integer support and before I fixed the
>> alignment.  I'll have a look at this, but Sven might be able to trace it
>> more effectively when he gets back.  Meanwhile, changing the USE_MMAP
>> #define in mem.c to #undef USE_MMAP should fix things up for the time
>> being.
>> 
> Here on my IRIX box (it's 64 bit but I am using -n32 i.e. 32 bit code)
> I get the same bus error,
> BUT mem.c does NOT use mmap here, since my <sys/mman.h> doesn't define
> MAP_ANONYMOUS
> 
> So what can we do?
> Helmut.
> 
In addition I found out

sizeof(struct heap) == 20  here, so with the definition
#define arena(X) ((char *) (X) + sizeof(struct heap))
in mem.c line 324

return arena(h) + n - size;

is nearly guaranteed to be misaligned here.


-- 
Helmut Jarausch
Lehrstuhl fuer Numerische Mathematik
Institute of Technology, RWTH Aachen
D 52056 Aachen, Germany

64_bit logical is broken (was Re: Glob problem with memory)

[jarausch]

in zsh.h we have
struct heap {
    struct heap *next;		/* next one                                  */
    size_t size;		/* size of heap                              */
    size_t used;		/* bytes used from the heap                  */
    struct heapstack *sp;	/* used by pushheap() to save the value used */
#ifdef PAD_64_BIT
    size_t dummy;		/* Make sure sizeof(heap) is a multiple of 8 */
#endif

with 
#ifndef LONG_IS_64_BIT
# define PAD_64_BIT 1
#endif

Now here

sizeof(long)== 4   so  PAD_64_BIT is defined as 1

but
sizeof(size_t) == 4  and sizeof(void*) == 4 too
so that the padding of struct heap is just wrong here.

What's the intent?

Helmut.

-- 
Helmut Jarausch
Lehrstuhl fuer Numerische Mathematik
Institute of Technology, RWTH Aachen
D 52056 Aachen, Germany

PATCH: struct heap 64-bit

[Clint Adams]

> What's the intent?

Helmut fixed this in 9186 but the patch was reversed.

This just gets rid of the overcompensation altogether.
Incidentally, this solves the bus error problem I was
experiencing on 64-bit SPARC Linux.

Index: Src/zsh.h
===================================================================
RCS file: /cvs/zsh/zsh/Src/zsh.h,v
retrieving revision 1.1.1.39
diff -u -r1.1.1.39 zsh.h
--- Src/zsh.h	1999/12/17 12:18:28	1.1.1.39
+++ Src/zsh.h	2000/01/04 15:51:39
@@ -1559,9 +1559,6 @@
     size_t size;		/* size of heap                              */
     size_t used;		/* bytes used from the heap                  */
     struct heapstack *sp;	/* used by pushheap() to save the value used */
-#ifdef PAD_64_BIT
-    size_t dummy;		/* Make sure sizeof(heap) is a multiple of 8 */
-#endif
 #define arena(X)	((char *) (X) + sizeof(struct heap))
 };
 

Re: PATCH: struct heap 64-bit

[jarausch]

On  4 Jan, Clint Adams wrote:
>> What's the intent?
> 
> Helmut fixed this in 9186 but the patch was reversed.
> 
> This just gets rid of the overcompensation altogether.
> Incidentally, this solves the bus error problem I was
> experiencing on 64-bit SPARC Linux.
> 
> Index: Src/zsh.h
> ===================================================================
> RCS file: /cvs/zsh/zsh/Src/zsh.h,v
> retrieving revision 1.1.1.39
> diff -u -r1.1.1.39 zsh.h
> --- Src/zsh.h	1999/12/17 12:18:28	1.1.1.39
> +++ Src/zsh.h	2000/01/04 15:51:39
> @@ -1559,9 +1559,6 @@
>      size_t size;		/* size of heap                              */
>      size_t used;		/* bytes used from the heap                  */
>      struct heapstack *sp;	/* used by pushheap() to save the value used */
> -#ifdef PAD_64_BIT
> -    size_t dummy;		/* Make sure sizeof(heap) is a multiple of 8 */
> -#endif
>  #define arena(X)	((char *) (X) + sizeof(struct heap))
>  };
>  

I don't think this is a good solution.
My patch added a comment and left the code commented.
When somebody adds a new member to struct heap next time, he/she will
then be reminded to reenable that code to pad the struct to a multiple
of 64 bits.

Helmut.

-- 
Helmut Jarausch
Lehrstuhl fuer Numerische Mathematik
Institute of Technology, RWTH Aachen
D 52056 Aachen, Germany

Re: PATCH: struct heap 64-bit

[Tanaka Akira]

In article <20000104105536.A8967@dman.com>,
  Clint Adams <schizo@debian.org> writes:

> This just gets rid of the overcompensation altogether.
> Incidentally, this solves the bus error problem I was
> experiencing on 64-bit SPARC Linux.

I think gcc extension should be used if it is available.

struct heap {
  ...
}
#if defined(PAD_64_BIT) && defined(__GNUC__)
  __attribute__ ((aligned (8)))
#endif
;
-- 
Tanaka Akira

Re: PATCH: struct heap 64-bit

[Clint Adams]

> struct heap {
>   ...
> }
> #if defined(PAD_64_BIT) && defined(__GNUC__)
>   __attribute__ ((aligned (8)))
> #endif
> ;

Can you do that on a struct definition?

Re: PATCH: struct heap 64-bit

[Tanaka Akira]

In article <20000104122533.A9908@dman.com>,
  Clint Adams <schizo@debian.org> writes:

> Can you do that on a struct definition?

Yes.

% cat t.c
struct s {
  char c;
} __attribute__ ((aligned (8)));

int main()
{
  printf("%d\n", sizeof(struct s));
  return 0;
}

% gcc t.c
% ./a.out
8
% 
-- 
Tanaka Akira