C word

[SiliconWizard]

Yup, this works on Gcc-v7-9 and GreenHills CC

Well, yes. It's correct. My bad again for omitting one identifier in my first post.

Note that struct declarations with a struct identifier are implicit forward declarations, so you can do the following if you so wish:

Code: [Select]

typedef struct Something
{
int n;
struct Something *pNext;

} Something_t;

An additional forward declaration of "struct Something;" is unnecessary.

[SiliconWizard]

Code: [Select]

typedef struct object_st object_t;
typedef object_t* p_object_t;
(...)


Just one remark on this piece of code. Several rules in safety-critical guidelines (including MISRA-C, and others, and myself  ), recommend NOT to declare pointer types, which hide the fact that a given object is a pointer. So this: "typedef object_t* p_object_t;" would be a no-no. The fact you cared to use a "p_" prefix mitigates this somewhat, but still...

If you work under rules which allow this, or even ENFORCE this, well. All I can say is we are a bunch of people who would disagree.

[legacy]

Type pointer is the perfect way to instrument checkers and debuggers, so we largely use it; AI-driven checkers are weak but strong enough to strickly check if what follows the "p_" prefix matches with the referenced item.

And they are largely better than human beings.

That's why we conider largely better than pushing a lot of "*" within the code; this especially if you consider that HOOD-ICEs might have problems at understanding it, while they have no problem with a type-pointer. Long story on the why, but basically because even Stood has less problems this way.

We follow a mash-up between MISRA and DO178B not just MISRA, hence everything can automate and simplify the process and improve the management of a product during its life-cycle is preferred.

[magic]

Accessing packed structs on the stack (local variables for instance...) royally sucks though, even when your accesses are perfectly aligned for each of the member you access (but obviously not necessarily to 16 bytes).

You know, some people actually pay money to get royally sucked off so further elaboration may be in order

Disassemble that crap (objdump -d xxx.o) and see what's generated. ARM assembly can't be rocket science, I suppose. There has to be some way of loading/storing 8 or 16 bits. OTOH, if you load/store 32bits unaligned, some ARMs supposedly may end up barrel shifting the word. Maybe wrong target core is specified?

[SiliconWizard]

@legacy: Oh, I was sure you had a good reason. I just respectfully disagree.
(I don't really see your point of having to push "*" or anything else; it's just part of the language. Are you complaining about having to push a lot of for's and while's? ...  )

Anyway, people can't agree on everything and that's alright.

Note: it doesn't come directly from MISRA-C, but from CERT-C.
Beyond the stylistic issue (hiding pointers), here is why it could go wrong: when you use qualifiers.

https://wiki.sei.cmu.edu/confluence/display/c/DCL05-C.+Use+typedefs+of+non-pointer+types+only

[legacy]

Beyond the stylistic issue (hiding pointers), here is why it could go wrong: when you use qualifiers.

Using type definitions (typedef) can often improve code readability. However, type definitions to pointer types can make it more difficult to write const-correct code because the const qualifier will be applied to the pointer type, not to the underlying declared type.

[legacy]

double pointers are banned and ... what have I now to fix now?

Code: [Select]

    int var = 789;
    int *ptr2;
    int **ptr1;  /* <---------- this is banned */
    ptr2 = &var;
    ptr1 = &ptr2;


Code: [Select]

    p_object_t* env; /* <------------- this is banned too */

A source FULL of double pointers ... 


note to me:
remember to ban-by-design any double pointers from any draft of the Arise-v2 HL, since tracking them on the HOOD-ICE is like putting the head into the microwave.

[legacy]

Code: [Select]

int fwritemem
(
    void *cookie,
    char *buf,
    int size
)
{
    /* funopen write */
    char **position  = (char * *) cookie;
    char *mem        = *(char * *) position;
    int  num_written = 0;
    for (; num_written < size && *buf != '\0'; num_written++)
    {
        *mem++ = *buf++;
    }
    *position = mem;
    return num_written;
}

