Am I going crazy? Problem with pointer arithmetic and types

Go To Last Post
45 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Freaks,

I think I've discovered my first real compiler bug here, but it's in a complex section of code and only occurs under a set of circumstances, making the creation of a simple test case difficult. Before I chalk it up to GCC being wonky, I thought I'd post my issue here for others to look and and decide if I just can't code, or if it really is a GCC issue.

The problem is in the descriptor processing code of my LUFA USB library. In the domain of USB, a "descriptor" is a special binary table which contains data that is sent to the USB host, describing the device's functionality. At the start of each descriptor is a fixed header giving the "type" of descriptor (there are several different types to describe the overall device, its interfaces and endpoints) and the descriptors size.

Part of my library's magic is the processing of a concatenated list of these descriptors through a function called "USB_GetNextDescriptorComp", which uses a user-defined callback routine to examine each descriptor in a list until the desired descriptor is found. If the user rejects the current descriptor via the callback, the USB_GetNextDescriptorComp function calls an inline function called "USB_GetNextDescriptor", which simply advances the descriptor table pointer ahead by the size of the current rejected descriptor, using the descriptor's size as reported by its fixed header.

Now, this is where things get wacky. Here's the source to the "USB_GetNextDescriptor" function:

static inline void USB_GetNextDescriptor(uint16_t* const BytesRem,
			                                         void** const CurrConfigLoc)
{
	uint16_t CurrDescriptorSize = DESCRIPTOR_CAST(*CurrConfigLoc, USB_Descriptor_Header_t).Size;

	*CurrConfigLoc += CurrDescriptorSize;
	*BytesRem      -= CurrDescriptorSize;
}

Standard stuff - the routine takes in a pointer to the bytes remaining counter and a pointer to the current location in the descriptor list, itself a pointer. The size of the current descriptor is obtained by its header, and the two parameters are adjusted accordingly.

This works great, but doesn't compile under C++ due to the void pointer arithmetic. I tried to solve that by using a cast in the following manner:

static inline void USB_GetNextDescriptor(uint16_t* const BytesRem,
			                                         void** const CurrConfigLoc)
{
	uint16_t CurrDescriptorSize = DESCRIPTOR_CAST(*CurrConfigLoc, USB_Descriptor_Header_t).Size;

	*((uint8_t**)CurrConfigLoc) += CurrDescriptorSize;
	*BytesRem      -= CurrDescriptorSize;
}

However this completely breaks the code, even though semantically it is doing the same thing - advancing a pointer a number of positions ahead. The type of the data being pointed to shouldn't actually come into play here other than to decide how much to advance the pointer by each time, however this causes the LSS to drastically change. GCC treats a void* destination type size as 1 byte when compiling in C mode (a GCC extension) which is the same as the size of a uint8_t, so the code with the cast should be exactly equivalent.

I've attached the two good (void**) and bad (uint8_t**) listing files to this post. If anyone can think of a reason why the cast to make the C++ compiler happy breaks the compiled output, I'd love to hear it.

Cheers!
- Dean :twisted:

Attachment(s): 

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It does seem like a bug. If you move the dereference op inside the first parenthesis does it make any difference?

(*(uint8_t**)CurrConfigLoc) += CurrDescriptorSize; 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

While this might be a compiler bug, I despise the "hacker style" C pointer arithmetics, which puts the compiler (hence its writers) through heavy syntactic exercise and is prone to errors.

Why can't you assign the pointer in question into a temporary uint16_t, increment, and then put it back (casting it into whatever that type really is)?

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:
While this might be a compiler bug, I despise the "hacker style" C pointer arithmetics, which puts the compiler (hence its writers) through heavy syntactic exercise and is prone to errors.

Why can't you assign the pointer in question into a temporary uint16_t, increment, and then put it back (casting it into whatever that type really is)?

JW

'despise' "hacker style"
Please elaborate.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

It does seem like a bug. If you move the dereference op inside the first parenthesis does it make any difference?

Code:
(*(uint8_t**)CurrConfigLoc) += CurrDescriptorSize;

No, and neither does creating a temporary uint8_t* to do the pointer increment, or even changing the type of the function parameter to uint8_t**. The only way to fix it I've found is to change the type of both the inline USB_GetNextDescriptor() and outer USB_GetNextDescriptorComp() functions to both be uint8_t** - only changing one or the other makes the problem reappear.

