gcc compiler warningS

[Crixus]

Hello all,

I have to say thank you for your work. It is a great help to have a way to follow to learning operating system.
I also have to apologize as i'm french, my english should be a little weird

This is my first post, i will hard try so, i think not the last one.

I hope that my first question isn't so stupid... i have some warning that i suppose are relevant in classic user development but not in kernel mode development.
i have found a piece of code on the xv6 MIT project that look like :

Code: Select all

void lgdt(struct Gdt *g, int size)
{
  volatile uint16_t pd[3];

  pd[0] = size-1; // 16 bit limit
  pd[1] = (uint16_t)g; // 32 bit base
  pd[2] = (uint16_t)g >> 16; // 32 bit base

  asm volatile("lgdt (%0)" : : "r" (pd));
}

my compiler said :

kernel/gdt.c:42:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
pd[1] = (uint16_t)g; // 32 bit base
^
kernel/gdt.c:43:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
pd[2] = (uint16_t)g >> 16; // 32 bit base

also i have found that code :

Code: Select all

#define SEG(type, base, lim, dpl) (struct segdesc)    \
{ ((lim) >> 12) & 0xffff, (uint)(base) & 0xffff,      \
  ((uint)(base) >> 16) & 0xff, type, 1, dpl, 1,       \
  (uint)(lim) >> 28, 0, 0, 1, 1, (uint)(base) >> 24 }

that i have rework as

Code: Select all

#define SET_GDT(base, limit, accessByte, flag) (struct Gdt){ \
  limit & 0xffff, \
  base & 0xffffff, \
  accessByte & 0xff, \
  (limit >> 16) & 0xf, \
  flag & 0xff, \
  (base >> 24) & 0xff \
}

My compiler say :

kernel/gdt.c:54:22: note: in expansion of macro ‘SET_GDT’
gdt[SEG_UDATA] = SET_GDT(0, 0xffffffff, STA_W + (1 << 4) + (DPL_USER << 5) ,0b0011);


I don"t understand what is about.

Bref, i have lot of warning that i don't know how to fix i that look to not appear a lot after some research on the web.

-------------------------------------------------

Code: Select all

#include <kernel/tty.h>
#include <kernel/gdt.h>

#include <stdio.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

#define DPL_USER    0x3     // User DPL

// Application segment type bits
#define STA_X       0x8     // Executable segment
#define STA_W       0x2     // Writeable (non-executable segments)
#define STA_R       0x2     // Readable (executable segments)

// System segment type bits
#define STS_T32A    0x9     // Available 32-bit TSS
#define STS_IG32    0xE     // 32-bit Interrupt Gate
#define STS_TG32    0xF     // 32-bit Trap Gate

// various segment selectors.
#define SEG_KCODE 1  // kernel code
#define SEG_KDATA 2  // kernel data+stack
#define SEG_UCODE 3  // user code
#define SEG_UDATA 4  // user data+stack
#define SEG_TSS   5  // this process's task state

#define SET_GDT(base, limit, accessByte, flag) (struct Gdt){ \
  limit & 0xffff, \
  base & 0xffffff, \
  accessByte & 0xff, \
  (limit >> 16) & 0xf, \
  flag & 0xff, \
  (base >> 24) & 0xff \
}

void lgdt(struct Gdt *g, int size)
{
  volatile uint16_t pd[3];

  pd[0] = size-1; // 16 bit limit
  pd[1] = (uint16_t)g; // 32 bit base
  pd[2] = (uint16_t)g >> 16; // 32 bit base

  asm volatile("lgdt (%0)" : : "r" (pd));
}


void createGdt(){
    struct Gdt* gdt;
    gdt[SEG_KCODE] = SET_GDT(0, 0xffffffff, (STA_X|STA_R) + (1 << 4) + (0 << 5) ,0b0011);
    gdt[SEG_KDATA] = SET_GDT(0, 0xffffffff, STA_W + (1 << 4) + (0 << 5) ,0b0011);
    gdt[SEG_UCODE] = SET_GDT(0, 0xffffffff, (STA_X|STA_R) + (1 << 4) + (DPL_USER << 5) ,0b0011);
    gdt[SEG_UDATA] = SET_GDT(0, 0xffffffff, STA_W + (1 << 4) + (DPL_USER << 5) ,0b0011);

    lgdt(gdt, sizeof(gdt));
}