the above comes from a bsd tool.

    char **position  = (char * *) cookie;

 

[SiliconWizard]

Disassemble that crap (objdump -d xxx.o) and see what's generated. ARM assembly can't be rocket science, I suppose. There has to be some way of loading/storing 8 or 16 bits. OTOH, if you load/store 32bits unaligned, some ARMs supposedly may end up barrel shifting the word. Maybe wrong target core is specified?

Oh, I did that. I also generate assembly from GCC (with -S option), as it produces more readable code with symbols. I use objdump on the final linked binary, as it allows to pinpoint which instruction is at which address. All I got from the exception handler was a few registers, FAR giving you the faulty address for mem access, and ELR the "link" address (which points you the address at which the exception occured).

I finally figured out that the culprit was this instruction: "ldr d0, [sp, 52]". Guess what?. It points to an address that is 4-byte aligned (which is fine, since I was accessing a 4-byte aligned, 32-bit field of a struct): sp (16-byte aligned) + 52 => a 4-byte aligned address.

It was not a stack alignment issue per se, just happened to be on the stack (so at first that misled me, because a pure alignment issue from my code didn't seem to be possible otherwise.)

Problem is that GCC emits this ldr d0, ... instruction. Which is a 64-bit transfer. Why so? Well. because it wants to optimize accesses, and I was actually accessing two successive 32-bit struct members, so it decided to access both in one shot. Problem is that this creates a non-aligned access. Is that great or what?

If you dig a little deeper, you figure out that GCC allows itself to do this, because AARCH64 CPUs allow non-aligned accesses. So GCC optimizes away, and goes on. Now why am I getting an exception if this is allowed by the CPU? Because, it is, but not always. You have to enable the MMU, from what I gathered, to allow unaligned accesses. Otherwise, you get an exception. And at the stage my code runs, the MMU is not yet enabled...

But you know what's worst? After all, it's not trivial indeed, but you could still say it's just a case of RTFM. Which would be fair enough. Except that... I enabled the "-mstrict-align" option, which is supposed to PREVENT GCC from assuming it can generate unaligned accesses. But guess what, it doesn't do squat here, and it still generates this 64-bit access. I think this looks like a nice bug.

Hope this can be helpful for others. If you ever work on ARM CPUs in 64-bit mode, and at a very low level. This one took me a while, all the more that I'm not ultra familiar with AARCH64 assembly, so this 64-bit access was not immediately obvious to spot without reading the IS manual...

The only fix for now is to cut down the opt. level to -O1 max, and the code runs with no exception. No fucked-up unaligned access is generated.
The above instruction is duely replaced with "ldr w0, [sp, 52]" and later "ldr w0, [sp, 56]", which is what I expected.
For the record, d0 is a 64-bit register, w0 a 32-bit one.

Starting with -O2, it's fucked up and the  "-mstrict-align" option appears to be ignored.

First time I run into a GCC bug in a long time here. It had served me very well until now. Maybe the official ARM version (AARCH64-ELF 8.3) is borked? I'll have to try with a custom-built one off a newer GCC version (9.1/9.2)...

Thanks for your attention. 

[SiliconWizard]

Code: [Select]

int fwritemem
(
    void *cookie,
    char *buf,
    int size
)
{
    /* funopen write */
    char **position  = (char * *) cookie;
    char *mem        = *(char * *) position;
    int  num_written = 0;
    for (; num_written < size && *buf != '\0'; num_written++)
    {
        *mem++ = *buf++;
    }
    *position = mem;
    return num_written;
}

the above comes from a bsd tool.

    char **position  = (char * *) cookie;

 

Well, the above code looks a bit hackish, but it's not THAT bad either. Although I would never write something like this.
If you have a rule of no double pointers, sure this is a problem. This sounds pretty strict; IIRC MISRA-C, for instance, forbids MORE than 2 levels. You're forbidding more than 1...

A double pointer here is just a table of pointers. Can be useful. Sure it would look nice to define intermediate types so there is only one level at a time.

The void * parameter is a bit weird here; since the function actually expects to get a table of pointers to char, the parameter should show this clearly. It's not like in just one-level cases in which void * can be useful (as long as there is no assumption about alignment in the function, meaning that only 8-bit accesses are allowed...), such as with the usual memcpy(), memset(), etc.

There are (admittedly confusing) but useless casts here:
char **position  = (char * *) cookie;
char *mem        = *(char * *) position;

Both are useless. You don't need to cast a void * as it's compatible with any pointer type in C. This includes a double pointer, which is still a pointer. So for the first line.
For the second line, they cast position to char **, position being already declared as a char **. It's just ridiculous.

char **position = cookie;
char *mem = *position;

is all that''s needed, and looks less clunky.
Of course, they could also have done some basic checks on the pointers being NULL for instance...

[magic]

Seems like a compiler bug then.
Mark the struct member volatile and see what happens

[SiliconWizard]

Following up on the AARCH64 GCC bug.

After considering filing a bug report, I searched first and found it: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71727
Looks like it's been there for 3 years at least. From what someone says, it was supposed to get fixed in GCC 7, but the case was never close, and I can confirm it's definitely NOT fixed. The bug report says at -O3, but I can confirm it's present at -O2 as well. Weird as this bug has remained rampant, and no clear situation. You can see someone asked to close the case end of 2018, but IT IS NOT fixed, and the status has not been updated since 2016. That doesn't really bode well...

We must now kindly request an account to file for bugs, so I did. Hoping I'll get one.

Others have recently run into the issue (but apparently not the bug, although they don't say if the option -mstrict-align, in their case, actually really fixed the problem, nor at which optimization level they build their tools...)

https://patchwork.kernel.org/patch/11133703/

[Nominal Animal]

Aaand now you know why I no longer submit patch fixes to GCC (unless I could discuss it with someone with commit access, I guess). 

[SiliconWizard]

(...)
the above comes from a bsd tool.

    char **position  = (char * *) cookie;

 

If you need to rewrite it and keep its interface intact (well, almost), this is how I would basically do it (using your pointer types style):

Code: [Select]

typedef void * p_GenericBuffer_t;
typedef char * p_szBuffer_t;
typedef const char * p_cszBuffer_t;

size_t fwritemem(p_GenericBuffer_t *pCookie, p_cszBuffer_t pBuffer, size_t nSize)
{
    char *pMem;
    size_t nBytesWritten;
   
    if ((pCookie == NULL) || (*pCookie == NULL) || (pBuffer == NULL))
    return 0;
   
    pMem = *pCookie;
   
    for (nBytesWritten = 0; (nBytesWritten < nSize) && (*pBuffer != '\0'); nBytesWritten++)
    {
        *pMem++ = *pBuffer++;
    }
   
    *pCookie = pMem;
   
    return nBytesWritten;
}

Note that the typedef'ed pointers need the qualifiers (ie.: const, volatile...) to be part of the typedef declaration to work as expected. This way, no problem (you can check here that the compiler won't let you write to the buffer pointed to by pBuffer). You just need one type by qualifier/set of qualifiers. And I'm even beginning to like your notation, or at least not dislike it as much as I used to.

Note that thanks to the pCookie declaration you can't pass anything else to pCookie than a pointer to a pointer (to anything...), instead of just a pointer to anything, which was pretty bad.

[SiliconWizard]

Aaand now you know why I no longer submit patch fixes to GCC (unless I could discuss it with someone with commit access, I guess). 

Well, that's good to know. I'm a bit disappointed. Does LLVM support AARCH64 targets? If so... I might consider it.
(For the record, I even built a cross GCC 9.2.0, and the bug is, unsurprisingly, still there...)

And maybe reporting it to ARM (since they disitribute "official" GCC binaries...) could get us further than to the GCC team directly? I'm sure they'd have more incentive to get this corrected...

[magic]

That's a different bug, involving vectorization of 64bit loads at O3. The fix has been committed and you can see the testcase is still there in 9.2.0.

I would file another one, stating specifically it concerns some different optimization which activates at O2 and combines 16b loads into 32b loads or whatever the problem is.

Also, have a short demo snippet ready and be sure not to look like a moron who doesn't know what he is doing
That's sadly the reality open source projects have to deal with today. Commercial customer support frankly too, that's why they treat you like an enemy

[SiliconWizard]

That's a different bug, involving vectorization of 64bit loads at O3.

Not it's not. It's the exact same bug: the fact that -mstrict-align doesn't always PREVENT unaligned accesses at certain optimization levels, which is what the overall bug is all about.
It is titled "-O3 -mstrict-align produces code which assumes unaligned vector accesses work" (it should add -O2, but whoever posted that probably didn't run into it or didn't care trying.)
It's exactly the same root problem.

We don't fucking care which exact optimization is concerned. It should activate NONE that can yield to unaligned accesses when "-mstrict-align" is enabled. Period. And again it's not just at -O3. I think they completely missed the point because they didn't look at the whole picture, but only a small part of it (not saying it's easy, GCC code is a monster), which is exactly what you just did as well. And it's probably why the ticket is not closed. It may have opened a can of worms.

