stdarg

[Neo]

Is there something wrong with my code for the 'stdarg' macros shown here

Code: Select all

typedef void *va_list;

#define va_rounded_size(T) \
  (((sizeof(T)+sizeof(int)-1)/sizeof(int))*sizeof(int))

#define va_start(ap,lastarg) \
  ( ap=(va_list)((char *)&(lastarg)+va_rounded_size(lastarg)) )

#define va_arg(ap, T) \
    (ap = (va_list) ((char *) (ap)+va_rounded_size(T)), \
     *((T *) (void *) ((char *) (ap)-va_rounded_size(T))))

I used them in a function like this

Code: Select all

void testf(char *msg, ...)
{
  va_list ap;
  int d;
  va_start(ap,msg);
  kprintf("\nmsg=0x%X ap=0x%x",&msg,ap);
  d=va_arg(ap,int);
  kprintf("\n%d @ %X",d,ap);
}

void kmain(void)
{
  setbgcolor(BLACK,FALSE);
  loadIDT();

  testf("Hello",10,20,30,40,50,60,70,80,90);
}

But never get the correct value of the second or subsequent arguments printed, instead I get something other weird value shown below.

Code: Select all

msg=0x104FF4 ap=0x104ff8
33095686 @ 104FFC

I first wrote this in a normal C program and tested it there and it worked fine, however it does not in my kernel.
I can't figure out what is wrong here, I hope someone here will be able to spot what I've done wrong, this has been bugging me for a while now.

[Dreamsmith]

33095686 = 0x1F90006, which almost looks like it might be an address... what good that does you, I know not...

[Pype.Clicker]

i'm suspicious at your 'va_start(ap,lastarg)' macro

( ap=(va_list)((char *)&(lastarg)+va_rounded_size(lastarg)) )

&(lastarg) is a char** (afaik) and thus +va_rounded... does add four times the value

you may like to know that there's a __builtin_next_arg(argument) function in GCC that will automatically gives you the next argument pointer...

[Dreamsmith]

( ap=(va_list)((char *)&(lastarg)+va_rounded_size(lastarg)) )

&(lastarg) is a char** (afaik) and thus +va_rounded... does add four times the value

Ah, but type casting operators take precedence over addition, so +va_rounded is being added to (char*)&(lastarg), not &(lastarg).

He said these macros are working fine in standard C programs he tried them with, it's just not working in his kernel. I suspect there's absolutely nothing wrong with the macros, the problem lies elsewhere. Using any odd compiler switches to invoke a different than standard calling convention? Or is it possible something else is clobbering values on the stack?

Even if you don't want to be dependent on GCC, just for testing purposes, you might want to use these macros:

Code: Select all

#define va_start(v,l)   __builtin_va_start(v,l)
#define va_end(v)       __builtin_va_end(v)
#define va_arg(v,l)     __builtin_va_arg(v,l)

If the problem goes away, it really was a problem with your macros. If it remains, you know the macros were a red herring, and you need to be looking elsewhere...

[Neo]

That gives mean 'undefined reference' i think because i use the '-nodefaultlibs' flag. Is there any other way to include these or do i have to remove that flag?

[Dreamsmith]

Add -lgcc to your flags. That should be perfectly safe to link to, even in a freestanding environment. However, it would surprise me greatly to discover __builtin_va_start and friends are in it...

Which version of GCC are you using?

[Neo]

no change

[Dreamsmith]

no change

Didn't think so. These are supposed to be built in to the compiler, not linked in from a library. -nostdlib/nodefaultlibs/etc should have no impact.

Are you using distcc by any chance? Mismatched GCC versions can cause this problem when using distcc...

You can always just dump the stack and look at things there. <stdarg.h> is a convenience header for writing portable Standard C code, it provides no necessary functions...

[Neo]

I copied the macros right from the DJGPP header files and the worked so i then started replacing it with my code and found that the 'va_start' macro was the culprit.
Heres the right one.

Code: Select all

#define va_start(ap,lastarg) \
  ( ap=((va_list)((char *)&lastarg)+va_rounded_size(lastarg)) )

Note the parentheses after surrounding the part excluding the 'va_rounded_size'.
Hope someone can explain it more clearly with the precedence etc.

[Dreamsmith]

