(MS VC++) Could you explain this to me ?

I am affraid that it is incorrect. The code above has worked in production for ~ 10 years for a smaller structure.
The purpose of the macro MY_TYPE is to translate the type of a member of the structure into the argument of the function SetType(...). In real life there are a bunch of overloading functions SetType(...) with different types of argument, and the union  MY_UNION represents all possible types. I believe that the difference for smaller and bigger structure is more understanble when you look at the Assembly code but I still cannot completely explain it.

Thanks.

It looks like you've inherited some "working", but horribly-designed code.

Any time one does wild-casting into a structure, there's lots of possibilities for errors.  Especially in a language like C, with no intrinsic array-bounds checking.


*Usually* when a program "works" but breaks when you change the size of something that should be completely unrelated, it tends to be that the program originally "worked" by pure dumb luck-- i.e. the struct member selections or array indexes just happened to point to valid memory by pure arithmetic chance.   Changing the size of something else changed the arrangement of things in memory just enough to cause the bad code to wipe out something.

Sorry. My understamding is that you did not not look at the assembly code. Sorry again but I don't need a "general" explanation.

grg99,

By the way, what kind of luck we are talking about , if you can copy and paste this code,shorten the number of strings in the structure BIG_STRUCT to 65  (instead of 71), this code will work perfectly  for you every time.

>grg99,By the way, what kind of luck we are talking about , if you can copy and paste this code,shorten the number of strings in the structure BIG_STRUCT to 65  (instead of 71), this code will work perfectly  for you every time.


"Works perfectly" in most environments usually means "hasnt crashed or made any obvious errors".

It's more than likely that some of those addressing macros/functions/casts are going awry, and not indexing to the addresses you expect.  For example, if you accidentally use an index on a pointer, instead of its base type, it may be indexing by 4 bytes, instead of your expected 1, 2, or some other size.  This will end up addressing some unexpected spot in your BIG_STRUCT, which is soo big, that the wildly incorrect  address may be still inside the structure and with a little more luck unused at that time.  Soo the program may "work", but it's using unexpected memory locations.  Any minor change in the size of the structure, or the indexing, or program data may end this honeymoon.    Been there, done that, have the tee shirt, the CD, and trophy.  

I've seen this many many a time.  Anytime a program is sensitive to minor changes in structure sizes, or adding debug lines, it's very very very very very very very very very very very very very very very very very very very very very very very very very very very very very often a case of wild pointers.

OK, you did not try this code. Thank you for your time. "Works perfectly" means that the code always produces the right result for you and for me and for all people on the Earth. In this particular case the result should be 5. This code (certainly not this sample, but the idea was the same) was written ~ 10 years ago by a recognized C++ guru. I personally don't like this code (I 've been 12+ years in C++ and 20 years in C). But the question is straight. Could you explain why this particular code ALWAYS works for the structue shorter than 66 string's fields and ALWAYSs causes Access Violation work if we have biger structure?
Please, don't respond if you don't know this particular answer.

Thanks.

Sorry, I forgot to show the disassembly for the call in question. This is the version that fails:

     int res = MY_TYPE(BIG_STRUCT, mode);
00411A85  mov         eax,dword ptr ds:[0042C040h]
00411A8A  push        eax  
00411A8B  call        SetType (4111B3h)

And this version does not:
     int res = MY_TYPE(BIG_STRUCT, mode);
00411A85  mov         eax,dword ptr ds:[0042BF40h]
00411A8A  push        eax  
00411A8B  call        SetType (4111B3h)

And the memory looks as this in the area of interest:

0x0042BFAE  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  .........................................
0x0042BFD7  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  .........................................
0x0042C000  ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??  .........................................
0x0042C029  ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??  .........................................

