Thread: Aligning memory

Managed to seemingly fix that issue by switching out the 1st a's for 1's, now I have what I believe the final issue at hand it it just confounds me:

Code:

			for ( a = 1,
				spot = page->init;
				spot < page->last;
				++a, spot = &(spot[1])
			)
			{
				if ( _data->data == spot->data.data ) break;
			}

Somehow spot manages to actually go BELOW page->init value (found that out with an extra condition there), the highest I've seen the value itself at is a little under 0x7fffffffffff, even if I consider overflow it should not do that because of the spot < page->last statement, any ideas peops?

Well I seem to have fixed the segfaults at least, I changed all the "!= page->last" to "< page->last" and change that memove line to

Code:

if ( (spot = &(page->init[1])) < page->last )
	(void)memmove( spot, page->init, (uintptr_t)(page->last - spot) );

which admittedly is easier to read anyways.
Now I'm just getting an invalid value somewhere, or at least incorrectly detected as invalid value anyways, a leak is better than a segfault but still gonna try and track down the cause

I give up for now, gonna take a break and play some games or read or something, if anyone wanna take a crack at it then here it is, it runs through to then end just fine, it just fails to find memory because the locations are somehow being overridden with 0

Found out why, turns out memmove was not doing it's job and moving data up, after some investigating I found that the pointer math was not doing as expected, apparently I had to cast them individually first, really annoying that is, they're still stored as integers at the end of the day, basic integer math should work, whoever though it was a decent idea to say otherwise needs good wack round the head.

Originally Posted by awsdert

Code:

            for ( a = 1,
                spot = page->init;
                spot < page->last;
                ++a, spot = &(spot[1])
            )
            {
                if ( _data->data == spot->data.data ) break;
            }

Why the obscure code? spot = &(spot[1]) is equivalent to ++spot, but the latter is (IMO) easier to read.

Originally Posted by laserlight

I'm puzzled: aren't you always passing pointers to memmove? The possible issue I see is that in two places you don't seem to be correctly computing the number of bytes to move (i.e., you didn't multiply by sizeof(std_spot)), but that is as you say, basic integer math that you simply failed to do.

I had the right math there, it's just clang didn't put the math there, instead it boiled it down to a comparison or something because the values that where at least 0x10 apart gave 1 on the instance I caught it (after passing it into want before hand), only after I cast them both to uintptr_t beforehand did I get the right output

Originally Posted by christop

Why the obscure code? spot = &(spot[1]) is equivalent to ++spot, but the latter is (IMO) easier to read.

Be cause I can only be sure the compiler will provide the correct assembly like that, ++spot is misleading and can instead cause the address to increment by only 1 byte whereas the spot structure is definitely more than one byte

Originally Posted by awsdert

Be cause I can only be sure the compiler will provide the correct assembly like that, ++spot is misleading and can instead cause the address to increment by only 1 byte whereas the spot structure is definitely more than one byte

You can rest assured that incrementing a pointer adds the size of the pointed-to object to the pointer. It's guaranteed by the standards, and it's common practice to use something like ++p to increment p to the next object in an array, regardless of the type of *p.

Consider the following equivalent statements:

Code:

spot = &(spot[1]);
spot = &(1[spot]); // yes, the array index operator is commutative!
spot = spot + 1;
spot = 1 + spot;
++spot;

Originally Posted by laserlight

...but if you want to be pedantic (as I see you cast the return value to void when unused)

Only do that on the memory being returned since that is same behaviour as malloc, I just need the execution permission in addition to read/write (wait for next thread to get clearer idea about that purpose(after this bug is resolved))

Originally Posted by laserlight

you can always cast: (size_t)(spot - init) * sizeof(*spot).

tried that with uintptr_t at 1st, as I said before the result was not what I expected from integer math so I had to do them individually, reason I don't use size_t is because I didn't want information being lost before the math occurs, the result will always fit inside size_t so I'm not worried about that but on other systems it is possible it would loose info so forcing the subtraction to the right size is better option

Originally Posted by laserlight

If we didn't have this, then to do even a simple subtraction of two pointers to different elements of an array would force you to divide by sizeof(type), i.e., you won't get the benefit of the pointer abstraction. In your case, by using memmove for bytes, you are deliberately forgoing the pointer abstraction to treat the memory as a contiguous sequence of bytes rather than a contiguous sequence of objects of some type in memory, hence you have to multiply by sizeof(type) after doing pointer subtraction, or cast in order to use integer subtraction of addresses.

But that's what you keep indexs for, they're supposed to used for those calculations then you just reference the node from the root, because it would be treated as an array at that point the compiler would have to apply the sizeof in the result without it being explicitly written with bytes. Well either way its results that matter to me not semantics.

Have you tried using valgrind and static analysis?

Code:

valgrind --tool=memcheck --track-origins=yes ./alloc

==9754== Conditional jump or move depends on uninitialised value(s)
==9754==    at 0x4D00397: mmap (in /usr/lib64/libc-2.29.so)
==9754==    by 0x4014FA: alloc_std_page (alloc.c:113)
==9754==    by 0x40180E: alloc_foo_page (alloc.c:177)
==9754==    by 0x401FD9: main (alloc.c:322)
==9754==  Uninitialised value was created by a heap allocation
==9754==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==9754==    by 0x4012F6: alloc_std_data (alloc.c:51)
==9754==    by 0x4013EC: alloc_std_list (alloc.c:78)
==9754==    by 0x40177A: alloc_foo_page (alloc.c:164)
==9754==    by 0x401FD9: main (alloc.c:322)
==9754==
==9754== Syscall param mmap(start) contains uninitialised byte(s)
==9754==    at 0x4D003A6: mmap (in /usr/lib64/libc-2.29.so)
==9754==    by 0x4014FA: alloc_std_page (alloc.c:113)
==9754==    by 0x40180E: alloc_foo_page (alloc.c:177)
==9754==    by 0x401FD9: main (alloc.c:322)
==9754==  Uninitialised value was created by a heap allocation
==9754==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==9754==    by 0x4012F6: alloc_std_data (alloc.c:51)
==9754==    by 0x4013EC: alloc_std_list (alloc.c:78)
==9754==    by 0x40177A: alloc_foo_page (alloc.c:164)
==9754==    by 0x401FD9: main (alloc.c:322)
==9754==

Lots of messages from clang, but these are the ones that stood out, to me, as being potentially problematic

Code:

alloc.c:223:14: Semantic Issue: warning: variable 'spot' may be uninitialized when used here
alloc.c:232:28: Semantic Issue: warning: variable 'page' may be uninitialized when used here
alloc.c:272:20: Semantic Issue: warning: cast from 'uchar *' (aka 'unsigned char *') to 'std_spot *' (aka 'struct spot *') increases required alignment from 1 to 8
alloc.c:287:25: Semantic Issue: warning: cast from 'uchar *' (aka 'unsigned char *') to 'std_spot *' (aka 'struct spot *') increases required alignment from 1 to 8