I'm certain this is a bug now, since the cast works fine on most of my demos - it's only on a random few that the compiler decides to break things in the manner shown. While it's less correct than a void**, I'll have to change over the routines to all use uint8_t** pointers instead to make C++ happy without breaking C.

Quote:

While this might be a compiler bug, I despise the "hacker style" C pointer arithmetics, which puts the compiler (hence its writers) through heavy syntactic exercise and is prone to errors.

Why can't you assign the pointer in question into a temporary uint16_t, increment, and then put it back (casting it into whatever that type really is)?

Two reasons:

1) It's less correct to play pointer arithmetic though an integer type - although intptr_t helps with this
2) That won't help with my actual problem, where the C++ compiler is unhappy with me modifying the destination of a void* without first casting to a concrete destination type - it's not the artimetic that's tripping me up, it's the changing of the pointer's destination type that breaks the code

The important thing to remember here is that I'm never double-dereferencing the pointer to get to the actual data, I'm only by-reference altering the pointer's destination address.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I think a little more explanation is in order, so wek and others can see what's going on.

The host application is requesting the USB device's descriptors, which are returned as a giant chunk of memory, typed as a uint8_t[] array for convenience. This array contains many differently-sized descriptors all containing the same standard descriptor header. In order to parse through the descriptors, my "comparator" search routine walks through the list of descriptors and fires a callback routine to the user application, which can decide whether the current descriptor is useful or not and needs further processing.

I've also got a set of other descriptor related routines, which allow the user to manually walk through the descriptor array if they choose - that is why USB_GetNextDescriptor() is an inline function and not just straight code placed inside USB_GetNextDescriptorComp().

Since the descriptor list is only stored as a uint8_t[] array for convenience and since technically the user app could pass a pointer of any type to the functions so long as it points to a contiguous block of memory containing a list of descriptors I use void* for the pointer type to the descriptor list that is passed to each descriptor function.

One other issue is that the user app and descriptor functions need to keep track of how many descriptors remain in the list, so that neither try to access uninitialized memory. To do that, a pointer to a "byte remaining" counter is used in each of the functions.

Lastly, so that the skipping of each descriptor is reflected in the user application, this void* to the current descriptor in the list is passed in itself as a pointer of type void**, so that when dereferenced the internal descriptor functions can modify where in the list the pointer is currently pointing to.

A single dereference of the void** yields the user application's void* to the descriptor list, and thus any change to that is reflected in the user app. However, while regular C is happy with additions and subtractions to a void* destination (it is treated as a pointer to a destination data type of 1 byte in size), C++ is not. I need to cast the void* to uint8_t* to make the compiler happy. That's where the problem is - telling the compiler that my void* is actually a uint8_t* is breaking the code spectacularly, but only in certain projects.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dean,

Just to be clear, I wasn't aiming anything at you. I just thought wek's comment seemed like a gratuitous slur and I was wondering how he might back up what he was saying.

Smiley

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Try using a reinterpret_cast. It should allow you to do what you want.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Try using a reinterpret_cast. It should allow you to do what you want.

Not in C - I'm making my C code compatible with C++, not doing a complete conversion over to C++.

The type of cast isn't the issue here anyway, it's the apparent compiler bug.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hmm. I've "solved" the problem by an ugly hack using a second internal inline function:

static inline void USB_GetNextDescriptorST(uint16_t* const BytesRem, uint8_t** CurrConfigLoc)
{
	uint16_t CurrDescriptorSize = DESCRIPTOR_CAST(*CurrConfigLoc, USB_Descriptor_Header_t).Size;
	
	*CurrConfigLoc += CurrDescriptorSize;
	*BytesRem      -= CurrDescriptorSize;
}
						  
static inline void USB_GetNextDescriptor(uint16_t* const BytesRem, void** CurrConfigLoc)
{
	/* Horrible workaround for a bug in GCC - in some circumstances, the code generated for the strongly-typed
	 * (uint8_t**) cast to avoid void pointer arithmetic (which is not allowed in C++) causes incorrect code to
	 * be generated. Performing the cast and using a secondary inline routine show here seems to avoid the
	 * problem.
	 */
	USB_GetNextDescriptorST(BytesRem, (uint8_t**)CurrConfigLoc);
}