Code: Select all

#ifndef GDT
#define GDT

struct Gdt
{
    unsigned int limit:16;
    unsigned int base:24;
    unsigned int accessByte:8;
    unsigned int limit2:4;
    unsigned int flags:4;
    unsigned int base2:8;
};

void createGdt();

#endif

Code: Select all

#include <stdio.h>

#include <kernel/tty.h>
#include <kernel/gdt.h>

void kernel_main(void) {
	terminal_initialize();
	printf("Hello, kernel World!\n");

	createGdt();
}

kernel/gdt.c: In function ‘lgdt’:
kernel/gdt.c:42:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
pd[1] = (uint16_t)g; // 32 bit base
^
kernel/gdt.c:43:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
pd[2] = (uint16_t)g >> 16; // 32 bit base
^
kernel/gdt.c: In function ‘createGdt’:
kernel/gdt.c:31:14: warning: suggest parentheses around ‘+’ in operand of ‘&’ [-Wparentheses]
accessByte & 0xff, \
^
kernel/gdt.c:51:22: note: in expansion of macro ‘SET_GDT’
gdt[SEG_KCODE] = SET_GDT(0, 0xffffffff, (STA_X|STA_R) + (1 << 4) + (0 << 5) ,0b0011);
^~~~~~~
kernel/gdt.c:31:14: warning: suggest parentheses around ‘+’ in operand of ‘&’ [-Wparentheses]
accessByte & 0xff, \
^
kernel/gdt.c:52:22: note: in expansion of macro ‘SET_GDT’
gdt[SEG_KDATA] = SET_GDT(0, 0xffffffff, STA_W + (1 << 4) + (0 << 5) ,0b0011);
^~~~~~~
kernel/gdt.c:31:14: warning: suggest parentheses around ‘+’ in operand of ‘&’ [-Wparentheses]
accessByte & 0xff, \
^
kernel/gdt.c:53:22: note: in expansion of macro ‘SET_GDT’
gdt[SEG_UCODE] = SET_GDT(0, 0xffffffff, (STA_X|STA_R) + (1 << 4) + (DPL_USER << 5) ,0b0011);
^~~~~~~
kernel/gdt.c:31:14: warning: suggest parentheses around ‘+’ in operand of ‘&’ [-Wparentheses]
accessByte & 0xff, \
^
kernel/gdt.c:54:22: note: in expansion of macro ‘SET_GDT’
gdt[SEG_UDATA] = SET_GDT(0, 0xffffffff, STA_W + (1 << 4) + (DPL_USER << 5) ,0b0011);
^~~~~~~
kernel/gdt.c:51:8: warning: ‘gdt’ is used uninitialized in this function [-Wuninitialized]
gdt[SEG_KCODE] = SET_GDT(0, 0xffffffff, (STA_X|STA_R) + (1 << 4) + (0 << 5) ,0b0011);

[nullplan]

Code: Select all

void lgdt(struct Gdt *g, int size)
{
  volatile uint16_t pd[3];

  pd[0] = size-1; // 16 bit limit
  pd[1] = (uint16_t)g; // 32 bit base
  pd[2] = (uint16_t)g >> 16; // 32 bit base

  asm volatile("lgdt (%0)" : : "r" (pd));
}

I doubt that this code works. GCC is warning you about the cast to an integer of a different size which you can get rid of by casting to uintptr_t. Indeed, I would suggest doing that: The assignment to pd[2] will always assign 0. The cast converts g from a pointer to a 16-bit integer (which it does by truncation), and then shifts it 16 bits anywhere, so the only possible result is zero. If you change both casts to uintptr_t then it is the assignment that does the truncation (although GCC may also warn about that), and the assignment happens after the calculation.

kernel/gdt.c:54:22: note: in expansion of macro ‘SET_GDT’
gdt[SEG_UDATA] = SET_GDT(0, 0xffffffff, STA_W + (1 << 4) + (DPL_USER << 5) ,0b0011);