[magic]

There is most likely no magic which can prevent a dozen of optimizations from issuing unaligned loads. There is a global disable flag, each module has to handle it correctly for the whole thing to work.

Maybe we should let the original submitter speak about what his bug is or isn't about:

Hello all,

When using aarch64 gcc with -O3 the optimization tries to make use of 16byte vfp unit commands for accessing data structures containing 8byte data members, where possible, as shown in the example below.

I only told you what to do if you want it fixed

[Nominal Animal]

I'm a bit disappointed.

When you have a patch that is tested to work, and just cannot get anyone with commit access to review and apply it, it becomes very frustrating.

If you are interested in workarounds, you could use
    #pragma GCC push_options
    #pragma GCC optimize("-O1")
    /* function implementation */
    #pragma GCC pop_options
around the affected functions, if -O1 stops GCC from miscompiling the code.

I fully expect that if a maintainer looks at that bug, they will close it because both the C standard and the architecture ABI allow it.  That's what usually happens; thinking about whether the allowed behaviour makes sense or not is .. atypical? rare? odd?.  Unless the behaviour hits code they themselves use or maintain, of course.

On the other hand, if you stalk a bit, and find someone who has commit access, and has fixed similar issues before, contacting them about a possible fix (if you have one) can make a HUGE difference.  Be brief, to the point, and leave links to bugs you've fixed, so they can evaluate your input with minimal effort.  Consider GCC a horribly badly managed organisation, and find a connection to a suitable person, in other words.  (I couldn't do that, because dealing with people instead of issues when solving a purely technical problem is one of my buttons.)