As you can see, this string (in my case #59) just passes the border to the unallocated memory.

It is exactly what I have for VC++ 6.0. The difference is the number of strings: I am OK with 66 strings. Moreover, when I look at dissassembly code I can see that compiler's table __cfltcvt_tab is used for small structure (I tested 3 member structure ) and __except_list table for bigger structure when the code still works (66 string fields for my VC++ 6.0). At this point of time I still believe that the problem is still related to some internal compiler's ot LINKEDIT limitations. What is lucky about this? The original C++ code is not very good, but it is still legitimate. What is wrong with it? Why does it work on all machines for structures with the size less than 16000 bytes or so? I don't think that the luckiness is a proper explanation. I need to decide what to do with the code that perfectly works for about 100 production systems and does not work for one new developer that happened to increase the length of the structure tremendously.

Thanks.

> What is lucky about this? The original C++ code is not very good, but it is still legitimate. What is wrong with it?
If you look at the disassembly in my previous comment, you see that the code tries to access an invalid memory location, in this case 0x0042BF40. The reason the code works with the fewer number of strings on your production machines is simply that the memory pointed to by the last member is then unused by whatever allocated it. "Luck" doesn't necessarily mean the errors fluctuate. As long as your compiled code remains the same, a faulty program could very well work "flawlessly", but even a very slight change to it could cause it to break down completely, as has already been pointed out in a previous comment. It could be dangerous to say that this code is "legitimate". It is, at best, a hack that uses the internal workings of the compiler to hide problems with invalid pointers.

I am still quite unclear with what the code should really do, but one way to get your sample program not to crash is allocating more memory to the MY_UNION union, like this:
typedef union
{
     int          i;
     char          c;
     short          t;
     char bigdata[8+257*71];
} MY_UNION;

Unfortunetely, you did not understand what the code does. I really don't have time to explain this to you in detail. But the point is in the union, only those types should be presented that we are going to point to, in the structure. For my example you can point to int, char or short in the structure. You cannot point to the string. I hope now it will be clearer for you. So your suggestion about union is obvious but completely useless (Sorry.)  

 static MY_UNION uni;

This allocates four bytes of space. It is a static structure, so it will be located in the uninitialized data area.

And the macro

  #define MY_TYPE(s,m)   SetType(((s *)&SOME_SPACE::uni)->m)

Takes the address of the four bytes mentioned above, and *pretends* that the four bytes are instead the huge structure of arrays. it then calculates offset of the "mode" member, which is WAY past the end of "uni".

The code is completely and utterly wrong. If it worked, it was pure luck, as the other experts have tried to tell you.

If you want an explanation for why you get the access violation, it is simple:

uni is 4 bytes. You are then calculating this pointer WAY past the end of uni. If you comment out a few of those arrays, your pointer ends up pointing to some weird area of the executable, like the import section or debug info. If you put those arrays back in, your pointer is so far past the uni structure, it is *past the end of the executable*. You are trying to access the mode member (which is not there), and your pointer is way out in space accessing memory that was never allocated or reserved, out past the end of the program.

adg,

It is not so obvious. When the program works (also for huge structures, like 16,000 bytes ), the value of int mode  goes as the argument of SetType(...). You can check it through a debugger. So basically not the whole structure's chunk of memory , but just integer "mode" goes to the 4 bytes that was reserved by static MY_UNION uni;

The executable is constructed from several sections:

.text: assembly code
.data: initialized data
.bss: uninitialized data (filled with zeros before program start)
.idata: import data
.rsrc: resources such as dialogs and icons
etc....

each of these sections is rounded up to the next higher multiple of 4KB. Now, your (4 byte) uni structure gets placed in the .bss section. If that is the only data in the program, .bss will probably end up being 4KB.

Now, if I access, say, 18248 bytes past the END of uni, my pointer will end up pointing to some garbage, probably so far ahead that it is past the end of the memory image. rpz demonstrated that it points well into the "idata" section, way past the end of "bss".

Reassuring us how good the original programmer was will NOT somehow magically make this code valid. It is completely wrong. It might be right in context (in the actual app), but the code listed here is completely invalid. Perhaps he obfuscated the code to buy some job security? Forgive me for being blunt, but myself and all the other people who posted here are trying to tell you that the code is not valid! :)

Here is an example of what you are doing:

----

static char aSmallArray[500];

int main()
{
    printf("This is going to crash!\n", aSmallArray[80000]);

    return 0;
}

----

See how it tries to read data WAY past the end of aSmallArray?

We don't need to convince you, your CPU attempts to lookup the address of the data (way off the end of the executable). The CPU finds that the paging tables say the memory (way off in space) is not allocated to any process. The CPU raises a paging exception, windows handles it, your program dies, end-of-story.

adg,

You did not respond my previous comment. Please talk from yourself, not from the whole community. Please try not to be rude. Don't take it personally.

If you can show me where it is ALLOCATING 16000+ bytes, then your code might be valid. You code allocates a 4-byte union, then accesses memory that is over 18000 bytes PAST the end of the union.

You were not entirely polite to rpz. (saying his suggestion was "useless")

adg,

Do you agree that only integer "mode" goes into static uni or not?

The real error is this line:

namespace SOME_SPACE { static MY_UNION uni; };

If you were to replace it with:

namespace SOME_SPACE { static BIG_STRUCT uni; };

and move it below the "typedef struct" it will work. It will allocate 18256 bytes for uni, and then the computer can add 18252 to the base address of uni and actually read uni!

....below the entire typeef struct, after the ;

I mean, after "} BIG_STRUCT;"

Why not use an array of arrays:

typedef struct
{
    int                    count1;
    char                 Buffers[71][257];
    int                    mode;
} BIG_STRUCT;

To the other experts:

A long time ago a VERY SMART fellow, an excellent programmer,  asked me for advice: his car stereo was blowing fuses at peculiar times.   I thought over the problem and gave him three plausible explanations of what could be wrong; each explanation was a bit subtle, but they were each logically consistent, fit all the known facts, and matched my experience with other similar situations.

All he could say is "BUT HOW CAN THAT BE?".   He was very smart, but had some mental block, he thought if every connection LOOKED okay, and he thought every wire went to the right place, and no speaker had a little thought balloon saying "I MAY LOOK OKAY FROM THE OUTSIDE, BUT I HAVE A SHORTED CROSSOVER CAPACITOR", he thought everything HAD to be okay.

There's not much you can do for such folk, they're stuck  in a mental logjam.

andrey_2007:
>> Do you agree that only integer "mode" goes into static uni or not?

uni is a union, and the largest member of that union is 4 bytes. Therefore uni is four bytes.

If you want proof, step into the program with the MSVC debugger, press Shift+F9, and type sizeof(uni) and press enter. It says 4, meaning uni is 4 bytes.

To get the address of uni in the debugger, you press Shift+F9 and type

&uni

I get 0x00427C50. That is the address where uni is stored.

Now, if you step into the assembly, you see that it reads address 0x0042C39C.

So if the beginning of uni is 0x00427C50 and uni is four bytes, then uni ends at 0x00427c54.

Your program is reading address 0x0042C39C, which is 18248 bytes past the end of uni, exactly what I calculated in previous postings.

The MY_TYPE macro accesses memory that is 18248 bytes past the end of uni. It is accessing something, but it is not accessing uni. On your compiler and my compiler, that just HAPPENS to be past the end of the executable. Apparently junges libraries are larger, so it doesn't crash there, but it is still accessing some crazy memory location, not uni.

So, to answer that question: No, there is no space in uni that is 18248 bytes from the beginning of uni, because uni is 4 bytes.

uni doesn't even have a "mode" member.

There is nowhere in that program listing where it stores any information. It *reads* a memory location, but that crashes it before it even touches "res". The memory address that the MY_TYPE macro calculates is so high that it is outside the entire program image, in unallocated space, causing a crash.

So integer 'mode' never goes into static uni.

The A20 kludge is a wraparound at the 1MEG line, not a segment wraparound. Even if it were running under a segmented architecture, it would still be accessing some random memory address, it would just be some random address within a given 64K area.

Since the program accesses memory that is way past the structure it is trying to read, if you add data after "uni", add at least 18252 bytes, then your accesses of "uni" will actually be reading some area of the variables after uni, and it won't crash. If you *write* to that pointer calculated by MY_TYPE, you will be corrupting the content of the variables declared after uni. If by chance your corruption of memory doesn't happen to disturb anything important, it might actually run. This is a type of bug that happens sometimes when working with pointers, where you calculate a completely wrong address and read/write the wrong address, causing extremely weird bugs to occur elsewhere in the program because you are corrupting unrelated variables. But you would never deliberately do that. You should allocate memory properly and calculate pointers to data areas you own.

>The A20 kludge is a wraparound at the 1MEG line, not a segment wraparound.

Thats right, it was expected that using a segment of > $F000 with high offsets would wrap around to low memory.
Same basic idea, only with the segment part, not the offset part.  What a weird world.

Way back in ancient days, whenever we ran BASIC on a PDP-8, afterwards one of the disks would be unreadable until a reboot.   Turned out BASIC was trying to write a flag of #0001 to the right offset, but the wrong memory bank (sort of like a segment).  That bashed address just happened to be smack dab in an important system disk info table.   Nobody  noticed the problem until they got enough disk devices to reach table entry #7, which was the one that got bashed.  The flag it was trying to set was a rather unimportant one, having to do with system swapping, so nobody noticed the flag wasnt getting set.   This stuff goes waaaaaay back, folks.

I have not abondoned this question. Do no delete or close.