Casting when passing the pointer to the secondary inline function doesn't seem to trigger the bug, and all works well. I didn't want to break the strong typing of my code nor make the coders use pointers to a destination datatype that really doesn't reflect what the data is, so this is the next best solution.

I'd submit a bug report for this, but they require a minimal test case, and this problem only occurs in some of my (large) demos -- no way I'll be able to tickle it using a contrived code sample.

Thanks for everyone's input on this. Just goes to show, if enough people cry "compiler bug", one of us has to be right eventually ;).

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

abcminiuser wrote:

No, and neither does creating a temporary uint8_t* to do the pointer increment, or even changing the type of the function parameter to uint8_t**. The only way to fix it I've found is to change the type of both the inline USB_GetNextDescriptor() and outer USB_GetNextDescriptorComp() functions to both be uint8_t** - only changing one or the other makes the problem reappear.

- Dean :twisted:

What exactly is broken? I can see that the compiled code samples are different, as expected*, but without analysing the whole thing I can't see what is broken.

*Expected, if you have any optimisation turned on, because the compiler is expected to optimise register allocation for pointers depending on their type. That is, all pointers to uint8_t are one class of variables, and all pointers to void are another class of variable, so it will naturally generate different register assignments when you change the pointer type, even if the pointer still points to objects which are the same size.

Having said that, I've had a few problems casting pointers for pointer arithmetic in gcc. What I do sometimes is wrap the cast and the called function into a dummy function - the wrapper function does whatever pointer casting is required, and does not do anything else, the inner function does pointer arithmetic on pointers without casting.

You shouldn't have to use a wrapper function to cast void** to uint8_t**, but it worked for me. Since I'm not absolutely sure that doing pointer arithmetic on a cast pointer is actually a defined operation in modern standard C, I've considered myself lucky that it does work, even when all static inlined.

Since I can make the inner function static inline, the outside wrapper doesn't add overhead (although it does optimise differently). It would get messy if you were trying to cast different kinds of pointers to uint8_t, but that would be messy however you did it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dean,

I've had a closer look at those two asm listings you've posted in the OP, and I fail to see a problem. IMHO in both cases *CurrConfigLoc += CurrDescriptorSize; works the same way. If you want I could try to concoct a detailed analysis.

So, if there is some problem, it's IMHO located elsewhere (thus may be not a compiler bug, with due respect).

What are the symptoms of "not working" and how are they different from "working"?

JW

PS. Joe: "I despise" and "hacker style" as in "I hate C". I would really like to talk about this with you and others over a glass of beer or other beverage of choice.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

What exactly is broken? I can see that the compiled code samples are different, as expected*, but without analysing the whole thing I can't see what is broken.

The two samples (with and without casting) should produce code that behaves identically, since I'm just playing with semantics (implicit one-byte destination type vs. explicit one byte destination type). On most of my demos this works exactly as advertised, with the assembly with and without the casting being exactly the same. With the troublesome demo, adding the cast - but not changing any other code - and comparing the before/after listings shows differences.

Running the "buggy" code results in a dead demo, with it indicating a descriptor parsing failure. From some analysis I did a while ago it looked like it was double-incrementing the pointer with the cast, but I can't be certain.

In any case, the cast should not affect the behavior of the code in any way whatsoever, but on certain demos it does.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Wek,

Essentially, the compiler is compiling code like the two fragments differently (example):

uint16_t** Test;
*Test++;
int16_t** Test;
*Test++;

Even though both should product identical code (since they are both pointing to datatypes of the same size, and the actual data is never referenced, only the pointer to it).

I've compiled both with and without cast examples with optimization turned off, and extracted only the code differences. This turned out to be just the code around the cast, with all other differences being address changes due to the different code path lengths.

My assembly's a little rusty, care to look over the two short snippets and tell me if they are equivelent?

WORKING:

*(CurrConfigLoc) += CurrDescriptorSize;
ldd	r30, Y+5	; 0x05
ldd	r31, Y+6	; 0x06
ld	r18, Z
ldd	r19, Z+1	; 0x01
ldd	r24, Y+1	; 0x01
ldd	r25, Y+2	; 0x02
add	r24, r18
adc	r25, r19
ldd	r30, Y+5	; 0x05
ldd	r31, Y+6	; 0x06
std	Z+1, r25	; 0x01
st	Z, r24