[SiliconWizard]

There is most likely no magic which can prevent a dozen of optimizations from issuing unaligned loads. There is a global disable flag, each module has to handle it correctly for the whole thing to work.

No magic indeed, just hard work. Didn't say it was easy. Just saying that when some bug like this happens, it's often good measure for developers to analyze all the possible impacts.
Here, an option not being correctly handled in all cases smells fishy - it's good measure to anticipate that it may be worse than just in ONE particular case someone submitted. (Otherwise you work like in an agile team, strictly sticking to the small issue you're handed with, and NEVER trying to see what could be behind it. Thus the same bug could be submitted again and again for almost ever, everytime with just a slight change  - but the same root cause. Nice, that gives work for some people undefinitely.)

And that said, I got a prompt reply and my account was created  like just one hour later. I submitted a new bug to make sure it would not get lost, described it, and just added a reference to the other one so they can see a link between them.

I'll see how that goes.

[SiliconWizard]

If you are interested in workarounds, you could use
    #pragma GCC push_options
    #pragma GCC optimize("-O1")
    /* function implementation */
    #pragma GCC pop_options
around the affected functions, if -O1 stops GCC from miscompiling the code.

Yeah, I know about that one... I just don't like that. Since the bug is there, there is no way of making sure it won't happen on any other part of the code. Thus sticking to -O1 for the whole code (at least the part that needs strict alignment) is the only option. Which wouldn't be horribly bad if I was actually SURE it won't happen with -O1. Or even -O0. Oh well...