Hmm. Unless I'm miscounting my parentheses, it looks like the value's getting cast as a va_list before the +va_rounded_size, which will work under GCC and presumably DJGPP, as it allows pointer arithmatic on void *'s, but that's definitely not portable...

[Neo]

Ok heres the code now, it should be portable too (not that i'm too concerned about that ) and i realised that it must have been the 'va_arg' macro not 'va_start' (trial and error is a lousy technique but works).

Code: Select all

typedef void *va_list;

#define va_start(ap,lastarg) \
  ( ap=(va_list)((char *)&(lastarg)+va_rounded_size(lastarg)) )

#define va_rounded_size(T) \
  (((sizeof(T)+sizeof(int)-1)/sizeof(int))*sizeof(int))

#define va_arg(ap, T) \
    (ap = (va_list)(((char *)(ap))+va_rounded_size(T)), \
     *((T *) (va_list)(((char *)(ap))-va_rounded_size(T))))

So i guess the bug was in the 'va_arg' macro. BTW can anyone test this in their kernel and let me know, please.

[Neo]

Another related doubt...
After finishing this 'stdarg.h' I had intended to enhance and rewrite my printf() function but am now wondering if this is necessary, does a kernel require such a complex function (printf() with all its bells and whistles), it seems more appropriate to use that for user apps. While IMHO kernel level functions should know exactly what they want to print so wouldn't it be better and safer to write individual kernel functions for printing strings, ints, floats etc.
Or would it be better to stick to the good old printf() ?

[Dreamsmith]

Now, a full-blown printf would be overkill, but having to call seperate functions for each data type you want to print, interspersed with string prints if you don't want it all run together, would be a major pain in the arse. I compromise with a "kprintf" that supports a subset of printf's type specifiers, specifically: %d, %x, %c, and %s. It also supports specifying a width and zero padding, ala %04X. But that's it, and that's all I've ever needed from kernel space. The whole thing's only three screens long (in my 80x37 xterm):

Code: Select all

int kprintf(char *fmt, ...)
{
   int *ap = (int *)&fmt + 1, width, zeropad, result = 0;
   if ( con.mutex ) acquireMutex(con.mutex, 0);
   while ( *fmt )
   {
      if ( *fmt == '%' && *++fmt != '%' )
      {
         zeropad = (*fmt == '0');
         width = 0;
         while ( *fmt >= '0' && *fmt <= '9' )
            width = width * 10 + (*fmt++ - '0');
         switch ( *fmt++ )
         {
            case 'd':
            {
               int arg = *ap++, neg = arg < 0, abs = neg ? -arg : arg;
               int tw = 1, base = 1;
               while ( tw < 10 && base * 10 - 1 < abs ) ++tw, base *= 10;
               if ( width > tw + neg )
               {
                  if ( neg && zeropad )
                  {
                     kputc('-');
                     ++result;
                     --width;
                     neg = 0;
                  }
                  while ( width > tw + neg )
                  {
                     kputc(zeropad ? '0' : ' ');
                     ++result;
                     --width;
                  }
               }
               if ( neg )
               {
                  kputc('-');
                  ++result;
               }
               while ( base )
               {
                  kputc(abs / base % 10 + '0');
                  ++result;
                  base /= 10;
               }
               break;
            }
            case 'x':
            case 'X':
            {
               int arg = *ap++;
               int tw = 1;
               while ( tw < 8 && arg >> (tw * 4) ) ++tw;
               while ( width > tw )
               {
                  kputc(zeropad ? '0' : ' ');
                  ++result;
                  --width;
               }
               while ( tw )
               {
                  kputc("0123456789ABCDEF"[arg >> (--tw * 4) & 15]);
                  ++result;
               }
               break;
            }
            case 's':
            {
               char *arg = (char *) *ap++;
               while ( *arg )
               {
                  kputc(*arg++);
                  ++result;
                  --width;
               }
               while ( width > 0 )
               {
                  kputc(' ');
                  ++result;
                  --width;
               }
               break;
            }
            case 'c':
            {
               char arg = (char) *ap++;
               kputc(arg);
               ++result;
               break;
            }
         }
      }
      else
      {
         kputc(*fmt++);
         ++result;
      }
   }
   if ( con.mutex ) releaseMutex(con.mutex, 0);
   return result;
}

Looking at it again, I see I also return the number of bytes output. I never use that, I could trim this further...