NOT WORKING:

*((uint8_t**)CurrConfigLoc) += CurrDescriptorSize;
ldd	r26, Y+5	; 0x05
ldd	r27, Y+6	; 0x06
ldd	r30, Y+5	; 0x05
ldd	r31, Y+6	; 0x06
ld	r18, Z
ldd	r19, Z+1	; 0x01
ldd	r24, Y+1	; 0x01
ldd	r25, Y+2	; 0x02
add	r24, r18
adc	r25, r19
adiw	r26, 0x01	; 1
st	X, r25
st	-X, r24

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, these snippets do the same (with different outcome in the registers, which might cause different behaviour of the subsequent code, of course).

---

But, having a look at those 2 snippets you posted originally, the problem is elsewhere: in how the parameter is handled in the ComparatorRoutine(*CurrConfigLoc) call (which as AFTER the increment). In the non-working example, the parameter (i.e. *CurrConfigLoc) is the original content of *CurrConfigLoc, from before the increment.

Now I am not versed enough in C semantics to be able to tell if this does constitute an error - it's not impossible that you've effectively "hidden" the modified variable behind the cast.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks Wek! This is why we need you assembly gurus around to supplement us code monkeys :).

Firing the callback using the previous pointer value would result in a failure like I'm seeing, as the routine would not properly advance like it should once the first match has been found.

It's not an issue with the inlining of the routine, as copy-pasting the code into the USB_GetDescriptorComp() function has the same issue. With no optimization there should be no reordering of statements, so the code should work as intended with the callback getting the updated location.

I don't know why the cast would break the code as it does since it's just modifying the destination data type. I still think this might be a bug since the problem disappears with the cast used as part of the function call (rather than inside it) however for now I'll just accept it as a quirk in my code and code around it.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Could you please post the exact source, asm .lst and command line (and avr-gcc version) for any of the "non-working" code, for future investigation?

Thanks.

Jan

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This line seems to be the problem in the code that is not working:

adiw   r26, 0x01   ; 1 

I don't see any reason for it to be incrementing the destination address?