I fully expect that if a maintainer looks at that bug, they will close it because both the C standard and the architecture ABI allow it.

I don't agree here. They have an option for that, "-mstrict-align". And many targets actually require strict alignment. The need is there. So either they remove the option, say fuck off that's allowed, and move on, or they fix it. This option is explicitely supported for aarch64. https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html

And as I pointed out, there were other occurences of similar bugs that were reported. The closest I posted has never been closed...

Thing is, GCC is free and open source, so you get what you get. BUT it's the official compiler supported by ARM, which is a commercial company, so that's a bit more annoying there. If I'm using ARM products, which compiler(s) am I supposed to use and be confident in?

[westfw]

They have an option for that, "-mstrict-align". And many targets actually require strict alignment. The need is there. So either they remove the option, say fuck off that's allowed, and move on, or they fix it.

Obviously correct.  How "-mstrict-align" behaves is not defined inside the C standard; it's strictly a gcc option.  If it exists, it should work correctly.
Sometimes one gets the feeling that Compiler Gurus lose site of things like "the need is there" while searching for some sort of academic purity or something.  :-(

That's a different bug, involving vectorization of 64bit loads at O3.

No it's not. It's the exact same bug: the fact that -mstrict-align doesn't always PREVENT unaligned accesses at certain optimization levels, which is what the overall bug is all about.

You won't get any faster response by claiming it's the same bug if they already actually fixed one instance, and the newly-found symptom requires patching a different section of code...
Go ahead an submit a new bug; if one of the developers thinks it's the same thing, they can mark it as so, but a new bug describing it as "simlar to this other thing (see, I did search for it!)" is a better way to get response...  (IMO, having been on both sides of similar arguments...)

[Nominal Animal]

Focusing the report on that -mstrict-align does not work, and that causes code to miscompile in situations where there are stricter alignment requirements than normally supported by the ABI, might work.  That said, if I lurked correctly, there seems to already be a dev with commit access (at least to the git mirror; not sure about the still-upstream SVN repo), which means there is hope to get this particular fix upstream.  To which version of GCC remains to be seen, though.

I must say that Steve Kargl did commit a gfortran bug fix the same day I posted it (with an existing test case) years ago, so GCC does have very good, very friendly developers, too.  I am just saying that getting their attention AND getting them to think about the bug and its fix, as a nobody off the interweb, just isn't easy.  Even when you get their attention, if the standard allows silly nasal daemons, their first reaction (in my very limited experience years ago) is usually to ignore the underlying problem, because the spec says they do not have to care.  That said, I suspect that people with better social skills have better luck with that than I had.

Years ago, I tried to get some discussion going for a new built-in complimentary to memcpy()/memmove(), one that would repeat the initial data in the buffer to the rest of the buffer.  This would be particularly useful for floating-point fills, as the initial assignment would catch any signaling conditions, and the new built-in handle the duplication of the value to the rest of the buffer.  It got no traction in glibc, because it really is something the C compiler should do internally when optimizing the C code; and no traction in GCC because the standard doesn't define one, and it isn't even in glibc.  Yet, if you look at linear algebra code, that function would make an actual difference on some processors; even changing the result of C:Fortran efficiency comparisons.  (Again, tests were done about a decade ago, so might not be true anymore.)

If linear algebra in C is something one of you is interested in, check these data structures I outlined a few years ago.  It's basically a way to describe vectors and matrices as references to refcounted data, with any order.  You can have a matrix with a vector of its diagonal elements, say -- not copies; to the exact same data --, and since the data is refcounted, both that matrix and vector are "first-order" data types, unlike e.g. GSL "views".  As long as you remember to discard each matrix and vector when you no longer need (those particular variables), the data is managed for you, almost like garbage collection.  (In fact, it does support allocation pools, so instead of managing it individually, you can discard an entire pool at once, ignoring the reference counts.)  You don't need to discard them in any particular order, either; whenever you are sure you won't access that matrix/vector variable is fine.  I was thinking of expanding this into a full linear algebra library (BLAS/LAPACK/GSL), optionally using IMKL or ACML as a back end, maybe even FFTW3 for 1D and 2D transforms on the data, but kinda just lost steam due to lack of interest...

[magic]

No magic indeed, just hard work. Didn't say it was easy. Just saying that when some bug like this happens, it's often good measure for developers to analyze all the possible impacts.
Here, an option not being correctly handled in all cases smells fishy - it's good measure to anticipate that it may be worse than just in ONE particular case someone submitted.

Yeah, maybe. In a one man project - certainly.
But if GCC is anything like Linux, there may not even be one person familiar with all the codebase. Something was reported about the vectorizer, some vectorizer guy picked it up and pushed a fix, we are happy now.
Maybe he left the ticket open to remember about checking other modules at some later opportunity
Maybe he is just lazy and doesn't close fixed bugs.
Your problem may be so old that nobody even suspected it to exist given the lack of complaints.
Or on the contrary, your bug may have been created after 2016.
There is a million possible reasons.

agile

Thing is, GCC is free and open source, so you get what you get. BUT it's the official compiler supported by ARM, which is a commercial company, so that's a bit more annoying there. If I'm using ARM products, which compiler(s) am I supposed to use and be confident in?

I think they offer some commercial compiler too if you don't mind the $$$. Then you presumably get some kind of official support.
And if you look around they may actually provide some means of reporting problems with their GCC build, particularly if it's some customized version like Linaro's.

Years ago, I tried to get some discussion going for a new built-in complimentary to memcpy()/memmove(), one that would repeat the initial data in the buffer to the rest of the buffer.  This would be particularly useful for floating-point fills, as the initial assignment would catch any signaling conditions, and the new built-in handle the duplication of the value to the rest of the buffer.

But you know, ignoring feature requests is not exactly the same as ignoring obvious bugs

[Gandalf_Sr]

An interesting thread.  The OP was asking about learning C and I can recommend this website https://publications.gbdirect.co.uk//c_book/

I'm an embedded hardware+firmware engineer and started years ago using assembly; at one point I (masochistically) wrote an entire alarm clock program in assembly (I still have it) and I've since re-written it all in C as an exercise in realizing how much simpler things can be.  Having written in, and warmed up to C (GCC) over the last 5 years, I am happy to have made the transition although there are still aspects that I'm unsure of.  In particular, pointers and passing pointers as arguments has caused me confusion and grief.  I've also been caught out a couple of times by running outside the array bounds, especially when dealing with 'strings' as arrays of char e.g.

char mystring[128];

C will let you write code that tries to access mystring[300] and, if you write to such a location, your program will often crash or hang. Unfortunately, solving such issues is never as simple as finding a line with an obvious out of bounds index such as I just gave.

If anyone wants to help in my evolving education, please feel free to offer reading subjects or advice.

In short, C is great when you're trying to control low-level stuff like ports and serial (I2C and SPI) devices but it can be a challenge to find bugs like the one above.

FWIW, I originally worked in Microchip's MPLAB (now MPLABX) but a few years ago, I moved to working with Cypress PSoC devices using Cypress' PSoC Creator (and now MODUS), all using the GCC (free) compiler.  I have to say I am VERY happy with PSoC Creator and the PSoC device family 4, 5, and now dual core 6; they are amazing devices.  My most complicated recent project is on PSoC 6 where I write 2 separate main.c programs, one for each core, with rules about which core starts first and gives the other core permisson to start and allows the core to 'own' peripherals.  Setting up shared RAM was not straightforward as it should have been but I figured it out and it works really well.