va_args trashing the stack...

[stonedzealot]

I know this has probably been mentioned many times before (e.g. http://www.mega-tokyo.com/forum/index.p ... 04;start=0 which I found unenlightening as the resolution was fuzzy) but I'll have to ask it again.

If every example seems to be using identical (or at least similar) va_arg code,and I can't see any real, substantial difference between the implementation code, how does one keep va_args from trashing the stack?

My current situation is with printf (which is why the above thread attracted me) printing fine and dandy before va_args, but then being utterly destroyed (...b0rked, if you will) when I add the va_arg code (even though all relevant translation code -ex: if(*message = '%'){, etc.- is in an if block). After running the code with and without the offending if block, I examined the stack and saw that after a few identical entries (telling me these aren't the problems) the stack after using v_args was significantly more full (about 30 bytes of info).

I doubt that it's the macro's fault (because people wouldn't use the macros if they were inherently faulty), so am I just missing a big piece of the va_args pie?

Of course, this could have nothing to do with va_args, but I've stripped the kernel down to nothing but printf statements and bootloader/kernel asm stub that I know is clean (jumps to kernel, sets up GDT, resets all selectors, sets up stack). Anyway...I'll talk to you all later. Hope this isn't just some retarded thing I'm not seeing...

[Pype.Clicker]

well, with such fuzzy information, i cannot help. Can you be more specific at the test you do and the way you code your va_* macros and how you use them in your printing function ?

[BI lazy]

the conclusion of the thread mentioned by you is logical but not fuzzy. It's classic debugging.

What does your kprintf look like? how do you pass parameters to the helper functions? How do you initiate the va_args before passing them to the printf-helper function?

[stonedzealot]

Just goes to show you, don't post on the forums when you're tired to the bone. Anyway here are the specifics. k_printf("%c", 0, 0, charvar) where charvar is a standard ASCII char would be a simple putch command printed to the very upper left corner as the code looks like this:

Code: Select all

unsigned int k_printf(char *message, unsigned X, unsigned Y, ...)
{
   int i = 160 * Y + X * 2;
   char ch;
   va_list args;
   va_start(args, Y);
   while(*message != 0)
   {
      if(*message = '%')
      {
         message++;
         if(*message = 'c')
         {
            ch = va_arg(args, char);
            putch(ch, i);
         }
      }
      else
         putch(*message, i);
      X++;
      message++;
      i = 160 * Y + X * 2;
   }
   va_end(args);
}

I test the %c code by putting a line into keyboard interrupt handler that will print the translated scan_code (e.g. k_printf("%c"), 10, 10, temp) where temp is the scan_code after being converted). This prints whatever I type on the keyboard, no problem. I realize that this could be replaced by a simpler call, k_printf(temp, 10, 10) if I just null terminated the char, but the point of this is to test va_args.

With the above code however, normal print statements (ones with no %s) are broken, outputting nothing to the screen. Whereas, if I take all of the va_arg code out, to make it look something like:

Code: Select all

unsigned int k_printf(char *message, unsigned X, unsigned Y)
{
   int i = 0; 
   while(*message != 0)
   {
      i = 160 * Y + X * 2;
      putch(*message, i);
      X++;
      message++;

   }
}

The printing works just fine, but va_args don't (as they're not implemented, of course).

So, to restate the question, how does adding this code manage to trash the stack and break the function when the function itself is sound and the implemented code is in an if block or is used in every other va_arg example I've found...

[rdragon]

I believe your problem lies here:

ch = va_arg(args, char);

As va_arg will take sizeof(char) to set itself up for the next call to va_arg, it will advance one byte. This is a problem, as when you push a char (1 byte) onto the stack, it is most usually pushed on as an int (4 bytes), and this will cause you serious problems.

You might try changing it to:

ch = (char) va_arg(args, int);

And that should clear up your problem.

[Pype.Clicker]

Code: Select all

      if(*message = '%')

should rather be

Code: Select all

if (*message == '%')

usual mistake that can be caught by turning on -Wall on the compiler.
You're introducing message modification and the function believes that every character is a '%c' because 'msg=%' evaluates to '%' which is non-null (thus true).

[Schol-R-LEA]

Another good way to catch this kind of error, at least when comparing a variable against a constant, is to have the constant on the left hand side of the equation:

Code: Select all

if ('%' ==  *message)

This way, if you use a single equal sign ('[tt]=[/tt]') by mistake, it will result in a compiler error, the cause of which should be easy to spot and fix.

[stonedzealot]

Ugh. My head hurts.

rdragon: Thanks for the suggestion, I thought that might've been it, but the va_args macros handled that like a charm, making this unnecessary.

Pype: Right again, I started using the -Wall command, it's very useful. I guess the error was just a throw back to Pascal, in which = is a comparative and := is the make equivalent sign.

Schol-R-Lea: That would be all right, but '%' == message just seems backwards to me...

Anyway, I think I'll stop coding after school and reserve it for the weekends because these are just silly things I miss becasuse I'm not thinking (school + theater crew = exhaustion of body and mind). Seriously, I just spent about a half hour implementing the corrections and failing, before I realized I was changing code that wasn't in the directory. Now that's pathetic. But what can you expect?

Oh yeah, forgot to mention (if you couldn't guess from context) that it works now.

[rdragon]

That is interesting, because in my case the macro did NOT handle things properly if I popped it off as a char rather than an int.

You might try calling the print function as

printf("%c%c%c%c%c", (char)0x36, (char)0x37);

if it prints out "37" then you are in the clear... though if it prints out 3...7 then it isn't handling things correctly.

Oddly enough, I took my va_arg macros right from the linux source code. It has been awhile since i've played with it though.

[Solar]

Another good way to catch this kind of error, at least when comparing a variable against a constant, is to have the constant on the left hand side of the equation:

I know that this is popular practice, but I don't like wrapping my mind around such stuff. Even less so since a simple compiler option should give you a warning if stuff like this happens.

[Pype.Clicker]

printf("%c%c%c%c%c", (char)0x36, (char)0x37);

if it prints out "37" then you are in the clear... though if it prints out 3...7 then it isn't handling things correctly.
with it though.

uh ? why so much "%c" ??
Remember that when shorts and chars are placed on the stack, they take a full integer size (but iirc, the highest part may hold some garbage). The same occur when you access them as arguments. So you're likely to require an 'int" type rather than a 'char' type for clean code.

Code: Select all

    #define va_arg(AP, TYPE)                                                \
 (AP = (__gnuc_va_list) ((char *) (AP) + __va_rounded_size (TYPE)),     \
  *((TYPE *) (void *) ((char *) (AP) - __va_rounded_size (TYPE))))

//... somewhere in 
void kprint(char *msg, ...)
{
 va_list args;
  va_start(args,msg);
  __kprint_internal(args,msg,NULL);
}

// target == NULL -> use default display buffer.
int __kprint_internal(va_list args, char *msg, char *target)
{
    while(*msg) {
    if (/* check you have %[^%] at *msg */) {
        void *arg=va_arg(args,void*); // all arguments are 32-bits on stack.
    switch(*msg) {
        case 's': {
             char * effective_arg = arg; break;
        }
        case 'c': {
              char effective_arg = (int)arg; break
        }
        case 'i' : {
               int effective_arg = (int)arg; break;
        }
    }
}

[Pype.Clicker]

oops. looks like i locked the thread by error ...

[rdragon]

Pype:

The reason for all the %c's was to test how the va_args macro is working. If it is reading things off, only stepping by a char rather than an int, then you would get an output of "3...7" where the . could be blank or garbage. Otherwise, if it is working correctly, it would print out "37..." meaning it is stepping correctly.

This was actually a problem I was running into for a little bit when I was using

case 'c': putchar((char)va_arg(list, char)); break;

rather than

case 'c': putchar((char)va_arg(list, int)); break;

which is what makes it work correctly. I noticed wangpeng is using

ch = va_arg(args, char);

and he claimed that it stepped properly, so I suggested he run that experiment to verify it, just to not cause more headache later.

[Pype.Clicker]

hmm ... i see ... (except 0x36 0x37 is likely to show '67' rather than '37' ). I guess the effective result will depend on the implementation of __va_rounded_size() macro ...

[rdragon]

Errr, yeah. 67. That is what I meant ;D

Hmm, I overlooked your va_arg macros before, I did not even realize the __va_rounded_size.

The macros i "obtained" do not even specify such a macro, just this:

typedef char* va_list;
#define va_start(list, lastarg) (list = (void*) (&(lastarg) + 1))
#define va_arg(list, type) ((type*)(list += sizeof(type)))[-1]
#define va_end(list)

Which would totally explain why i got the results I did