That is what it says in addition to the main warning further above. It will say something like:

Code: Select all

<at the definition of SET_GDT>: weird stuff happens
<at the use of SET_GDT>: because of this instantiation

Case in point:

kernel/gdt.c: In function ‘createGdt’:
kernel/gdt.c:31:14: warning: suggest parentheses around ‘+’ in operand of ‘&’ [-Wparentheses]
accessByte & 0xff, \
^
kernel/gdt.c:51:22: note: in expansion of macro ‘SET_GDT’
gdt[SEG_KCODE] = SET_GDT(0, 0xffffffff, (STA_X|STA_R) + (1 << 4) + (0 << 5) ,0b0011);
^~~~~~~

Yeah, you need to put parentheses around all macro arguments in a macro expansion unless you know what you are doing. In this case it works out since + binds more tightly than &, but it is still good form to add parentheses, since not everyone always wants to look at a precedence table first. Add them in the definition of SET_GDT like this:

Code: Select all

#define SET_GDT(base, limit, accessByte, flag) (struct Gdt){ \
  (limit) & 0xffff, \
  (base) & 0xffffff, \
  (accessByte) & 0xff, \
  ((limit) >> 16) & 0xf, \
  (flag) & 0xff, \
  ((base) >> 24) & 0xff \
}

[Crixus]

Thx for remembering me that cast apply before shift.
Thx to helping me to fix warnings with parenthesis.

I have no warning now with :

Code: Select all

#include <kernel/tty.h>
#include <kernel/gdt.h>

#define DPL_USER    0x3     // User DPL

// Application segment type bits
#define STA_X       0x8     // Executable segment
#define STA_W       0x2     // Writeable (non-executable segments)
#define STA_R       0x2     // Readable (executable segments)

// System segment type bits
#define STS_T32A    0x9     // Available 32-bit TSS
#define STS_IG32    0xE     // 32-bit Interrupt Gate
#define STS_TG32    0xF     // 32-bit Trap Gate

// various segment selectors.
#define SEG_KCODE 1  // kernel code
#define SEG_KDATA 2  // kernel data+stack
#define SEG_UCODE 3  // user code
#define SEG_UDATA 4  // user data+stack
#define SEG_TSS   5  // this process's task state

#define SET_GDT(base, limit, accessByte, flag) (struct Gdt){ \
  (limit) & (0xffff), \
  (base) & (0xffffff), \
  (accessByte) & (0xff), \
  (limit >> 16) & (0x0f), \
  (flag) & (0xff), \
  (base >> 24) & (0xff) \
}

void lgdt(struct Gdt *g, int size)
{
  struct LgdtDescriptor lgdtDescriptor = {
    (size-1) & (0xffff),
    (intptr_t)g
  };

  asm volatile("lgdt (%0)" : : "r" (&lgdtDescriptor));
}


void createGdt(){
    gdt[SEG_KCODE] = SET_GDT(0, 0xffffffff, (STA_X|STA_R) + (1 << 4) + (0 << 5) ,0b0011);
    gdt[SEG_KDATA] = SET_GDT(0, 0xffffffff, STA_W + (1 << 4) + (0 << 5) ,0b0011);
    gdt[SEG_UCODE] = SET_GDT(0, 0xffffffff, (STA_X|STA_R) + (1 << 4) + (DPL_USER << 5) ,0b0011);
    gdt[SEG_UDATA] = SET_GDT(0, 0xffffffff, STA_W + (1 << 4) + (DPL_USER << 5) ,0b0011);

    lgdt(gdt, sizeof(gdt));
}

Code: Select all

#ifndef GDT
#define GDT

#include <stdio.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

struct Gdt
{
    unsigned int limit : 16;
    unsigned int base : 24;
    unsigned int accessByte : 8;
    unsigned int limit2 : 4;
    unsigned int flags : 4;
    unsigned int base2 : 8;
};

struct LgdtDescriptor
{
    int16_t limit;
    intptr_t base;
};

struct Gdt* gdt;

void createGdt();

#endif