edit: scratch that, I didn't notice they were storing them differently which should have the same result.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It's all part of my LUFA project which is open source, and thus easy to get to. Just click the "Download Source" button on the GitHub page (http://github.com/abcminiuser/lu...) to get the entire LUFA source tree.

The demo in question is located in Demos/Host/LowLevel/MouseHostWithParser. Install the latest WinAVR and run "make all" to build, including the .lss file.

The casting operation is performed in LUFA/Drivers/USB/HighLevel/ConfigDescriptor.h.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Okay, but this is the working variant, isn't it? For us, pervert types, this is uninteresting ;-)

How to make it non-working?

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Attached is the old (working) ConfigDescriptor.h file, before I used the second inline function to mask the problem. Drop it into the LUFA/Drivers/USB/HighLevel/ directory.

To break it, replace the line:

*CurrConfigLoc += CurrDescriptorSize;

In the GetNextDescriptor() function with:

*((uint8_t**)CurrConfigLoc) += CurrDescriptorSize;

And recompile. Also note that the same change made with most of the other demos in the Demos/Host/* directories don't suffer the same problems despite using very similar code.

- Dean :twisted:

Attachment(s): 

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

abcminiuser wrote:
I don't know why the cast would break the code as it does since it's just modifying the destination data type.

Since the destination variable has two different types, I wouldn't assume that they are the same variable at all.

That is, I wouldn't assume that incrementing a uint_8t* or uint8_t** pointer would have any effect on the void* or void** version of the same pointer.

My naive reading of the C standard was that when you cast a pointer to a different type, the result was undefined behaviour.

Apart from the heap allocation routines, which are defined to return a different kind of void pointer.

There was also some kind of odd exception for "char" pointers.

Note, "unsigned char" is not "char" even if char is unsigned.

Note, although I say "undefined behaviour", the specific intent was to allow the compiler to re-order and discard code.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

which are defined to return a different kind of void pointer.

Eh? "void *" is "void *" is "void *" isn't it?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

melbourne wrote:
abcminiuser wrote:
I don't know why the cast would break the code as it does since it's just modifying the destination data type.

Since the destination variable has two different types, I wouldn't assume that they are the same variable at all.
I am reading the standard now for two days to find evidence for this, none so far.

Contrary, 6.2.5 #27 says, "A pointer to void shall have the same representation and alignment requirements as a pointer to a character type." (uint8_t IS a character type, see 6.2.5 #15). While this still does not imply that their behaviour must be the same in all regards, unless somebody finds better explanation of how a cast-dereferenced-pointer shall behave, I'm now quite inclined to call the observed behaviour a bug.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Quote:

which are defined to return a different kind of void pointer.

Eh? "void *" is "void *" is "void *" isn't it?

"The effective type of an object for an access to its stored value is the declared type of the object, if any"

The void pointers we are discussing here have an effective type: the void pointers supplied by the memory allocation routines do not.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

melbourne wrote:
clawson wrote:
Quote:

which are defined to return a different kind of void pointer.

Eh? "void *" is "void *" is "void *" isn't it?

"The effective type of an object for an access to its stored value is the declared type of the object, if any"

The void pointers we are discussing here have an effective type: the void pointers supplied by the memory allocation routines do not.


Which verse of the standard do you quote?

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:

Contrary, 6.2.5 #27 says, "A pointer to void shall have the same representation and alignment requirements as a pointer to a character type." (uint8_t IS a character type, see 6.2.5 #15). While this still does not imply that their behaviour must be the same in all regards, unless somebody finds better explanation of how a cast-dereferenced-pointer shall behave, I'm now quite inclined to call the observed behaviour a bug.

Well, I've been trying to avoid the question about if it is a bug or not.

But just to be clear, I thought that void** and uint8_t** were compatible types, and that char** and uint8_t** were compatible types, and I didn't think that you should have to use a dummy function to do the cast between compatible types.

However, it seems likely that a pointer does wind up pointing to something that is not a character type somewhere else in the program, so I'm open on the question as to if the void** has a non-compatible effective type.

My naive reading of the standard is, that if the void** has a non-compatible effective type, the behaviour of the program is undefined. (6.5.6, 6.5.7)

(I won't ask you to try to explain those sections to me, but I won't be hurt if you disagree)

In any case, bug or not, I confirm that it does seem to have been my experience, so when a pointer has two different types, I wouldn't assume that they are the same variable at all.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Wait, so the standard says that any void* casting tomfoolery is undefined, except for the library memory functions? That doesn't make sense -- otherwise, casting a variable to a different type would be invalid, breaking billions of lines of code in things like network stacks.

Incidentally, the actual data is indeed read in as a uint8_t* array - I just make the function take in a voud** reference to that array so that any storage could be used. This is because the descriptors aren't really any one particular type, but a series of differently-sized tables collated together.

Essentially, this is what is happening on the breaking code:

uint8_t Descriptors[512];
Fetch_Descriptors(Descriptors);

void* CurrentDescriptor = Descriptors;

USB_GetNextDescriptor(&CurrentDescriptor);

So while the intermediate pointer is a void, both the original data type and the cast-to type in the USB_GetNextDescriptor() function are identical.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

otherwise, casting a variable to a different type would be invalid, breaking billions of lines of code in things like network stacks.

Yup, you just have to look at a complex software stack like the Linux kernel to see examples of pointers being passed about as void * and then cast to their interpretation type on delivery. The C standard simply cannot say that this is invalid. Hence my previous comment about void * being void * being void *. It's the universal "get out of jail free" card that lets you easily pass pointers without having to worry about type interpretation. As far as I know all void * are created equal.

typedef enum {
 TYPE_CHAR,
 TYPE_INT
} var_t;

void function(void * ptr, var_t type);

char c;
int n;

function((void *)&c, TYPE_CHAR);
function((void *)&n, TYPE_INT);

...

void function(void * ptr, var_t type) {
 switch(type) {
  case TYPE_CHAR:
    (*((char *)ptr))++;
    break;
  case TYPE_INT:
    (*((int *)ptr))++;
    break;
 }
}

In this the same ptr is used to pass both a "char *" and an "int *" with it being made anonymous (cast to void *) at the point of the function invocation then cast back to the correct interpretation at the point of usage.

Is the C standard telling us this isn't a valid thing to do? If it isn't what exactly makes malloc() "special" in that it's return value may be cast from void * to any intepretation you choose. How does the C compiler know that malloc() is any different to any other function that might return a void * pointer?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'd have tried

*CurrConfigLoc = CurrDescriptorSize + (char*)(*CurrConfigLoc);

as a workaround.
The single cast is guaranteed to be a no op.

"SCSI is NOT magic. There are *fundamental technical
reasons* why it is necessary to sacrifice a young
goat to your SCSI chain now and then." -- John Woods

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I'd have tried
Code:
*CurrConfigLoc = CurrDescriptorSize + (char*)(*CurrConfigLoc);
as a workaround.
The single cast is guaranteed to be a no op.

Ooh, that's clever - I should have seen the obvious solution. Testing it shows that it seems to work, and I'd wager it does -- no doubt casting a void* once is a more tested operation in GCC than L-Value casts of pointers-to-pointers-to-void.

I still think this is a bug, but I'm happy with either the multiple-function workaround, or Michael's solution.

I'll add your name to the changelog, Michael.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Yup, you just have to look at a complex software stack like the Linux kernel to see examples of pointers being passed about as void * and then cast to their interpretation type on delivery.

This is invalid argumentation, Cliff. Linux kernel is not designed to be portable nor is it a benchmark of the standard. I'd even risk to say, contrary.

clawson wrote:
The C standard simply cannot say that this is invalid.

The standard says that generally, you cannot cast around pointers to different objects and expect that they will be universally valid. However, there are exceptions, void* and [character type]* - they share alignment and representation; and void* can be converted to any other pointer type freely (i.e. it won't result in error or warning; however, the resulting pointer may not be a valid pointer for the target type if misaligned for example). An easy example, in some architectures, ints must be aligned to word/dword boundaries. If you take & of a byte at an odd address into char*, convert to void* and to int*, int* may be an invalid address and dereferencing it on that architecture may result in exception.

Strictly speaking, in an "evil" but compliant compiler, pointers to int may be represented differently than pointers to char; say, int are words and int* are word addresses (i.e. half of byte addresses). If you convert int* to void* to int*, nothing happens, as the standard requires this behaviour; my malicious compiler would simply leave the value unchanged. The same for char*. However, if you convert int* to void* to char*, my malicious compiler, not changing the value, would result with char* containing in fact half the byte address you would expect. The standard also has a special requirement that if you convert int* to char* (i.e. directly, not through void*), then the result must point to the first byte of the int which my malicious compiler would handle through an exceptional mechanism (multiplying the int* value by 2), but the conversion from char* to int* may be implementation defined (and hence have unexpected outcome, in my malicious compiler I would simply copy the byte address to int* pointing completely elsewhere).

This is some of the reasons why I don't like (a.k.a. despise) "hacker's pointer magic", and C as such.

Most of this above is supported by 6.3.2.3 and 6.2.6.2 #27.

Quote:
How does the C compiler know that malloc() is any different to any other function that might return a void * pointer?

The standard does put requirements on the standard library functions; in fact a rather excessive part of the standard, chapter 7 (and some of the appendices), is devoted to them. The void* pointer returned by malloc() and kin must fulfill the following requirement:
7.20.3 wrote:
The pointer returned if the allocation
succeeds is suitably aligned so that it may be assigned to a pointer to any type of object and then used to access such an object or an array of such objects in the space allocated (until the space is explicitly deallocated).

So, it is not an "any void*", it must be prepared by the library function in a careful way so that it sucesfully undergoes any future conversion (I admit that in my malicious compiler it would be non-trivial to conform to this requirement malloc() ;-) But it's not required for freestanding implementations, so that's where my malicious compiler would go :-P )

All this said, I am now quite convinced that the original problem presented by Dean IS a compiler bug (given the special treatment the standard provides for void* and [character type]*).

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you to everyone - especially Wek - for the continued insight you've all given me. This is/has been a very interesting (if frustrating) discussion and I've learned a lot about C that I wasn't aware of.

Quote:

If you take & of a byte at an odd address into char*, convert to void* and to int*, int* may be an invalid address and dereferencing it on that architecture may result in exception.

Ah, I'm aware of the alignment issues that would cause exceptions on e.g. ARM processors. However, I thought that given a "sane" cast, correct behaviour would occur. Specifically, so long as the types are aligned properly due to the programmer being careful, I can't see any reason for things to not work.

Quote:

This is invalid argumentation, Cliff. Linux kernel is not designed to be portable nor is it a benchmark of the standard. I'd even risk to say, contrary.

I'd take issue with this. Linux has been ported to a billion different architectures, so the kernel can be compiled for a PC, toaster, fighter plane or anything inbetween. If that's not portable, what is?

Quote:

This is some of the reasons why I don't like (a.k.a. despise) "hacker's pointer magic", and C as such.

You are going to hate my LUFA codebase. A lot.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Strictly speaking, in an "evil" but compliant compiler, pointers to int may be represented differently than pointers to char; say, int are words and int* are word addresses (i.e. half of byte addresses).

Then this theoretical discussion is moot isn't it? The subject of this forum is avr-gcc and unless someone knows of evidence to the contrary then all it's pointers are 16 bits wide and there's no question of alignment issues as they are all byte aligned.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

abcminiuser wrote:
I wrote:

This is invalid argumentation, Cliff. Linux kernel is not designed to be portable nor is it a benchmark of the standard. I'd even risk to say, contrary.

I'd take issue with this. Linux has been ported to a billion different architectures, so the kernel can be compiled for a PC, toaster, fighter plane or anything inbetween. If that's not portable, what is?


It's not the hardware portability in question here, but software. I know of no case of linux kernel being compiled with other than gcc. It means, whatever standard violations might in the kernel be, gcc will always cover them up.

Dean wrote:
I wrote:
This is some of the reasons why I don't like (a.k.a. despise) "hacker's pointer magic", and C as such.

You are going to hate my LUFA codebase. A lot.


Well, you know, together with C, I don't like AVRs and use them only because I get paid for it. So, unless somebody pays me to work with LUFA, this is not really a threat... But thanks for the warning... :-P

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Quote:

Strictly speaking, in an "evil" but compliant compiler, pointers to int may be represented differently than pointers to char; say, int are words and int* are word addresses (i.e. half of byte addresses).

Then this theoretical discussion is moot isn't it? The subject of this forum is avr-gcc and unless someone knows of evidence to the contrary then all it's pointers are 16 bits wide and there's no question of alignment issues as they are all byte aligned.

Well, for example, function pointers would benefit from being word addresses. What would prevent an agile avr-gcc developer to reimplement them as such, say since the next issue of WinAVR? One should be aware of what is cast to stone by the standard, and what is just incidentally "it is so".

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Okay, I think this is the reference that we were looking for:

Quote:
From ISO/ANSI 9899-1990 6.3.4, cast operators, footnote 44: "A cast does not yield an lvalue. [...]".

It seems some compilers allow you to cast an lvalue. I did see references that this "feature was depreciated by gcc". I guess the bug would be if it didn't give some sort of warning or error?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

CirMicro wrote:
Okay, I think this is the reference that we were looking for:
Quote:
From ISO/ANSI 9899-1990 6.3.4, cast operators, footnote 44: "A cast does not yield an lvalue. [...]".

It seems some compilers allow you to cast an lvalue. I did see references that this "feature was depreciated by gcc". I guess the bug would be if it didn't give some sort of warning or error?


This is not the case here.

The line in question is:

Dean wrote:

*((uint8_t**)CurrConfigLoc) += CurrDescriptorSize;


The expression on the left is a dereferenced cast. Casts don't constitute an lvalue, but they can be dereferenced, and dereferenced pointers do yield an lvalue (6.5.3.2).

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

abcminiuser wrote:
I'd take issue with this. Linux has been ported to a billion different architectures, so the kernel can be compiled for a PC, toaster, fighter plane or anything inbetween. If that's not portable, what is?
Nyet
Only 956,237,923.7

"SCSI is NOT magic. There are *fundamental technical
reasons* why it is necessary to sacrifice a young
goat to your SCSI chain now and then." -- John Woods

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

These are not relevant for our case.

In both these examples, an attempt is to increment the cast pointer, rather than the dereferenced object (which is Dean's case).

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:
These are not relevant for our case.

In both these examples, an attempt is to increment the cast pointer, rather than the dereferenced object (which is Dean's case).

JW

I don't know what you are looking at but the example I see is incrementing the location being dereferenced.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

CirMicro wrote:
I don't know what you are looking at

*((some_type *) ptr)++;
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:
CirMicro wrote:
I don't know what you are looking at

*((some_type *) ptr)++;

oops, I added my own parenthesis :roll:

(*((some_type *) ptr))++;